View Javadoc
1   /**
2    * BSD-style license; for more info see http://pmd.sourceforge.net/license.html
3    */
4   package net.sourceforge.pmd.lang.java.rule.design;
5   
6   import net.sourceforge.pmd.lang.ast.Node;
7   import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression;
8   import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression;
9   import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression;
10  import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression;
11  import net.sourceforge.pmd.lang.java.ast.ASTExpression;
12  import net.sourceforge.pmd.lang.java.ast.ASTIfStatement;
13  import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression;
14  import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix;
15  import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus;
16  import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule;
17  import net.sourceforge.pmd.lang.rule.properties.BooleanProperty;
18  
19  /**
20   * if (x != y) { diff(); } else { same(); } and<br>
21   * (!x ? diff() : same());.
22   * <p/>
23   * XPath can handle the easy cases, e.g.:
24   * 
25   * <pre>
26   *    //IfStatement[
27   *      Statement[2]
28   *      and Expression[
29   *        EqualityExpression[@Image="!="] or
30   *        UnaryExpressionNotPlusMinus[@Image="!"]]]
31   * </pre>
32   * 
33   * but "&amp;&amp;" and "||" are difficult, since we need a match for <i>all</i>
34   * children instead of just one. This can be done by using a double-negative,
35   * e.g.:
36   * 
37   * <pre>
38   *    not(*[not(<i>matchme</i>)])
39   * </pre>
40   * 
41   * Still, XPath is unable to handle arbitrarily nested cases, since it lacks
42   * recursion, e.g.:
43   * 
44   * <pre>
45   * if (((x != !y)) || !(x)) {
46   *     diff();
47   * } else {
48   *     same();
49   * }
50   * </pre>
51   */
52  public class ConfusingTernaryRule extends AbstractJavaRule {
53      private static BooleanProperty ignoreElseIfProperty = new BooleanProperty("ignoreElseIf",
54              "Ignore conditions with an else-if case", Boolean.FALSE, 0);
55  
56      public ConfusingTernaryRule() {
57          super();
58          definePropertyDescriptor(ignoreElseIfProperty);
59      }
60  
61      public Object visit(ASTIfStatement node, Object data) {
62          // look for "if (match) ..; else .."
63          if (node.jjtGetNumChildren() == 3) {
64              Node inode = node.jjtGetChild(0);
65              if (inode instanceof ASTExpression && inode.jjtGetNumChildren() == 1) {
66                  Node jnode = inode.jjtGetChild(0);
67                  if (isMatch(jnode)) {
68  
69                      if (!getProperty(ignoreElseIfProperty)
70                              || !(node.jjtGetChild(2).jjtGetChild(0) instanceof ASTIfStatement)
71                              && !(node.jjtGetParent().jjtGetParent() instanceof ASTIfStatement)) {
72                          addViolation(data, node);
73                      }
74                  }
75              }
76          }
77          return super.visit(node, data);
78      }
79  
80      public Object visit(ASTConditionalExpression node, Object data) {
81          // look for "match ? .. : .."
82          if (node.jjtGetNumChildren() > 0) {
83              Node inode = node.jjtGetChild(0);
84              if (isMatch(inode)) {
85                  addViolation(data, node);
86              }
87          }
88          return super.visit(node, data);
89      }
90  
91      // recursive!
92      private static boolean isMatch(Node node) {
93          return isUnaryNot(node) || isNotEquals(node) || isConditionalWithAllMatches(node)
94                  || isParenthesisAroundMatch(node);
95      }
96  
97      private static boolean isUnaryNot(Node node) {
98          // look for "!x"
99          return node instanceof ASTUnaryExpressionNotPlusMinus && "!".equals(node.getImage());
100     }
101 
102     private static boolean isNotEquals(Node node) {
103         // look for "x != y"
104         return node instanceof ASTEqualityExpression && "!=".equals(node.getImage());
105     }
106 
107     private static boolean isConditionalWithAllMatches(Node node) {
108         // look for "match && match" or "match || match"
109         if (!(node instanceof ASTConditionalAndExpression) && !(node instanceof ASTConditionalOrExpression)) {
110             return false;
111         }
112         int n = node.jjtGetNumChildren();
113         if (n <= 0) {
114             return false;
115         }
116         for (int i = 0; i < n; i++) {
117             Node inode = node.jjtGetChild(i);
118             // recurse!
119             if (!isMatch(inode)) {
120                 return false;
121             }
122         }
123         // all match
124         return true;
125     }
126 
127     private static boolean isParenthesisAroundMatch(Node node) {
128         // look for "(match)"
129         if (!(node instanceof ASTPrimaryExpression) || node.jjtGetNumChildren() != 1) {
130             return false;
131         }
132         Node inode = node.jjtGetChild(0);
133         if (!(inode instanceof ASTPrimaryPrefix) || inode.jjtGetNumChildren() != 1) {
134             return false;
135         }
136         Node jnode = inode.jjtGetChild(0);
137         if (!(jnode instanceof ASTExpression) || jnode.jjtGetNumChildren() != 1) {
138             return false;
139         }
140         Node knode = jnode.jjtGetChild(0);
141         // recurse!
142         return isMatch(knode);
143     }
144 }