From b769629a100337ba94ab29b18916fa1db3b13ed3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 15 Aug 2019 01:43:13 +0200 Subject: [PATCH 1/8] Split UnaryExpr into Prefix and Postfix Among the different possible categorisations of unary expressions, this is probably the most logical and easiest to document. Here's a comparison of different possible categorisations. Note: `_++` is the postfix increment operator, while `++_` is the prefix one - idem for decrement. The last one is the one implemented by this commit. \## 6.0.x ``` Unary = { -, + } UnaryNotPlusMinus { !, ~ } PreIncrement = { ++_ } PreDecrement { --_ } Postfix { _++, _++ } ``` * Very assymmetric, splits operators based on parsing concerns \## Before #1890: ``` Unary = { -, + , !, ~ } PreIncrement = { ++_ } PreDecrement { --_ } Postfix { _++, _++ } ``` * Minor simplification \## #1890: ``` Unary = Prefix \ { ++_, --_ }) Increment ( { ++ , -- } x (postfix, prefix) ) ``` * Names are weird (IncrementExpr may be decrement, Unary != Increment even though semantically, Increment \subset Unary) * No possibility to introduce a supertype (what would it be?) * But easy to match all increment/decrement expressions \## JLS (also, Eclipse): ``` Prefix = { !, ~, -, +, ++_, --_ } Postfix = { _++, _-- } ``` * Both can have super interface UnaryExpr * This allows matching all increment/decrement expressions easily too * Easiest to document, JLS like, AST like * Fits well with `InfixExpr` --- pmd-java/etc/grammar/Java.jjt | 19 ++- .../pmd/lang/java/ast/ASTAssignableExpr.java | 4 +- .../pmd/lang/java/ast/ASTExpression.java | 4 +- .../lang/java/ast/ASTIncrementExpression.java | 109 -------------- .../lang/java/ast/ASTPostfixExpression.java | 51 +++++++ .../lang/java/ast/ASTPrefixExpression.java | 57 ++++++++ .../pmd/lang/java/ast/ASTUnaryExpression.java | 59 ++------ .../pmd/lang/java/ast/IncrementOp.java | 37 ----- .../java/ast/JavaParserVisitorAdapter.java | 12 +- .../pmd/lang/java/ast/UnaryOp.java | 134 ++++++++++++------ .../AvoidReassigningLoopVariablesRule.java | 6 +- .../errorprone/AssignmentInOperandRule.java | 10 +- .../java/symboltable/JavaNameOccurrence.java | 9 +- .../typeresolution/ClassTypeResolver.java | 82 +++++------ .../pmd/lang/java/ast/ASTLiteralTest.kt | 11 +- .../ast/ASTMultiplicativeExpressionTest.kt | 4 +- .../lang/java/ast/ASTSwitchExpressionTests.kt | 3 +- .../lang/java/ast/ASTUnaryExpressionTest.kt | 66 +++++---- .../pmd/lang/java/ast/TestExtensions.kt | 22 +-- 19 files changed, 345 insertions(+), 354 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTIncrementExpression.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java create mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/IncrementOp.java diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index 72cd939fec..ca1c96f1c5 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2495,23 +2495,24 @@ void MultiplicativeExpression() #void: void UnaryExpression() #void: {} { - (("+" {jjtThis.setImage("+");} | "-" {jjtThis.setImage("-");}) UnaryExpression()) #UnaryExpression + (("+" {jjtThis.setOp(UnaryOp.PrefixOp.UNARY_PLUS);} | "-" {jjtThis.setOp(UnaryOp.PrefixOp.UNARY_MINUS);}) UnaryExpression()) #PrefixExpression | PrefixIncrementExpression() | UnaryExpressionNotPlusMinus() } -void PrefixIncrementExpression() #IncrementExpression: +void PrefixIncrementExpression() #PrefixExpression: {} { - ( "++" {jjtThis.setIncrement();} | "--" ) - { jjtThis.setPrefix(); } + ("++" {jjtThis.setOp(UnaryOp.PrefixOp.PRE_INCREMENT);} + | "--" {jjtThis.setOp(UnaryOp.PrefixOp.PRE_DECREMENT);} + ) PrimaryExpression() } void UnaryExpressionNotPlusMinus() #void: {} { - (( "~" {jjtThis.setImage("~");} | "!" {jjtThis.setImage("!");}) UnaryExpression()) #UnaryExpression + (( "~" {jjtThis.setOp(UnaryOp.PrefixOp.COMPLEMENT);} | "!" {jjtThis.setOp(UnaryOp.PrefixOp.NEGATION);}) UnaryExpression()) #PrefixExpression // There's no separate production for CastExpression, because that would force // us to repeat the lookahead // Those lookaheads are quite expensive, they're run to disambiguate between @@ -2540,7 +2541,13 @@ void PostfixExpression() #void: {} { - PrimaryExpression() [LOOKAHEAD(1) ("++" {jjtThis.setIncrement();} | "--") #IncrementExpression(1) ] + PrimaryExpression() + [ + LOOKAHEAD(1) + ("++" {jjtThis.setOp(UnaryOp.PostfixOp.POST_INCREMENT);} + | "--" {jjtThis.setOp(UnaryOp.PostfixOp.POST_DECREMENT);} + ) #PostfixExpression(1) + ] } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAssignableExpr.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAssignableExpr.java index 7040a04bd2..55e52b62c1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAssignableExpr.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAssignableExpr.java @@ -30,7 +30,7 @@ public interface ASTAssignableExpr extends ASTPrimaryExpression { /** * Returns how this expression is accessed in the enclosing expression. * If this expression occurs as the left-hand-side of an {@linkplain ASTAssignmentExpression assignment}, - * or as the target of an {@linkplain ASTIncrementExpression increment or decrement expression}, + * or as the target of an {@linkplain ASTUnaryExpression increment or decrement expression}, * this method returns {@link AccessType#WRITE}. Otherwise the value is just {@linkplain AccessType#READ read}. */ @NonNull @@ -38,7 +38,7 @@ public interface ASTAssignableExpr extends ASTPrimaryExpression { Node parent = this.jjtGetParent(); - if (parent instanceof ASTIncrementExpression + if (parent instanceof ASTUnaryExpression && !((ASTUnaryExpression) parent).getOperator().isPure() || jjtGetChildIndex() == 0 && parent instanceof ASTAssignmentExpression) { return AccessType.WRITE; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java index 3adc1327be..09c35881e2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java @@ -20,8 +20,8 @@ package net.sourceforge.pmd.lang.java.ast; * | {@link ASTConditionalExpression ConditionalExpression} * | {@link ASTInfixExpression InfixExpression} * | {@link ASTInstanceOfExpression InstanceOfExpression} - * | {@link ASTUnaryExpression UnaryExpression} | {@link ASTIncrementExpression PrefixIncrement} | {@link ASTCastExpression CastExpression} - * | {@link ASTIncrementExpression PostfixIncrement} + * | {@link ASTPrefixExpression PrefixExpression} | {@link ASTCastExpression CastExpression} + * | {@link ASTPostfixExpression PostfixExpression} * | {@link ASTSwitchExpression SwitchExpression} * | {@link ASTPrimaryExpression PrimaryExpression} * diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTIncrementExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTIncrementExpression.java deleted file mode 100644 index 77467aed9c..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTIncrementExpression.java +++ /dev/null @@ -1,109 +0,0 @@ -/* - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - -/** - * Represents an increment or decrement operation on a variable. This represents - * both the postfix and prefix forms. - * - *

Since 7.0, this node replaces the nodes PostfixExpression, PreDecrementExpression, - * and PreIncrementExpression. - * - *

In prefix form, this has the same precedence as {@linkplain ASTUnaryExpression UnaryExpression}. - * In postfix form, this has a precedence greater as {@linkplain ASTUnaryExpression UnaryExpression}, - * and lower as {@linkplain ASTPrimaryExpression PrimaryExpression}. - * - * 

