Using PMD to scan explicit/implicit casting of BigDecimal to String

At work, a project needed to be upgraded from Java 1.4 to Java 1.6. One of the big issues introduce in Java 1.5 was the behavior change of the toString method of BigDecimal. Under certain situation, it now returns the string in using scientific notation. This change was based on JSR 13. While the change is justifiable, some question if the expert group underestimated the behavioral compatibility impact, especially with programs that interface with databases.

Anyway, one way (the way we used at work) to solve the issue is to use the new method toPlainString. We grep the code to find all the toString method calls and manually change them to toPlainString. But not only this is prone to error, there are many cases that can't be detected. For example, this code doesn't call the toString method explicitly but still affected by the compatibility issue :

String s = "" + bd;

There ought to be a better (and automated) way to scan the source tree. So during a long weekend, I spent a day to play with PMD. Below is a rule that I implemented to do the job. Part of it is based on a sample found in the PMD package.But note that this still CANNOT find all the cases of BigDecimal.toString (more about that later).

import java.util.HashMap;
import java.util.Map;
import java.util.Set;
import java.util.List;

import net.sourceforge.pmd.lang.ast.Node;
import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration;
import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator;
import net.sourceforge.pmd.lang.java.ast.ASTType;
import net.sourceforge.pmd.lang.java.ast.ASTLiteral;
import net.sourceforge.pmd.lang.java.ast.ASTName;
import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId;
import net.sourceforge.pmd.lang.java.ast.ASTAdditiveExpression;
import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
import net.sourceforge.pmd.lang.java.symboltable.NameOccurrence;
import net.sourceforge.pmd.util.CollectionUtil;


/**
 *
 */
public class ScanBigDecimal extends AbstractJavaRule {

    /**
     * These are the BigDecimal methods
     */
    private static final Set<String> BIG_DECIMAL_METHODS = CollectionUtil.asSet(new String[] { ".toString" });

    /**
     * These are the classes that the rule can apply to
     */
    private static final Map<String, Set<String>> MAP_CLASSES = new HashMap<String, Set<String>>();
    static {
        MAP_CLASSES.put("java.math.BigDecimal", BIG_DECIMAL_METHODS);
        MAP_CLASSES.put("BigDecimal", BIG_DECIMAL_METHODS);
    }

    @Override
    public Object visit(ASTLocalVariableDeclaration node, Object data) {

        ASTVariableDeclaratorId var = getDeclaration(node);
        if (var == null) {
            return super.visit(node, data);
        }
        String variableName = var.getImage();
        for (NameOccurrence no: var.getUsages()) {
            // FIXME - getUsages will return everything with the same name as the variable,
            // see JUnit test, case 6. Changing to Node below, revisit when getUsages is fixed
            Node sn = no.getLocation();
            Node primaryExpression = sn.jjtGetParent().jjtGetParent();
            Class<? extends Node> parentClass = primaryExpression.jjtGetParent().getClass();

            // see if we are calling the methods we are interested in
            String methodCall = sn.getImage().substring(variableName.length());
            ASTType nodeType = node.getTypeNode();
            if (nodeType != null && MAP_CLASSES.get(nodeType.getTypeImage()).contains(methodCall)) {
                addViolation(data, sn);
            }
        }
        return super.visit(node, data);
    }

    @Override
    public Object visit(ASTAdditiveExpression node, Object data) {

        boolean hasLiteral = true;
        boolean hasClass = false;

        if (node == null) {
            return super.visit(node, data);
        }

        for (int i = 0; i < node.jjtGetNumChildren(); i++) {
            Node child = node.jjtGetChild(i);

            if (child.hasDescendantOfType(ASTLiteral.class)) {
                hasLiteral = true;
            } else {
                ASTName nameNode = (ASTName)child.getFirstDescendantOfType(ASTName.class);

                if (nameNode != null && findDeclaration(node, nameNode.getImage()) != null) {
                    hasClass = true;
                }
            }

            if (hasLiteral && hasClass) {
                addViolation(data, node);
                break;
            }
        }

        return super.visit(node, data);
    }


