diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a9d8201b0a..e9e0362965 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -17,6 +17,7 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues * java-bestpractices + * [#808](https://github.com/pmd/pmd/issues/808): \[java] AccessorMethodGeneration false positives with compile time constants * [#1555](https://github.com/pmd/pmd/issues/1555): \[java] UnusedImports false positive for method parameter type in @see Javadoc * java-codestyle * [#1543](https://github.com/pmd/pmd/issues/1543): \[java] LinguisticNaming should ignore overriden methods @@ -37,6 +38,7 @@ This is a {{ site.pmd.release_type }} release. * [#1628](https://github.com/pmd/pmd/pull/1628): \[java] LinguisticNaming should ignore overriden methods - [Shubham](https://github.com/Shubham-2k17) * [#1634](https://github.com/pmd/pmd/pull/1634): \[java] BeanMembersShouldSerializeRule does not recognize lombok accessors - [Shubham](https://github.com/Shubham-2k17) * [#1635](https://github.com/pmd/pmd/pull/1635): \[java] UnsynchronizedStaticFormatter reports commons lang FastDateFormat - [Shubham](https://github.com/Shubham-2k17) +* [#1637](https://github.com/pmd/pmd/pull/1637): \[java] Compile time constants initialized by literals avoided by AccessorMethodGenerationRule - [Shubham](https://github.com/Shubham-2k17) * [#1640](https://github.com/pmd/pmd/pull/1640): \[java] Update instead of override classHasLombokAnnotation flag - [Phokham Nonava](https://github.com/fluxroot) {% endtocmaker %} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AccessorMethodGenerationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AccessorMethodGenerationRule.java index 56cad28680..b84491331d 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AccessorMethodGenerationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AccessorMethodGenerationRule.java @@ -4,12 +4,21 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; +import java.util.ArrayList; import java.util.List; import java.util.Map; +import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTLiteral; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; +import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; +import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer; import net.sourceforge.pmd.lang.java.ast.AbstractJavaAccessNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.symboltable.AbstractJavaScope; @@ -22,6 +31,8 @@ import net.sourceforge.pmd.lang.symboltable.NameOccurrence; public class AccessorMethodGenerationRule extends AbstractJavaRule { + private List cache = new ArrayList<>(); + @Override public Object visit(final ASTCompilationUnit node, final Object data) { final SourceFileScope file = node.getScope().getEnclosingScope(SourceFileScope.class); @@ -57,6 +68,30 @@ public class AccessorMethodGenerationRule extends AbstractJavaRule { return; } + if (node.isFinal()) { + for (final ASTVariableDeclarator varDecl: node.findChildrenOfType(ASTVariableDeclarator.class)) { + if (varDecl.hasInitializer()) { + ASTVariableInitializer varInit = varDecl.getInitializer(); + List initExpression = varInit.findDescendantsOfType(ASTExpression.class); + boolean isConstantExpression = true; + constantCheck: + for (ASTExpression exp: initExpression) { + List primaryExpressions = exp.findDescendantsOfType(ASTPrimaryExpression.class); + for (ASTPrimaryExpression expression: primaryExpressions) { + if (!isCompileTimeConstant(expression)) { + isConstantExpression = false; + break constantCheck; + } + } + } + if (isConstantExpression) { + cache.add(varDecl.getName()); + return; + } + } + } + } + for (final NameOccurrence no : occurrences) { ClassScope usedAtScope = no.getLocation().getScope().getEnclosingScope(ClassScope.class); @@ -66,4 +101,39 @@ public class AccessorMethodGenerationRule extends AbstractJavaRule { } } } + + public boolean isCompileTimeConstant(ASTPrimaryExpression expressions) { + // function call detected + List suffix = expressions.findDescendantsOfType(ASTPrimarySuffix.class); + if (!suffix.isEmpty()) { + return false; + } + + // single node expression + List nameNodes = expressions.findDescendantsOfType(ASTName.class); + List literalNodes = expressions.findDescendantsOfType(ASTLiteral.class); + if (nameNodes.size() + literalNodes.size() < 2) { + for (ASTName node: nameNodes) { + // TODO : use the symbol table to get the declaration of the referenced var and check it + if (!cache.contains(node.getImage())) { + return false; + } + } + return true; + } + + // multiple node expression + List subExpressions = expressions.findDescendantsOfType(ASTPrimaryExpression.class); + for (ASTPrimaryExpression exp: subExpressions) { + if (!isCompileTimeConstant(exp)) { + return false; + } + } + return true; + } + + @Override + public void end(RuleContext ctx) { + cache.clear(); + } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AccessorMethodGeneration.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AccessorMethodGeneration.xml index bf1b8fd638..b053b9a31e 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AccessorMethodGeneration.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AccessorMethodGeneration.xml @@ -190,6 +190,46 @@ public class Foo implements Parcelable { return value == 1; } }; +} + ]]> + java 10 + + + #808 - [java] AccessorMethodGeneration false positives with compile time constants + 4 + 25,26,27,28 + java 10