- *
- * IncrementExpr ::= {@linkplain ASTAssignableExpr AssignableExpr} ( "++" | "--" )
- *                 | ( "++" | "--" ) {@linkplain ASTAssignableExpr AssignableExpr}
- *
- *
- */ -public final class ASTIncrementExpression extends AbstractJavaExpr implements ASTExpression, LeftRecursiveNode { - - private boolean isPrefix; - private boolean isIncrement; - - ASTIncrementExpression(int id) { - super(id); - } - - ASTIncrementExpression(JavaParser p, int id) { - super(p, id); - } - - @Override - public Object jjtAccept(JavaParserVisitor visitor, Object data) { - return visitor.visit(this, data); - } - - - @Override - public void jjtAccept(SideEffectingVisitor visitor, T data) { - visitor.visit(this, data); - } - - - void setPrefix() { - this.isPrefix = true; - } - - - void setIncrement() { - this.isIncrement = true; - } - - /** - * Returns true if this a prefix assignment, {@literal e.g.} {@code ++i}, {@code --i}. - */ - public boolean isPrefix() { - return isPrefix; - } - - /** - * Returns true if this a postfix assignment, {@literal e.g.} {@code i++}, {@code i--}. - */ - public boolean isPostfix() { - return !isPrefix; - } - - /** - * Returns the operator of this expression. - */ - public IncrementOp getOp() { - return isIncrement ? IncrementOp.INCREMENT : IncrementOp.DECREMENT; - } - - /** - * Returns the name of the operator. - */ - public String getOpName() { - return getOp().name(); - } - - /** - * Returns true if this is an increment expression ({@link #getOp()} is {@link IncrementOp#INCREMENT}). - */ - public boolean isIncrement() { - return isIncrement; - } - - /** - * Returns true if this is a decrement expression ({@link #getOp()} is {@link IncrementOp#DECREMENT}). - */ - public boolean isDecrement() { - return !isIncrement; - } - - - /** - * Returns the expression assigned by this expression. - */ - public ASTAssignableExpr getOperand() { - return (ASTAssignableExpr) jjtGetChild(0); - } - -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java new file mode 100644 index 0000000000..726eb90eaa --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java @@ -0,0 +1,51 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.ast; + + +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp; + +/** + * Represents a unary postfix operation on a value. + * This has a precedence greater than {@link ASTCastExpression CastExpression}. + * + * 
+ *
+ * PostfixExpression ::= {@link ASTPrimaryExpression PrimaryExpression} ({@link PostfixOp "++" | "--"})
+ *
+ *
+ */ +public final class ASTPostfixExpression extends AbstractJavaExpr implements ASTUnaryExpression { + + private PostfixOp operator; + + ASTPostfixExpression(int id) { + super(id); + } + + ASTPostfixExpression(JavaParser p, int id) { + super(p, id); + } + + @Override + public Object jjtAccept(JavaParserVisitor visitor, Object data) { + return visitor.visit(this, data); + } + + + @Override + public void jjtAccept(SideEffectingVisitor visitor, T data) { + visitor.visit(this, data); + } + + @Override + public PostfixOp getOperator() { + return operator; + } + + void setOp(PostfixOp op) { + this.operator = op; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java new file mode 100644 index 0000000000..e0fcde67f2 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java @@ -0,0 +1,57 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.ast; + + +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp; + +/** + * Represents a unary prefix operation on a value. + * This has a precedence greater than {@link ASTInfixExpression}. + * + *

Prefix expressions have the same precedence as {@linkplain ASTCastExpression CastExpression}. + * + * 

+ *
+ * PrefixExpression ::= {@link PrefixOp} UnaryExpression
+ *
+ *
+ */ +public final class ASTPrefixExpression extends AbstractJavaExpr implements ASTUnaryExpression { + + private PrefixOp operator; + + ASTPrefixExpression(int id) { + super(id); + } + + ASTPrefixExpression(JavaParser p, int id) { + super(p, id); + } + + @Override + public Object jjtAccept(JavaParserVisitor visitor, Object data) { + return visitor.visit(this, data); + } + + + @Override + public void jjtAccept(SideEffectingVisitor visitor, T data) { + visitor.visit(this, data); + } + + @Override + public PrefixOp getOperator() { + return operator; + } + + void setOp(PrefixOp op) { + this.operator = op; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java index 75f3eaf12d..4f4bd42b72 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java @@ -5,64 +5,31 @@ package net.sourceforge.pmd.lang.java.ast; -import java.util.Objects; - /** - * Represents a unary prefix operation on a value. - * This has a precedence greater than {@link ASTMultiplicativeExpression}. - * - *

UnaryExpression has the same precedence as the prefix forms of {@linkplain ASTIncrementExpression IncrementExpression}, - * and {@linkplain ASTCastExpression CastExpression}. - * - *

Note that the child of this node is not necessarily a UnaryExpression, - * rather, it can be an expression with an operator precedence greater or equal - * to a UnaryExpression. + * Represents a unary operation on a value. The syntactic form may be + * prefix or postfix, which are represented with different nodes. * * 

  *
- * UnaryExpression ::= {@link UnaryOp} UnaryExpression
+ * UnaryExpression ::= {@link ASTPrefixExpression PrefixExpression}
+ *                   | {@link ASTPostfixExpression PostfixExpression}
  *
  *
*/ -public final class ASTUnaryExpression extends AbstractJavaExpr implements ASTExpression { +public interface ASTUnaryExpression extends ASTExpression { - private UnaryOp operator; - - ASTUnaryExpression(int id) { - super(id); - } - - ASTUnaryExpression(JavaParser p, int id) { - super(p, id); - } - - @Override - public Object jjtAccept(JavaParserVisitor visitor, Object data) { - return visitor.visit(this, data); - } - - - @Override - public void jjtAccept(SideEffectingVisitor visitor, T data) { - visitor.visit(this, data); - } - - - public ASTExpression getOperand() { + /** + * Returns the expression nested within this expression. + */ + default ASTExpression getOperand() { return (ASTExpression) jjtGetChild(0); } - @Override - public void setImage(String image) { - super.setImage(image); - this.operator = Objects.requireNonNull(UnaryOp.fromImage(image)); - } - /** - * Returns the constant representing the operator of this expression. + * Returns the constant representing the operator. */ - public UnaryOp getOperator() { - return operator; - } + UnaryOp getOperator(); + + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/IncrementOp.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/IncrementOp.java deleted file mode 100644 index 65a2c45568..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/IncrementOp.java +++ /dev/null @@ -1,37 +0,0 @@ -/* - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - -/** - * An increment operator for {@link ASTIncrementExpression IncrementExpression}. - * - * 
- *
- * IncrementOp ::= "++" | "--"
- *
- *
- * - * @see AssignmentOp - * @see UnaryOp - */ -public enum IncrementOp { - /** "++" */ - INCREMENT("++"), - /** "--" */ - DECREMENT("--"); - - private final String code; - - - IncrementOp(String code) { - this.code = code; - } - - @Override - public String toString() { - return this.code; - } - -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java index a3d37bba79..2b81edc93e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java @@ -112,12 +112,20 @@ public class JavaParserVisitorAdapter implements JavaParserVisitor { return visit((ASTExpression) node, data); } - - @Override public Object visit(ASTUnaryExpression node, Object data) { return visit((ASTExpression) node, data); } + @Override + public Object visit(ASTPrefixExpression node, Object data) { + return visit((ASTUnaryExpression) node, data); + } + + @Override + public Object visit(ASTPostfixExpression node, Object data) { + return visit((ASTUnaryExpression) node, data); + } + @Override public Object visit(ASTCastExpression node, Object data) { return visit((ASTExpression) node, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java index 68c2f861a9..bace1285d2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java @@ -1,63 +1,115 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ package net.sourceforge.pmd.lang.java.ast; -import static java.util.stream.Collectors.collectingAndThen; -import static java.util.stream.Collectors.toMap; - -import java.util.Arrays; -import java.util.Collections; -import java.util.Map; - - /** - * A unary operator for {@link ASTUnaryExpression}. + * A unary operator, either prefix or postfix. This is used by {@link ASTUnaryExpression UnaryExpression} + * to abstract over the syntactic form of the operator. * * 
  *
- * UnaryOp ::= "+" | "-" | "~" | "!"
+ * UnaryOp ::= {@link PrefixOp} | {@link PostfixOp}
  *
  *
* - * @see BinaryOp - * @see AssignmentOp */ -public enum UnaryOp { - /** "+" */ - UNARY_PLUS("+"), - /** "-" */ - UNARY_MINUS("-"), - /** "~" */ - BITWISE_INVERSE("~"), - /** "!" */ - BOOLEAN_NOT("!"); +public interface UnaryOp { - private static final Map LOOKUP = - Arrays.stream(values()) - .collect( - collectingAndThen( - toMap(Object::toString, op -> op), - Collections::unmodifiableMap - ) - ); - - private final String code; + /** + * Returns true if this operator is pure, ie the evaluation of + * the unary expression doesn't produce side-effects. Only increment + * and decrement operators are impure. + * + *

This can be used to fetch all increment or decrement operations, + * regardless of whether they're postfix or prefix. E.g. + * 

{@code
+     *  node.findDescendantsOfType(ASTUnaryExpression.class).stream().anyMatch(it -> !it.getOperator().isPure())
+     * }
+ * + * TODO update example for node streams + */ + boolean isPure(); - UnaryOp(String code) { - this.code = code; + /** + * A prefix operator for {@link ASTPrefixExpression}. + * + * 
+     *
+     * PrefixOp ::= "+" | "-" | "~" | "!" | "++" | "--"
+     *
+     *
+ * + * @see BinaryOp + * @see AssignmentOp + */ + enum PrefixOp implements UnaryOp { + /** Unary numeric promotion operator {@code "+"}. */ + UNARY_PLUS("+"), + /** Arithmetic negation operation {@code "-"}. */ + UNARY_MINUS("-"), + /** Bitwise complement operator {@code "~"}. */ + COMPLEMENT("~"), + /** Logical complement operator {@code "!"}. */ + NEGATION("!"), + + /** Prefix increment operator {@code "++"}. */ + PRE_INCREMENT("++"), + /** Prefix decrement operator {@code "--"}. */ + PRE_DECREMENT("--"); + + private final String code; + + PrefixOp(String code) { + this.code = code; + } + + @Override + public boolean isPure() { + return this != PRE_INCREMENT && this != PRE_DECREMENT; + } + + @Override + public String toString() { + return this.code; + } + } - @Override - public String toString() { - return this.code; - } + /** + * A postfix operator for {@link ASTPostfixExpression}. + * + * 
+     *
+     * PostfixOp ::= "++" | "--"
+     *
+     *
+ * + * @see BinaryOp + * @see AssignmentOp + */ + enum PostfixOp implements UnaryOp { + /** Postfix increment operator {@code "++"}. */ + POST_INCREMENT("++"), + /** Postfix decrement operator {@code "--"}. */ + POST_DECREMENT("--"); + private final String code; - // parser only for now - static UnaryOp fromImage(String image) { - return LOOKUP.get(image); + PostfixOp(String code) { + this.code = code; + } + + @Override + public boolean isPure() { + return false; + } + + @Override + public String toString() { + return this.code; + } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java index 32e42e9548..4f092eea5d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningLoopVariablesRule.java @@ -26,7 +26,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTForInit; import net.sourceforge.pmd.lang.java.ast.ASTForStatement; import net.sourceforge.pmd.lang.java.ast.ASTForUpdate; import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; -import net.sourceforge.pmd.lang.java.ast.ASTIncrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; @@ -34,6 +33,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTStatement; import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement; +import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; import net.sourceforge.pmd.lang.java.rule.performance.AbstractOptimizationRule; @@ -138,8 +138,8 @@ public class AvoidReassigningLoopVariablesRule extends AbstractOptimizationRule */ private void checkIncrementAndDecrement(Object data, Set loopVariables, ASTStatement loopBody, IgnoreFlags... ignoreFlags) { - for (ASTIncrementExpression expression : loopBody.findDescendantsOfType(ASTIncrementExpression.class)) { - if (ignoreNode(expression, loopBody, ignoreFlags)) { + for (ASTUnaryExpression expression : loopBody.findDescendantsOfType(ASTUnaryExpression.class)) { + if (expression.getOperator().isPure() || ignoreNode(expression, loopBody, ignoreFlags)) { continue; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java index 91f0197250..c05bf3ba2a 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AssignmentInOperandRule.java @@ -11,7 +11,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTForStatement; import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; -import net.sourceforge.pmd.lang.java.ast.ASTIncrementExpression; +import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -59,8 +59,7 @@ public class AssignmentInOperandRule extends AbstractJavaRule { || parent instanceof ASTForStatement && parent.jjtGetChild(1) == node && !getProperty(ALLOW_FOR_DESCRIPTOR)) && (node.hasDescendantOfType(ASTAssignmentOperator.class) - || !getProperty(ALLOW_INCREMENT_DECREMENT_DESCRIPTOR) - && node.hasDescendantOfType(ASTIncrementExpression.class))) { + || !getProperty(ALLOW_INCREMENT_DECREMENT_DESCRIPTOR) && hasIncrement(node))) { addViolation(data, node); return data; @@ -68,6 +67,11 @@ public class AssignmentInOperandRule extends AbstractJavaRule { return super.visit(node, data); } + private boolean hasIncrement(ASTExpression node) { + // todo use node streams + return node.findDescendantsOfType(ASTUnaryExpression.class).stream().anyMatch(it -> !it.getOperator().isPure()); + } + public boolean allowsAllAssignments() { return getProperty(ALLOW_IF_DESCRIPTOR) && getProperty(ALLOW_FOR_DESCRIPTOR) && getProperty(ALLOW_WHILE_DESCRIPTOR) && getProperty(ALLOW_INCREMENT_DECREMENT_DESCRIPTOR); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java index 56c8b1a40a..1825b97b78 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java @@ -7,13 +7,14 @@ package net.sourceforge.pmd.lang.java.symboltable; import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTExpression; -import net.sourceforge.pmd.lang.java.ast.ASTIncrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTMethodReference; import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTPostfixExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; import net.sourceforge.pmd.lang.java.ast.ASTResource; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; +import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; @@ -120,7 +121,7 @@ public class JavaNameOccurrence implements NameOccurrence { } private boolean isStandAlonePostfix(Node primaryExpression) { - if (!(primaryExpression instanceof ASTIncrementExpression && ((ASTIncrementExpression) primaryExpression).isPostfix()) + if (!(primaryExpression instanceof ASTPostfixExpression && !((ASTPostfixExpression) primaryExpression).getOperator().isPure()) || !(primaryExpression.jjtGetParent() instanceof ASTStatementExpression)) { return false; } @@ -152,7 +153,7 @@ public class JavaNameOccurrence implements NameOccurrence { Node p = l.jjtGetParent(); Node gp = p.jjtGetParent(); Node node = gp.jjtGetParent(); - if (node instanceof ASTIncrementExpression) { + if (node instanceof ASTUnaryExpression && !((ASTUnaryExpression) node).getOperator().isPure()) { return true; } @@ -174,7 +175,7 @@ public class JavaNameOccurrence implements NameOccurrence { } // catch this.i++ or ++this.i - return gp instanceof ASTIncrementExpression; + return node instanceof ASTUnaryExpression && !((ASTUnaryExpression) node).getOperator().isPure(); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 2e54957d62..e188186967 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -55,7 +55,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTForStatement; import net.sourceforge.pmd.lang.java.ast.ASTFormalParameter; import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTInclusiveOrExpression; -import net.sourceforge.pmd.lang.java.ast.ASTIncrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTInstanceOfExpression; import net.sourceforge.pmd.lang.java.ast.ASTIntersectionType; import net.sourceforge.pmd.lang.java.ast.ASTLiteral; @@ -89,6 +88,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTYieldStatement; import net.sourceforge.pmd.lang.java.ast.InternalApiBridge; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; import net.sourceforge.pmd.lang.java.ast.TypeNode; +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp; import net.sourceforge.pmd.lang.java.symboltable.ClassScope; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; @@ -110,11 +110,14 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { private static final Map> PRIMITIVE_TYPES; private static final Map JAVA_LANG; - + private final PMDASMClassLoader pmdClassLoader; private Map staticFieldImageToTypeDef; private Map> staticNamesToClasses; private List importOnDemandStaticClasses; private ASTCompilationUnit currentAcu; + private Map importedClasses; + private List importedOnDemand; + static { // Note: Assumption here that primitives come from same parent @@ -169,10 +172,6 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { JAVA_LANG = Collections.unmodifiableMap(theJavaLang); } - private final PMDASMClassLoader pmdClassLoader; - private Map importedClasses; - private List importedOnDemand; - public ClassTypeResolver() { this(ClassTypeResolver.class.getClassLoader()); @@ -283,7 +282,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param node * * @return The index in the array produced by splitting the node's name by '.', which is not part of the - * class name found. Example: com.package.SomeClass.staticField.otherField, return would be 3 + * class name found. Example: com.package.SomeClass.staticField.otherField, return would be 3 */ private int searchNodeNameForClass(TypeNode node) { // this is the index from which field/method names start in the dotSplitImage array @@ -293,7 +292,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // First try: com.package.SomeClass.staticField.otherField // Second try: com.package.SomeClass.staticField // Third try: com.package.SomeClass <- found a class! - for (String reducedImage = node.getImage();;) { + for (String reducedImage = node.getImage(); ; ) { populateType(node, reducedImage); if (node.getType() != null) { break; // we found a class! @@ -370,8 +369,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // TODO: remove this if branch, it's only purpose is to make JUnitAssertionsShouldIncludeMessage's tests pass // as the code is not compiled there and symbol table works on uncompiled code if (node.getNameDeclaration() != null - && previousType == null // if it's not null, then let other code handle things - && node.getNameDeclaration().getNode() instanceof TypeNode) { + && previousType == null // if it's not null, then let other code handle things + && node.getNameDeclaration().getNode() instanceof TypeNode) { // Carry over the type (including generics) from the declaration JavaTypeDefinition nodeType = ((TypeNode) node.getNameDeclaration().getNode()).getTypeDefinition(); if (nodeType != null) { @@ -471,7 +470,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Node prefix = node.jjtGetParent(); if (prefix instanceof ASTPrimaryPrefix - && prefix.jjtGetParent().jjtGetNumChildren() >= 2) { + && prefix.jjtGetParent().jjtGetNumChildren() >= 2) { return prefix.jjtGetParent().jjtGetChild(1).getFirstChildOfType(ASTArguments.class); } @@ -490,7 +489,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @return JavaTypeDefinition of the resolved field or null if it could not be found. */ private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class - accessingClass) { + accessingClass) { while (typeToSearch != null && typeToSearch.getType() != Object.class) { try { final Field field = typeToSearch.getType().getDeclaredField(fieldImage); @@ -502,7 +501,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } catch (final LinkageError e) { if (LOG.isLoggable(Level.WARNING)) { String message = "Error during type resolution of field '" + fieldImage + "' in " - + typeToSearch.getType() + " due to: " + e; + + typeToSearch.getType() + " due to: " + e; LOG.log(Level.WARNING, message); } // TODO : report a missing class once we start doing that... @@ -527,7 +526,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @return Type def. of the field, or null if it could not be resolved. */ private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class - accessingClass) { + accessingClass) { if (accessingClass == null) { return null; } @@ -535,7 +534,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { for (/* empty */; scope != null; scope = scope.getParent()) { // search each enclosing scope one by one for (Map.Entry> entry - : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { + : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { if (entry.getKey().getImage().equals(image)) { ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); @@ -557,8 +556,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { try { // get the superclass type def. ot the Class the ClassScope belongs to JavaTypeDefinition superClass - = getSuperClassTypeDefinition(((ClassScope) scope).getClassDeclaration().getNode(), - null); + = getSuperClassTypeDefinition(((ClassScope) scope).getClassDeclaration().getNode(), + null); // TODO: check if anonymous classes are class scope // try searching this type def. @@ -784,28 +783,17 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTUnaryExpression node, Object data) { super.visit(node, data); - switch (node.getOperator()) { - case BOOLEAN_NOT: + if (node.getOperator() == PrefixOp.NEGATION) { populateType(node, "boolean"); - break; - case BITWISE_INVERSE: + } else if (node.getOperator() == PrefixOp.COMPLEMENT) { rollupTypeUnary(node); - break; - default: + } else { rollupTypeUnaryNumericPromotion(node); - break; } return data; } - @Override - public Object visit(ASTIncrementExpression node, Object data) { - super.visit(node, data); - rollupTypeUnary(node); - return data; - } - @Override public Object visit(ASTCastExpression node, Object data) { super.visit(node, data); @@ -924,8 +912,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return (TypeNode) node; // anonymous class declaration } else if (node instanceof ASTAllocationExpression // is anonymous class declaration - && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // array cant be anonymous - && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor + && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // array cant be anonymous + && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor return (TypeNode) node; } @@ -961,8 +949,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Node previousNode = null; for (; node != null; previousNode = node, node = node.jjtGetParent()) { if (node instanceof ASTClassOrInterfaceDeclaration // class declaration - // is the class we are looking for or caller requested first class - && (((TypeNode) node).getType() == clazz || clazz == null)) { + // is the class we are looking for or caller requested first class + && (((TypeNode) node).getType() == clazz || clazz == null)) { ASTExtendsList extendsList = node.getFirstChildOfType(ASTExtendsList.class); @@ -974,9 +962,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // anonymous class declaration } else if (clazz == null // callers requested any class scope - && node instanceof ASTAllocationExpression // is anonymous class decl - && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // arrays can't be anonymous - && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor + && node instanceof ASTAllocationExpression // is anonymous class decl + && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // arrays can't be anonymous + && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor return node.getFirstChildOfType(ASTClassOrInterfaceType.class).getTypeDefinition(); } } @@ -1236,7 +1224,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Class type = ((TypeNode) child).getType(); if (type != null) { if ("byte".equals(type.getName()) || "short".equals(type.getName()) - || "char".equals(type.getName())) { + || "char".equals(type.getName())) { populateType(typeNode, "int"); } else { InternalApiBridge.setTypeDefinition(typeNode, ((TypeNode) child).getTypeDefinition()); @@ -1278,7 +1266,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // Yeah, String is not numeric, but easiest place to handle // it, only affects ASTAdditiveExpression if (type1 != null && "java.lang.String".equals(type1.getName()) - || type2 != null && "java.lang.String".equals(type2.getName())) { + || type2 != null && "java.lang.String".equals(type2.getName())) { populateType(typeNode, "java.lang.String"); } } @@ -1314,7 +1302,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // we found the class, but there is a problem with it (see https://github.com/pmd/pmd/issues/1131) if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + qualifiedName + " from on demand import, " - + "with an incomplete classpath.", e); + + "with an incomplete classpath.", e); } return; } @@ -1323,7 +1311,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (myType == null && qualifiedName != null && qualifiedName.contains(".")) { // try if the last part defines a inner class String qualifiedNameInner = qualifiedName.substring(0, qualifiedName.lastIndexOf('.')) + "$" - + qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1); + + qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1); try { myType = pmdClassLoader.loadClass(qualifiedNameInner); } catch (ClassNotFoundException ignored) { @@ -1332,7 +1320,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // we found the class, but there is a problem with it (see https://github.com/pmd/pmd/issues/1131) if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + qualifiedNameInner + " from on demand import, " - + "with an incomplete classpath.", e); + + "with an incomplete classpath.", e); } return; } @@ -1368,8 +1356,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (parent instanceof ASTTypeParameters) { // if type parameter defined in the same < > typeParameters = (ASTTypeParameters) parent; } else if (parent instanceof ASTConstructorDeclaration - || parent instanceof ASTMethodDeclaration - || parent instanceof ASTClassOrInterfaceDeclaration) { + || parent instanceof ASTMethodDeclaration + || parent instanceof ASTClassOrInterfaceDeclaration) { typeParameters = parent.getFirstChildOfType(ASTTypeParameters.class); } @@ -1409,7 +1397,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } catch (LinkageError e2) { if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + fullyQualifiedClassName + " from on demand import, " - + "with an incomplete classpath.", e2); + + "with an incomplete classpath.", e2); } return null; } @@ -1425,7 +1413,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } catch (LinkageError e) { if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + fullClassName + " from on demand import, " - + "with an incomplete classpath.", e); + + "with an incomplete classpath.", e); } } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt index bc5e50499f..423f708a7c 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt @@ -9,6 +9,7 @@ import net.sourceforge.pmd.lang.java.ast.JavaVersion.* import net.sourceforge.pmd.lang.java.ast.JavaVersion.Companion.Earliest import net.sourceforge.pmd.lang.java.ast.JavaVersion.Companion.Latest import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.UNARY_MINUS /** * @author Clément Fournier @@ -232,8 +233,8 @@ $delim it::getImage shouldBe "0b0000_0010" } - "-0X0000_000f" should matchExpr { - it::getOperator shouldBe UnaryOp.UNARY_MINUS + "-0X0000_000f" should matchExpr { + it::getOperator shouldBe UNARY_MINUS it::getOperand shouldBe number(INT) { it::getImage shouldBe "0X0000_000f" it::getValueAsInt shouldBe 15 @@ -282,8 +283,8 @@ $delim it::getImage shouldBe "12f" } - "-3_456.123_456" should matchExpr { - it::getOperator shouldBe UnaryOp.UNARY_MINUS + "-3_456.123_456" should matchExpr { + it::getOperator shouldBe UNARY_MINUS it::getOperand shouldBe number(DOUBLE) { it::getValueAsInt shouldBe 3456 @@ -361,7 +362,7 @@ $delim "0x0_0__0F" should parseAs(hex15i) "-0X0000_000f" should parseAs { - unaryExpr(UnaryOp.UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { hex15i() } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt index 954e2eddba..c7aa074a07 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt @@ -6,7 +6,7 @@ package net.sourceforge.pmd.lang.java.ast import net.sourceforge.pmd.lang.java.ast.BinaryOp.* import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx -import net.sourceforge.pmd.lang.java.ast.UnaryOp.UNARY_MINUS +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.UNARY_MINUS class ASTMultiplicativeExpressionTest : ParserTestSpec({ @@ -30,7 +30,7 @@ class ASTMultiplicativeExpressionTest : ParserTestSpec({ int(1) int(2) } - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { int(4) } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt index 9857986015..1623e15ed1 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt @@ -7,6 +7,7 @@ package net.sourceforge.pmd.lang.java.ast import net.sourceforge.pmd.lang.ast.test.shouldBe import net.sourceforge.pmd.lang.java.ast.BinaryOp.* import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.UNARY_MINUS /** @@ -145,7 +146,7 @@ class ASTSwitchExpressionTests : ParserTestSpec({ } } "-switch (day) {default -> 6;}" should parseAs { - unaryExpr(UnaryOp.UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { switchExpr() } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt index 9b0528dc5c..8608fc3acd 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt @@ -1,12 +1,13 @@ package net.sourceforge.pmd.lang.java.ast -import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType.READ import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType.WRITE +import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType import net.sourceforge.pmd.lang.java.ast.BinaryOp.ADD -import net.sourceforge.pmd.lang.java.ast.IncrementOp.* import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx -import net.sourceforge.pmd.lang.java.ast.UnaryOp.* +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp.POST_DECREMENT +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp.POST_INCREMENT +import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.* /** * Nodes that previously corresponded to ASTAllocationExpression. @@ -20,25 +21,25 @@ class ASTUnaryExpressionTest : ParserTestSpec({ inContext(ExpressionParsingCtx) { "-2" should parseAs { - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { number() } } "-2" should parseAs { - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { number() } } "-2" should parseAs { - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { number() } } "-2" should parseAs { - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { number() } } @@ -51,7 +52,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "2 + -2" should parseAs { additiveExpr(ADD) { number() - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { number() } } @@ -60,7 +61,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "2 +-2" should parseAs { additiveExpr(ADD) { number() - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { number() } } @@ -69,7 +70,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "2 + +2" should parseAs { additiveExpr(ADD) { number() - unaryExpr(UNARY_PLUS) { + prefixExpr(UNARY_PLUS) { number() } } @@ -102,7 +103,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { classType("p") - unaryExpr(BITWISE_INVERSE) { + prefixExpr(COMPLEMENT) { variableAccess("q", READ) } } @@ -112,14 +113,14 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { classType("p") - unaryExpr(BOOLEAN_NOT) { + prefixExpr(NEGATION) { variableAccess("q", READ) } } } "(p)++" should parseAs { - postfixMutation(INCREMENT) { + postfixExpr(POST_INCREMENT) { parenthesized { variableAccess("p", WRITE) } @@ -131,7 +132,16 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "i+++i" should parseAs { additiveExpr(ADD) { - postfixMutation(INCREMENT) { + postfixExpr(POST_INCREMENT) { + variableAccess("i", WRITE) + } + variableAccess("i", READ) + } + } + + "i---i" should parseAs { + additiveExpr(ADD) { + postfixExpr(POST_DECREMENT) { variableAccess("i", WRITE) } variableAccess("i", READ) @@ -150,7 +160,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - unaryExpr(UNARY_PLUS) { + prefixExpr(UNARY_PLUS) { variableAccess("q", READ) } } @@ -160,7 +170,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - unaryExpr(UNARY_MINUS) { + prefixExpr(UNARY_MINUS) { variableAccess("q", READ) } } @@ -170,7 +180,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - prefixMutation(INCREMENT) { + prefixExpr(PRE_INCREMENT) { variableAccess("q", WRITE) } } @@ -180,7 +190,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - prefixMutation(DECREMENT) { + prefixExpr(PRE_DECREMENT) { variableAccess("q", WRITE) } } @@ -198,34 +208,34 @@ class ASTUnaryExpressionTest : ParserTestSpec({ inContext(ExpressionParsingCtx) { "!!true" should parseAs { - unaryExpr(BOOLEAN_NOT) { - unaryExpr(BOOLEAN_NOT) { + prefixExpr(NEGATION) { + prefixExpr(NEGATION) { boolean(true) } } } "~~1" should parseAs { - unaryExpr(BITWISE_INVERSE) { - unaryExpr(BITWISE_INVERSE) { + prefixExpr(COMPLEMENT) { + prefixExpr(COMPLEMENT) { number() } } } "-~1" should parseAs { - unaryExpr(UNARY_MINUS) { - unaryExpr(BITWISE_INVERSE) { + prefixExpr(UNARY_MINUS) { + prefixExpr(COMPLEMENT) { number() } } } "-+-+1" should parseAs { - unaryExpr(UNARY_MINUS) { - unaryExpr(UNARY_PLUS) { - unaryExpr(UNARY_MINUS) { - unaryExpr(UNARY_PLUS) { + prefixExpr(UNARY_MINUS) { + prefixExpr(UNARY_PLUS) { + prefixExpr(UNARY_MINUS) { + prefixExpr(UNARY_PLUS) { number() } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt index 543e303c6e..0bb6e1a9a2 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt @@ -107,29 +107,19 @@ fun TreeNodeWrapper.parenthesized(depth: } -fun TreeNodeWrapper.unaryExpr(op: UnaryOp, baseExpr: TreeNodeWrapper.() -> ASTExpression): ASTExpression = - child { +fun TreeNodeWrapper.prefixExpr(op: UnaryOp.PrefixOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = + child { it::getOperator shouldBe op it::getOperand shouldBe baseExpr() } - -fun TreeNodeWrapper.postfixMutation(op: IncrementOp, baseExpr: ValuedNodeSpec) = - incrementExpr(op, isPrefix = false, baseExpr = baseExpr) - -fun TreeNodeWrapper.prefixMutation(op: IncrementOp, baseExpr: ValuedNodeSpec) = - incrementExpr(op, isPrefix = true, baseExpr = baseExpr) - -fun TreeNodeWrapper.incrementExpr(op: IncrementOp, isPrefix: Boolean, baseExpr: ValuedNodeSpec) = - child { - it::getOp shouldBe op - it::isPostfix shouldBe !isPrefix - it::isPrefix shouldBe isPrefix - it::isDecrement shouldBe (op == IncrementOp.DECREMENT) - it::isIncrement shouldBe (op == IncrementOp.INCREMENT) +fun TreeNodeWrapper.postfixExpr(op: UnaryOp.PostfixOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = + child { + it::getOperator shouldBe op it::getOperand shouldBe baseExpr() } + fun TreeNodeWrapper.typeParamList(contents: NodeSpec) = child(nodeSpec = contents) From 555c98d621e5c21ffaf9ee06c749b3f018182888 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 16 Aug 2019 13:46:46 +0200 Subject: [PATCH 2/8] Test precedence of cast --- .../lang/java/ast/ASTPostfixExpression.java | 2 +- .../pmd/lang/java/rule/AbstractJavaRule.java | 4 +++ .../lang/java/ast/ASTUnaryExpressionTest.kt | 34 ++++++++++++++----- 3 files changed, 30 insertions(+), 10 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java index 726eb90eaa..9e7f75605b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java @@ -17,7 +17,7 @@ import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp; * * */ -public final class ASTPostfixExpression extends AbstractJavaExpr implements ASTUnaryExpression { +public final class ASTPostfixExpression extends AbstractJavaExpr implements ASTUnaryExpression, LeftRecursiveNode { private PostfixOp operator; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java index 3e25375df1..fb06526815 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java @@ -36,6 +36,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTRelationalExpression; import net.sourceforge.pmd.lang.java.ast.ASTShiftExpression; import net.sourceforge.pmd.lang.java.ast.ASTTypeArgument; +import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus; import net.sourceforge.pmd.lang.java.ast.ASTWildcardBounds; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitor; @@ -139,6 +140,9 @@ public abstract class AbstractJavaRule extends AbstractRule implements JavaParse return JavaParserVisitor.super.visit(node, data); } + public Object visit(ASTUnaryExpression node, Object data) { + return visit((ASTExpression) node, data); + } @Deprecated public Object visit(ASTConditionalOrExpression node, Object data) { diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt index 8608fc3acd..67421991ea 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt @@ -4,17 +4,12 @@ import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType.READ import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.AccessType.WRITE import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType import net.sourceforge.pmd.lang.java.ast.BinaryOp.ADD +import net.sourceforge.pmd.lang.java.ast.BinaryOp.SUB import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp.POST_DECREMENT import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp.POST_INCREMENT import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.* -/** - * Nodes that previously corresponded to ASTAllocationExpression. - * - * @author Clément Fournier - * @since 7.0.0 - */ class ASTUnaryExpressionTest : ParserTestSpec({ parserTest("Simple unary expressions") { @@ -76,6 +71,27 @@ class ASTUnaryExpressionTest : ParserTestSpec({ } } + "+(int)-a" should parseAs { + prefixExpr(UNARY_PLUS) { + castExpr { + primitiveType(PrimitiveType.INT) + prefixExpr(UNARY_MINUS) { + variableAccess("a") + } + } + } + } + "+-(int)a" should parseAs { + prefixExpr(UNARY_PLUS) { + prefixExpr(UNARY_MINUS) { + castExpr { + primitiveType(PrimitiveType.INT) + variableAccess("a") + } + } + } + } + "2 ++ 2" shouldNot parse() "2 -- 2" shouldNot parse() } @@ -140,7 +156,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ } "i---i" should parseAs { - additiveExpr(ADD) { + additiveExpr(SUB) { postfixExpr(POST_DECREMENT) { variableAccess("i", WRITE) } @@ -148,8 +164,8 @@ class ASTUnaryExpressionTest : ParserTestSpec({ } } - // "++i++" doesn't compile so don't test it - + // "++i++" parses, but doesn't compile, so don't test it + // same for eg "p+++++q" (which doesn't parse) PrimitiveType .values() From 8744fa953df1055f8d4697da5caaecd9c61da45f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 26 Sep 2019 07:20:54 +0200 Subject: [PATCH 3/8] Checkstyle --- .../pmd/lang/java/typeresolution/ClassTypeResolver.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index e188186967..1956c0aab3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -292,7 +292,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // First try: com.package.SomeClass.staticField.otherField // Second try: com.package.SomeClass.staticField // Third try: com.package.SomeClass <- found a class! - for (String reducedImage = node.getImage(); ; ) { + for (String reducedImage = node.getImage(); ;) { populateType(node, reducedImage); if (node.getType() != null) { break; // we found a class! From 928be5de6fc18d9d2970b5fc17c8f93f774b0032 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 19 Nov 2019 16:28:39 +0100 Subject: [PATCH 4/8] Fix style --- .../lang/java/ast/ASTPrefixExpression.java | 4 -- .../pmd/lang/java/ast/ASTUnaryExpression.java | 8 +--- .../typeresolution/ClassTypeResolver.java | 47 +++++++++---------- .../lang/java/ast/ASTLambdaExpressionTest.kt | 4 -- 4 files changed, 25 insertions(+), 38 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java index e0fcde67f2..8fd0c42225 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java @@ -2,10 +2,6 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - package net.sourceforge.pmd.lang.java.ast; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java index 4f4bd42b72..2f9b05d4f1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java @@ -18,17 +18,13 @@ package net.sourceforge.pmd.lang.java.ast; */ public interface ASTUnaryExpression extends ASTExpression { - /** - * Returns the expression nested within this expression. - */ + /** Returns the expression nested within this expression. */ default ASTExpression getOperand() { return (ASTExpression) jjtGetChild(0); } - /** - * Returns the constant representing the operator. - */ + /** Returns the constant representing the operator. */ UnaryOp getOperator(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 1956c0aab3..0f20988cd0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -110,11 +110,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { private static final Map> PRIMITIVE_TYPES; private static final Map JAVA_LANG; - private final PMDASMClassLoader pmdClassLoader; private Map staticFieldImageToTypeDef; private Map> staticNamesToClasses; private List importOnDemandStaticClasses; private ASTCompilationUnit currentAcu; + + private final PMDASMClassLoader pmdClassLoader; private Map importedClasses; private List importedOnDemand; @@ -369,8 +370,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // TODO: remove this if branch, it's only purpose is to make JUnitAssertionsShouldIncludeMessage's tests pass // as the code is not compiled there and symbol table works on uncompiled code if (node.getNameDeclaration() != null - && previousType == null // if it's not null, then let other code handle things - && node.getNameDeclaration().getNode() instanceof TypeNode) { + && previousType == null // if it's not null, then let other code handle things + && node.getNameDeclaration().getNode() instanceof TypeNode) { // Carry over the type (including generics) from the declaration JavaTypeDefinition nodeType = ((TypeNode) node.getNameDeclaration().getNode()).getTypeDefinition(); if (nodeType != null) { @@ -470,7 +471,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Node prefix = node.jjtGetParent(); if (prefix instanceof ASTPrimaryPrefix - && prefix.jjtGetParent().jjtGetNumChildren() >= 2) { + && prefix.jjtGetParent().jjtGetNumChildren() >= 2) { return prefix.jjtGetParent().jjtGetChild(1).getFirstChildOfType(ASTArguments.class); } @@ -488,8 +489,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * * @return JavaTypeDefinition of the resolved field or null if it could not be found. */ - private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class - accessingClass) { + private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class accessingClass) { while (typeToSearch != null && typeToSearch.getType() != Object.class) { try { final Field field = typeToSearch.getType().getDeclaredField(fieldImage); @@ -501,7 +501,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } catch (final LinkageError e) { if (LOG.isLoggable(Level.WARNING)) { String message = "Error during type resolution of field '" + fieldImage + "' in " - + typeToSearch.getType() + " due to: " + e; + + typeToSearch.getType() + " due to: " + e; LOG.log(Level.WARNING, message); } // TODO : report a missing class once we start doing that... @@ -525,8 +525,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * * @return Type def. of the field, or null if it could not be resolved. */ - private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class - accessingClass) { + private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class accessingClass) { if (accessingClass == null) { return null; } @@ -912,8 +911,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { return (TypeNode) node; // anonymous class declaration } else if (node instanceof ASTAllocationExpression // is anonymous class declaration - && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // array cant be anonymous - && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor + && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // array cant be anonymous + && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor return (TypeNode) node; } @@ -949,8 +948,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Node previousNode = null; for (; node != null; previousNode = node, node = node.jjtGetParent()) { if (node instanceof ASTClassOrInterfaceDeclaration // class declaration - // is the class we are looking for or caller requested first class - && (((TypeNode) node).getType() == clazz || clazz == null)) { + // is the class we are looking for or caller requested first class + && (((TypeNode) node).getType() == clazz || clazz == null)) { ASTExtendsList extendsList = node.getFirstChildOfType(ASTExtendsList.class); @@ -962,9 +961,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // anonymous class declaration } else if (clazz == null // callers requested any class scope - && node instanceof ASTAllocationExpression // is anonymous class decl - && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // arrays can't be anonymous - && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor + && node instanceof ASTAllocationExpression // is anonymous class decl + && node.getFirstChildOfType(ASTArrayDimsAndInits.class) == null // arrays can't be anonymous + && !(previousNode instanceof ASTArguments)) { // we might come out of the constructor return node.getFirstChildOfType(ASTClassOrInterfaceType.class).getTypeDefinition(); } } @@ -1266,7 +1265,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // Yeah, String is not numeric, but easiest place to handle // it, only affects ASTAdditiveExpression if (type1 != null && "java.lang.String".equals(type1.getName()) - || type2 != null && "java.lang.String".equals(type2.getName())) { + || type2 != null && "java.lang.String".equals(type2.getName())) { populateType(typeNode, "java.lang.String"); } } @@ -1302,7 +1301,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // we found the class, but there is a problem with it (see https://github.com/pmd/pmd/issues/1131) if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + qualifiedName + " from on demand import, " - + "with an incomplete classpath.", e); + + "with an incomplete classpath.", e); } return; } @@ -1311,7 +1310,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (myType == null && qualifiedName != null && qualifiedName.contains(".")) { // try if the last part defines a inner class String qualifiedNameInner = qualifiedName.substring(0, qualifiedName.lastIndexOf('.')) + "$" - + qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1); + + qualifiedName.substring(qualifiedName.lastIndexOf('.') + 1); try { myType = pmdClassLoader.loadClass(qualifiedNameInner); } catch (ClassNotFoundException ignored) { @@ -1320,7 +1319,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // we found the class, but there is a problem with it (see https://github.com/pmd/pmd/issues/1131) if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + qualifiedNameInner + " from on demand import, " - + "with an incomplete classpath.", e); + + "with an incomplete classpath.", e); } return; } @@ -1356,8 +1355,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { if (parent instanceof ASTTypeParameters) { // if type parameter defined in the same < > typeParameters = (ASTTypeParameters) parent; } else if (parent instanceof ASTConstructorDeclaration - || parent instanceof ASTMethodDeclaration - || parent instanceof ASTClassOrInterfaceDeclaration) { + || parent instanceof ASTMethodDeclaration + || parent instanceof ASTClassOrInterfaceDeclaration) { typeParameters = parent.getFirstChildOfType(ASTTypeParameters.class); } @@ -1397,7 +1396,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } catch (LinkageError e2) { if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + fullyQualifiedClassName + " from on demand import, " - + "with an incomplete classpath.", e2); + + "with an incomplete classpath.", e2); } return null; } @@ -1413,7 +1412,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { } catch (LinkageError e) { if (LOG.isLoggable(Level.FINE)) { LOG.log(Level.FINE, "Tried to load class " + fullClassName + " from on demand import, " - + "with an incomplete classpath.", e); + + "with an incomplete classpath.", e); } } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLambdaExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLambdaExpressionTest.kt index 7505a8787d..f1a7d2dbfc 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLambdaExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLambdaExpressionTest.kt @@ -2,10 +2,6 @@ * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - package net.sourceforge.pmd.lang.java.ast import net.sourceforge.pmd.lang.ast.test.shouldBe From 76439a47f26dd5624b68d7aab717795c7acbaca5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 9 Dec 2019 14:54:05 +0100 Subject: [PATCH 5/8] Use a single node for unary exprs --- pmd-java/etc/grammar/Java.jjt | 16 +- .../pmd/lang/java/ast/ASTExpression.java | 4 +- .../lang/java/ast/ASTPostfixExpression.java | 51 ------ .../lang/java/ast/ASTPrefixExpression.java | 53 ------- .../pmd/lang/java/ast/ASTUnaryExpression.java | 59 +++++-- .../java/ast/JavaParserVisitorAdapter.java | 12 +- .../pmd/lang/java/ast/UnaryOp.java | 145 +++++++----------- .../java/symboltable/JavaNameOccurrence.java | 14 +- .../typeresolution/ClassTypeResolver.java | 6 +- .../pmd/lang/java/ast/ASTLiteralTest.kt | 6 +- .../pmd/lang/java/ast/TestExtensions.kt | 8 +- 11 files changed, 134 insertions(+), 240 deletions(-) delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java delete mode 100644 pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java diff --git a/pmd-java/etc/grammar/Java.jjt b/pmd-java/etc/grammar/Java.jjt index fea265803d..2f155de407 100644 --- a/pmd-java/etc/grammar/Java.jjt +++ b/pmd-java/etc/grammar/Java.jjt @@ -2505,16 +2505,16 @@ void MultiplicativeExpression() #void: void UnaryExpression() #void: {} { - (("+" {jjtThis.setOp(UnaryOp.PrefixOp.UNARY_PLUS);} | "-" {jjtThis.setOp(UnaryOp.PrefixOp.UNARY_MINUS);}) UnaryExpression()) #PrefixExpression + (("+" {jjtThis.setOp(UnaryOp.UNARY_PLUS);} | "-" {jjtThis.setOp(UnaryOp.UNARY_MINUS);}) UnaryExpression()) #UnaryExpression | PrefixIncrementExpression() | UnaryExpressionNotPlusMinus() } -void PrefixIncrementExpression() #PrefixExpression: +void PrefixIncrementExpression() #UnaryExpression: {} { - ("++" {jjtThis.setOp(UnaryOp.PrefixOp.PRE_INCREMENT);} - | "--" {jjtThis.setOp(UnaryOp.PrefixOp.PRE_DECREMENT);} + ("++" {jjtThis.setOp(UnaryOp.PRE_INCREMENT);} + | "--" {jjtThis.setOp(UnaryOp.PRE_DECREMENT);} ) PrimaryExpression() } @@ -2522,7 +2522,7 @@ void PrefixIncrementExpression() #PrefixExpression: void UnaryExpressionNotPlusMinus() #void: {} { - (( "~" {jjtThis.setOp(UnaryOp.PrefixOp.COMPLEMENT);} | "!" {jjtThis.setOp(UnaryOp.PrefixOp.NEGATION);}) UnaryExpression()) #PrefixExpression + (( "~" {jjtThis.setOp(UnaryOp.COMPLEMENT);} | "!" {jjtThis.setOp(UnaryOp.NEGATION);}) UnaryExpression()) #UnaryExpression // There's no separate production for CastExpression, because that would force // us to repeat the lookahead // Those lookaheads are quite expensive, they're run to disambiguate between @@ -2554,9 +2554,9 @@ void PostfixExpression() #void: PrimaryExpression() [ LOOKAHEAD(1) - ("++" {jjtThis.setOp(UnaryOp.PostfixOp.POST_INCREMENT);} - | "--" {jjtThis.setOp(UnaryOp.PostfixOp.POST_DECREMENT);} - ) #PostfixExpression(1) + ("++" {jjtThis.setOp(UnaryOp.POST_INCREMENT);} + | "--" {jjtThis.setOp(UnaryOp.POST_DECREMENT);} + ) #UnaryExpression(1) ] } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java index 390a55b048..72ff4467c9 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTExpression.java @@ -27,8 +27,8 @@ package net.sourceforge.pmd.lang.java.ast; * | {@link ASTAssignmentExpression AssignmentExpression} * | {@link ASTConditionalExpression ConditionalExpression} * | {@link ASTInfixExpression InfixExpression} - * | {@link ASTPrefixExpression PrefixExpression} | {@link ASTCastExpression CastExpression} - * | {@link ASTPostfixExpression PostfixExpression} + * | {@link ASTUnaryExpression PrefixExpression} | {@link ASTCastExpression CastExpression} + * | {@link ASTUnaryExpression PostfixExpression} * | {@link ASTSwitchExpression SwitchExpression} * | {@link ASTPrimaryExpression PrimaryExpression} * diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java deleted file mode 100644 index 9e7f75605b..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPostfixExpression.java +++ /dev/null @@ -1,51 +0,0 @@ -/* - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - - -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp; - -/** - * Represents a unary postfix operation on a value. - * This has a precedence greater than {@link ASTCastExpression CastExpression}. - * - * 
- *
- * PostfixExpression ::= {@link ASTPrimaryExpression PrimaryExpression} ({@link PostfixOp "++" | "--"})
- *
- *
- */ -public final class ASTPostfixExpression extends AbstractJavaExpr implements ASTUnaryExpression, LeftRecursiveNode { - - private PostfixOp operator; - - ASTPostfixExpression(int id) { - super(id); - } - - ASTPostfixExpression(JavaParser p, int id) { - super(p, id); - } - - @Override - public Object jjtAccept(JavaParserVisitor visitor, Object data) { - return visitor.visit(this, data); - } - - - @Override - public void jjtAccept(SideEffectingVisitor visitor, T data) { - visitor.visit(this, data); - } - - @Override - public PostfixOp getOperator() { - return operator; - } - - void setOp(PostfixOp op) { - this.operator = op; - } -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java deleted file mode 100644 index 8fd0c42225..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTPrefixExpression.java +++ /dev/null @@ -1,53 +0,0 @@ -/* - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.ast; - - -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp; - -/** - * Represents a unary prefix operation on a value. - * This has a precedence greater than {@link ASTInfixExpression}. - * - *

Prefix expressions have the same precedence as {@linkplain ASTCastExpression CastExpression}. - * - * 

- *
- * PrefixExpression ::= {@link PrefixOp} UnaryExpression
- *
- *
- */ -public final class ASTPrefixExpression extends AbstractJavaExpr implements ASTUnaryExpression { - - private PrefixOp operator; - - ASTPrefixExpression(int id) { - super(id); - } - - ASTPrefixExpression(JavaParser p, int id) { - super(p, id); - } - - @Override - public Object jjtAccept(JavaParserVisitor visitor, Object data) { - return visitor.visit(this, data); - } - - - @Override - public void jjtAccept(SideEffectingVisitor visitor, T data) { - visitor.visit(this, data); - } - - @Override - public PrefixOp getOperator() { - return operator; - } - - void setOp(PrefixOp op) { - this.operator = op; - } -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java index 2f9b05d4f1..13e84b8bf6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java @@ -7,25 +7,66 @@ package net.sourceforge.pmd.lang.java.ast; /** * Represents a unary operation on a value. The syntactic form may be - * prefix or postfix, which are represented with different nodes. + * prefix or postfix, which are represented with the same nodes, even + * though they have different precedences. * * 
  *
- * UnaryExpression ::= {@link ASTPrefixExpression PrefixExpression}
- *                   | {@link ASTPostfixExpression PostfixExpression}
+ * UnaryExpression ::= PrefixExpression | PostfixExpression
+ *
+ * PrefixExpression  ::= {@link UnaryOp PrefixOp} UnaryExpression
+ *
+ * PostfixExpression ::= {@link ASTExpression Expression} ({@link UnaryOp PostfixOp})
  *
  *
*/ -public interface ASTUnaryExpression extends ASTExpression { +public class ASTUnaryExpression extends AbstractJavaExpr implements LeftRecursiveNode { - /** Returns the expression nested within this expression. */ - default ASTExpression getOperand() { - return (ASTExpression) jjtGetChild(0); + private UnaryOp operator; + + ASTUnaryExpression(int id) { + super(id); + } + + ASTUnaryExpression(JavaParser p, int id) { + super(p, id); } - /** Returns the constant representing the operator. */ - UnaryOp getOperator(); + @Override + public Object jjtAccept(JavaParserVisitor visitor, Object data) { + return visitor.visit(this, data); + } + @Override + public void jjtAccept(SideEffectingVisitor visitor, T data) { + visitor.visit(this, data); + } + + + /** Returns the expression nested within this expression. */ + public ASTExpression getOperand() { + return (ASTExpression) jjtGetChild(0); + } + + /** Returns the constant representing the operator. */ + public UnaryOp getOperator() { + return operator; + } + + /** + * Returns true if this is a prefix expression. + * + * @deprecated XPath-attribute only, use {@code getOperator().isPrefix()} in java code. + */ + @Deprecated + public boolean isPrefix() { + return getOperator().isPrefix(); + } + + void setOp(UnaryOp op) { + this.operator = op; + } + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java index a5ea15b344..7bb9ee6057 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java @@ -106,21 +106,11 @@ public class JavaParserVisitorAdapter implements JavaParserVisitor { return visit((ASTExpression) node, data); } - + @Override public Object visit(ASTUnaryExpression node, Object data) { return visit((ASTExpression) node, data); } - @Override - public Object visit(ASTPrefixExpression node, Object data) { - return visit((ASTUnaryExpression) node, data); - } - - @Override - public Object visit(ASTPostfixExpression node, Object data) { - return visit((ASTUnaryExpression) node, data); - } - @Override public Object visit(ASTCastExpression node, Object data) { return visit((ASTExpression) node, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java index 5ac2f19b26..f80ef6d872 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java @@ -10,12 +10,40 @@ package net.sourceforge.pmd.lang.java.ast; * * 
  *
- * UnaryOp ::= {@link PrefixOp} | {@link PostfixOp}
+ * UnaryOp ::= PrefixOp | PostfixOp
  *
- *
+ * PrefixOp ::= "+" | "-" | "~" | "!" | "++" | "--" * + * PostfixOp ::= "++" | "--" + * + * */ -public interface UnaryOp extends InternalInterfaces.OperatorLike { +public enum UnaryOp implements InternalInterfaces.OperatorLike { + /** Unary numeric promotion operator {@code "+"}. */ + UNARY_PLUS("+"), + /** Arithmetic negation operation {@code "-"}. */ + UNARY_MINUS("-"), + /** Bitwise complement operator {@code "~"}. */ + COMPLEMENT("~"), + /** Logical complement operator {@code "!"}. */ + NEGATION("!"), + + /** Prefix increment operator {@code "++"}. */ + PRE_INCREMENT("++"), + /** Prefix decrement operator {@code "--"}. */ + PRE_DECREMENT("--"), + + /** Postfix increment operator {@code "++"}. */ + POST_INCREMENT("++"), + /** Postfix decrement operator {@code "--"}. */ + POST_DECREMENT("--"); + + + private final String code; + + UnaryOp(String code) { + this.code = code; + } /** * Returns true if this operator is pure, ie the evaluation of @@ -30,96 +58,29 @@ public interface UnaryOp extends InternalInterfaces.OperatorLike { * * TODO update example for node streams */ - boolean isPure(); - - - /** - * A prefix operator for {@link ASTPrefixExpression}. - * - * 
-     *
-     * PrefixOp ::= "+" | "-" | "~" | "!" | "++" | "--"
-     *
-     *
- * - * @see BinaryOp - * @see AssignmentOp - */ - enum PrefixOp implements UnaryOp { - /** Unary numeric promotion operator {@code "+"}. */ - UNARY_PLUS("+"), - /** Arithmetic negation operation {@code "-"}. */ - UNARY_MINUS("-"), - /** Bitwise complement operator {@code "~"}. */ - COMPLEMENT("~"), - /** Logical complement operator {@code "!"}. */ - NEGATION("!"), - - /** Prefix increment operator {@code "++"}. */ - PRE_INCREMENT("++"), - /** Prefix decrement operator {@code "--"}. */ - PRE_DECREMENT("--"); - - private final String code; - - PrefixOp(String code) { - this.code = code; - } - - @Override - public boolean isPure() { - return this != PRE_INCREMENT && this != PRE_DECREMENT; - } - - @Override - public String getToken() { - return code; - } - - @Override - public String toString() { - return this.code; - } - + public boolean isPure() { + return this.ordinal() < PRE_INCREMENT.ordinal(); } - /** - * A postfix operator for {@link ASTPostfixExpression}. - * - * 
-     *
-     * PostfixOp ::= "++" | "--"
-     *
-     *
- * - * @see BinaryOp - * @see AssignmentOp - */ - enum PostfixOp implements UnaryOp { - /** Postfix increment operator {@code "++"}. */ - POST_INCREMENT("++"), - /** Postfix decrement operator {@code "--"}. */ - POST_DECREMENT("--"); - - private final String code; - - PostfixOp(String code) { - this.code = code; - } - - @Override - public boolean isPure() { - return false; - } - - @Override - public String getToken() { - return code; - } - - @Override - public String toString() { - return this.code; - } + /** Returns true if this is a prefix operator. */ + public boolean isPrefix() { + return this.ordinal() < POST_INCREMENT.ordinal(); } + + /** Returns true if this is a postfix operator. */ + public boolean isPostfix() { + return !isPrefix(); + } + + + @Override + public String getToken() { + return code; + } + + @Override + public String toString() { + return this.code; + } + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java index 1825b97b78..5986ef9743 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symboltable/JavaNameOccurrence.java @@ -9,7 +9,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTMethodReference; import net.sourceforge.pmd.lang.java.ast.ASTName; -import net.sourceforge.pmd.lang.java.ast.ASTPostfixExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; import net.sourceforge.pmd.lang.java.ast.ASTResource; @@ -121,13 +120,20 @@ public class JavaNameOccurrence implements NameOccurrence { } private boolean isStandAlonePostfix(Node primaryExpression) { - if (!(primaryExpression instanceof ASTPostfixExpression && !((ASTPostfixExpression) primaryExpression).getOperator().isPure()) - || !(primaryExpression.jjtGetParent() instanceof ASTStatementExpression)) { + if (!(primaryExpression instanceof ASTUnaryExpression)) { + return false; + } + + ASTUnaryExpression unaryExpr = (ASTUnaryExpression) primaryExpression; + + if (unaryExpr.getOperator().isPure() + || unaryExpr.getOperator().isPrefix() + || !(primaryExpression.jjtGetParent() instanceof ASTStatementExpression)) { return false; } ASTPrimaryPrefix pf = (ASTPrimaryPrefix) ((ASTPrimaryExpression) primaryExpression.jjtGetChild(0)) - .jjtGetChild(0); + .jjtGetChild(0); if (pf.usesThisModifier()) { return true; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index 0f20988cd0..bebba753b1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -88,7 +88,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTYieldStatement; import net.sourceforge.pmd.lang.java.ast.InternalApiBridge; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; import net.sourceforge.pmd.lang.java.ast.TypeNode; -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp; +import net.sourceforge.pmd.lang.java.ast.UnaryOp; import net.sourceforge.pmd.lang.java.symboltable.ClassScope; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; import net.sourceforge.pmd.lang.java.typeresolution.typedefinition.JavaTypeDefinition; @@ -782,9 +782,9 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { public Object visit(ASTUnaryExpression node, Object data) { super.visit(node, data); - if (node.getOperator() == PrefixOp.NEGATION) { + if (node.getOperator() == UnaryOp.NEGATION) { populateType(node, "boolean"); - } else if (node.getOperator() == PrefixOp.COMPLEMENT) { + } else if (node.getOperator() == UnaryOp.COMPLEMENT) { rollupTypeUnary(node); } else { rollupTypeUnaryNumericPromotion(node); diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt index 423f708a7c..be60428fd8 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt @@ -9,7 +9,7 @@ import net.sourceforge.pmd.lang.java.ast.JavaVersion.* import net.sourceforge.pmd.lang.java.ast.JavaVersion.Companion.Earliest import net.sourceforge.pmd.lang.java.ast.JavaVersion.Companion.Latest import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.UNARY_MINUS +import net.sourceforge.pmd.lang.java.ast.UnaryOp.UNARY_MINUS /** * @author Clément Fournier @@ -233,7 +233,7 @@ $delim it::getImage shouldBe "0b0000_0010" } - "-0X0000_000f" should matchExpr { + "-0X0000_000f" should matchExpr { it::getOperator shouldBe UNARY_MINUS it::getOperand shouldBe number(INT) { it::getImage shouldBe "0X0000_000f" @@ -283,7 +283,7 @@ $delim it::getImage shouldBe "12f" } - "-3_456.123_456" should matchExpr { + "-3_456.123_456" should matchExpr { it::getOperator shouldBe UNARY_MINUS it::getOperand shouldBe number(DOUBLE) { diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt index bf7fdeb6a6..ec7a3c681c 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt @@ -123,14 +123,14 @@ fun TreeNodeWrapper.methodCall(inside: NodeSpec) = } -fun TreeNodeWrapper.prefixExpr(op: UnaryOp.PrefixOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = - child { +fun TreeNodeWrapper.prefixExpr(op: UnaryOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = + child { it::getOperator shouldBe op it::getOperand shouldBe baseExpr() } -fun TreeNodeWrapper.postfixExpr(op: UnaryOp.PostfixOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = - child { +fun TreeNodeWrapper.postfixExpr(op: UnaryOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = + child { it::getOperator shouldBe op it::getOperand shouldBe baseExpr() } From 03e9b64f10e2c7388faf2ace6a929de2e8604c5d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Mon, 9 Dec 2019 15:13:26 +0100 Subject: [PATCH 6/8] Remove whitespace changes --- .../pmd/lang/java/ast/ASTUnaryExpression.java | 11 ++-- .../java/ast/JavaParserVisitorAdapter.java | 1 + .../pmd/lang/java/ast/UnaryOp.java | 3 + .../pmd/lang/java/rule/AbstractJavaRule.java | 4 -- .../typeresolution/ClassTypeResolver.java | 28 ++++---- .../pmd/lang/java/ast/ASTLiteralTest.kt | 2 +- .../ast/ASTMultiplicativeExpressionTest.kt | 4 +- .../lang/java/ast/ASTSwitchExpressionTests.kt | 2 +- .../lang/java/ast/ASTUnaryExpressionTest.kt | 64 +++++++++---------- .../pmd/lang/java/ast/TestExtensions.kt | 8 +-- 10 files changed, 61 insertions(+), 66 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java index 13e84b8bf6..e740f72f8f 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpression.java @@ -20,7 +20,7 @@ package net.sourceforge.pmd.lang.java.ast; * * */ -public class ASTUnaryExpression extends AbstractJavaExpr implements LeftRecursiveNode { +public final class ASTUnaryExpression extends AbstractJavaExpr implements LeftRecursiveNode { private UnaryOp operator; @@ -50,10 +50,6 @@ public class ASTUnaryExpression extends AbstractJavaExpr implements LeftRecursiv return (ASTExpression) jjtGetChild(0); } - /** Returns the constant representing the operator. */ - public UnaryOp getOperator() { - return operator; - } /** * Returns true if this is a prefix expression. @@ -65,6 +61,11 @@ public class ASTUnaryExpression extends AbstractJavaExpr implements LeftRecursiv return getOperator().isPrefix(); } + /** Returns the constant representing the operator of this expression. */ + public UnaryOp getOperator() { + return operator; + } + void setOp(UnaryOp op) { this.operator = op; } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java index 7bb9ee6057..0361dfbf08 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/JavaParserVisitorAdapter.java @@ -106,6 +106,7 @@ public class JavaParserVisitorAdapter implements JavaParserVisitor { return visit((ASTExpression) node, data); } + @Override public Object visit(ASTUnaryExpression node, Object data) { return visit((ASTExpression) node, data); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java index f80ef6d872..d6156bd929 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java @@ -17,6 +17,9 @@ package net.sourceforge.pmd.lang.java.ast; * PostfixOp ::= "++" | "--" * * + * + * @see BinaryOp + * @see AssignmentOp */ public enum UnaryOp implements InternalInterfaces.OperatorLike { /** Unary numeric promotion operator {@code "+"}. */ diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java index 43c21f2049..5e5935f16c 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/AbstractJavaRule.java @@ -36,7 +36,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTRelationalExpression; import net.sourceforge.pmd.lang.java.ast.ASTShiftExpression; import net.sourceforge.pmd.lang.java.ast.ASTTypeArgument; -import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus; import net.sourceforge.pmd.lang.java.ast.ASTWildcardBounds; import net.sourceforge.pmd.lang.java.ast.JavaParserVisitor; @@ -126,9 +125,6 @@ public abstract class AbstractJavaRule extends AbstractRule implements JavaParse return JavaParserVisitor.super.visit(node, data); } - public Object visit(ASTUnaryExpression node, Object data) { - return visit((ASTExpression) node, data); - } // REMOVE ME // deprecated stuff kept for compatibility with existing visitors, not matched by anything diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java index bebba753b1..3be0da41d7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/ClassTypeResolver.java @@ -110,16 +110,12 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { private static final Map> PRIMITIVE_TYPES; private static final Map JAVA_LANG; + private Map staticFieldImageToTypeDef; private Map> staticNamesToClasses; private List importOnDemandStaticClasses; private ASTCompilationUnit currentAcu; - private final PMDASMClassLoader pmdClassLoader; - private Map importedClasses; - private List importedOnDemand; - - static { // Note: Assumption here that primitives come from same parent // ClassLoader regardless of what ClassLoader we are passed @@ -173,6 +169,10 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { JAVA_LANG = Collections.unmodifiableMap(theJavaLang); } + private final PMDASMClassLoader pmdClassLoader; + private Map importedClasses; + private List importedOnDemand; + public ClassTypeResolver() { this(ClassTypeResolver.class.getClassLoader()); @@ -283,7 +283,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * @param node * * @return The index in the array produced by splitting the node's name by '.', which is not part of the - * class name found. Example: com.package.SomeClass.staticField.otherField, return would be 3 + * class name found. Example: com.package.SomeClass.staticField.otherField, return would be 3 */ private int searchNodeNameForClass(TypeNode node) { // this is the index from which field/method names start in the dotSplitImage array @@ -293,7 +293,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { // First try: com.package.SomeClass.staticField.otherField // Second try: com.package.SomeClass.staticField // Third try: com.package.SomeClass <- found a class! - for (String reducedImage = node.getImage(); ;) { + for (String reducedImage = node.getImage();;) { populateType(node, reducedImage); if (node.getType() != null) { break; // we found a class! @@ -489,7 +489,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * * @return JavaTypeDefinition of the resolved field or null if it could not be found. */ - private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class accessingClass) { + private JavaTypeDefinition getFieldType(JavaTypeDefinition typeToSearch, String fieldImage, Class + accessingClass) { while (typeToSearch != null && typeToSearch.getType() != Object.class) { try { final Field field = typeToSearch.getType().getDeclaredField(fieldImage); @@ -525,7 +526,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { * * @return Type def. of the field, or null if it could not be resolved. */ - private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class accessingClass) { + private JavaTypeDefinition getTypeDefinitionOfVariableFromScope(Scope scope, String image, Class + accessingClass) { if (accessingClass == null) { return null; } @@ -533,7 +535,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { for (/* empty */; scope != null; scope = scope.getParent()) { // search each enclosing scope one by one for (Map.Entry> entry - : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { + : scope.getDeclarations(VariableNameDeclaration.class).entrySet()) { if (entry.getKey().getImage().equals(image)) { ASTType typeNode = entry.getKey().getDeclaratorId().getTypeNode(); @@ -555,8 +557,8 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { try { // get the superclass type def. ot the Class the ClassScope belongs to JavaTypeDefinition superClass - = getSuperClassTypeDefinition(((ClassScope) scope).getClassDeclaration().getNode(), - null); + = getSuperClassTypeDefinition(((ClassScope) scope).getClassDeclaration().getNode(), + null); // TODO: check if anonymous classes are class scope // try searching this type def. @@ -1223,7 +1225,7 @@ public class ClassTypeResolver extends JavaParserVisitorAdapter { Class type = ((TypeNode) child).getType(); if (type != null) { if ("byte".equals(type.getName()) || "short".equals(type.getName()) - || "char".equals(type.getName())) { + || "char".equals(type.getName())) { populateType(typeNode, "int"); } else { InternalApiBridge.setTypeDefinition(typeNode, ((TypeNode) child).getTypeDefinition()); diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt index be60428fd8..f79f70cb99 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTLiteralTest.kt @@ -362,7 +362,7 @@ $delim "0x0_0__0F" should parseAs(hex15i) "-0X0000_000f" should parseAs { - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { hex15i() } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt index c7aa074a07..954e2eddba 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTMultiplicativeExpressionTest.kt @@ -6,7 +6,7 @@ package net.sourceforge.pmd.lang.java.ast import net.sourceforge.pmd.lang.java.ast.BinaryOp.* import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.UNARY_MINUS +import net.sourceforge.pmd.lang.java.ast.UnaryOp.UNARY_MINUS class ASTMultiplicativeExpressionTest : ParserTestSpec({ @@ -30,7 +30,7 @@ class ASTMultiplicativeExpressionTest : ParserTestSpec({ int(1) int(2) } - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { int(4) } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt index 1623e15ed1..5328e14e86 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt @@ -146,7 +146,7 @@ class ASTSwitchExpressionTests : ParserTestSpec({ } } "-switch (day) {default -> 6;}" should parseAs { - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { switchExpr() } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt index 67421991ea..5900063c87 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTUnaryExpressionTest.kt @@ -6,9 +6,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType.PrimitiveType import net.sourceforge.pmd.lang.java.ast.BinaryOp.ADD import net.sourceforge.pmd.lang.java.ast.BinaryOp.SUB import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp.POST_DECREMENT -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PostfixOp.POST_INCREMENT -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.* +import net.sourceforge.pmd.lang.java.ast.UnaryOp.* class ASTUnaryExpressionTest : ParserTestSpec({ @@ -16,25 +14,25 @@ class ASTUnaryExpressionTest : ParserTestSpec({ inContext(ExpressionParsingCtx) { "-2" should parseAs { - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { number() } } "-2" should parseAs { - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { number() } } "-2" should parseAs { - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { number() } } "-2" should parseAs { - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { number() } } @@ -47,7 +45,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "2 + -2" should parseAs { additiveExpr(ADD) { number() - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { number() } } @@ -56,7 +54,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "2 +-2" should parseAs { additiveExpr(ADD) { number() - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { number() } } @@ -65,25 +63,25 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "2 + +2" should parseAs { additiveExpr(ADD) { number() - prefixExpr(UNARY_PLUS) { + unaryExpr(UNARY_PLUS) { number() } } } "+(int)-a" should parseAs { - prefixExpr(UNARY_PLUS) { + unaryExpr(UNARY_PLUS) { castExpr { primitiveType(PrimitiveType.INT) - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { variableAccess("a") } } } } "+-(int)a" should parseAs { - prefixExpr(UNARY_PLUS) { - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_PLUS) { + unaryExpr(UNARY_MINUS) { castExpr { primitiveType(PrimitiveType.INT) variableAccess("a") @@ -119,7 +117,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { classType("p") - prefixExpr(COMPLEMENT) { + unaryExpr(COMPLEMENT) { variableAccess("q", READ) } } @@ -129,14 +127,14 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { classType("p") - prefixExpr(NEGATION) { + unaryExpr(NEGATION) { variableAccess("q", READ) } } } "(p)++" should parseAs { - postfixExpr(POST_INCREMENT) { + unaryExpr(POST_INCREMENT) { parenthesized { variableAccess("p", WRITE) } @@ -148,7 +146,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "i+++i" should parseAs { additiveExpr(ADD) { - postfixExpr(POST_INCREMENT) { + unaryExpr(POST_INCREMENT) { variableAccess("i", WRITE) } variableAccess("i", READ) @@ -157,7 +155,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ "i---i" should parseAs { additiveExpr(SUB) { - postfixExpr(POST_DECREMENT) { + unaryExpr(POST_DECREMENT) { variableAccess("i", WRITE) } variableAccess("i", READ) @@ -176,7 +174,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - prefixExpr(UNARY_PLUS) { + unaryExpr(UNARY_PLUS) { variableAccess("q", READ) } } @@ -186,7 +184,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - prefixExpr(UNARY_MINUS) { + unaryExpr(UNARY_MINUS) { variableAccess("q", READ) } } @@ -196,7 +194,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - prefixExpr(PRE_INCREMENT) { + unaryExpr(PRE_INCREMENT) { variableAccess("q", WRITE) } } @@ -206,7 +204,7 @@ class ASTUnaryExpressionTest : ParserTestSpec({ castExpr { primitiveType(type) - prefixExpr(PRE_DECREMENT) { + unaryExpr(PRE_DECREMENT) { variableAccess("q", WRITE) } } @@ -224,34 +222,34 @@ class ASTUnaryExpressionTest : ParserTestSpec({ inContext(ExpressionParsingCtx) { "!!true" should parseAs { - prefixExpr(NEGATION) { - prefixExpr(NEGATION) { + unaryExpr(NEGATION) { + unaryExpr(NEGATION) { boolean(true) } } } "~~1" should parseAs { - prefixExpr(COMPLEMENT) { - prefixExpr(COMPLEMENT) { + unaryExpr(COMPLEMENT) { + unaryExpr(COMPLEMENT) { number() } } } "-~1" should parseAs { - prefixExpr(UNARY_MINUS) { - prefixExpr(COMPLEMENT) { + unaryExpr(UNARY_MINUS) { + unaryExpr(COMPLEMENT) { number() } } } "-+-+1" should parseAs { - prefixExpr(UNARY_MINUS) { - prefixExpr(UNARY_PLUS) { - prefixExpr(UNARY_MINUS) { - prefixExpr(UNARY_PLUS) { + unaryExpr(UNARY_MINUS) { + unaryExpr(UNARY_PLUS) { + unaryExpr(UNARY_MINUS) { + unaryExpr(UNARY_PLUS) { number() } } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt index ec7a3c681c..a0597c86bb 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/TestExtensions.kt @@ -123,13 +123,7 @@ fun TreeNodeWrapper.methodCall(inside: NodeSpec) = } -fun TreeNodeWrapper.prefixExpr(op: UnaryOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = - child { - it::getOperator shouldBe op - it::getOperand shouldBe baseExpr() - } - -fun TreeNodeWrapper.postfixExpr(op: UnaryOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = +fun TreeNodeWrapper.unaryExpr(op: UnaryOp, baseExpr: TreeNodeWrapper.() -> ASTExpression) = child { it::getOperator shouldBe op it::getOperand shouldBe baseExpr() From 07c53cd28cb3774c223ab831cb02b3665a1c309c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 10 Dec 2019 13:12:08 +0100 Subject: [PATCH 7/8] Fix tests --- .../sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt index 5328e14e86..6e3a30caf5 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTSwitchExpressionTests.kt @@ -7,7 +7,7 @@ package net.sourceforge.pmd.lang.java.ast import net.sourceforge.pmd.lang.ast.test.shouldBe import net.sourceforge.pmd.lang.java.ast.BinaryOp.* import net.sourceforge.pmd.lang.java.ast.ParserTestCtx.Companion.ExpressionParsingCtx -import net.sourceforge.pmd.lang.java.ast.UnaryOp.PrefixOp.UNARY_MINUS +import net.sourceforge.pmd.lang.java.ast.UnaryOp.UNARY_MINUS /** From 98576448821b369e56c6aaa0670de68a833cb224 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 15 Dec 2019 01:34:48 +0100 Subject: [PATCH 8/8] Fix compilation --- .../net/sourceforge/pmd/lang/java/ast/UnaryOp.java | 11 +++++++++++ .../rule/bestpractices/ForLoopCanBeForeachRule.java | 12 ++++++------ 2 files changed, 17 insertions(+), 6 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java index 4ce49dad59..170911d0bc 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/UnaryOp.java @@ -64,6 +64,17 @@ public enum UnaryOp implements InternalInterfaces.OperatorLike { return this.ordinal() < PRE_INCREMENT.ordinal(); } + /** Returns true if this is one of {@link #PRE_INCREMENT} or {@link #POST_INCREMENT}. */ + public boolean isIncrement() { + return this == PRE_INCREMENT || this == POST_INCREMENT; + } + + /** Returns true if this is one of {@link #PRE_DECREMENT} or {@link #POST_DECREMENT}. */ + public boolean isDecrement() { + return this == PRE_DECREMENT || this == POST_DECREMENT; + } + + /** Returns true if this is a prefix operator. */ public boolean isPrefix() { return this.ordinal() < POST_INCREMENT.ordinal(); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java index 93eeef41de..ded18ba4d6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/ForLoopCanBeForeachRule.java @@ -18,7 +18,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTForInit; import net.sourceforge.pmd.lang.java.ast.ASTForStatement; import net.sourceforge.pmd.lang.java.ast.ASTForUpdate; -import net.sourceforge.pmd.lang.java.ast.ASTIncrementExpression; import net.sourceforge.pmd.lang.java.ast.ASTLiteral; import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; @@ -28,6 +27,8 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTRelationalExpression; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.ASTStatementExpressionList; +import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTVariableAccess; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; @@ -131,11 +132,10 @@ public class ForLoopCanBeForeachRule extends AbstractJavaRule { .children(ASTStatementExpressionList.class) .filter(it -> it.jjtGetNumChildren() == 1) .children(ASTStatementExpression.class) - .children(ASTIncrementExpression.class) - .filter(ASTIncrementExpression::isIncrement) - .children(ASTPrimaryExpression.class) - .children(ASTPrimaryPrefix.class) - .children(ASTName.class) + .children(ASTUnaryExpression.class) + .filter(it -> it.getOperator().isIncrement()) + .map(ASTUnaryExpression::getOperand) + .filterIs(ASTVariableAccess.class) .firstOpt() .map(Node::getImage); }