diff --git a/.travis.yml b/.travis.yml index 2276f63309..5ff4e3b8b8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -49,7 +49,7 @@ jobs: env: BUILD=publish before_install: - - bash .travis/before_install.sh "11.0.7+10" + - bash .travis/before_install.sh "11.0.8+10" - source ${HOME}/java.env install: true before_script: true diff --git a/.travis/build-coveralls.sh b/.travis/build-coveralls.sh index b58629a5fa..c3834512a7 100755 --- a/.travis/build-coveralls.sh +++ b/.travis/build-coveralls.sh @@ -4,7 +4,7 @@ set -e source .travis/logger.sh source .travis/common-functions.sh -VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:1.5.0:exec) +VERSION=$(get_pom_version) log_info "Building PMD Coveralls.io report ${VERSION} on branch ${TRAVIS_BRANCH}" if ! travis_isPush; then diff --git a/.travis/build-deploy.sh b/.travis/build-deploy.sh index 18dc1c824a..f4bbddc2d5 100755 --- a/.travis/build-deploy.sh +++ b/.travis/build-deploy.sh @@ -7,7 +7,7 @@ source .travis/github-releases-api.sh source .travis/sourceforge-api.sh source .travis/regression-tester.sh -VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:1.5.0:exec) +VERSION=$(get_pom_version) log_info "Building PMD ${VERSION} on branch ${TRAVIS_BRANCH}" MVN_BUILD_FLAGS="-B -V" diff --git a/.travis/build-doc.sh b/.travis/build-doc.sh index 1d81fd2bff..581fc64064 100755 --- a/.travis/build-doc.sh +++ b/.travis/build-doc.sh @@ -8,7 +8,7 @@ source .travis/sourceforge-api.sh source .travis/pmd-code-api.sh function main() { - VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:1.5.0:exec) + VERSION=$(get_pom_version) log_info "Building PMD Documentation ${VERSION} on branch ${TRAVIS_BRANCH}" # diff --git a/.travis/build-publish.sh b/.travis/build-publish.sh index e93f47a270..5599103585 100644 --- a/.travis/build-publish.sh +++ b/.travis/build-publish.sh @@ -6,7 +6,7 @@ source .travis/common-functions.sh source .travis/github-releases-api.sh source .travis/sourceforge-api.sh -VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:1.5.0:exec) +VERSION=$(get_pom_version) log_info "PMD Release ${VERSION}" if ! travis_isPush; then diff --git a/.travis/build-sonar.sh b/.travis/build-sonar.sh index 7e51aaf46d..36cc2cc6b4 100755 --- a/.travis/build-sonar.sh +++ b/.travis/build-sonar.sh @@ -4,7 +4,7 @@ set -e source .travis/logger.sh source .travis/common-functions.sh -VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:1.5.0:exec) +VERSION=$(get_pom_version) log_info "Building PMD Sonar ${VERSION} on branch ${TRAVIS_BRANCH}" if ! travis_isPush; then diff --git a/.travis/common-functions.sh b/.travis/common-functions.sh index e46731feb6..3148dd9796 100755 --- a/.travis/common-functions.sh +++ b/.travis/common-functions.sh @@ -53,3 +53,7 @@ function travis_isWindows() { return 1 fi } + +function get_pom_version() { + echo $(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:3.0.0:exec) +} diff --git a/do-release.sh b/do-release.sh index 128b65b129..25d5785ed3 100755 --- a/do-release.sh +++ b/do-release.sh @@ -25,7 +25,7 @@ echo "Releasing PMD" echo "-------------------------------------------" # see also https://gist.github.com/pdunnavant/4743895 -CURRENT_VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:1.5.0:exec) +CURRENT_VERSION=$(./mvnw -q -Dexec.executable="echo" -Dexec.args='${project.version}' --non-recursive org.codehaus.mojo:exec-maven-plugin:3.0.0:exec) RELEASE_VERSION=${CURRENT_VERSION%-SNAPSHOT} MAJOR=$(echo $RELEASE_VERSION | cut -d . -f 1) MINOR=$(echo $RELEASE_VERSION | cut -d . -f 2) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 4cc19b0952..64435a1374 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,7 +14,26 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy +#### New Rules + +* The new Java rule {% rule "java/bestpractices/UnusedAssignment" %} (`java-bestpractices`) finds assignments + to variables, that are never used and are useless. The new rule is supposed to entirely replace + {% rule "java/errorprone/DataflowAnomalyAnalysis" %}. + ### Fixed Issues +* apex + * [#2610](https://github.com/pmd/pmd/pull/2610): \[apex] Support top-level enums in rules +* apex-bestpractices + * [#2626](https://github.com/pmd/pmd/issues/2626): \[apex] UnusedLocalVariable - false positive on case insensitivity allowed in Apex +* core + * [#710](https://github.com/pmd/pmd/issues/710): \[core] Review used dependencies + * [#2594](https://github.com/pmd/pmd/issues/2594): \[core] Update exec-maven-plugin and align it in all project +* java-design + * [#2174](https://github.com/pmd/pmd/issues/2174): \[java] LawOfDemeter: False positive with 'this' pointer + * [#2189](https://github.com/pmd/pmd/issues/2189): \[java] LawOfDemeter: False positive when casting to derived class +* java-performance + * [#1736](https://github.com/pmd/pmd/issues/1736): \[java] UseStringBufferForStringAppends: False positive if only one concatenation + * [#2207](https://github.com/pmd/pmd/issues/2207): \[java] AvoidInstantiatingObjectsInLoops: False positive - should not flag objects when assigned to lists/arrays ### API Changes @@ -29,6 +48,10 @@ This is a {{ site.pmd.release_type }} release. ### External Contributions +* [#2558](https://github.com/pmd/pmd/pull/2558): \[java] Fix issue #1736 and issue #2207 - [Young Chan](https://github.com/YYoungC) +* [#2560](https://github.com/pmd/pmd/pull/2560): \[java] Fix false positives of LawOfDemeter: this and cast expressions - [xioayuge](https://github.com/xioayuge) +* [#2590](https://github.com/pmd/pmd/pull/2590): Update libraries snyk is referring to as `unsafe` - [Artem Krosheninnikov](https://github.com/KroArtem) +* [#2597](https://github.com/pmd/pmd/pull/2597): \[dependencies] Fix issue #2594, update exec-maven-plugin everywhere - [Artem Krosheninnikov](https://github.com/KroArtem) {% endtocmaker %} diff --git a/pmd-apex-jorje/pom.xml b/pmd-apex-jorje/pom.xml index de533a5c87..638cea2eb3 100644 --- a/pmd-apex-jorje/pom.xml +++ b/pmd-apex-jorje/pom.xml @@ -54,12 +54,12 @@ ch.qos.logback logback-classic - 1.1.7 + 1.2.3 ch.qos.logback logback-core - 1.1.7 + 1.2.3 com.google.code.findbugs @@ -89,7 +89,6 @@ org.antlr antlr-runtime - 3.5.2 org.antlr @@ -119,7 +118,7 @@ org.yaml snakeyaml - 1.17 + 1.26 aopalliance diff --git a/pmd-apex/pom.xml b/pmd-apex/pom.xml index 08b66db721..2bf2041268 100644 --- a/pmd-apex/pom.xml +++ b/pmd-apex/pom.xml @@ -1,125 +1,98 @@ - - 4.0.0 - pmd-apex - PMD Apex + + 4.0.0 + pmd-apex + PMD Apex - - net.sourceforge.pmd - pmd - 6.26.0-SNAPSHOT - ../ - + + net.sourceforge.pmd + pmd + 6.26.0-SNAPSHOT + ../ + - - 8 - + + 8 + - - - - ${basedir}/src/main/resources - true - - - - - maven-resources-plugin - - false - - ${*} - - - - - - - - net.sourceforge.pmd - pmd-core - - - - ${project.groupId} - pmd-apex-jorje - ${project.version} - lib - - - - ${project.groupId} - pmd-apex-jorje - ${project.version} - pom - - - - commons-io - commons-io - - - - - net.sourceforge.saxon - saxon - - - - org.hamcrest - hamcrest - test - - - junit - junit - test - - - com.github.stefanbirkner - system-rules - test - - - org.junit.vintage - junit-vintage-engine - test - - - net.sourceforge.pmd - pmd-test - test - - - net.sourceforge.pmd - pmd-lang-test - test - - - - - - designer - + + + + ${basedir}/src/main/resources + true + + - - org.codehaus.mojo - exec-maven-plugin - 1.4.0 - - net.sourceforge.pmd.util.designer.Designer - true - - - - net.sourceforge.pmd - pmd-java - ${project.version} - - - + + maven-resources-plugin + + false + + ${*} + + + - - - + + + + net.sourceforge.pmd + pmd-core + + + org.antlr + antlr-runtime + + + + ${project.groupId} + pmd-apex-jorje + ${project.version} + lib + + + + ${project.groupId} + pmd-apex-jorje + ${project.version} + pom + + + + commons-io + commons-io + + + org.apache.commons + commons-lang3 + + + + + org.hamcrest + hamcrest + test + + + junit + junit + test + + + com.github.stefanbirkner + system-rules + test + + + net.sourceforge.pmd + pmd-test + test + + + net.sourceforge.pmd + pmd-lang-test + test + + diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetrics.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetrics.java index d8dd2ec984..9c22ae27e5 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetrics.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/metrics/ApexMetrics.java @@ -7,8 +7,6 @@ package net.sourceforge.pmd.lang.apex.metrics; import java.util.ArrayList; import java.util.List; -import org.checkerframework.checker.nullness.qual.NonNull; - import net.sourceforge.pmd.lang.apex.ast.ASTMethod; import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; import net.sourceforge.pmd.lang.apex.ast.ASTUserClassOrInterface; @@ -136,7 +134,6 @@ public final class ApexMetrics { } - @NonNull public static List findOps(ASTUserClassOrInterface node) { List candidates = node.findChildrenOfType(ASTMethod.class); List result = new ArrayList<>(candidates); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/multifile/ApexMultifileVisitor.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/multifile/ApexMultifileVisitor.java index 66d796d408..e24af12287 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/multifile/ApexMultifileVisitor.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/multifile/ApexMultifileVisitor.java @@ -8,6 +8,7 @@ import java.util.Stack; import net.sourceforge.pmd.lang.apex.ast.ASTMethod; import net.sourceforge.pmd.lang.apex.ast.ASTUserClassOrInterface; +import net.sourceforge.pmd.lang.apex.ast.ASTUserEnum; import net.sourceforge.pmd.lang.apex.ast.ASTUserTrigger; import net.sourceforge.pmd.lang.apex.ast.ApexParserVisitorReducedAdapter; @@ -41,6 +42,12 @@ public class ApexMultifileVisitor extends ApexParserVisitorReducedAdapter { } + @Override + public Object visit(ASTUserEnum node, Object data) { + return data; // ignore + } + + @Override public Object visit(ASTMethod node, Object data) { stack.peek().addOperation(node.getQualifiedName().getOperation(), node.getSignature()); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java index e9f2c7b721..04eb3cea4e 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/UnusedLocalVariableRule.java @@ -1,4 +1,4 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ @@ -7,6 +7,8 @@ package net.sourceforge.pmd.lang.apex.rule.bestpractices; import java.util.ArrayList; import java.util.List; +import org.apache.commons.lang3.StringUtils; + import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.apex.ast.ASTReferenceExpression; import net.sourceforge.pmd.lang.apex.ast.ASTVariableDeclaration; @@ -41,7 +43,7 @@ public class UnusedLocalVariableRule extends AbstractApexRule { continue; } - if (usage.hasImageEqualTo(variableName)) { + if (StringUtils.equalsIgnoreCase(variableName, usage.getImage())) { return data; } } diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/StdCyclomaticComplexityRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/StdCyclomaticComplexityRule.java index cd9bad69c8..a70fc7f626 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/StdCyclomaticComplexityRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/design/StdCyclomaticComplexityRule.java @@ -129,13 +129,6 @@ public class StdCyclomaticComplexityRule extends AbstractApexRule { @Override public Object visit(ASTUserEnum node, Object data) { - entryStack.push(new Entry()); - super.visit(node, data); - Entry classEntry = entryStack.pop(); - if (classEntry.getComplexityAverage() >= reportLevel || classEntry.highestDecisionPoints >= reportLevel) { - addViolation(data, node, new String[] { "class", node.getImage(), - classEntry.getComplexityAverage() + "(Highest = " + classEntry.highestDecisionPoints + ')', }); - } return data; } diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRuleTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRuleTest.java new file mode 100644 index 0000000000..896f647c18 --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/AbstractApexRuleTest.java @@ -0,0 +1,86 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule; + +import static org.junit.Assert.assertEquals; + +import java.util.Collections; + +import org.junit.Test; + +import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.lang.apex.ast.ASTAnonymousClass; +import net.sourceforge.pmd.lang.apex.ast.ASTUserClass; +import net.sourceforge.pmd.lang.apex.ast.ASTUserEnum; +import net.sourceforge.pmd.lang.apex.ast.ASTUserInterface; +import net.sourceforge.pmd.lang.apex.ast.ASTUserTrigger; +import net.sourceforge.pmd.lang.apex.ast.ApexNode; +import net.sourceforge.pmd.lang.apex.ast.ApexParserTestBase; + +import apex.jorje.semantic.ast.compilation.Compilation; + +public class AbstractApexRuleTest extends ApexParserTestBase { + + @Test + public void shouldVisitTopLevelClass() { + run("class Foo { }"); + } + + @Test + public void shouldVisitTopLevelInterface() { + run("interface Foo { }"); + } + + @Test + public void shouldVisitTopLevelTrigger() { + run("trigger Foo on Account (before insert, before update) { }"); + } + + @Test + public void shouldVisitTopLevelEnum() { + run("enum Foo { }"); + } + + private void run(String code) { + ApexNode node = parse(code); + RuleContext ctx = new RuleContext(); + ctx.setLanguageVersion(apex.getDefaultVersion()); + TopLevelRule rule = new TopLevelRule(); + rule.apply(Collections.singletonList(node), ctx); + assertEquals(1, ctx.getReport().size()); + } + + private static class TopLevelRule extends AbstractApexRule { + @Override + public Object visit(ASTUserClass node, Object data) { + addViolation(data, node); + return data; + } + + @Override + public Object visit(ASTUserInterface node, Object data) { + addViolation(data, node); + return data; + } + + @Override + public Object visit(ASTUserTrigger node, Object data) { + addViolation(data, node); + return data; + } + + @Override + public Object visit(ASTUserEnum node, Object data) { + addViolation(data, node); + return data; + } + + @Override + public Object visit(ASTAnonymousClass node, Object data) { + addViolation(data, node); + return data; + } + } +} diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml index 1888c60fda..5f39923098 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/UnusedLocalVariable.xml @@ -94,6 +94,20 @@ public class Foo { trigger leadOwnerUpdate on Lead (after update) { for(Lead Id : Trigger.new) { } +} + ]]> + + + + [apex] UnusedLocalVariable - false positive on case insensitivity allowed in Apex #2626 + 0 + diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/ClassNamingConventions.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/ClassNamingConventions.xml index c5095ffec8..7eeac5de01 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/ClassNamingConventions.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/codestyle/xml/ClassNamingConventions.xml @@ -63,6 +63,14 @@ public class Foo { ]]> + + top-level enum all is well + 0 + + + test class default is title case 1 @@ -121,6 +129,17 @@ public class Foo { ]]> + + top-level enum default is title case + 1 + + The enum name 'fooEnum' doesn't match '[A-Z][a-zA-Z0-9_]*' + + + + custom test class pattern [a-zA-Z0-9_]+ @@ -157,4 +176,22 @@ public class FOO_CLASS { } public interface FOO_INTERFACE { } ]]> + + + custom enum pattern + E[a-zA-Z0-9_]+ + 0 + + + + + custom enum pattern + E[a-zA-Z0-9_]+ + 1 + + diff --git a/pmd-core/pom.xml b/pmd-core/pom.xml index 0089efa13a..9590252ed0 100644 --- a/pmd-core/pom.xml +++ b/pmd-core/pom.xml @@ -97,6 +97,12 @@ ant provided + + net.java.dev.javacc + javacc + provided + + org.antlr antlr4-runtime @@ -122,10 +128,7 @@ --> true - - net.java.dev.javacc - javacc - + net.sourceforge.saxon saxon diff --git a/pmd-core/src/main/resources/rulesets/releases/6260.xml b/pmd-core/src/main/resources/rulesets/releases/6260.xml new file mode 100644 index 0000000000..d4200e1e0c --- /dev/null +++ b/pmd-core/src/main/resources/rulesets/releases/6260.xml @@ -0,0 +1,13 @@ + + + + +This ruleset contains links to rules that are new in PMD v6.26.0 + + + + + diff --git a/pmd-cpp/pom.xml b/pmd-cpp/pom.xml index 51d5111c0b..9f25873b2f 100644 --- a/pmd-cpp/pom.xml +++ b/pmd-cpp/pom.xml @@ -70,21 +70,12 @@ net.sourceforge.pmd pmd-core - - commons-io - commons-io - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-cs/pom.xml b/pmd-cs/pom.xml index 7d6939ed6e..9ed215bb18 100644 --- a/pmd-cs/pom.xml +++ b/pmd-cs/pom.xml @@ -36,8 +36,8 @@ pmd-core - commons-io - commons-io + org.antlr + antlr4-runtime @@ -45,11 +45,6 @@ junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-dart/pom.xml b/pmd-dart/pom.xml index e2fbafc4b8..e3b0d8482e 100644 --- a/pmd-dart/pom.xml +++ b/pmd-dart/pom.xml @@ -31,17 +31,13 @@ - - org.antlr - antlr4-runtime - net.sourceforge.pmd pmd-core - commons-io - commons-io + org.antlr + antlr4-runtime @@ -49,11 +45,6 @@ junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-dist/pom.xml b/pmd-dist/pom.xml index 96fca07038..ea56d1ccb6 100644 --- a/pmd-dist/pom.xml +++ b/pmd-dist/pom.xml @@ -230,13 +230,13 @@ - org.hamcrest - hamcrest + junit + junit test - junit - junit + org.hamcrest + hamcrest test diff --git a/pmd-doc/pom.xml b/pmd-doc/pom.xml index 22f43c9c0b..318dfa95b3 100644 --- a/pmd-doc/pom.xml +++ b/pmd-doc/pom.xml @@ -24,7 +24,6 @@ org.codehaus.mojo exec-maven-plugin - 1.6.0 generate-rule-docs @@ -100,18 +99,18 @@ org.yaml snakeyaml - 1.19 + 1.26 - - org.hamcrest - hamcrest - test - junit junit test + + org.hamcrest + hamcrest + test + diff --git a/pmd-fortran/pom.xml b/pmd-fortran/pom.xml index 9e451b1c53..4a94623d7c 100644 --- a/pmd-fortran/pom.xml +++ b/pmd-fortran/pom.xml @@ -29,21 +29,12 @@ net.sourceforge.pmd pmd-core - - commons-io - commons-io - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-go/pom.xml b/pmd-go/pom.xml index 773a98e3b3..311df23eeb 100644 --- a/pmd-go/pom.xml +++ b/pmd-go/pom.xml @@ -29,25 +29,20 @@ - - org.antlr - antlr4-runtime - net.sourceforge.pmd pmd-core + + org.antlr + antlr4-runtime + junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-groovy/pom.xml b/pmd-groovy/pom.xml index 655f884a74..969a99e55e 100644 --- a/pmd-groovy/pom.xml +++ b/pmd-groovy/pom.xml @@ -26,30 +26,20 @@ - - org.codehaus.groovy - groovy - - - commons-io - commons-io - - net.sourceforge.pmd pmd-core + + org.codehaus.groovy + groovy + junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-java/pom.xml b/pmd-java/pom.xml index 5ff10aa5fd..f3e4264c30 100644 --- a/pmd-java/pom.xml +++ b/pmd-java/pom.xml @@ -107,10 +107,6 @@ - - net.java.dev.javacc - javacc - net.sourceforge.pmd pmd-core @@ -132,13 +128,6 @@ commons-lang3 - - net.sourceforge.saxon - saxon - dom - runtime - - net.sourceforge.pmd @@ -150,23 +139,6 @@ pmd-test test - - org.slf4j - slf4j-api - test - - - org.apache.logging.log4j - log4j-api - 2.12.1 - test - - - org.apache.logging.log4j - log4j-core - 2.12.1 - test - org.hamcrest hamcrest @@ -177,11 +149,6 @@ junit test - - org.junit.vintage - junit-vintage-engine - test - com.github.stefanbirkner @@ -193,6 +160,66 @@ ant-testutil test + + + com.github.oowekyala.treeutils + tree-matchers + test + + + com.github.oowekyala.treeutils + tree-printers + test + + + io.kotlintest + kotlintest-assertions + test + + + io.kotlintest + kotlintest-core + test + + + com.google.guava + guava + test + + + org.jetbrains.kotlin + kotlin-stdlib + test + + + org.jetbrains.kotlin + kotlin-test + test + + + org.jetbrains + annotations + test + + + + + org.slf4j + slf4j-api + test + + + org.apache.logging.log4j + log4j-api + 2.13.3 + test + + + org.apache.logging.log4j + log4j-core + 2.13.3 + test + org.assertj assertj-core diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java index 7f410b84c8..f98fd3295e 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnnotation.java @@ -26,7 +26,7 @@ import net.sourceforge.pmd.annotation.InternalApi; public class ASTAnnotation extends AbstractJavaTypeNode { private static final List UNUSED_RULES - = Arrays.asList("UnusedPrivateField", "UnusedLocalVariable", "UnusedPrivateMethod", "UnusedFormalParameter"); + = Arrays.asList("UnusedPrivateField", "UnusedLocalVariable", "UnusedPrivateMethod", "UnusedFormalParameter", "UnusedAssignment"); private static final List SERIAL_RULES = Arrays.asList("BeanMembersShouldSerialize", "MissingSerialVersionUID"); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCatchStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCatchStatement.java index f8df8abe83..98e8a30b78 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCatchStatement.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTCatchStatement.java @@ -101,4 +101,10 @@ public class ASTCatchStatement extends AbstractJavaNode { return getFirstDescendantOfType(ASTVariableDeclaratorId.class).getImage(); } + /** + * Returns the declarator id for the exception parameter. + */ + public ASTVariableDeclaratorId getExceptionId() { + return getFirstChildOfType(ASTFormalParameter.class).getVariableDeclaratorId(); + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTConditionalExpression.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTConditionalExpression.java index 521b5828f0..1d4a84d645 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTConditionalExpression.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTConditionalExpression.java @@ -74,7 +74,7 @@ public class ASTConditionalExpression extends AbstractJavaTypeNode { * Returns the node that represents the guard of this conditional. * That is the expression before the '?'. */ - public Node getCondition() { + public JavaNode getCondition() { return getChild(0); } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java index a304d3f9b6..6fad0e2722 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTTryStatement.java @@ -49,7 +49,7 @@ public class ASTTryStatement extends AbstractJavaNode { * Returns the body of this try statement. */ public ASTBlock getBody() { - return (ASTBlock) getChild(1); + return (ASTBlock) getFirstChildOfType(ASTBlock.class); } /** diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentRule.java new file mode 100644 index 0000000000..540bd27afa --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentRule.java @@ -0,0 +1,1345 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + + +import static net.sourceforge.pmd.lang.java.rule.codestyle.ConfusingTernaryRule.unwrapParentheses; + +import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collections; +import java.util.Comparator; +import java.util.Deque; +import java.util.HashMap; +import java.util.HashSet; +import java.util.Iterator; +import java.util.List; +import java.util.Map; +import java.util.Objects; +import java.util.Set; + +import net.sourceforge.pmd.RuleContext; +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; +import net.sourceforge.pmd.lang.java.ast.ASTBlock; +import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.java.ast.ASTBreakStatement; +import net.sourceforge.pmd.lang.java.ast.ASTCatchStatement; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression; +import net.sourceforge.pmd.lang.java.ast.ASTConditionalExpression; +import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression; +import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTContinueStatement; +import net.sourceforge.pmd.lang.java.ast.ASTDoStatement; +import net.sourceforge.pmd.lang.java.ast.ASTEnumBody; +import net.sourceforge.pmd.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTFinallyStatement; +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.ASTFormalParameter; +import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; +import net.sourceforge.pmd.lang.java.ast.ASTInitializer; +import net.sourceforge.pmd.lang.java.ast.ASTLabeledStatement; +import net.sourceforge.pmd.lang.java.ast.ASTLambdaExpression; +import net.sourceforge.pmd.lang.java.ast.ASTLocalVariableDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTName; +import net.sourceforge.pmd.lang.java.ast.ASTPostfixExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPreDecrementExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPreIncrementExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; +import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; +import net.sourceforge.pmd.lang.java.ast.ASTResourceSpecification; +import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; +import net.sourceforge.pmd.lang.java.ast.ASTStatement; +import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchExpression; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabel; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabeledRule; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement; +import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement; +import net.sourceforge.pmd.lang.java.ast.ASTTryStatement; +import net.sourceforge.pmd.lang.java.ast.ASTTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclarator; +import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; +import net.sourceforge.pmd.lang.java.ast.ASTVariableInitializer; +import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; +import net.sourceforge.pmd.lang.java.ast.ASTYieldStatement; +import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.symboltable.ClassScope; +import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; +import net.sourceforge.pmd.lang.symboltable.Scope; +import net.sourceforge.pmd.properties.PropertyDescriptor; +import net.sourceforge.pmd.properties.PropertyFactory; + +@SuppressWarnings("PMD.DontCallSuperVisitWhenUsingRuleChain") +public class UnusedAssignmentRule extends AbstractJavaRule { + + /* + Detects unused assignments. This performs a reaching definition + analysis. This makes the assumption that there is no dead code. + + Since we have the reaching definitions at each variable usage, we + could also use that to detect other kinds of bug, eg conditions + that are always true, or dereferences that will always NPE. In + the general case though, this is complicated and better left to + a DFA library, eg google Z3. + + This analysis may be used as-is to detect switch labels that + fall-through, which could be useful to improve accuracy of other + rules. + + TODO + * labels on arbitrary statements (currently only loops) + * explicit ctor call (hard to impossible without type res, + or at least proper graph algorithms like toposort) + -> this is pretty invisible as it causes false negatives, not FPs + * test ternary expr + + DONE + * conditionals + * loops + * switch + * loop labels + * try/catch/finally + * lambdas + * constructors + initializers + * anon class + * test this.field in ctors + * foreach var should be reassigned from one iter to another + * test local class/anonymous class + * shortcut conditionals have their own control-flow + * parenthesized expressions + * conditional exprs in loops + * ignore variables that start with 'ignore' + + */ + + private static final PropertyDescriptor CHECK_PREFIX_INCREMENT = + PropertyFactory.booleanProperty("checkUnusedPrefixIncrement") + .desc("Report expressions like ++i that may be replaced with (i + 1)") + .defaultValue(false) + .build(); + + private static final PropertyDescriptor REPORT_UNUSED_VARS = + PropertyFactory.booleanProperty("reportUnusedVariables") + .desc("Report variables that are only initialized, and never read at all. " + + "The rule UnusedVariable already cares for that, but you can enable it if needed") + .defaultValue(false) + .build(); + + public UnusedAssignmentRule() { + definePropertyDescriptor(CHECK_PREFIX_INCREMENT); + definePropertyDescriptor(REPORT_UNUSED_VARS); + addRuleChainVisit(ASTCompilationUnit.class); + } + + @Override + public Object visit(ASTCompilationUnit node, Object data) { + for (JavaNode child : node.children()) { + if (child instanceof ASTTypeDeclaration) { + + ASTAnyTypeDeclaration typeDecl = (ASTAnyTypeDeclaration) child.getChild(child.getNumChildren() - 1); + GlobalAlgoState result = new GlobalAlgoState(); + typeDecl.jjtAccept(ReachingDefsVisitor.ONLY_LOCALS, new SpanInfo(result)); + + reportFinished(result, (RuleContext) data); + } + } + + return data; + } + + private void reportFinished(GlobalAlgoState result, RuleContext ruleCtx) { + if (result.usedAssignments.size() < result.allAssignments.size()) { + Set unused = result.allAssignments; + // note that this mutates allAssignments, so the global + // state is unusable after this + unused.removeAll(result.usedAssignments); + + for (AssignmentEntry entry : unused) { + if (isIgnorablePrefixIncrement(entry.rhs)) { + continue; + } + + Set killers = result.killRecord.get(entry); + final String reason; + if (killers == null || killers.isEmpty()) { + // var went out of scope before being used (no assignment kills it, yet it's unused) + + if (entry.var.isField()) { + // assignments to fields don't really go out of scope + continue; + } else if (suppressUnusedVariableRuleOverlap(entry)) { + // see REPORT_UNUSED_VARS property + continue; + } + // This is a "DU" anomaly, the others are "DD" + reason = null; + } else if (killers.size() == 1) { + AssignmentEntry k = killers.iterator().next(); + if (k.rhs.equals(entry.rhs)) { + // assignment reassigns itself, only possible in a loop + if (suppressUnusedVariableRuleOverlap(entry)) { + continue; + } else if (entry.rhs instanceof ASTVariableDeclaratorId) { + reason = null; // unused foreach variable + } else { + reason = "reassigned every iteration"; + } + } else { + reason = "overwritten on line " + k.rhs.getBeginLine(); + } + } else { + reason = joinLines("overwritten on lines ", killers); + } + if (reason == null && hasExplicitIgnorableName(entry.var.getName())) { + // Then the variable is never used (cf UnusedVariable) + // We ignore those that start with "ignored", as that is standard + // practice for exceptions, and may be useful for resources/foreach vars + continue; + } + addViolationWithMessage(ruleCtx, entry.rhs, makeMessage(entry, reason, entry.var.isField())); + } + } + } + + private boolean hasExplicitIgnorableName(String name) { + return name.startsWith("ignored") + || "_".equals(name); // before java 9 it's ok + } + + private boolean suppressUnusedVariableRuleOverlap(AssignmentEntry entry) { + return !getProperty(REPORT_UNUSED_VARS) && (entry.rhs instanceof ASTVariableInitializer + || entry.rhs instanceof ASTVariableDeclaratorId); + } + + private static String getKind(ASTVariableDeclaratorId id) { + if (id.isField()) { + return "field"; + } else if (id.isResourceDeclaration()) { + return "resource"; + } else if (id.isExceptionBlockParameter()) { + return "exception parameter"; + } else if (id.getNthParent(3) instanceof ASTForStatement) { + return "loop variable"; + } else if (id.isFormalParameter()) { + return "parameter"; + } + return "variable"; + } + + private boolean isIgnorablePrefixIncrement(JavaNode assignment) { + if (assignment instanceof ASTPreIncrementExpression + || assignment instanceof ASTPreDecrementExpression) { + // the variable value is used if it was found somewhere else + // than in statement position + return !getProperty(CHECK_PREFIX_INCREMENT) && !(assignment.getParent() instanceof ASTStatementExpression); + } + return false; + } + + private static String makeMessage(AssignmentEntry assignment, /* Nullable */ String reason, boolean isField) { + // if reason is null, then the variable is unused (at most assigned to) + + String varName = assignment.var.getName(); + StringBuilder result = new StringBuilder(64); + if (assignment.rhs instanceof ASTVariableInitializer) { + result.append(isField ? "the field initializer for" + : "the initializer for variable"); + } else if (assignment.rhs instanceof ASTVariableDeclaratorId) { + if (reason != null) { + result.append("the initial value of "); + } + result.append(getKind(assignment.var)); + } else { + if (assignment.rhs instanceof ASTPreIncrementExpression + || assignment.rhs instanceof ASTPreDecrementExpression + || assignment.rhs instanceof ASTPostfixExpression) { + result.append("the updated value of "); + } else { + result.append("the value assigned to "); + } + result.append(isField ? "field" : "variable"); + } + result.append(" ''").append(varName).append("''"); + result.append(" is never used"); + if (reason != null) { + result.append(" (").append(reason).append(")"); + } + result.setCharAt(0, Character.toUpperCase(result.charAt(0))); + return result.toString(); + } + + private static String joinLines(String prefix, Set killers) { + StringBuilder sb = new StringBuilder(prefix); + ArrayList sorted = new ArrayList<>(killers); + Collections.sort(sorted, new Comparator() { + @Override + public int compare(AssignmentEntry o1, AssignmentEntry o2) { + int lineRes = Integer.compare(o1.rhs.getBeginLine(), o2.rhs.getBeginLine()); + return lineRes != 0 ? lineRes + : Integer.compare(o1.rhs.getBeginColumn(), o2.rhs.getBeginColumn()); + } + }); + + sb.append(sorted.get(0).rhs.getBeginLine()); + for (int i = 1; i < sorted.size() - 1; i++) { + sb.append(", ").append(sorted.get(i).rhs.getBeginLine()); + } + sb.append(" and ").append(sorted.get(sorted.size() - 1).rhs.getBeginLine()); + + return sb.toString(); + } + + private static class ReachingDefsVisitor extends JavaParserVisitorAdapter { + + + static final ReachingDefsVisitor ONLY_LOCALS = new ReachingDefsVisitor(null); + + // The class scope for the "this" reference, used to find fields + // of this class + // null if we're not processing instance/static initializers, + // so in methods we don't care about fields + // If not null, fields are effectively treated as locals + private final ClassScope enclosingClassScope; + + private ReachingDefsVisitor(ClassScope scope) { + this.enclosingClassScope = scope; + } + + + // following deals with control flow structures + + @Override + public Object visit(JavaNode node, Object data) { + + for (JavaNode child : node.children()) { + // each output is passed as input to the next (most relevant for blocks) + data = child.jjtAccept(this, data); + } + + return data; + } + + @Override + public Object visit(ASTBlock node, final Object data) { + // variables local to a loop iteration must be killed before the + // next iteration + + SpanInfo state = (SpanInfo) data; + Set localsToKill = new HashSet<>(); + + for (JavaNode child : node.children()) { + // each output is passed as input to the next (most relevant for blocks) + state = acceptOpt(child, state); + if (child instanceof ASTBlockStatement + && child.getChild(0) instanceof ASTLocalVariableDeclaration) { + ASTLocalVariableDeclaration local = (ASTLocalVariableDeclaration) child.getChild(0); + for (ASTVariableDeclaratorId id : local) { + localsToKill.add(id); + } + } + } + + for (ASTVariableDeclaratorId var : localsToKill) { + state.deleteVar(var); + } + + return state; + } + + @Override + public Object visit(ASTSwitchStatement node, Object data) { + return processSwitch(node, (SpanInfo) data, node.getTestedExpression()); + } + + @Override + public Object visit(ASTSwitchExpression node, Object data) { + return processSwitch(node, (SpanInfo) data, node.getChild(0)); + } + + private SpanInfo processSwitch(JavaNode switchLike, SpanInfo data, JavaNode testedExpr) { + GlobalAlgoState global = data.global; + SpanInfo before = acceptOpt(testedExpr, data); + + global.breakTargets.push(before.fork()); + + SpanInfo current = before; + for (int i = 1; i < switchLike.getNumChildren(); i++) { + JavaNode child = switchLike.getChild(i); + if (child instanceof ASTSwitchLabel) { + current = before.fork().absorb(current); + } else if (child instanceof ASTSwitchLabeledRule) { + current = acceptOpt(child.getChild(1), before.fork()); + current = global.breakTargets.doBreak(current, null); // process this as if it was followed by a break + } else { + // statement in a regular fallthrough switch block + current = acceptOpt(child, current); + } + } + + before = global.breakTargets.pop(); + + // join with the last state, which is the exit point of the + // switch, if it's not closed by a break; + return before.absorb(current); + } + + @Override + public Object visit(ASTIfStatement node, Object data) { + SpanInfo before = (SpanInfo) data; + return makeConditional(before, node.getCondition(), node.getThenBranch(), node.getElseBranch()); + } + + @Override + public Object visit(ASTConditionalExpression node, Object data) { + SpanInfo before = (SpanInfo) data; + return makeConditional(before, node.getCondition(), node.getChild(1), node.getChild(2)); + } + + // This will be much easier with the 7.0 grammar..... + SpanInfo makeConditional(SpanInfo before, JavaNode condition, JavaNode thenBranch, JavaNode elseBranch) { + SpanInfo thenState = before.fork(); + SpanInfo elseState = elseBranch != null ? before.fork() : before; + + linkConditional(before, condition, thenState, elseState, true); + + thenState = acceptOpt(thenBranch, thenState); + elseState = acceptOpt(elseBranch, elseState); + + return elseState.absorb(thenState); + } + + /* + * This recursive procedure translates shortcut conditionals + * that occur in condition position in the following way: + * + * if (a || b) if (a) + * else ~> else + * if (b) + * else + * + * + * if (a && b) if (a) + * else ~> if (b) + * else + * else + * + * The new conditions are recursively processed to translate + * bigger conditions, like `a || b && c` + * + * This is how it works, but the and branch are + * visited only once, because it's not done in this method, but + * in makeConditional. + * + * @return the state in which all expressions have been evaluated + * Eg for `a || b`, this is the `else` state (all evaluated to false) + * Eg for `a && b`, this is the `then` state (all evaluated to true) + * + */ + private SpanInfo linkConditional(SpanInfo before, JavaNode condition, SpanInfo thenState, SpanInfo elseState, boolean isTopLevel) { + if (condition == null) { + return before; + } + condition = unwrapParentheses(condition); + + if (condition instanceof ASTConditionalOrExpression) { + return visitShortcutOrExpr(condition, before, thenState, elseState); + } else if (condition instanceof ASTConditionalAndExpression) { + // To mimic a shortcut AND expr, swap the thenState and the elseState + // See explanations in method + return visitShortcutOrExpr(condition, before, elseState, thenState); + } else if (condition instanceof ASTExpression && condition.getNumChildren() == 1) { + return linkConditional(before, condition.getChild(0), thenState, elseState, isTopLevel); + } else { + SpanInfo state = acceptOpt(condition, before); + if (isTopLevel) { + thenState.absorb(state); + elseState.absorb(state); + } + return state; + } + } + + SpanInfo visitShortcutOrExpr(JavaNode orExpr, + SpanInfo before, + SpanInfo thenState, + SpanInfo elseState) { + + // if ( || || ... || ) + // else + // + // in , we are sure that at least was evaluated, + // but really any prefix of ... is possible so they're all merged + + // in , we are sure that all of ... were evaluated (to false) + + // If you replace || with &&, then the above holds if you swap and + // So this method handles the OR expr, the caller can swap the arguments to make an AND + + // --- + // This method side effects on thenState and elseState to + // set the variables. + + Iterator iterator = orExpr.children().iterator(); + + SpanInfo cur = before; + do { + JavaNode cond = iterator.next(); + cur = linkConditional(cur, cond, thenState, elseState, false); + thenState.absorb(cur); + } while (iterator.hasNext()); + + elseState.absorb(cur); + + return cur; + } + + + @Override + public Object visit(ASTTryStatement node, Object data) { + final SpanInfo before = (SpanInfo) data; + ASTFinallyStatement finallyClause = node.getFinallyClause(); + + /* + + try () { + + } catch (IOException e) { + + } finally { + + } + + + There is a path -> -> -> -> + and for each catch, -> -> -> + + Except that abrupt completion before the jumps + to the and completes abruptly for the same + reason (if the completes normally), which + means it doesn't go to + */ + + if (finallyClause != null) { + before.myFinally = before.forkEmpty(); + } + + ASTResourceSpecification resources = node.getFirstChildOfType(ASTResourceSpecification.class); + + SpanInfo bodyState = acceptOpt(resources, before.fork()); + bodyState = acceptOpt(node.getBody(), bodyState); + + SpanInfo exceptionalState = null; + for (ASTCatchStatement catchClause : node.getCatchClauses()) { + SpanInfo current = acceptOpt(catchClause, before.fork().absorb(bodyState)); + exceptionalState = current.absorb(exceptionalState); + } + + SpanInfo finalState; + finalState = bodyState.absorb(exceptionalState); + if (finallyClause != null) { + // this represents the finally clause when it was entered + // because of abrupt completion + // since we don't know when it terminated we must join it with before + SpanInfo abruptFinally = before.myFinally.absorb(before); + acceptOpt(finallyClause, abruptFinally); + before.myFinally = null; + + // this is the normal finally + finalState = acceptOpt(finallyClause, finalState); + } + + // In the 7.0 grammar, the resources should be explicitly + // used here. For now they don't trigger anything as their + // node is not a VariableDeclaratorId. There's a test to + // check that. + + return finalState; + } + + @Override + public Object visit(ASTCatchStatement node, Object data) { + SpanInfo result = (SpanInfo) visit((JavaNode) node, data); + result.deleteVar(node.getExceptionId()); + return result; + } + + @Override + public Object visit(ASTLambdaExpression node, Object data) { + // Lambda expression have control flow that is separate from the method + // So we fork the context, but don't join it + + // Reaching definitions of the enclosing context still reach in the lambda + // Since those definitions are [effectively] final, they actually can't be + // killed, but they can be used in the lambda + + SpanInfo before = (SpanInfo) data; + + JavaNode lambdaBody = node.getChild(node.getNumChildren() - 1); + // if it's an expression, then no assignments may occur in it, + // but it can still use some variables of the context + acceptOpt(lambdaBody, before.forkCapturingNonLocal()); + return before; + } + + @Override + public Object visit(ASTWhileStatement node, Object data) { + return handleLoop(node, (SpanInfo) data, null, node.getCondition(), null, node.getBody(), true, null); + } + + @Override + public Object visit(ASTDoStatement node, Object data) { + return handleLoop(node, (SpanInfo) data, null, node.getCondition(), null, node.getBody(), false, null); + } + + @Override + public Object visit(ASTForStatement node, Object data) { + ASTStatement body = node.getBody(); + if (node.isForeach()) { + // the iterable expression + JavaNode init = node.getChild(1); + ASTVariableDeclaratorId foreachVar = ((ASTLocalVariableDeclaration) node.getChild(0)).iterator().next(); + return handleLoop(node, (SpanInfo) data, init, null, null, body, true, foreachVar); + } else { + ASTForInit init = node.getFirstChildOfType(ASTForInit.class); + ASTExpression cond = node.getCondition(); + ASTForUpdate update = node.getFirstChildOfType(ASTForUpdate.class); + return handleLoop(node, (SpanInfo) data, init, cond, update, body, true, null); + } + } + + + private SpanInfo handleLoop(JavaNode loop, + SpanInfo before, + JavaNode init, + JavaNode cond, + JavaNode update, + JavaNode body, + boolean checkFirstIter, + ASTVariableDeclaratorId foreachVar) { + final GlobalAlgoState globalState = before.global; + + SpanInfo breakTarget = before.forkEmpty(); + SpanInfo continueTarget = before.forkEmpty(); + pushTargets(loop, breakTarget, continueTarget); + + // perform a few "iterations", to make sure that assignments in + // the body can affect themselves in the next iteration, and + // that they affect the condition, etc + + before = acceptOpt(init, before); + if (checkFirstIter && cond != null) { // false for do-while + SpanInfo ifcondTrue = before.forkEmpty(); + linkConditional(before, cond, ifcondTrue, breakTarget, true); + before = ifcondTrue; + } + + if (foreachVar != null) { + // in foreach loops, the loop variable is assigned before the first iteration + before.assign(foreachVar, foreachVar); + } + + + // make the defs of the body reach the other parts of the loop, + // including itself + SpanInfo iter = acceptOpt(body, before.fork()); + + if (foreachVar != null && iter.hasVar(foreachVar)) { + // in foreach loops, the loop variable is reassigned on each update + iter.assign(foreachVar, foreachVar); + } else { + iter = acceptOpt(update, iter); + } + + linkConditional(iter, cond, iter, breakTarget, true); + iter = acceptOpt(body, iter); + + + breakTarget = globalState.breakTargets.peek(); + continueTarget = globalState.continueTargets.peek(); + if (!continueTarget.symtable.isEmpty()) { + // make assignments before a continue reach the other parts of the loop + + linkConditional(continueTarget, cond, continueTarget, breakTarget, true); + + continueTarget = acceptOpt(body, continueTarget); + continueTarget = acceptOpt(update, continueTarget); + } + + SpanInfo result = popTargets(loop, breakTarget, continueTarget); + result = result.absorb(iter); + if (checkFirstIter) { + // if the first iteration is checked, + // then it could be false on the first try, meaning + // the definitions before the loop reach after too + result = result.absorb(before); + } + + if (foreachVar != null) { + result.deleteVar(foreachVar); + } + + return result; + } + + private void pushTargets(JavaNode loop, SpanInfo breakTarget, SpanInfo continueTarget) { + GlobalAlgoState globalState = breakTarget.global; + globalState.breakTargets.unnamedTargets.push(breakTarget); + globalState.continueTargets.unnamedTargets.push(continueTarget); + + Node parent = loop.getNthParent(2); + while (parent instanceof ASTLabeledStatement) { + String label = parent.getImage(); + globalState.breakTargets.namedTargets.put(label, breakTarget); + globalState.continueTargets.namedTargets.put(label, continueTarget); + parent = parent.getNthParent(2); + } + } + + private SpanInfo popTargets(JavaNode loop, SpanInfo breakTarget, SpanInfo continueTarget) { + GlobalAlgoState globalState = breakTarget.global; + globalState.breakTargets.unnamedTargets.pop(); + globalState.continueTargets.unnamedTargets.pop(); + + SpanInfo total = breakTarget.absorb(continueTarget); + + Node parent = loop.getNthParent(2); + while (parent instanceof ASTLabeledStatement) { + String label = parent.getImage(); + total = total.absorb(globalState.breakTargets.namedTargets.remove(label)); + total = total.absorb(globalState.continueTargets.namedTargets.remove(label)); + parent = parent.getNthParent(2); + } + return total; + } + + private SpanInfo acceptOpt(JavaNode node, SpanInfo before) { + return node == null ? before : (SpanInfo) node.jjtAccept(this, before); + } + + @Override + public Object visit(ASTContinueStatement node, Object data) { + SpanInfo state = (SpanInfo) data; + return state.global.continueTargets.doBreak(state, node.getImage()); + } + + @Override + public Object visit(ASTBreakStatement node, Object data) { + SpanInfo state = (SpanInfo) data; + return state.global.breakTargets.doBreak(state, node.getImage()); + } + + @Override + public Object visit(ASTYieldStatement node, Object data) { + super.visit(node, data); // visit expression + + SpanInfo state = (SpanInfo) data; + // treat as break, ie abrupt completion + link reaching defs to outer context + return state.global.breakTargets.doBreak(state, null); + } + + + // both of those exit the scope of the method/ctor, so their assignments go dead + + @Override + public Object visit(ASTThrowStatement node, Object data) { + super.visit(node, data); + return ((SpanInfo) data).abruptCompletion(null); + } + + @Override + public Object visit(ASTReturnStatement node, Object data) { + super.visit(node, data); + return ((SpanInfo) data).abruptCompletion(null); + } + + // following deals with assignment + + @Override + public Object visit(ASTFormalParameter node, Object data) { + if (!node.isExplicitReceiverParameter()) { + ASTVariableDeclaratorId id = node.getVariableDeclaratorId(); + ((SpanInfo) data).assign(id, id); + } + return data; + } + + @Override + public Object visit(ASTVariableDeclarator node, Object data) { + ASTVariableDeclaratorId var = node.getVariableId(); + ASTVariableInitializer rhs = node.getInitializer(); + if (rhs != null) { + rhs.jjtAccept(this, data); + ((SpanInfo) data).assign(var, rhs); + } else { + ((SpanInfo) data).assign(var, node.getVariableId()); + } + return data; + } + + + @Override + public Object visit(ASTExpression node, Object data) { + return checkAssignment(node, data); + } + + @Override + public Object visit(ASTStatementExpression node, Object data) { + return checkAssignment(node, data); + } + + public Object checkAssignment(JavaNode node, Object data) { + SpanInfo result = (SpanInfo) data; + if (node.getNumChildren() == 3) { + // assignment + assert node.getChild(1) instanceof ASTAssignmentOperator; + + // visit the rhs as it is evaluated before + JavaNode rhs = node.getChild(2); + result = acceptOpt(rhs, result); + + ASTVariableDeclaratorId lhsVar = getVarFromExpression(node.getChild(0), true); + if (lhsVar != null) { + // in that case lhs is a normal variable (array access not supported) + + if (node.getChild(1).getImage().length() >= 2) { + // compound assignment, to use BEFORE assigning + result.use(lhsVar); + } + + result.assign(lhsVar, rhs); + } else { + result = acceptOpt(node.getChild(0), result); + } + return result; + } else { + return visit(node, data); + } + } + + @Override + public Object visit(ASTPreDecrementExpression node, Object data) { + return checkIncOrDecrement(node, (SpanInfo) data); + } + + @Override + public Object visit(ASTPreIncrementExpression node, Object data) { + return checkIncOrDecrement(node, (SpanInfo) data); + } + + @Override + public Object visit(ASTPostfixExpression node, Object data) { + return checkIncOrDecrement(node, (SpanInfo) data); + } + + private SpanInfo checkIncOrDecrement(JavaNode unary, SpanInfo data) { + ASTVariableDeclaratorId var = getVarFromExpression(unary.getChild(0), true); + if (var != null) { + data.use(var); + data.assign(var, unary); + } + return data; + } + + // variable usage + + @Override + public Object visit(ASTPrimaryExpression node, Object data) { + SpanInfo state = (SpanInfo) visit((JavaNode) node, data); // visit subexpressions + + ASTVariableDeclaratorId var = getVarFromExpression(node, false); + if (var != null) { + state.use(var); + } + return state; + } + + /** + * Get the variable accessed from a primary. + */ + private ASTVariableDeclaratorId getVarFromExpression(JavaNode primary, boolean inLhs) { + + if (primary instanceof ASTPrimaryExpression) { + ASTPrimaryPrefix prefix = (ASTPrimaryPrefix) primary.getChild(0); + + + // this.x = 2; + if (prefix.usesThisModifier() && this.enclosingClassScope != null) { + if (primary.getNumChildren() < 2 || primary.getNumChildren() > 2 && inLhs) { + return null; + } + + ASTPrimarySuffix suffix = (ASTPrimarySuffix) primary.getChild(1); + if (suffix.getImage() == null) { + // catches arrays and such + return null; + } + + return findVar(primary.getScope(), true, suffix.getImage()); + } else { + if (prefix.getNumChildren() > 0 && prefix.getChild(0) instanceof ASTName) { + String prefixImage = prefix.getChild(0).getImage(); + String varname = identOf(inLhs, prefixImage); + if (primary.getNumChildren() > 1) { + if (primary.getNumChildren() > 2 && inLhs) { + // this is for chains like `foo.m().field = 3` + return null; + } + ASTPrimarySuffix suffix = (ASTPrimarySuffix) primary.getChild(1); + if (suffix.isArguments()) { + // then the prefix has the method name + varname = methodLhsName(prefixImage); + } else if (suffix.isArrayDereference() && inLhs) { + return null; + } + } + return findVar(prefix.getScope(), false, varname); + } + } + } + + return null; + } + + private static String identOf(boolean inLhs, String str) { + int i = str.indexOf('.'); + if (i < 0) { + return str; + } else if (inLhs) { + // a qualified name in LHS, so + // the assignment doesn't assign the variable but one of its fields + return null; + } + return str.substring(0, i); + } + + private static String methodLhsName(String name) { + int i = name.indexOf('.'); + return i < 0 ? null // no lhs, the name is just the method name + : name.substring(0, i); + } + + private ASTVariableDeclaratorId findVar(Scope scope, boolean isField, String name) { + if (name == null) { + return null; + } + + if (isField) { + return getFromSingleScope(enclosingClassScope, name); + } + + while (scope != null) { + ASTVariableDeclaratorId result = getFromSingleScope(scope, name); + if (result != null) { + if (scope instanceof ClassScope && scope != enclosingClassScope) { // NOPMD CompareObjectsWithEqual this is what we want + // don't handle fields + return null; + } + return result; + } + + scope = scope.getParent(); + } + + return null; + } + + private ASTVariableDeclaratorId getFromSingleScope(Scope scope, String name) { + if (scope != null) { + for (VariableNameDeclaration decl : scope.getDeclarations(VariableNameDeclaration.class).keySet()) { + if (decl.getImage().equals(name)) { + return (ASTVariableDeclaratorId) decl.getNode(); + } + } + } + return null; + } + + + // ctor/initializer handling + + // this is the common denominator between anonymous class & astAnyTypeDeclaration on master + + @Override + public Object visit(ASTClassOrInterfaceBody node, Object data) { + visitTypeBody(node, (SpanInfo) data); + return data; // type doesn't contribute anything to the enclosing control flow + } + + @Override + public Object visit(ASTEnumBody node, Object data) { + visitTypeBody(node, (SpanInfo) data); + return data; // type doesn't contribute anything to the enclosing control flow + } + + + private void visitTypeBody(JavaNode typeBody, SpanInfo data) { + List declarations = typeBody.findChildrenOfType(ASTAnyTypeBodyDeclaration.class); + processInitializers(declarations, data, (ClassScope) typeBody.getScope()); + + for (ASTAnyTypeBodyDeclaration decl : declarations) { + JavaNode d = decl.getDeclarationNode(); + if (d instanceof ASTMethodDeclaration) { + ONLY_LOCALS.acceptOpt(d, data.forkCapturingNonLocal()); + } else if (d instanceof ASTAnyTypeDeclaration) { + JavaNode body = d.getChild(d.getNumChildren() - 1); + visitTypeBody(body, data.forkEmptyNonLocal()); + } + } + } + + + private static void processInitializers(List declarations, + SpanInfo beforeLocal, + ClassScope scope) { + + ReachingDefsVisitor visitor = new ReachingDefsVisitor(scope); + + // All field initializers + instance initializers + SpanInfo ctorHeader = beforeLocal.forkCapturingNonLocal(); + // All static field initializers + static initializers + SpanInfo staticInit = beforeLocal.forkEmptyNonLocal(); + + List ctors = new ArrayList<>(); + + for (ASTAnyTypeBodyDeclaration declaration : declarations) { + JavaNode node = declaration.getDeclarationNode(); + + final boolean isStatic; + if (node instanceof ASTFieldDeclaration) { + isStatic = ((ASTFieldDeclaration) node).isStatic(); + } else if (node instanceof ASTInitializer) { + isStatic = ((ASTInitializer) node).isStatic(); + } else if (node instanceof ASTConstructorDeclaration) { + ctors.add((ASTConstructorDeclaration) node); + continue; + } else { + continue; + } + + if (isStatic) { + staticInit = visitor.acceptOpt(node, staticInit); + } else { + ctorHeader = visitor.acceptOpt(node, ctorHeader); + } + } + + + SpanInfo ctorEndState = ctors.isEmpty() ? ctorHeader : null; + for (ASTConstructorDeclaration ctor : ctors) { + SpanInfo state = visitor.acceptOpt(ctor, ctorHeader.forkCapturingNonLocal()); + ctorEndState = ctorEndState == null ? state : ctorEndState.absorb(state); + } + + // assignments that reach the end of any constructor must + // be considered used + for (VariableNameDeclaration field : visitor.enclosingClassScope.getVariableDeclarations().keySet()) { + ASTVariableDeclaratorId var = (ASTVariableDeclaratorId) field.getNode(); + if (field.getAccessNodeParent().isStatic()) { + staticInit.use(var); + } + ctorEndState.use(var); + } + } + } + + /** + * The shared state for all {@link SpanInfo} instances in the same + * toplevel class. + */ + private static class GlobalAlgoState { + + final Set allAssignments; + final Set usedAssignments; + + // track which assignments kill which + // assignment -> killers(assignment) + final Map> killRecord; + + final TargetStack breakTargets = new TargetStack(); + // continue jumps to the condition check, while break jumps to after the loop + final TargetStack continueTargets = new TargetStack(); + + private GlobalAlgoState(Set allAssignments, + Set usedAssignments, + Map> killRecord) { + this.allAssignments = allAssignments; + this.usedAssignments = usedAssignments; + this.killRecord = killRecord; + + } + + private GlobalAlgoState() { + this(new HashSet(), + new HashSet(), + new HashMap>()); + } + } + + // Information about a variable in a code span. + static class VarLocalInfo { + + Set reachingDefs; + + VarLocalInfo(Set reachingDefs) { + this.reachingDefs = reachingDefs; + } + + VarLocalInfo absorb(VarLocalInfo other) { + if (other == this) { + return this; + } + Set merged = new HashSet<>(reachingDefs.size() + other.reachingDefs.size()); + merged.addAll(reachingDefs); + merged.addAll(other.reachingDefs); + return new VarLocalInfo(merged); + } + + @Override + public String toString() { + return "VarLocalInfo{reachingDefs=" + reachingDefs + '}'; + } + + public VarLocalInfo copy() { + return new VarLocalInfo(this.reachingDefs); + } + } + + /** + * Information about a span of code. + */ + private static class SpanInfo { + + // spans are arranged in a tree, to look for enclosing finallies + // when abrupt completion occurs. Blocks that have non-local + // control-flow (lambda bodies, anonymous classes, etc) aren't + // linked to the outer parents. + final SpanInfo parent; + + // If != null, then abrupt completion in this span of code (and any descendant) + // needs to go through the finally span (the finally must absorb it) + SpanInfo myFinally = null; + + final GlobalAlgoState global; + + final Map symtable; + + private SpanInfo(GlobalAlgoState global) { + this(null, global, new HashMap()); + } + + private SpanInfo(SpanInfo parent, + GlobalAlgoState global, + Map symtable) { + this.parent = parent; + this.global = global; + this.symtable = symtable; + } + + boolean hasVar(ASTVariableDeclaratorId var) { + return symtable.containsKey(var); + } + + void assign(ASTVariableDeclaratorId var, JavaNode rhs) { + AssignmentEntry entry = new AssignmentEntry(var, rhs); + VarLocalInfo previous = symtable.put(var, new VarLocalInfo(Collections.singleton(entry))); + if (previous != null) { + // those assignments were overwritten ("killed") + for (AssignmentEntry killed : previous.reachingDefs) { + if (killed.rhs instanceof ASTVariableDeclaratorId + && killed.rhs.getParent() instanceof ASTVariableDeclarator + && killed.rhs != rhs) { + continue; + } + // java8: computeIfAbsent + Set killers = global.killRecord.get(killed); + if (killers == null) { + killers = new HashSet<>(1); + global.killRecord.put(killed, killers); + } + killers.add(entry); + } + } + global.allAssignments.add(entry); + } + + void use(ASTVariableDeclaratorId var) { + VarLocalInfo info = symtable.get(var); + // may be null for implicit assignments, like method parameter + if (info != null) { + global.usedAssignments.addAll(info.reachingDefs); + } + } + + void deleteVar(ASTVariableDeclaratorId var) { + symtable.remove(var); + } + + // Forks duplicate this context, to preserve the reaching defs + // of the current context while analysing a sub-block + // Forks must be merged later if control flow merges again, see ::absorb + + SpanInfo fork() { + return doFork(this, copyTable()); + } + + SpanInfo forkEmpty() { + return doFork(this, new HashMap()); + } + + + SpanInfo forkEmptyNonLocal() { + return doFork(null, new HashMap()); + } + + SpanInfo forkCapturingNonLocal() { + return doFork(null, copyTable()); + } + + private Map copyTable() { + HashMap copy = new HashMap<>(this.symtable.size()); + for (ASTVariableDeclaratorId var : this.symtable.keySet()) { + copy.put(var, this.symtable.get(var).copy()); + } + return copy; + } + + private SpanInfo doFork(SpanInfo parent, Map reaching) { + return new SpanInfo(parent, this.global, reaching); + } + + SpanInfo abruptCompletion(SpanInfo target) { + // if target == null then this will unwind all the parents + SpanInfo parent = this; + while (parent != target && parent != null) { // NOPMD CompareObjectsWithEqual this is what we want + if (parent.myFinally != null) { + parent.myFinally.absorb(this); + } + parent = parent.parent; + } + + this.symtable.clear(); + return this; + } + + + SpanInfo absorb(SpanInfo other) { + // Merge reaching defs of the other scope into this + // This is used to join paths after the control flow has forked + + // a spanInfo may be absorbed several times so this method should not + // destroy the parameter + if (other == this || other == null || other.symtable.isEmpty()) { + return this; + } + + // we don't have to double the capacity since they're normally of the same size + // (vars are deleted when exiting a block) + Set keysUnion = new HashSet<>(this.symtable.keySet()); + keysUnion.addAll(other.symtable.keySet()); + + for (ASTVariableDeclaratorId var : keysUnion) { + VarLocalInfo thisInfo = this.symtable.get(var); + VarLocalInfo otherInfo = other.symtable.get(var); + if (thisInfo == otherInfo) { // NOPMD CompareObjectsWithEqual this is what we want + continue; + } + if (otherInfo != null && thisInfo != null) { + this.symtable.put(var, thisInfo.absorb(otherInfo)); + } else if (otherInfo != null) { + this.symtable.put(var, otherInfo.copy()); + } + } + return this; + } + + @Override + public String toString() { + return symtable.toString(); + } + } + + static class TargetStack { + + final Deque unnamedTargets = new ArrayDeque<>(); + final Map namedTargets = new HashMap<>(); + + + void push(SpanInfo state) { + unnamedTargets.push(state); + } + + SpanInfo pop() { + return unnamedTargets.pop(); + } + + SpanInfo peek() { + return unnamedTargets.getFirst(); + } + + SpanInfo doBreak(SpanInfo data, /* nullable */ String label) { + // basically, reaching defs at the point of the break + // also reach after the break (wherever it lands) + SpanInfo target; + if (label == null) { + target = unnamedTargets.getFirst(); + } else { + target = namedTargets.get(label); + } + + if (target != null) { // otherwise CT error + target.absorb(data); + } + return data.abruptCompletion(target); + } + } + + static class AssignmentEntry { + + final ASTVariableDeclaratorId var; + + // this is not necessarily an expression, it may be also the + // variable declarator of a foreach loop + final JavaNode rhs; + + AssignmentEntry(ASTVariableDeclaratorId var, JavaNode rhs) { + this.var = var; + this.rhs = rhs; + } + + @Override + public String toString() { + return var.getName() + " := " + rhs; + } + + @Override + public boolean equals(Object o) { + if (this == o) { + return true; + } + if (o == null || getClass() != o.getClass()) { + return false; + } + AssignmentEntry that = (AssignmentEntry) o; + return Objects.equals(rhs, that.rhs); + } + + @Override + public int hashCode() { + return rhs.hashCode(); + } + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedImportsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedImportsRule.java index ce0966823e..d14baf6077 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedImportsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedImportsRule.java @@ -1,4 +1,4 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ @@ -20,7 +20,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.Comment; -import net.sourceforge.pmd.lang.java.ast.DummyJavaNode; import net.sourceforge.pmd.lang.java.ast.FormalComment; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; @@ -83,16 +82,16 @@ public class UnusedImportsRule extends AbstractJavaRule { for (Pattern p : PATTERNS) { Matcher m = p.matcher(comment.getImage()); while (m.find()) { - String s = m.group(1); + String fullname = m.group(1); - if (s != null) { // may be null for "@see #" and "@link #" - imports.remove(new ImportWrapper(s, s, new DummyJavaNode(-1))); + if (fullname != null) { // may be null for "@see #" and "@link #" + imports.remove(new ImportWrapper(fullname, fullname)); } if (m.groupCount() > 1) { - s = m.group(2); - if (s != null) { - String[] params = s.split("\\s*,\\s*"); + fullname = m.group(2); + if (fullname != null) { + String[] params = fullname.split("\\s*,\\s*"); for (String param : params) { final int firstDot = param.indexOf('.'); final String expectedImportName; @@ -101,7 +100,7 @@ public class UnusedImportsRule extends AbstractJavaRule { } else { expectedImportName = param.substring(0, firstDot); } - imports.remove(new ImportWrapper(param, expectedImportName, new DummyJavaNode(-1))); + imports.remove(new ImportWrapper(param, expectedImportName)); } } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java index 95e0c05fb6..b0ef3262e1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/ConfusingTernaryRule.java @@ -16,6 +16,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus; +import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -65,9 +66,9 @@ public class ConfusingTernaryRule extends AbstractJavaRule { public Object visit(ASTIfStatement node, Object data) { // look for "if (match) ..; else .." if (node.getNumChildren() == 3) { - Node inode = node.getChild(0); + JavaNode inode = node.getChild(0); if (inode instanceof ASTExpression && inode.getNumChildren() == 1) { - Node jnode = inode.getChild(0); + JavaNode jnode = inode.getChild(0); if (isMatch(jnode)) { if (!getProperty(ignoreElseIfProperty) @@ -85,7 +86,7 @@ public class ConfusingTernaryRule extends AbstractJavaRule { public Object visit(ASTConditionalExpression node, Object data) { // look for "match ? .. : .." if (node.getNumChildren() > 0) { - Node inode = node.getChild(0); + JavaNode inode = node.getChild(0); if (isMatch(inode)) { addViolation(data, node); } @@ -94,9 +95,9 @@ public class ConfusingTernaryRule extends AbstractJavaRule { } // recursive! - private static boolean isMatch(Node node) { - return isUnaryNot(node) || isNotEquals(node) || isConditionalWithAllMatches(node) - || isParenthesisAroundMatch(node); + private static boolean isMatch(JavaNode node) { + node = unwrapParentheses(node); + return isUnaryNot(node) || isNotEquals(node) || isConditionalWithAllMatches(node); } private static boolean isUnaryNot(Node node) { @@ -109,7 +110,7 @@ public class ConfusingTernaryRule extends AbstractJavaRule { return node instanceof ASTEqualityExpression && "!=".equals(node.getImage()); } - private static boolean isConditionalWithAllMatches(Node node) { + private static boolean isConditionalWithAllMatches(JavaNode node) { // look for "match && match" or "match || match" if (!(node instanceof ASTConditionalAndExpression) && !(node instanceof ASTConditionalOrExpression)) { return false; @@ -119,7 +120,7 @@ public class ConfusingTernaryRule extends AbstractJavaRule { return false; } for (int i = 0; i < n; i++) { - Node inode = node.getChild(i); + JavaNode inode = node.getChild(i); // recurse! if (!isMatch(inode)) { return false; @@ -129,21 +130,31 @@ public class ConfusingTernaryRule extends AbstractJavaRule { return true; } - private static boolean isParenthesisAroundMatch(Node node) { + /** + * Extracts the outermost node that is not a parenthesized + * expression. + * + * @deprecated This is internal API, because it will be removed in PMD 7. + * In PMD 7 there are no additional layers for parentheses in the Java tree. + */ + @Deprecated + public static JavaNode unwrapParentheses(final JavaNode top) { + JavaNode node = top; // look for "(match)" if (!(node instanceof ASTPrimaryExpression) || node.getNumChildren() != 1) { - return false; + return top; } - Node inode = node.getChild(0); - if (!(inode instanceof ASTPrimaryPrefix) || inode.getNumChildren() != 1) { - return false; + node = node.getChild(0); + if (!(node instanceof ASTPrimaryPrefix) || node.getNumChildren() != 1) { + return top; } - Node jnode = inode.getChild(0); - if (!(jnode instanceof ASTExpression) || jnode.getNumChildren() != 1) { - return false; + node = node.getChild(0); + if (!(node instanceof ASTExpression) || node.getNumChildren() != 1) { + return top; } - Node knode = jnode.getChild(0); - // recurse! - return isMatch(knode); + node = node.getChild(0); + + // recurse to unwrap another layer if possible + return unwrapParentheses(node); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java index 2d91046194..69c0beffed 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/LawOfDemeterRule.java @@ -15,6 +15,7 @@ import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTAssignmentOperator; import net.sourceforge.pmd.lang.java.ast.ASTBlock; +import net.sourceforge.pmd.lang.java.ast.ASTCastExpression; import net.sourceforge.pmd.lang.java.ast.ASTForStatement; import net.sourceforge.pmd.lang.java.ast.ASTLiteral; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; @@ -146,7 +147,9 @@ public class LawOfDemeterRule extends AbstractJavaRule { if (firstMethodCallInChain.isNotBuilder()) { List suffixes = findSuffixesWithoutArguments(expression); for (ASTPrimarySuffix suffix : suffixes) { - result.add(new MethodCall(expression, suffix)); + if (!expression.hasDescendantOfType(ASTCastExpression.class)) { + result.add(new MethodCall(expression, suffix)); + } } } } @@ -175,7 +178,7 @@ public class LawOfDemeterRule extends AbstractJavaRule { private static List findSuffixesWithoutArguments(ASTPrimaryExpression expr) { List result = new ArrayList<>(); if (hasRealPrefix(expr)) { - List suffixes = expr.findDescendantsOfType(ASTPrimarySuffix.class); + List suffixes = expr.findChildrenOfType(ASTPrimarySuffix.class); for (ASTPrimarySuffix suffix : suffixes) { if (!suffix.isArguments()) { result.add(suffix); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java index eee41ce882..6a8be010f1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidInstantiatingObjectsInLoopsRule.java @@ -4,38 +4,114 @@ package net.sourceforge.pmd.lang.java.rule.performance; +import java.util.Collection; + import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; +import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTBlock; +import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; +import net.sourceforge.pmd.lang.java.ast.ASTBreakStatement; import net.sourceforge.pmd.lang.java.ast.ASTDoStatement; import net.sourceforge.pmd.lang.java.ast.ASTForInit; import net.sourceforge.pmd.lang.java.ast.ASTForStatement; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; +import net.sourceforge.pmd.lang.java.ast.ASTStatementExpression; import net.sourceforge.pmd.lang.java.ast.ASTThrowStatement; import net.sourceforge.pmd.lang.java.ast.ASTWhileStatement; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; -public class AvoidInstantiatingObjectsInLoopsRule extends AbstractOptimizationRule { +public class AvoidInstantiatingObjectsInLoopsRule extends AbstractJavaRule { + public AvoidInstantiatingObjectsInLoopsRule() { + addRuleChainVisit(ASTAllocationExpression.class); + } + + /** + * This method is used to check whether user instantiates variables + * which are not assigned to arrays/lists in loops. + * @param node This is the expression of part of java code to be checked. + * @param data This is the data to return. + * @return Object This returns the data passed in. If violation happens, violation is added to data. + */ @Override public Object visit(ASTAllocationExpression node, Object data) { - if (insideLoop(node) && fourthParentNotThrow(node) && fourthParentNotReturn(node)) { + if (notInsideLoop(node)) { + return data; + } + + if (fourthParentNotThrow(node) + && fourthParentNotReturn(node) + && notArrayAssignment(node) + && notCollectionAccess(node) + && notBreakFollowing(node)) { addViolation(data, node); } return data; } + private boolean notArrayAssignment(ASTAllocationExpression node) { + if (node.getNthParent(4) instanceof ASTStatementExpression) { + ASTPrimaryExpression assignee = node.getNthParent(4).getFirstChildOfType(ASTPrimaryExpression.class); + ASTPrimarySuffix suffix = assignee.getFirstChildOfType(ASTPrimarySuffix.class); + return suffix == null || !suffix.isArrayDereference(); + } + return true; + } + + private boolean notCollectionAccess(ASTAllocationExpression node) { + if (node.getNthParent(4) instanceof ASTArgumentList && node.getNthParent(8) instanceof ASTStatementExpression) { + ASTStatementExpression statement = (ASTStatementExpression) node.getNthParent(8); + return !TypeHelper.isA(statement, Collection.class); + } + return true; + } + + private boolean notBreakFollowing(ASTAllocationExpression node) { + ASTBlockStatement blockStatement = node.getFirstParentOfType(ASTBlockStatement.class); + if (blockStatement != null) { + ASTBlock block = blockStatement.getFirstParentOfType(ASTBlock.class); + if (block.getNumChildren() > blockStatement.getIndexInParent() + 1) { + ASTBlockStatement next = (ASTBlockStatement) block.getChild(blockStatement.getIndexInParent() + 1); + if (next.getNumChildren() == 1 && next.getChild(0).getNumChildren() == 1) { + return !(next.getChild(0).getChild(0) instanceof ASTBreakStatement); + } + } + } + return true; + } + + /** + * This method is used to check whether this expression is a throw statement. + * @param node This is the expression of part of java code to be checked. + * @return boolean This returns Whether the fourth parent of node is an instance of throw statement. + */ private boolean fourthParentNotThrow(ASTAllocationExpression node) { - return !(node.getParent().getParent().getParent().getParent() instanceof ASTThrowStatement); + return !(node.getNthParent(4) instanceof ASTThrowStatement); } + /** + * This method is used to check whether this expression is a return statement. + * @param node This is the expression of part of java code to be checked. + * @return boolean This returns Whether the fourth parent of node is an instance of return statement. + */ private boolean fourthParentNotReturn(ASTAllocationExpression node) { - return !(node.getParent().getParent().getParent().getParent() instanceof ASTReturnStatement); + return !(node.getNthParent(4) instanceof ASTReturnStatement); } - private boolean insideLoop(ASTAllocationExpression node) { + /** + * This method is used to check whether this expression is not in a loop. + * @param node This is the expression of part of java code to be checked. + * @return boolean false if the given node is inside a loop, true otherwise + */ + private boolean notInsideLoop(ASTAllocationExpression node) { Node n = node.getParent(); while (n != null) { if (n instanceof ASTDoStatement || n instanceof ASTWhileStatement || n instanceof ASTForStatement) { - return true; + return false; } else if (n instanceof ASTForInit) { /* * init part is not technically inside the loop. Skip parent @@ -55,6 +131,6 @@ public class AvoidInstantiatingObjectsInLoopsRule extends AbstractOptimizationRu } n = n.getParent(); } - return false; + return true; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UseStringBufferForStringAppendsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UseStringBufferForStringAppendsRule.java index 283a38d3b0..30619175ba 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UseStringBufferForStringAppendsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UseStringBufferForStringAppendsRule.java @@ -30,6 +30,12 @@ public class UseStringBufferForStringAppendsRule extends AbstractJavaRule { addRuleChainVisit(ASTVariableDeclaratorId.class); } + /** + * This method is used to check whether user appends string directly instead of using StringBuffer or StringBuilder + * @param node This is the expression of part of java code to be checked. + * @param data This is the data to return. + * @return Object This returns the data passed in. If violation happens, violation is added to data. + */ @Override public Object visit(ASTVariableDeclaratorId node, Object data) { if (!TypeHelper.isA(node, String.class) || node.isArray() @@ -37,6 +43,9 @@ public class UseStringBufferForStringAppendsRule extends AbstractJavaRule { return data; } + // Remember how often we the variable has been used + int usageCounter = 0; + for (NameOccurrence no : node.getUsages()) { Node name = no.getLocation(); ASTStatementExpression statement = name.getFirstParentOfType(ASTStatementExpression.class); @@ -74,17 +83,36 @@ public class UseStringBufferForStringAppendsRule extends AbstractJavaRule { if (statement.getNumChildren() > 0 && statement.getChild(0) instanceof ASTPrimaryExpression) { ASTName astName = statement.getChild(0).getFirstDescendantOfType(ASTName.class); if (astName != null) { + ASTAssignmentOperator assignmentOperator = statement + .getFirstDescendantOfType(ASTAssignmentOperator.class); if (astName.equals(name)) { - ASTAssignmentOperator assignmentOperator = statement - .getFirstDescendantOfType(ASTAssignmentOperator.class); if (assignmentOperator != null && assignmentOperator.isCompound()) { - addViolation(data, assignmentOperator); + if (isWithinLoop(name)) { + // always report within a loop + addViolation(data, assignmentOperator); + } else { + usageCounter++; + if (usageCounter > 1) { + // only report, if it is not the first time + addViolation(data, assignmentOperator); + } + } } - } else if (astName.getImage().equals(name.getImage())) { - ASTAssignmentOperator assignmentOperator = statement - .getFirstDescendantOfType(ASTAssignmentOperator.class); + } else if (astName.hasImageEqualTo(name.getImage())) { if (assignmentOperator != null && !assignmentOperator.isCompound()) { - addViolation(data, astName); + if (isWithinLoop(name)) { + // always report within a loop + addViolation(data, assignmentOperator); + } else { + usageCounter++; + if (usageCounter > 1) { + // only report, if it is not the first time + addViolation(data, assignmentOperator); + } + } + } else if (assignmentOperator != null && assignmentOperator.isCompound() + && usageCounter >= 1) { + addViolation(data, assignmentOperator); } } } @@ -98,4 +126,8 @@ public class UseStringBufferForStringAppendsRule extends AbstractJavaRule { && name.getFirstParentOfType(ASTWhileStatement.class) == null && name.getFirstParentOfType(ASTDoStatement.class) == null; } + + private boolean isWithinLoop(Node name) { + return !isNotWithinLoop(name); + } } diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 9c2dbf7138..6bc9247e8e 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1298,6 +1298,121 @@ class Foo{ + + + Reports assignments to variables that are never used before the variable is overwritten, + or goes out of scope. Unused assignments are those for which + 1. The variable is never read after the assignment, or + 2. The assigned value is always overwritten by other assignments before the next read of + the variable. + + The rule doesn't consider assignments to fields except for those of `this` in a constructor, + or static fields of the current class in static initializers. + + The rule may be suppressed with the standard `@SuppressWarnings("unused")` tag. + + The rule subsumes {% rule "UnusedLocalVariable" %}, and {% rule "UnusedFormalParameter" %}. + Those violations are filtered + out by default, in case you already have enabled those rules, but may be enabled with the property + `reportUnusedVariables`. Variables whose name starts with `ignored` are filtered out, as + is standard practice for exceptions. + + 3 + + + + + + + + + + + + + + --> + diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentTest.java new file mode 100644 index 0000000000..7d363ef084 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedAssignmentTest.java @@ -0,0 +1,11 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import net.sourceforge.pmd.testframework.PmdRuleTst; + +public class UnusedAssignmentTest extends PmdRuleTst { + // no additional unit tests +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/ClassScopeTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/ClassScopeTest.java index a847fa13ec..ad32bbabcc 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/ClassScopeTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/symboltable/ClassScopeTest.java @@ -1,4 +1,4 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ @@ -19,8 +19,9 @@ import net.sourceforge.pmd.PMD; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; 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.ASTVariableDeclaratorId; -import net.sourceforge.pmd.lang.java.ast.DummyJavaNode; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.symboltable.testdata.InnerClass; import net.sourceforge.pmd.lang.java.symboltable.testdata.InnerClass.TheInnerClass; @@ -69,23 +70,23 @@ public class ClassScopeTest extends BaseNonParserTest { @Test public void testCantContainsSuperToString() { - ClassNameDeclaration classDeclaration = new ClassNameDeclaration(null); + ASTCompilationUnit cu = java.parse("class Foo { public String toString() { return super.toString(); } }"); + ClassNameDeclaration classDeclaration = new ClassNameDeclaration(cu.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class)); ClassScope s = new ClassScope("Foo", classDeclaration); - JavaNode node = new DummyJavaNode(1); - node.setImage("super.toString"); - assertFalse(s.contains(new JavaNameOccurrence(node, node.getImage()))); + JavaNode node = cu.getFirstDescendantOfType(ASTPrimaryExpression.class); // "super.toString();" + assertFalse(s.contains(new JavaNameOccurrence(node, "super.toString"))); } @Test public void testContainsStaticVariablePrefixedWithClassName() { - ClassNameDeclaration classDeclaration = new ClassNameDeclaration(null); + ASTCompilationUnit cu = java.parse("class Foo { static int X; public int bar() { return Foo.X; } }"); + + ClassNameDeclaration classDeclaration = new ClassNameDeclaration(cu.getFirstDescendantOfType(ASTClassOrInterfaceDeclaration.class)); ClassScope s = new ClassScope("Foo", classDeclaration); - ASTVariableDeclaratorId node = new ASTVariableDeclaratorId(1); - node.setImage("X"); + ASTVariableDeclaratorId node = cu.getFirstDescendantOfType(ASTVariableDeclaratorId.class); s.addDeclaration(new VariableNameDeclaration(node)); - JavaNode node2 = new DummyJavaNode(2); - node2.setImage("Foo.X"); + JavaNode node2 = cu.getFirstDescendantOfType(ASTName.class); assertTrue(s.contains(new JavaNameOccurrence(node2, node2.getImage()))); } diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTCatchStatementTest.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTCatchStatementTest.kt index 80eb0b2c3a..8c4d34181f 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTCatchStatementTest.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ASTCatchStatementTest.kt @@ -3,7 +3,6 @@ package net.sourceforge.pmd.lang.java.ast import io.kotlintest.matchers.collections.shouldContainExactly import io.kotlintest.should import io.kotlintest.shouldBe -import io.kotlintest.specs.FunSpec 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 diff --git a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParserTestSpec.kt b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParserTestSpec.kt index d228c5d0d1..741b2bff65 100644 --- a/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParserTestSpec.kt +++ b/pmd-java/src/test/kotlin/net/sourceforge/pmd/lang/java/ast/ParserTestSpec.kt @@ -3,7 +3,6 @@ package net.sourceforge.pmd.lang.java.ast import io.kotlintest.AbstractSpec import io.kotlintest.TestContext import io.kotlintest.TestType -import io.kotlintest.specs.IntelliMarker import net.sourceforge.pmd.lang.ast.test.Assertions import io.kotlintest.should as kotlintestShould @@ -15,7 +14,7 @@ import io.kotlintest.should as kotlintestShould * * @author Clément Fournier */ -abstract class ParserTestSpec(body: ParserTestSpec.() -> Unit) : AbstractSpec(), IntelliMarker { +abstract class ParserTestSpec(body: ParserTestSpec.() -> Unit) : AbstractSpec() { init { body() diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedAssignment.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedAssignment.xml new file mode 100644 index 0000000000..f637fb8f9d --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedAssignment.xml @@ -0,0 +1,2786 @@ + + + + + ok + 0 + + + + + DD anomaly + 1 + 3 + + The initializer for variable 'i' is never used (overwritten on line 4) + + + + + + DU anomaly + 0 + + + + DU anomaly (reportUnusedVariables) + true + 1 + + + + + UR anomaly + 0 + + + + + Conditional flow 0 + true + 2 + 3,4 + + The initializer for variable 'j' is never used (overwritten on line 6) + The initializer for variable 'z' is never used + + + + + + Conditional flow 1 + true + 1 + 4 + + The initializer for variable 'z' is never used + + + + + + Conditional flow 2 + 1 + 3 + + The initializer for variable 'j' is never used (overwritten on lines 6 and 8) + + + + + + Conditional flow with abrupt throw + 2 + 3,6 + + The initializer for variable 'j' is never used (overwritten on lines 6 and 9) + The value assigned to variable 'j' is never used + + + + + + Conditional flow with abrupt return + 2 + 3,6 + + The initializer for variable 'j' is never used (overwritten on lines 6 and 9) + The value assigned to variable 'j' is never used + + + + + + Local variable in loop + true + 4 + 2,10,11,19 + + Parameter 'args' is never used + The initializer for variable 'fail' is never used (overwritten on line 19) + Loop variable 'j' is never used + The value assigned to variable 'fail' is never used (reassigned every iteration) + + + + + + #408 Assert statements causing + 0 + + + + + #1905 [java] DataflowAnomalyAnalysis Rule in right order : Case 1. DU-Anomaly(b) + 1 + 6 + + The value assigned to variable 'b' is never used + + + + + + For loop + 0 + + + + + For loop 2 + 2 + 3,5 + + The initializer for variable 'a' is never used (overwritten on line 5) + The value assigned to variable 'a' is never used (reassigned every iteration) + + + + + + For loop 3 + 0 + + + + + For loop 4 + 0 + + + + + Foreach + 1 + 5 + + The value assigned to variable 'a' is never used (overwritten on line 4) + + + + + + Foreach unused + true + 2 + 2,4 + + Parameter 'args' is never used + Loop variable 'a' is never used + + + + + + While loop 1 + 0 + + + + + While loop 2 + 2 + 4,7 + + The initializer for variable 'i' is never used (overwritten on line 7) + The value assigned to variable 'i' is never used (reassigned every iteration) + + + + + While loop with break + 1 + 7 + + The value assigned to variable 'i' is never used + + = 30) { + i = a + 1; // unused + break; + } + a = a + 3; + i = i + 1; // used by itself + } + } +} + ]]> + + + + While loop without break (control case) + 1 + = 30) { + i += a + 1; // unused by below + // break; // no break here + } + a = a + 3; + i = a + 2; // used by above + } + } +} + ]]> + + + + While loop without break 2 (control case) + 1 + 12 + + The value assigned to variable 'i' is never used (overwritten on line 16) + + = 30) { + i += a + 1; // unused because of i = a + 2 + // break outer; + } + a = a + 3; + i = a + 2; // killed by below + } + + i = 2; // used by print + } + + System.out.println(i); // uses i = i + 1 + } +} + ]]> + + + + While loop without break 2 (control case) + 1 + 12 + + The value assigned to variable 'i' is never used (overwritten on line 19) + + = 30) { + i += a + 1; // unused because of i = 2 + break; + } + a = a + 3; + i = a + 2; // used by i += a + 1 + } + + i = 2; // used by print + } + + System.out.println(i); // uses i = i + 1 + } +} + ]]> + + + + While loop with named break 2 + 0 + = 30) { + i += a + 1; // used by print + break outer; + } + a = a + 3; + i = a + 2; // used by i += a + 1 + } + + i = 2; // used by print + } + + System.out.println(i); // uses i = i + 1 + } +} + ]]> + + + + + While loop with continue + 0 + = 30) { + i = a + 1; // used by below + continue; + } + a = a + 3; + i = i + 1; // used by itself + } + } +} + ]]> + + + + While loop with continue 2 + 0 + = 30) { + a = i + 1; // used by loop condition + continue; + } + i++; // used by itself + } + } +} + ]]> + + + + While loop with break (control for continue test above) + 1 + 7 + + The value assigned to variable 'a' is never used + + = 30) { + a = i + 1; // unused + break; + } + i++; // used by itself + } + } +} + ]]> + + + + Do while 0 + 0 + + + + + Do while 1 + 1 + 3 + + The initializer for variable 'a' is never used (overwritten on line 6) + + + + + + Do while with break + 2 + 7,8 + + The value assigned to variable 'i' is never used + The value assigned to variable 'a' is never used + + = 20) { + i = 4; + a *= 5; + break; + } + + a = i + 3; + i += 3; + } while (i < 30); + } +} + ]]> + + + + Do while with continue + 0 + = 20) { + i = 4; // used by condition + a *= 5; + continue; + } + + a = i + 3; + i += 3; + } while (i < 30); + } +} + ]]> + + + + Switch statement 0 + 4 + 6,8,10,12 + + The value assigned to variable 'a' is never used + The value assigned to variable 'a' is never used + The value assigned to variable 'a' is never used + The value assigned to variable 'a' is never used + + + + + + Switch statement 1 + 0 + + + + + Switch statement 2 + 0 + 0) break; // else fallthrough + case 2 : a = 2; break; + case 3 : a = 3; break; + default : a = a + 1; + } + + System.out.println(a); + } +} + ]]> + + + + Switch fallthrough + 1 + 6 + + The value assigned to variable 'a' is never used (overwritten on line 8) + + + + + + Switch fallthrough 2 + 1 + 9 + + The value assigned to variable 'a' is never used + + + + + + Switch non-fallthrough + 0 + a = 1; + case 2 -> a = 2; + case 3 -> a = 3; + default -> a = a + 1; + } + System.out.println(a); + } +} + ]]> + + + + Switch non-fallthrough blocks + 1 + 9 + + The value assigned to variable 'i' is never used + + a = 1; + case 2 -> { + if (args.length > 0) { + i = 4; + break; + } + a = 2; + } + case 3 -> a = 3; + default -> a = a + 1; + } + System.out.println(a); + } +} + ]]> + + + + Switch expr non-fallthrough + 4 + 6,7,8,9 + + The value assigned to variable 'a' is never used (overwritten on line 4) + The value assigned to variable 'a' is never used (overwritten on line 4) + The value assigned to variable 'a' is never used (overwritten on line 4) + The value assigned to variable 'a' is never used (overwritten on line 4) + + a = 1; + case 2 -> a = 2; + case 3 -> a = 3; + default -> a = a + 1; + }; + + System.out.println(a); + } +} + ]]> + + + + Switch expr with yield + 4 + 6,9,13,14 + + The value assigned to variable 'a' is never used (overwritten on line 4) + The updated value of variable 'a' is never used (overwritten on line 4) + The value assigned to variable 'a' is never used (overwritten on line 4) + The value assigned to variable 'a' is never used (overwritten on line 4) + + a = 1; + case 2 -> { + if (a > 0) { + yield a++; + } + yield 4; + } + case 3 -> a = 3; + default -> a = a + 1; + }; + + System.out.println(a); + } +} + ]]> + + + + Usage as LHS of method + 1 + 5 + + The value assigned to variable 't1' is never used + + + + + + Assignment in operand + 1 + 6 + + The value assigned to variable 't1' is never used + + + + + + Assignment in operand 2 + 1 + 7 + + The value assigned to variable 't1' is never used + + + + + + Assignment in operand 3 + 0 + + + + + Assignment in operand 4 + true + 3 + 2,4,6 + + Parameter 'args' is never used + The initializer for variable 't2' is never used + The value assigned to variable 't1' is never used + + + + + + #1749 DD False Positive in DataflowAnomalyAnalysis + 1 + 4 + + The value assigned to variable 'a' is never used + + + + + Compound assignment + 1 + 4 + + The value assigned to variable 'a' is never used + + + + + Another case + 2 + 3,5 + + The initializer for variable 'iter' is never used (overwritten on line 4) + The value assigned to variable 'iter' is never used + + + + + + Var usage in lambda (#1304) + 0 + dummySet) { + captured = captured.trim(); + return dummySet.stream().noneMatch(value -> value.equalsIgnoreCase(captured)); + } + +} ]]> + + + + Try/catch + 1 + 4 + + The initializer for variable 'a' is never used (overwritten on lines 6 and 8) + + + + + + Try with several catches + 0 + + + + + Try with resources: resources should be used + 0 + + + + + Definitions in try block reach catch blocks + 0 + + + + + Try/catch finally + 0 + + + + Try/catch finally 3 + 1 + 4 + + The initializer for variable 'a' is never used (overwritten on lines 6 and 8) + + + + + Try/catch finally in loop + 0 + 10) { + try (Reader r = new StringReader("")) { + r.read(); + } catch (IOException e) { + a = -1; // used in finally even if break + break; + } finally { + a++; + } + } + return a; + } + +} + ]]> + + + Abstract method NPE + 0 + + + + FP in finally + 0 + + + + + Lambda captured var use + 0 + decode() { + Flux> splitEvents = splitEvts(); + + return map(events -> { + return unmarshal(events.append(splitEvents)); + }); + } + +} + ]]> + + + Lambda assignment + 2 + 5,6 + + The initializer for variable 'k' is never used (overwritten on line 6) + The value assigned to variable 'k' is never used + + { + int k = 0; + return k = 2; + }); + } + +} + ]]> + + + Lambda returns 2 + true + 2 + 4,7 + + The initializer for variable 'splitEvents' is never used + The value assigned to variable 'events' is never used + + decode() { + Flux> splitEvents = splitEvts(); + + return map(events -> { + events = events.normalize(); + return dontUseEvents(); + }); + } + +} + ]]> + + + FP in try + 0 + + + + + Field initializers 0 + 0 + + + + + Field initializers 1 + 1 + 3 + + The field initializer for 'f1' is never used (overwritten on line 4) + + + + + + Field initializers 1 + 1 + 3 + + The field initializer for 'f1' is never used (overwritten on line 4) + + + + + + Field initializers and ctor + 1 + 3 + + The field initializer for 'f1' is never used (overwritten on lines 7 and 11) + + + + + + Field initializers and ctor with this + 1 + 3 + + The field initializer for 'f1' is never used (overwritten on lines 7 and 11) + + + + + + Field initializers and ctor with this, shadowing + 1 + 3 + + The field initializer for 'f1' is never used (overwritten on lines 7 and 11) + + + + + + Field initializers and ctor with this, field access + 0 + + + + + Field initializers and ctor + 2 + 3,11 + + The field initializer for 'f1' is never used (overwritten on line 11) + The value assigned to field 'f1' is never used (overwritten on lines 7 and 15) + + + + + + Static initializer + 0 + ejbRefClass; + + private static Class webServiceRefClass; + + static { + try { + Class clazz = (Class) Class.forName("javax.xml.ws.WebServiceRef"); + webServiceRefClass = clazz; + } catch (ClassNotFoundException ex) { + webServiceRefClass = null; + } + + try { + Class clazz = Class.forName("javax.ejb.EJB"); + ejbRefClass = clazz; + } catch (ClassNotFoundException ex) { + ejbRefClass = null; + } + } + + + private static Class other = webServiceRefClass; + +} + ]]> + + + + + FP with anonymous classes on the way + 0 + OBJECT_METHODS = new ArrayList(); + + static { + Method privateLookupIn; + Method lookupDefineClass; + Method classLoaderDefineClass; + ProtectionDomain protectionDomain; + Throwable throwable = null; + try { + privateLookupIn = (Method) AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Object run() throws Exception { + try { + return MethodHandles.class.getMethod("privateLookupIn", Class.class, MethodHandles.Lookup.class); + } + catch (NoSuchMethodException ex) { + return null; + } + } + }); + lookupDefineClass = (Method) AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Object run() throws Exception { + try { + return MethodHandles.Lookup.class.getMethod("defineClass", byte[].class); + } + catch (NoSuchMethodException ex) { + return null; + } + } + }); + classLoaderDefineClass = (Method) AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Object run() throws Exception { + return ClassLoader.class.getDeclaredMethod("defineClass", + String.class, byte[].class, Integer.TYPE, Integer.TYPE, ProtectionDomain.class); + } + }); + protectionDomain = getProtectionDomain(ReflectUtils.class); + AccessController.doPrivileged(new PrivilegedExceptionAction() { + public Object run() throws Exception { + Method[] methods = Object.class.getDeclaredMethods(); + for (Method method : methods) { + if ("finalize".equals(method.getName()) + || (method.getModifiers() & (Modifier.FINAL | Modifier.STATIC)) > 0) { + continue; + } + OBJECT_METHODS.add(method); + } + return null; + } + }); + } + catch (Throwable t) { + privateLookupIn = null; + lookupDefineClass = null; + classLoaderDefineClass = null; + protectionDomain = null; + throwable = t; + } + privateLookupInMethod = privateLookupIn; + lookupDefineClassMethod = lookupDefineClass; + classLoaderDefineClassMethod = classLoaderDefineClass; + PROTECTION_DOMAIN = protectionDomain; + THROWABLE = throwable; + } + +} + ]]> + + + + FP with array access 0 + 0 + map, String name, int[] arr) { + Integer index = map.get(name); + arr[index] = 4; + } + +} + ]]> + + + + FP with array access 1 + 1 + 2 + + The initial value of parameter 'arr' is never used (overwritten on line 3) + + + + + + FP with long field access + 0 + + + + + FP with long access 2 + 1 + 2 + + The initial value of parameter 'arr' is never used (overwritten on line 3) + + + + + FP with long access 3 + 1 + 2 + + The initial value of parameter 'arr' is never used (overwritten on line 3) + + + + + + FN with casts + 1 + + + + + + SuppressWarnings test (local) + 0 + + + + + SuppressWarnings test (method) + 0 + + + + + + SuppressWarnings test (class) + 0 + + + + + + Post-increment behavior + 1 + 5 + + The updated value of variable 'i' is never used + + + + + + + Post-increment behavior 2 + 1 + 6 + + The updated value of variable 'i' is never used + + + + + + + + Pre-increment behavior + 1 + 5 + + The updated value of variable 'i' is never used + + + + + + Pre-increment behavior + true + 1 + 5 + + The updated value of variable 'i' is never used + + + + + + + Pre-increment behavior 2 + 0 + + + + + + Pre-increment behavior 2 + true + 1 + 6 + + The updated value of variable 'i' is never used + + + + + + Pre-increment behavior 2 + true + 1 + 6 + + The updated value of variable 'i' is never used + + + + + + + Test local class + true + 2 + 4,6 + + The initializer for variable 'shadowed' is never used + The field initializer for 'f' is never used (overwritten on line 8) + + + + + + Test anonymous class + 3 + 4,5,6 + + The initializer for variable 'shadowed' is never used (overwritten on line 5) + The value assigned to variable 'shadowed' is never used + The field initializer for 'f' is never used (overwritten on line 8) + + + + + + Test shortcut AND + 1 + 5 + + The initializer for variable 'k' is never used (overwritten on line 9) + + + + + + Test shortcut OR + true + 0 + + + + + Test shortcut OR + 3 + 5,7,8 + + The initializer for variable 'i' is never used (overwritten on line 7) + The value assigned to variable 'j' is never used (overwritten on line 8) + The value assigned to variable 'j' is never used + + + + + + + Test shortcut OR 2 + 1 + + The initializer for variable 'i' is never used (overwritten on line 7) + + + + + + + Test shortcut AND + 2 + 5,7 + + The initializer for variable 'i' is never used (overwritten on line 7) + The value assigned to variable 'j' is never used (overwritten on line 8) + + + + + + Test shortcut AND 2 + 1 + + The initializer for variable 'i' is never used (overwritten on line 7) + + + + + + Nested boolean logic 1 + 1 + + The value assigned to variable 'i' is never used (overwritten on line 7) + + 0 || ((i = 2) < (j = i) && (j = k) == i) ) { + // reaching: i = 1, i = 2, j = k (not j = i) + } else { + // reaching: i = 2, j = k, j = i (not i = 1) + log(j); + log(i); + } + } + } + + ]]> + + + Nested boolean logic 2 + 1 + + The value assigned to variable 'i' is never used (overwritten on line 7) + + 0 && ((i = 2) < (j = i) || (j = k) == i) ) { + // reaching: i = 2, j = i, j = k (not i = 1) + log(i); + } else { + // reaching: i = 1, i = 2, j = k, j = i + log(j); + } + } + } + + ]]> + + + + FP with argument + 1 + + The initializer for variable 'i' is never used (overwritten on line 7) + + + + + + DU anomaly false positive? #1304 + 0 + dummySet) { + final String trimmedValue = trimToNull(stringValue); + return dummySet.stream() + .noneMatch(value -> value.equalsIgnoreCase(trimmedValue)); + } + + } + ]]> + + + + DataflowAnomalyAnalysis DU false positive #399 + 0 + + + + + DataflowAnomalyAnalysis: DD false positive #400 + 0 + args) { + T res = zeroS();// + + + + DU Anomaly #1107 + 0 + + + + + + DataflowAnomalyAnalysis: DD false positive for arrays #1251 + 0 + + + + + DU false positive in DataflowAnomalyAnalysis #1606 + 0 + = 0); + + if (n < 2) { + return n; + } else { + int a = 0; + int b = 1; + final int m = n - 1; + + for (int i = 0; i < m; i++) { + final int c = a; + a = b; + b = c + b; + } + + return b; + } + } + } + ]]> + + + + false-positive in DD-part of DataflowAnomalyAnalysis #1675 + 0 + formatterClass = findClass(formatterClassName); + formatter = formatterClass.getDeclaredConstructor().newInstance(); + } catch (ClassNotFoundException | NoSuchMethodException | IllegalAccessException | InstantiationException | InvocationTargetException ignored) { + } + } + setFormatter((formatter == null) ? new SimpleFormatter() : formatter); + } + } + ]]> + + + + DU false positive in DataflowAnomalyAnalysis #1682 + 0 + { + bufferedMessages.forEach(bufferedMessage -> sendMessage(discordManager, bufferedMessage)); + return null; + }); + sendMessage(discordManager, logMessage); + } + } + } + ]]> + + + + + DataflowAnomalyAnalysis has false positive for object initialised outside loop. #2131 + 0 + list = new ArrayList<>(); + + public void run() { + String str = Thread.currentThread().getName() + " Element : %d"; + for (int i = 0; i < 10_000; i++) { + list.add(String.format(str, i)); + } + } + + public void runAgain() { + String str = Thread.currentThread().getName() + " Element : %d"; + for (int i = 0; i < 10_000; i++) + list.add(String.format(str, i)); + } + + public void runOnceMore() { + String str = Thread.currentThread().getName() + " Element : %d"; + list = IntStream.range(0, 10_000) + .mapToObj(i -> String.format(str, i)) + .collect(Collectors.toList()); + } + } + ]]> + + + + + ClassCastException with annotated foreach var + true + 4 + + + + + + Catch in loop + 0 + + + + + Catch in loop (reportUnusedVariables) + true + 1 + 8 + + Exception parameter 'e' is never used + + + + + + + Catch in loop (reportUnusedVariables) + true + 6 + + + + + + + Unused formal value + true + 1 + 2 + + The initial value of parameter 'i' is never used (overwritten on line 3) + + + + + + Unused formal value + false + 1 + + The initial value of parameter 'i' is never used (overwritten on line 3) + + + + + + Test ignored name 0 + true + 1 + + + + + Test ignored name 1 + true + 0 + + + + + Test ignored name 2 + true + 0 + + java 7 + + + + Test ignored name 2 + true + 1 + + + + + Test ignored name 2 + false + 0 + + + + + Test annot suppression + true + 0 + + + + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/LawOfDemeter.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/LawOfDemeter.xml index 56d22f3699..8f8e0b5f40 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/LawOfDemeter.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/LawOfDemeter.xml @@ -288,6 +288,60 @@ public class Test { .withFoo("foo") .build(); } +} + ]]> + + + + [java] LawOfDemeter: False positive with 'this' pointer #2174 + 0 + + + + + [java] LawOfDemeter: False positive when casting to derived class #2189 + 0 + > tasks; + + public void cancelTasks(ForkJoinTask cancelTask) { + for (ForkJoinTask task : tasks) { + if (!task.equals(cancelTask)) { + task.cancel(true); + ((SearchNumberTask) task).writeCancelMessage(); // wrong violation: method chain calls + } + } + } } ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml index cb9ada5759..d8ddc23c75 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidInstantiatingObjectsInLoops.xml @@ -89,8 +89,7 @@ public class Foo { ]]> - - + #1215 AvoidInstantiatingObjectsInLoops matches the right side of a list iteration loop @@ -124,6 +123,112 @@ public class TestInstantiationInLoop { System.out.println(filename); } } +} + ]]> + + + + [java] False positive: AvoidInstantiatingObjectsInLoops should not flag objects with different parameters or objects assigned or passed as parameters #2207 + 0 + + + + + False positive when assigning to a list/array (see #2207 and #1043) + 0 + cars = new ArrayList<>(); + for(int i = 0; i < 3; ++i) { + cars.add(new Car()); + } + } +} + ]]> + + + + False negative with break in other for-loop + 1 + 7 + getFilteredMessages( + String fileName, FileContents fileContents, DetailAST rootAST) { + final SortedSet result = new TreeSet<>(messages); + for (LocalizedMessage element : messages) { + final TreeWalkerAuditEvent event = + new TreeWalkerAuditEvent(fileContents, fileName, element, rootAST); + for (TreeWalkerFilter filter : filters) { + if (!filter.accept(event)) { + result.remove(element); + break; + } + } + } + return result; + } +} + ]]> + + + + Instantiation in loop condition + 1 + 3 + 0) { + } + } +} + ]]> + + + + false negative in anonymous classes + 2 + 5,14 + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UseStringBufferForStringAppends.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UseStringBufferForStringAppends.xml index e430e4a433..33a85af08c 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UseStringBufferForStringAppends.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UseStringBufferForStringAppends.xml @@ -7,14 +7,15 @@ failure case 1 - 6 + 7 @@ -60,15 +61,16 @@ public class Foo { - failure case + failure case, constructor 1 - 5 + 6 @@ -77,28 +79,30 @@ public class Foo { static failure case 1 - 5 + 6 - reference self - 1 - 5 + reference self inside for loop + 2 + 5,6 + + + + [java] UseStringBufferForStringAppends: False positive if only one concatenation #1736 + 0 + diff --git a/pmd-java8/pom.xml b/pmd-java8/pom.xml index 6507278282..53aea03a99 100644 --- a/pmd-java8/pom.xml +++ b/pmd-java8/pom.xml @@ -56,15 +56,15 @@ pmd-core - - org.hamcrest - hamcrest - test - junit junit test + + org.hamcrest + hamcrest + test + diff --git a/pmd-javascript/pom.xml b/pmd-javascript/pom.xml index a97cff755b..9bdb9e9bc8 100644 --- a/pmd-javascript/pom.xml +++ b/pmd-javascript/pom.xml @@ -83,10 +83,6 @@ commons-io commons-io - - net.sourceforge.saxon - saxon - junit @@ -98,11 +94,6 @@ pmd-test test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-lang-test diff --git a/pmd-jsp/pom.xml b/pmd-jsp/pom.xml index a01fdccf22..860bf37c85 100644 --- a/pmd-jsp/pom.xml +++ b/pmd-jsp/pom.xml @@ -72,33 +72,16 @@ - - net.java.dev.javacc - javacc - net.sourceforge.pmd pmd-core - - commons-io - commons-io - - - net.sourceforge.saxon - saxon - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-kotlin/pom.xml b/pmd-kotlin/pom.xml index bce62d34f6..5fa13a4a8e 100644 --- a/pmd-kotlin/pom.xml +++ b/pmd-kotlin/pom.xml @@ -31,17 +31,13 @@ - - org.antlr - antlr4-runtime - net.sourceforge.pmd pmd-core - commons-io - commons-io + org.antlr + antlr4-runtime @@ -49,11 +45,6 @@ junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-lang-test/pom.xml b/pmd-lang-test/pom.xml index 26669a0869..6fa4a14da8 100644 --- a/pmd-lang-test/pom.xml +++ b/pmd-lang-test/pom.xml @@ -89,11 +89,38 @@ pmd-core + + org.apache.commons + commons-lang3 + + + commons-io + commons-io + + + + io.kotlintest + kotlintest-assertions + compile + + + org.jetbrains.kotlin + kotlin-test + compile + + + + org.jetbrains + annotations + compile + + org.hamcrest hamcrest @@ -129,25 +156,15 @@ compile - - io.kotlintest - kotlintest-runner-junit5 - compile - - com.github.oowekyala.treeutils tree-matchers - 2.1.0 compile - - - net.sourceforge.pmd - pmd-java - 6.12.0 - test + com.github.oowekyala.treeutils + tree-printers + compile diff --git a/pmd-lua/pom.xml b/pmd-lua/pom.xml index a519014e08..827030946b 100644 --- a/pmd-lua/pom.xml +++ b/pmd-lua/pom.xml @@ -31,17 +31,13 @@ - - org.antlr - antlr4-runtime - net.sourceforge.pmd pmd-core - commons-io - commons-io + org.antlr + antlr4-runtime @@ -49,11 +45,6 @@ junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-matlab/pom.xml b/pmd-matlab/pom.xml index cd3585c4bb..8b1824078c 100644 --- a/pmd-matlab/pom.xml +++ b/pmd-matlab/pom.xml @@ -70,21 +70,12 @@ net.sourceforge.pmd pmd-core - - commons-io - commons-io - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-modelica/pom.xml b/pmd-modelica/pom.xml index ddb1f877e6..3c3f5de689 100644 --- a/pmd-modelica/pom.xml +++ b/pmd-modelica/pom.xml @@ -79,8 +79,8 @@ - org.junit.vintage - junit-vintage-engine + junit + junit test @@ -93,5 +93,41 @@ pmd-test test + + + com.github.oowekyala.treeutils + tree-matchers + test + + + com.github.oowekyala.treeutils + tree-printers + test + + + org.jetbrains.kotlin + kotlin-stdlib + test + + + org.jetbrains.kotlin + kotlin-test + test + + + org.jetbrains + annotations + test + + + io.kotlintest + kotlintest-assertions + test + + + io.kotlintest + kotlintest-core + test + diff --git a/pmd-modelica/src/test/kotlin/net/sourceforge/pmd/lang/modelica/ast/ModelicaCoordsTest.kt b/pmd-modelica/src/test/kotlin/net/sourceforge/pmd/lang/modelica/ast/ModelicaCoordsTest.kt index 6ad15f5bb4..dfa62e29b5 100644 --- a/pmd-modelica/src/test/kotlin/net/sourceforge/pmd/lang/modelica/ast/ModelicaCoordsTest.kt +++ b/pmd-modelica/src/test/kotlin/net/sourceforge/pmd/lang/modelica/ast/ModelicaCoordsTest.kt @@ -6,14 +6,14 @@ package net.sourceforge.pmd.lang.modelica.ast import io.kotlintest.should import io.kotlintest.shouldBe -import io.kotlintest.specs.FunSpec +import io.kotlintest.specs.AbstractFunSpec import net.sourceforge.pmd.lang.LanguageRegistry import net.sourceforge.pmd.lang.ast.Node import net.sourceforge.pmd.lang.ast.test.matchNode import net.sourceforge.pmd.lang.ast.test.shouldBe import java.io.StringReader -class ModelicaCoordsTest : FunSpec({ +class ModelicaCoordsTest : AbstractFunSpec({ test("Test line/column numbers for implicit nodes") { diff --git a/pmd-objectivec/pom.xml b/pmd-objectivec/pom.xml index 2bbbb7cbf5..7606c1db5d 100644 --- a/pmd-objectivec/pom.xml +++ b/pmd-objectivec/pom.xml @@ -70,21 +70,12 @@ net.sourceforge.pmd pmd-core - - commons-io - commons-io - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-perl/pom.xml b/pmd-perl/pom.xml index 2eb7fd9c34..49cc40f80a 100644 --- a/pmd-perl/pom.xml +++ b/pmd-perl/pom.xml @@ -30,10 +30,20 @@ pmd-core + + junit + junit + test + net.sourceforge.pmd pmd-test test + + net.sourceforge.pmd + pmd-lang-test + test + diff --git a/pmd-php/pom.xml b/pmd-php/pom.xml index a985180d05..26259bb0d6 100644 --- a/pmd-php/pom.xml +++ b/pmd-php/pom.xml @@ -40,5 +40,10 @@ pmd-test test + + net.sourceforge.pmd + pmd-lang-test + test + diff --git a/pmd-plsql/pom.xml b/pmd-plsql/pom.xml index 3f44ed5a1d..334c8559dd 100644 --- a/pmd-plsql/pom.xml +++ b/pmd-plsql/pom.xml @@ -80,10 +80,6 @@ - - net.java.dev.javacc - javacc - net.sourceforge.pmd pmd-core @@ -92,21 +88,12 @@ commons-io commons-io - - net.sourceforge.saxon - saxon - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-python/pom.xml b/pmd-python/pom.xml index 1340131928..c0bd0bb459 100644 --- a/pmd-python/pom.xml +++ b/pmd-python/pom.xml @@ -70,21 +70,12 @@ net.sourceforge.pmd pmd-core - - commons-io - commons-io - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-ruby/pom.xml b/pmd-ruby/pom.xml index fc6778050e..bcebcdcd0f 100644 --- a/pmd-ruby/pom.xml +++ b/pmd-ruby/pom.xml @@ -16,21 +16,12 @@ net.sourceforge.pmd pmd-core - - commons-io - commons-io - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-scala-modules/pmd-scala-common/pom.xml b/pmd-scala-modules/pmd-scala-common/pom.xml index de9580fe17..4304adf8c6 100644 --- a/pmd-scala-modules/pmd-scala-common/pom.xml +++ b/pmd-scala-modules/pmd-scala-common/pom.xml @@ -103,17 +103,20 @@ net.sourceforge.pmd pmd-core + + commons-io + commons-io + + + org.apache.commons + commons-lang3 + junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test @@ -124,5 +127,42 @@ pmd-lang-test test + + + + com.github.oowekyala.treeutils + tree-matchers + test + + + com.github.oowekyala.treeutils + tree-printers + test + + + io.kotlintest + kotlintest-assertions + test + + + io.kotlintest + kotlintest-core + test + + + org.jetbrains.kotlin + kotlin-stdlib + test + + + org.jetbrains.kotlin + kotlin-test + test + + + org.jetbrains + annotations + test + diff --git a/pmd-scala-modules/pmd-scala-common/src/test/kotlin/net/sourceforge/pmd/lang/scala/ast/ScalaTreeTests.kt b/pmd-scala-modules/pmd-scala-common/src/test/kotlin/net/sourceforge/pmd/lang/scala/ast/ScalaTreeTests.kt index 21825ff9ea..32d808af26 100644 --- a/pmd-scala-modules/pmd-scala-common/src/test/kotlin/net/sourceforge/pmd/lang/scala/ast/ScalaTreeTests.kt +++ b/pmd-scala-modules/pmd-scala-common/src/test/kotlin/net/sourceforge/pmd/lang/scala/ast/ScalaTreeTests.kt @@ -5,14 +5,14 @@ package net.sourceforge.pmd.lang.scala.ast import io.kotlintest.should -import io.kotlintest.specs.FunSpec +import io.kotlintest.specs.AbstractFunSpec import net.sourceforge.pmd.lang.LanguageRegistry import net.sourceforge.pmd.lang.ast.Node import net.sourceforge.pmd.lang.ast.test.matchNode import net.sourceforge.pmd.lang.ast.test.shouldBe import java.io.StringReader -class ScalaTreeTests : FunSpec({ +class ScalaTreeTests : AbstractFunSpec({ test("Test line/column numbers") { diff --git a/pmd-scala-modules/pmd-scala_2.12/pom.xml b/pmd-scala-modules/pmd-scala_2.12/pom.xml index 1a912de417..80aabc12b3 100644 --- a/pmd-scala-modules/pmd-scala_2.12/pom.xml +++ b/pmd-scala-modules/pmd-scala_2.12/pom.xml @@ -25,9 +25,19 @@ + + org.scala-lang + scala-library + ${scalaVersion}.10 + org.scalameta - scalameta_${scalaVersion} + parsers_${scalaVersion} + ${scalameta.version} + + + org.scalameta + trees_${scalaVersion} ${scalameta.version} diff --git a/pmd-scala-modules/pmd-scala_2.13/pom.xml b/pmd-scala-modules/pmd-scala_2.13/pom.xml index 7a72d501e5..ffbd77819a 100644 --- a/pmd-scala-modules/pmd-scala_2.13/pom.xml +++ b/pmd-scala-modules/pmd-scala_2.13/pom.xml @@ -25,9 +25,19 @@ + + org.scala-lang + scala-library + ${scalaVersion}.3 + org.scalameta - scalameta_${scalaVersion} + parsers_${scalaVersion} + ${scalameta.version} + + + org.scalameta + trees_${scalaVersion} ${scalameta.version} diff --git a/pmd-swift/pom.xml b/pmd-swift/pom.xml index 7744be7696..f33680c761 100644 --- a/pmd-swift/pom.xml +++ b/pmd-swift/pom.xml @@ -31,17 +31,13 @@ - - org.antlr - antlr4-runtime - net.sourceforge.pmd pmd-core - commons-io - commons-io + org.antlr + antlr4-runtime @@ -49,11 +45,6 @@ junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-visualforce/pom.xml b/pmd-visualforce/pom.xml index 7c3b8b7d23..c5fb203ea0 100644 --- a/pmd-visualforce/pom.xml +++ b/pmd-visualforce/pom.xml @@ -72,33 +72,16 @@ - - net.java.dev.javacc - javacc - net.sourceforge.pmd pmd-core - - commons-io - commons-io - - - net.sourceforge.saxon - saxon - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pmd-vm/pom.xml b/pmd-vm/pom.xml index 0e92c868d2..ded0e269b3 100644 --- a/pmd-vm/pom.xml +++ b/pmd-vm/pom.xml @@ -80,10 +80,6 @@ - - net.java.dev.javacc - javacc - net.sourceforge.pmd pmd-core @@ -92,10 +88,6 @@ org.apache.commons commons-lang3 - - net.sourceforge.saxon - saxon - junit diff --git a/pmd-xml/pom.xml b/pmd-xml/pom.xml index 4876d5561b..7632c6e37d 100644 --- a/pmd-xml/pom.xml +++ b/pmd-xml/pom.xml @@ -35,40 +35,24 @@ - - org.antlr - antlr4-runtime - net.sourceforge.pmd pmd-core - net.sourceforge.saxon - saxon + org.antlr + antlr4-runtime commons-io commons-io - - net.sourceforge.saxon - saxon - dom - runtime - - junit junit test - - org.junit.vintage - junit-vintage-engine - test - net.sourceforge.pmd pmd-test diff --git a/pom.xml b/pom.xml index 277c3f1a07..4e0026061d 100644 --- a/pom.xml +++ b/pom.xml @@ -86,11 +86,12 @@ ${maven.compiler.test.target} 1.3.0 + 3.1.8 0.10.1 5.0 - 2.22.1 + 3.0.0-M5 8.30 3.1.1 3.13.0 @@ -259,12 +260,31 @@ ${project.build.testResources[0].directory} + + + + org.junit.vintage + junit-vintage-engine + 5.6.2 + + + + io.kotlintest + kotlintest-runner-junit5 + ${kotlintest.version} + + org.codehaus.mojo build-helper-maven-plugin 3.0.0 + + org.codehaus.mojo + exec-maven-plugin + 3.0.0 + org.apache.maven.plugins maven-source-plugin @@ -594,6 +614,12 @@ antlr4-runtime ${antlr.version} + + + org.antlr + antlr-runtime + 3.5.2 + org.apache.ant ant @@ -658,6 +684,7 @@ net.java.dev.javacc javacc ${javacc.version} + provided commons-io @@ -695,6 +722,26 @@ test + + com.github.oowekyala.treeutils + tree-matchers + 2.1.0 + test + + + com.github.oowekyala.treeutils + tree-printers + 2.1.0 + test + + + + com.google.guava + guava + 18.0 + test + + org.hamcrest hamcrest @@ -748,40 +795,6 @@ test - - - org.junit.jupiter - junit-jupiter-api - 5.5.0 - test - - - org.junit.jupiter - junit-jupiter-engine - 5.5.0 - test - - - - org.junit.platform - junit-platform-commons - 1.5.0 - - - org.junit.platform - junit-platform-launcher - 1.5.0 - test - - - - - org.junit.vintage - junit-vintage-engine - 5.5.0 - test - - org.jetbrains.kotlin @@ -789,32 +802,48 @@ ${kotlin.version} test - org.jetbrains.kotlin kotlin-stdlib-jdk8 ${kotlin.version} test - org.jetbrains.kotlin kotlin-reflect ${kotlin.version} test - org.jetbrains.kotlin kotlin-test-junit ${kotlin.version} test + + org.jetbrains.kotlin + kotlin-test + ${kotlin.version} + test + io.kotlintest - kotlintest-runner-junit5 - 3.1.8 + kotlintest-assertions + ${kotlintest.version} + test + + + io.kotlintest + kotlintest-core + ${kotlintest.version} + test + + + + org.jetbrains + annotations + 13.0 test