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",
55              Boolean.FALSE, 0);
56  
57      public ConfusingTernaryRule() {
58          super();
59          definePropertyDescriptor(ignoreElseIfProperty);
60      }
61  
62      public Object visit(ASTIfStatement node, Object data) {
63          // look for "if (match) ..; else .."
64          if (node.jjtGetNumChildren() == 3) {
65              Node inode = node.jjtGetChild(0);
66              if (inode instanceof ASTExpression && inode.jjtGetNumChildren() == 1) {
67                  Node jnode = inode.jjtGetChild(0);
68                  if (isMatch(jnode)) {
69  
70                      if (!getProperty(ignoreElseIfProperty)
71                              || (
72                                  !(node.jjtGetChild(2).jjtGetChild(0) instanceof ASTIfStatement)
73                                  &&
74                                  !(node.jjtGetParent().jjtGetParent() instanceof ASTIfStatement)
75                                 )
76                          ) {
77                          addViolation(data, node);
78                      }
79                  }
80              }
81          }
82          return super.visit(node, data);
83      }
84  
85      public Object visit(ASTConditionalExpression node, Object data) {
86          // look for "match ? .. : .."
87          if (node.jjtGetNumChildren() > 0) {
88              Node inode = node.jjtGetChild(0);
89              if (isMatch(inode)) {
90                  addViolation(data, node);
91              }
92          }
93          return super.visit(node, data);
94      }
95  
96      // recursive!
97      private static boolean isMatch(Node node) {
98          return
99                  isUnaryNot(node) ||
100                 isNotEquals(node) ||
101                 isConditionalWithAllMatches(node) ||
102                 isParenthesisAroundMatch(node);
103     }
104 
105     private static boolean isUnaryNot(Node node) {
106         // look for "!x"
107         return
108                 node instanceof ASTUnaryExpressionNotPlusMinus &&
109                 "!".equals(node.getImage());
110     }
111 
112     private static boolean isNotEquals(Node node) {
113         // look for "x != y"
114         return
115                 node instanceof ASTEqualityExpression &&
116                 "!=".equals(node.getImage());
117     }
118 
119     private static boolean isConditionalWithAllMatches(Node node) {
120         // look for "match && match" or "match || match"
121         if (!(node instanceof ASTConditionalAndExpression) &&
122                 !(node instanceof ASTConditionalOrExpression)) {
123             return false;
124         }
125         int n = node.jjtGetNumChildren();
126         if (n <= 0) {
127             return false;
128         }
129         for (int i = 0; i < n; i++) {
130             Node inode = node.jjtGetChild(i);
131             // recurse!
132             if (!isMatch(inode)) {
133                 return false;
134             }
135         }
136         // all match
137         return true;
138     }
139 
140     private static boolean isParenthesisAroundMatch(Node node) {
141         // look for "(match)"
142         if (!(node instanceof ASTPrimaryExpression) ||
143                 (node.jjtGetNumChildren() != 1)) {
144             return false;
145         }
146         Node inode = node.jjtGetChild(0);
147         if (!(inode instanceof ASTPrimaryPrefix) ||
148                 (inode.jjtGetNumChildren() != 1)) {
149             return false;
150         }
151         Node jnode = inode.jjtGetChild(0);
152         if (!(jnode instanceof ASTExpression) ||
153                 (jnode.jjtGetNumChildren() != 1)) {
154             return false;
155         }
156         Node knode = jnode.jjtGetChild(0);
157         // recurse!
158         return isMatch(knode);
159     }
160 }