    /**
     * This method checks the variable declaration if it is on a class we care
     * about. If it is, it returns the DeclaratorId
     *
     * @param node
     *            The ASTLocalVariableDeclaration which is a problem
     * @return ASTVariableDeclaratorId
     */
    private ASTVariableDeclaratorId getDeclaration(ASTLocalVariableDeclaration node) {
        ASTType type = node.getTypeNode();
        if (MAP_CLASSES.keySet().contains(type.getTypeImage())) {
            return node.getFirstDescendantOfType(ASTVariableDeclaratorId.class);
        }
        return null;
    }

    private ASTVariableDeclaratorId findDeclaration(Node node, String varName) {
        if (node == null || varName == null) {
            return null;
        }

        Node parent = node.getFirstParentOfType(ASTClassOrInterfaceBodyDeclaration.class);
        if (parent != null) {
            List<ASTVariableDeclaratorId> list = parent.findDescendantsOfType(ASTVariableDeclaratorId.class);

            for (ASTVariableDeclaratorId declarator : list) {
                if (varName.equals(declarator.getImage())) {
                    ASTType nodeType = null;
                    if (declarator.jjtGetParent() instanceof ASTFormalParameter) {
                        nodeType = ((ASTFormalParameter)(declarator.jjtGetParent())).getTypeNode();
                    } else if (declarator.jjtGetParent() instanceof ASTLocalVariableDeclaration) {
                        nodeType = ((ASTLocalVariableDeclaration)(declarator.jjtGetParent())).getTypeNode();
                    } else if (declarator.jjtGetParent() instanceof ASTVariableDeclarator) {
                        nodeType = ((ASTLocalVariableDeclaration)(declarator.jjtGetParent().jjtGetParent())).getTypeNode();
                    } else {
                        System.err.println("Unknown type");
                    }

                    if (nodeType != null && MAP_CLASSES.keySet().contains(nodeType.getTypeImage())) {
                        return declarator;
                    }
                }
            }
        }

        return null;
    }
}

Here is the ruleset file:

<?xml version="1.0"?>
        <ruleset name="My custom rules"
            xmlns="http://pmd.sourceforge.net/ruleset/2.0.0"
            xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
            xsi:schemaLocation="http://pmd.sourceforge.net/ruleset/2.0.0 http://pmd.sourceforge.net/ruleset_2_0_0.xsd"
            xsi:noNamespaceSchemaLocation="http://pmd.sourceforge.net/ruleset_2_0_0.xsd">
            <rule name="ScanBigDecimal"
                  message="Avoid using BigDecimal.toString or implicit casting to string"
                  class="ScanBigDecimal">
              <description>
              Avoid using BigDecimal.toString or implicit casting to string
              </description>
                <priority>3</priority>

              <example>
        <![CDATA[
            }
        ]]>
              </example>
            </rule>
        </ruleset>

Command to test the rule:

java -cp "C:/Users/clarence/Documents/MyApps/pmd-bin-5.0.0/lib/*";. net.sourceforge.pmd.PMD C:/Users/clarence/Documents/MyTest/pmd/MyTestCase.java text bigdecimal.xml

Below is a sample test case. As said, there are cases that cannot be detected by my PMD rule. For example, concatenating a string with a BigDecimal return value from method call. Or, passing a BigDecimal to a method that takes an Object parameter and internally invoking the toString method.

Anyway, feel free to take the code and make it better~!

import java.math.BigDecimal;

public class MyTestCase {

    public static void main(String args) {
        BigDecimal bd = new BigDecimal("12.345");

        // will trigger error
        System.out.println("This is the number: " + bd);

        // this type can't be detected...
        System.out.println(bd);

        // will trigger error;
        String s = "" + bd;
        String s2 = "" + bd.toString();
        String s3 = bd + "";
        String s4 = bd + MyTestCase.returnStr();

        // can't be detected
        String s5 = "" + returnBigDecimal("12.34");
        System.err.println(returnString(bd));

        // this wont trigger warning
        String s6 = "" + bd.toPlainString();

        String s7 = "" + "a" + "b";

    }

    private static String returnStr() {
        return "hello";
    }

    private static String returnString(BigDecimal bd) {
        return "" + bd;
    }

    private static BigDecimal returnBigDecimal(String num) {
        return new BigDecimal(num);
    }
}