From dd3a8a3514a1a4552291ae40a9a4e0cefa6ababd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 27 Apr 2021 22:56:59 +0200 Subject: [PATCH 01/80] [java] Make CompareObjectWithEquals allow comparing against constants #3205 Implementation strategy is to look for all caps name. Resolving the symbol to know if the field is constant or not may be done in pmd 7 (the rule is a Java rule there). --- .../resources/category/java/errorprone.xml | 3 + .../xml/CompareObjectsWithEquals.xml | 63 ++++++++++++++++++- 2 files changed, 65 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index d440ac8cc6..ca401f5435 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -1118,6 +1118,9 @@ Use equals() to compare object references; avoid comparing them with ==. [not(PrimarySuffix)] [ancestor::MethodDeclaration[@Name = 'equals']]) ] + (: Is not a field access with an all-caps identifier :) + [not(PrimaryExpression[not(PrimarySuffix) and PrimaryPrefix/Name[upper-case(@Image)=@Image] + or PrimaryExpression/PrimarySuffix[last()][upper-case(@Image)=@Image]])] ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CompareObjectsWithEquals.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CompareObjectsWithEquals.xml index 813301e972..b60d09038e 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CompareObjectsWithEquals.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CompareObjectsWithEquals.xml @@ -112,7 +112,7 @@ package net.sourceforge.pmd.lang.java.rule.errorprone.compareobjectswithequals; public class CompareObjectsWithEqualsSample { void array(int[] a, String[] b) { - if (a[1] == b[1]) {} // int == String - this comparison doesn't make sense + if (a[1] == b[1]) {} // int == String - this comparison doesn't make sense (and doesn't compile...) } void array2(int[] c, int[] d) { if (c[1] == d[1]) {} @@ -365,4 +365,65 @@ public class EnumTest { ]]> + + static constant #3205 + 0 + + + + + static constant in other class #3205 + 0 + + + + constant field on some object #3205 + 0 + + + + constant field on some object, more complicated expr #3205 + 0 + + + From eb89176ff826db72f7f5a9804766d9e73674ce9a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 27 Apr 2021 23:08:40 +0200 Subject: [PATCH 02/80] [java] Enhance CompareObjectsWithEquals with list of exceptions #3110 --- .../resources/category/java/errorprone.xml | 21 ++++++++++++++----- .../xml/CompareObjectsWithEquals.xml | 12 +++++++++++ 2 files changed, 28 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index ca401f5435..3b4aeeceae 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -1100,19 +1100,30 @@ public class Bar { class="net.sourceforge.pmd.lang.rule.XPathRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#compareobjectswithequals"> -Use equals() to compare object references; avoid comparing them with ==. +Use `equals()` to compare object references; avoid comparing them with `==`. + +Since comparing objects with named constants is useful in some cases (eg, when +defining constants for sentinel values), the rule ignores comparisons against +fields with all-caps name (eg `this == SENTINEL`), which is a common naming +convention for constant fields. + +You may allow some types to be compared by reference by listing the exceptions +in the `typesThatCompareByReference` property. 3 + + java.lang.Enum,java.lang.Class + + + Property typesThatCompareByReference #3110 + java.lang.String + 0 + + From 6e750bf927ef99826c500f00a6d5990a18951744 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 29 Apr 2021 11:25:56 +0200 Subject: [PATCH 03/80] [java] UnnecessaryImport false positive for on-demand imports Fixes #2655 --- docs/pages/release_notes.md | 3 +++ .../rule/codestyle/UnnecessaryImportRule.java | 17 ++++++++++++++++- .../codestyle/unnecessaryimport/package1/U.java | 13 +++++++++++++ .../codestyle/unnecessaryimport/package2/C.java | 11 +++++++++++ .../rule/codestyle/xml/UnnecessaryImport.xml | 16 ++++++++++++++++ 5 files changed, 59 insertions(+), 1 deletion(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package1/U.java create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package2/C.java diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..56d4d34445 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues +* java-codestyle + * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports + ### API Changes ### External Contributions diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java index a311692615..67c7e95ea5 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryImportRule.java @@ -24,6 +24,7 @@ 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.FormalComment; +import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.ast.internal.ImportWrapper; import net.sourceforge.pmd.lang.java.ast.internal.PrettyPrintingUtil; @@ -161,7 +162,7 @@ public class UnnecessaryImportRule extends AbstractJavaRule { * Remove the import wrapper that imports the name referenced by the * given node. */ - protected void check(Node referenceNode, RuleContext ruleCtx) { + protected void check(JavaNode referenceNode, RuleContext ruleCtx) { if (imports.isEmpty()) { return; } @@ -184,6 +185,20 @@ public class UnnecessaryImportRule extends AbstractJavaRule { } } + // check on-demand imports + it = imports.iterator(); + while (it.hasNext()) { + ImportWrapper i = it.next(); + if (!i.isStaticOnDemand() && i.isOnDemand()) { + String possibleClassName = i.getFullName() + "." + candName; + Class possibleClazz = referenceNode.getRoot().getClassTypeResolver() + .loadClassOrNull(possibleClassName); + if (possibleClazz != null) { + it.remove(); + } + } + } + // check static on-demand imports it = imports.iterator(); while (it.hasNext()) { diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package1/U.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package1/U.java new file mode 100644 index 0000000000..902d084b08 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package1/U.java @@ -0,0 +1,13 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.codestyle.unnecessaryimport.package1; + +import net.sourceforge.pmd.lang.java.rule.codestyle.unnecessaryimport.package2.*; // SUPPRESS CHECKSTYLE needed for test case + +public class U { + private void g() { + String k = C.V; + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package2/C.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package2/C.java new file mode 100644 index 0000000000..19710af874 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/unnecessaryimport/package2/C.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.codestyle.unnecessaryimport.package2; + +public class C { + private C() { } + + public static final String V = ""; +} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryImport.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryImport.xml index 56d86c23cf..f94c1ff429 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryImport.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryImport.xml @@ -897,4 +897,20 @@ class NPEImport {} } ]]> + + + [java] UnnecessaryImport false positive for on-demand imports #2655 + 0 + + From cab260ed226c7f1e6d2526e62426f41e0608ad48 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 29 Apr 2021 14:59:13 +0200 Subject: [PATCH 04/80] [javascript] Update Rhino library to 1.7.13 * Convert pmd-javascript to a java8 module * Fixes #699 * Fixes #2081 --- BUILDING.md | 3 ++- docs/pages/pmd/devdocs/building.md | 3 ++- docs/pages/pmd/userdocs/installation.md | 8 +++++++- docs/pages/release_notes.md | 11 +++++++++++ pmd-dist/pom.xml | 10 +++++----- .../net/sourceforge/pmd/it/BinaryDistributionIT.java | 6 +++--- pmd-javascript/pom.xml | 5 +++++ .../pmd/lang/ecmascript/ast/EcmascriptParserTest.java | 10 ++++++++++ pom.xml | 7 +------ 9 files changed, 46 insertions(+), 17 deletions(-) diff --git a/BUILDING.md b/BUILDING.md index 8654ec8953..6579e3ca2e 100644 --- a/BUILDING.md +++ b/BUILDING.md @@ -16,7 +16,8 @@ This will create the zip files in the directory `pmd-dist/target`: That's all ! -**Note:** While Java 11 is required for building, running PMD only requires Java 7 (or Java 8 for Apex and the Designer). +**Note:** While Java 11 is required for building, running PMD only requires Java 7 +(or Java 8 for Apex, JavaScript, Scala, Visualforce, and the Designer). **Note:** With PMD 6.24.0, we are creating [Reproducible Builds](https://reproducible-builds.org/). Since we use [Maven](https://maven.apache.org/guides/mini/guide-reproducible-builds.html) for building, the following diff --git a/docs/pages/pmd/devdocs/building.md b/docs/pages/pmd/devdocs/building.md index d9a7bcbc11..97196f2179 100644 --- a/docs/pages/pmd/devdocs/building.md +++ b/docs/pages/pmd/devdocs/building.md @@ -12,7 +12,8 @@ author: Tom Copeland, Xavier Le Vourch * JDK 11 or higher -{% include note.html content="While Java 11 is required for building, running PMD only requires Java 7 (or Java 8 for Apex, Scala, Visualforce, and the Designer)." %} +{% include note.html content="While Java 11 is required for building, running PMD only requires Java 7 +(or Java 8 for Apex, JavaScript, Scala, Visualforce, and the Designer)." %} You’ll need to either check out the source code or download the latest source release. Assuming you’ve got the latest source release, unzip it to a directory: diff --git a/docs/pages/pmd/userdocs/installation.md b/docs/pages/pmd/userdocs/installation.md index 7339d1b986..264b3dbdc0 100644 --- a/docs/pages/pmd/userdocs/installation.md +++ b/docs/pages/pmd/userdocs/installation.md @@ -11,7 +11,13 @@ sidebar: pmd_sidebar ### Requirements -* [Java JRE](http://www.oracle.com/technetwork/java/javase/downloads/index.html) 1.7 or higher +* [Java JRE](http://www.oracle.com/technetwork/java/javase/downloads/index.html), + OpenJDK from [Azul](https://www.azul.com/downloads/zulu-community/) + or [AdoptOpenJDK](https://adoptopenjdk.net/) 1.7 or higher. + + **Note:** For analyzing Apex, JavaScript, Scala or VisualForce or running the [Designer](pmd_userdocs_extending_designer_reference.html) + at least Java 8 is required. + * A zip archiver, e.g.: * For Windows: [Winzip](http://winzip.com) or the free [7-zip](http://www.7-zip.org/) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..3dce1681cf 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,8 +14,19 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy +#### Javascript module now requires at least Java 8 + +The latest version of [Rhino](https://github.com/mozilla/rhino), the implementation of JavaScript we use +for parsing JavaScript code, requires at least Java 8. Therefore we decided to upgrade the pmd-javascript +module to Java 8 as well. This means, that from now on, a Java 8 or later runtime is required in order +to analyze JavaScript code. Note, that PMD core still stays the at Java 7. + ### Fixed Issues +* pmd-javascript + * [#699](https://github.com/pmd/pmd/issues/699): \[javascript] Update Rhino library to 1.7.13 + * [#2081](https://github.com/pmd/pmd/issues/2081): \[javascript] Failing with OutOfMemoryError parsing a Javascript file + ### API Changes ### External Contributions diff --git a/pmd-dist/pom.xml b/pmd-dist/pom.xml index 44c0ed3eb7..39a4eec1ff 100644 --- a/pmd-dist/pom.xml +++ b/pmd-dist/pom.xml @@ -138,11 +138,6 @@ pmd-java ${project.version} - - net.sourceforge.pmd - pmd-javascript - ${project.version} - net.sourceforge.pmd pmd-jsp @@ -249,6 +244,11 @@ pmd-apex ${project.version} + + net.sourceforge.pmd + pmd-javascript + ${project.version} + net.sourceforge.pmd pmd-scala_2.13 diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java index d5bff3f134..f95dc88391 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java @@ -25,10 +25,10 @@ public class BinaryDistributionIT extends AbstractBinaryDistributionTest { private static final String SUPPORTED_LANGUAGES_PMD; static { - // note: apex, visualforce, and scala require java8 + // note: apex, javascript, visualforce, and scala require java8 if (PMDExecutor.isJava7Test()) { - SUPPORTED_LANGUAGES_CPD = "Supported languages: [cpp, cs, dart, ecmascript, fortran, go, groovy, java, jsp, kotlin, lua, matlab, modelica, objectivec, perl, php, plsql, python, ruby, swift, xml]"; - SUPPORTED_LANGUAGES_PMD = "ecmascript, java, jsp, modelica, plsql, pom, vm, wsdl, xml, xsl"; + SUPPORTED_LANGUAGES_CPD = "Supported languages: [cpp, cs, dart, fortran, go, groovy, java, jsp, kotlin, lua, matlab, modelica, objectivec, perl, php, plsql, python, ruby, swift, xml]"; + SUPPORTED_LANGUAGES_PMD = "java, jsp, modelica, plsql, pom, vm, wsdl, xml, xsl"; } else { SUPPORTED_LANGUAGES_CPD = "Supported languages: [apex, cpp, cs, dart, ecmascript, fortran, go, groovy, java, jsp, kotlin, lua, matlab, modelica, objectivec, perl, php, plsql, python, ruby, scala, swift, vf, xml]"; SUPPORTED_LANGUAGES_PMD = "apex, ecmascript, java, jsp, modelica, plsql, pom, scala, vf, vm, wsdl, xml, xsl"; diff --git a/pmd-javascript/pom.xml b/pmd-javascript/pom.xml index 589a819970..534a496f49 100644 --- a/pmd-javascript/pom.xml +++ b/pmd-javascript/pom.xml @@ -11,6 +11,10 @@ ../ + + 8 + + @@ -78,6 +82,7 @@ org.mozilla rhino + 1.7.13 commons-io diff --git a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java index b4ee5913cf..f6fe15ce3f 100644 --- a/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java +++ b/pmd-javascript/src/test/java/net/sourceforge/pmd/lang/ecmascript/ast/EcmascriptParserTest.java @@ -6,6 +6,7 @@ package net.sourceforge.pmd.lang.ecmascript.ast; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertTrue; import java.io.Reader; @@ -181,4 +182,13 @@ public class EcmascriptParserTest extends EcmascriptParserTestBase { ASTAssignment infix = rootNode.getFirstDescendantOfType(ASTAssignment.class); assertEquals("^=", infix.getImage()); } + + /** + * [javascript] Failing with OutOfMemoryError parsing a Javascript file #2081 + */ + @Test(timeout = 5000L) + public void shouldNotFailWithOutOfMemory() { + ASTAstRoot rootNode = js.parse("(``\n);"); + assertNotNull(rootNode); + } } diff --git a/pom.xml b/pom.xml index 068c80b77c..c03411d3e2 100644 --- a/pom.xml +++ b/pom.xml @@ -687,11 +687,6 @@ 9.1.0.8 dom - - org.mozilla - rhino - 1.7.7.2 - net.java.dev.javacc javacc @@ -1066,7 +1061,6 @@ pmd-groovy pmd-lua pmd-java - pmd-javascript pmd-jsp pmd-kotlin pmd-matlab @@ -1086,6 +1080,7 @@ pmd-apex-jorje pmd-apex pmd-java8 + pmd-javascript pmd-doc pmd-lang-test pmd-scala From 16e70bd8f6024fda39a9c082ad409cf3aab0ae7b Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 29 Apr 2021 15:08:47 +0200 Subject: [PATCH 05/80] [javascript] Fix unit tests --- .../ast/testdata/jquery-selector.txt | 348 +++++++++++++----- 1 file changed, 251 insertions(+), 97 deletions(-) diff --git a/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/ast/testdata/jquery-selector.txt b/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/ast/testdata/jquery-selector.txt index 92588d6387..5ddb5321df 100644 --- a/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/ast/testdata/jquery-selector.txt +++ b/pmd-javascript/src/test/resources/net/sourceforge/pmd/lang/ecmascript/ast/testdata/jquery-selector.txt @@ -1,4 +1,5 @@ +- AstRoot + +- Comment +- EmptyStatement +- EmptyStatement +- EmptyStatement @@ -9,6 +10,7 @@ +- EmptyStatement +- EmptyStatement +- EmptyStatement + +- Comment +- EmptyStatement +- EmptyStatement +- EmptyStatement @@ -338,6 +340,7 @@ | | +- IfStatement | | | +- Name | | | +- Scope + | | | +- Comment | | | +- ReturnStatement | | | +- Name | | +- ReturnStatement @@ -451,6 +454,7 @@ | | +- InfixExpression | | +- Name | | +- ArrayLiteral + | +- Comment | +- IfStatement | | +- InfixExpression | | | +- InfixExpression @@ -491,6 +495,8 @@ | | +- IfStatement | | +- Name | | +- Scope + | | +- Comment + | | +- Comment | | +- IfStatement | | | +- InfixExpression | | | | +- InfixExpression @@ -505,6 +511,7 @@ | | | | | +- Name | | | | +- Name | | | +- Scope + | | | +- Comment | | | +- IfStatement | | | +- ParenthesizedExpression | | | | +- Assignment @@ -513,59 +520,62 @@ | | | | +- Name | | | | +- NumberLiteral | | | +- Scope + | | | | +- Comment | | | | +- IfStatement - | | | | +- InfixExpression - | | | | | +- Name - | | | | | +- NumberLiteral - | | | | +- Scope - | | | | | +- IfStatement - | | | | | | +- ParenthesizedExpression - | | | | | | | +- Assignment - | | | | | | | +- Name - | | | | | | | +- FunctionCall - | | | | | | | +- PropertyGet - | | | | | | | | +- Name - | | | | | | | | +- Name - | | | | | | | +- Name - | | | | | | +- Scope - | | | | | | +- ExpressionStatement - | | | | | | +- FunctionCall - | | | | | | +- PropertyGet - | | | | | | | +- Name - | | | | | | | +- Name - | | | | | | +- Name - | | | | | | +- Name - | | | | | +- ReturnStatement - | | | | | +- Name - | | | | +- Scope - | | | | +- IfStatement - | | | | +- InfixExpression - | | | | | +- Name - | | | | | +- InfixExpression - | | | | | +- ParenthesizedExpression - | | | | | | +- Assignment - | | | | | | +- Name - | | | | | | +- FunctionCall - | | | | | | +- PropertyGet - | | | | | | | +- Name - | | | | | | | +- Name - | | | | | | +- Name - | | | | | +- FunctionCall - | | | | | +- PropertyGet - | | | | | | +- Name - | | | | | | +- Name - | | | | | +- Name - | | | | | +- Name - | | | | +- Scope - | | | | +- ExpressionStatement - | | | | | +- FunctionCall - | | | | | +- PropertyGet - | | | | | | +- Name - | | | | | | +- Name - | | | | | +- Name - | | | | | +- Name - | | | | +- ReturnStatement - | | | | +- Name + | | | | | +- InfixExpression + | | | | | | +- Name + | | | | | | +- NumberLiteral + | | | | | +- Scope + | | | | | | +- IfStatement + | | | | | | | +- ParenthesizedExpression + | | | | | | | | +- Assignment + | | | | | | | | +- Name + | | | | | | | | +- FunctionCall + | | | | | | | | +- PropertyGet + | | | | | | | | | +- Name + | | | | | | | | | +- Name + | | | | | | | | +- Name + | | | | | | | +- Scope + | | | | | | | +- ExpressionStatement + | | | | | | | +- FunctionCall + | | | | | | | +- PropertyGet + | | | | | | | | +- Name + | | | | | | | | +- Name + | | | | | | | +- Name + | | | | | | | +- Name + | | | | | | +- ReturnStatement + | | | | | | | +- Name + | | | | | | +- Comment + | | | | | +- Scope + | | | | | +- IfStatement + | | | | | +- InfixExpression + | | | | | | +- Name + | | | | | | +- InfixExpression + | | | | | | +- ParenthesizedExpression + | | | | | | | +- Assignment + | | | | | | | +- Name + | | | | | | | +- FunctionCall + | | | | | | | +- PropertyGet + | | | | | | | | +- Name + | | | | | | | | +- Name + | | | | | | | +- Name + | | | | | | +- FunctionCall + | | | | | | +- PropertyGet + | | | | | | | +- Name + | | | | | | | +- Name + | | | | | | +- Name + | | | | | | +- Name + | | | | | +- Scope + | | | | | +- ExpressionStatement + | | | | | | +- FunctionCall + | | | | | | +- PropertyGet + | | | | | | | +- Name + | | | | | | | +- Name + | | | | | | +- Name + | | | | | | +- Name + | | | | | +- ReturnStatement + | | | | | +- Name + | | | | +- Comment | | | +- IfStatement | | | +- ElementGet | | | | +- Name @@ -583,7 +593,8 @@ | | | | | | +- Name | | | | | +- Name | | | | +- ReturnStatement - | | | | +- Name + | | | | | +- Name + | | | | +- Comment | | | +- IfStatement | | | +- InfixExpression | | | | +- ParenthesizedExpression @@ -636,6 +647,13 @@ | | | +- Assignment | | | +- Name | | | +- Name + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment | | +- IfStatement | | | +- InfixExpression | | | | +- InfixExpression @@ -654,6 +672,7 @@ | | | | | +- Name | | | | +- Name | | | +- Scope + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- Name @@ -670,6 +689,8 @@ | | | | | +- Name | | | | | +- Name | | | | +- Name + | | | +- Comment + | | | +- Comment | | | +- IfStatement | | | | +- InfixExpression | | | | | +- InfixExpression @@ -680,6 +701,7 @@ | | | | | +- Name | | | | | +- Name | | | | +- Scope + | | | | +- Comment | | | | +- IfStatement | | | | +- ParenthesizedExpression | | | | | +- Assignment @@ -800,6 +822,7 @@ | +- Name | +- Name | +- Name + +- Comment +- FunctionNode | +- Name | +- Block @@ -812,6 +835,7 @@ | | +- Name | | +- Name | | +- Block + | | +- Comment | | +- IfStatement | | | +- InfixExpression | | | | +- FunctionCall @@ -825,6 +849,7 @@ | | | | +- Name | | | | +- Name | | | +- Scope + | | | +- Comment | | | +- ExpressionStatement | | | +- UnaryExpression | | | +- ElementGet @@ -844,6 +869,7 @@ | | +- Name | +- ReturnStatement | +- Name + +- Comment +- FunctionNode | +- Name | +- Name @@ -856,6 +882,7 @@ | | +- KeywordLiteral | +- ReturnStatement | +- Name + +- Comment +- FunctionNode | +- Name | +- Name @@ -875,6 +902,7 @@ | | +- Name | | +- Name | +- Name + +- Comment +- FunctionNode | +- Name | +- Name @@ -900,19 +928,31 @@ | | +- Name | | +- Name | +- Name + +- Comment +- FunctionNode | +- Name | +- Name | +- Block + | +- Comment | +- ReturnStatement | +- FunctionNode | +- Name | +- Block + | +- Comment + | +- Comment + | +- Comment | +- IfStatement | | +- InfixExpression | | | +- StringLiteral | | | +- Name | | +- Scope + | | | +- Comment + | | | +- Comment + | | | +- Comment + | | | +- Comment + | | | +- Comment + | | | +- Comment + | | | +- Comment | | | +- IfStatement | | | | +- InfixExpression | | | | | +- PropertyGet @@ -924,6 +964,7 @@ | | | | | | +- Name | | | | | +- KeywordLiteral | | | | +- Scope + | | | | +- Comment | | | | +- IfStatement | | | | | +- InfixExpression | | | | | | +- StringLiteral @@ -971,11 +1012,14 @@ | | | | | +- Name | | | | +- Name | | | +- ReturnStatement - | | | +- InfixExpression - | | | +- PropertyGet - | | | | +- Name - | | | | +- Name - | | | +- Name + | | | | +- InfixExpression + | | | | +- PropertyGet + | | | | | +- Name + | | | | | +- Name + | | | | +- Name + | | | +- Comment + | | | +- Comment + | | | +- Comment | | +- IfStatement | | +- InfixExpression | | | +- StringLiteral @@ -989,6 +1033,7 @@ | | +- Name | +- ReturnStatement | +- KeywordLiteral + +- Comment +- FunctionNode | +- Name | +- Name @@ -1028,6 +1073,7 @@ | | +- PropertyGet | | +- Name | | +- Name + | +- Comment | +- WhileLoop | +- UnaryExpression | | +- Name @@ -1056,6 +1102,7 @@ | +- ElementGet | +- Name | +- Name + +- Comment +- FunctionNode | +- Name | +- Name @@ -1071,6 +1118,7 @@ | | | +- Name | | +- StringLiteral | +- Name + +- Comment +- FunctionNode | +- Name | +- Name @@ -1088,6 +1136,11 @@ | | | | +- Name | | | +- Name | | +- Name + | +- Comment + | +- Comment + | +- Comment + | +- Comment + | +- Comment | +- IfStatement | | +- InfixExpression | | | +- InfixExpression @@ -1119,6 +1172,12 @@ | | | +- Name | | | +- Name | | +- Name + | +- Comment + | +- Comment + | +- Comment + | +- Comment + | +- Comment + | +- Comment | +- IfStatement | +- InfixExpression | | +- InfixExpression @@ -1137,6 +1196,7 @@ | | | +- Name | | +- Name | +- Scope + | +- Comment | +- ExpressionStatement | +- FunctionCall | +- PropertyGet @@ -1288,11 +1348,12 @@ | | | | +- StringLiteral | | | +- Scope | | | | +- ReturnStatement - | | | | +- FunctionCall - | | | | +- PropertyGet - | | | | | +- Name - | | | | | +- Name - | | | | +- Name + | | | | | +- FunctionCall + | | | | | +- PropertyGet + | | | | | | +- Name + | | | | | | +- Name + | | | | | +- Name + | | | | +- Comment | | | +- Scope | | | +- ReturnStatement | | | +- FunctionCall @@ -1376,6 +1437,7 @@ | | | | | +- Name | | | | +- Name | | | | +- Name + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- ElementGet @@ -1431,6 +1493,7 @@ | | | +- FunctionNode | | | +- Name | | | +- Block + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- ElementGet @@ -1454,6 +1517,7 @@ | | | | | | +- NumberLiteral | | | | | +- StringLiteral | | | | +- Scope + | | | | | +- Comment | | | | | +- IfStatement | | | | | | +- UnaryExpression | | | | | | | +- ElementGet @@ -1502,26 +1566,27 @@ | | | | | | | +- NumberLiteral | | | | | | +- StringLiteral | | | | | +- ExpressionStatement - | | | | | +- Assignment - | | | | | +- ElementGet - | | | | | | +- Name - | | | | | | +- NumberLiteral - | | | | | +- UnaryExpression - | | | | | +- ParenthesizedExpression - | | | | | +- InfixExpression - | | | | | +- ParenthesizedExpression - | | | | | | +- InfixExpression - | | | | | | +- ElementGet - | | | | | | | +- Name - | | | | | | | +- NumberLiteral - | | | | | | +- ElementGet - | | | | | | +- Name - | | | | | | +- NumberLiteral - | | | | | +- InfixExpression - | | | | | +- ElementGet - | | | | | | +- Name - | | | | | | +- NumberLiteral - | | | | | +- StringLiteral + | | | | | | +- Assignment + | | | | | | +- ElementGet + | | | | | | | +- Name + | | | | | | | +- NumberLiteral + | | | | | | +- UnaryExpression + | | | | | | +- ParenthesizedExpression + | | | | | | +- InfixExpression + | | | | | | +- ParenthesizedExpression + | | | | | | | +- InfixExpression + | | | | | | | +- ElementGet + | | | | | | | | +- Name + | | | | | | | | +- NumberLiteral + | | | | | | | +- ElementGet + | | | | | | | +- Name + | | | | | | | +- NumberLiteral + | | | | | | +- InfixExpression + | | | | | | +- ElementGet + | | | | | | | +- Name + | | | | | | | +- NumberLiteral + | | | | | | +- StringLiteral + | | | | | +- Comment | | | | +- IfStatement | | | | +- ElementGet | | | | | +- Name @@ -1572,19 +1637,20 @@ | | | | +- NumberLiteral | | | +- Scope | | | | +- ExpressionStatement - | | | | +- Assignment - | | | | +- ElementGet - | | | | | +- Name - | | | | | +- NumberLiteral - | | | | +- InfixExpression - | | | | +- ElementGet - | | | | | +- Name - | | | | | +- NumberLiteral - | | | | +- InfixExpression - | | | | +- ElementGet - | | | | | +- Name - | | | | | +- NumberLiteral - | | | | +- StringLiteral + | | | | | +- Assignment + | | | | | +- ElementGet + | | | | | | +- Name + | | | | | | +- NumberLiteral + | | | | | +- InfixExpression + | | | | | +- ElementGet + | | | | | | +- Name + | | | | | | +- NumberLiteral + | | | | | +- InfixExpression + | | | | | +- ElementGet + | | | | | | +- Name + | | | | | | +- NumberLiteral + | | | | | +- StringLiteral + | | | | +- Comment | | | +- IfStatement | | | +- InfixExpression | | | | +- Name @@ -1620,6 +1686,7 @@ | | | | +- Name | | | | +- Name | | | +- Scope + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- ElementGet @@ -2046,6 +2113,7 @@ | | | +- IfStatement | | | +- Name | | | +- Scope + | | | +- Comment | | | +- IfStatement | | | | +- Name | | | | +- Scope @@ -2079,6 +2147,7 @@ | | | | | | +- Scope | | | | | | +- ReturnStatement | | | | | | +- KeywordLiteral + | | | | | +- Comment | | | | | +- ExpressionStatement | | | | | +- Assignment | | | | | +- Name @@ -2106,11 +2175,13 @@ | | | | +- PropertyGet | | | | +- Name | | | | +- Name + | | | +- Comment | | | +- IfStatement | | | | +- InfixExpression | | | | | +- Name | | | | | +- Name | | | | +- Scope + | | | | | +- Comment | | | | | +- ExpressionStatement | | | | | | +- Assignment | | | | | | +- Name @@ -2187,6 +2258,7 @@ | | | | | | +- Name | | | | | | +- Name | | | | | +- Scope + | | | | | +- Comment | | | | | +- IfStatement | | | | | +- InfixExpression | | | | | | +- InfixExpression @@ -2212,6 +2284,7 @@ | | | | | | +- Name | | | | | +- BreakStatement | | | | +- Scope + | | | | +- Comment | | | | +- IfStatement | | | | | +- Name | | | | | +- Scope @@ -2257,6 +2330,7 @@ | | | | | +- Name | | | | | +- KeywordLiteral | | | | +- Scope + | | | | +- Comment | | | | +- WhileLoop | | | | +- ParenthesizedExpression | | | | | +- Assignment @@ -2299,6 +2373,7 @@ | | | | | +- UnaryExpression | | | | | +- Name | | | | +- Scope + | | | | +- Comment | | | | +- IfStatement | | | | | +- Name | | | | | +- Scope @@ -2329,6 +2404,7 @@ | | | | | +- Name | | | | +- Scope | | | | +- BreakStatement + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- Name @@ -2356,6 +2432,10 @@ | | +- Name | | +- Name | | +- Block + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment | | +- VariableDeclaration | | | +- VariableInitializer | | | | +- Name @@ -2381,6 +2461,9 @@ | | | +- InfixExpression | | | +- StringLiteral | | | +- Name + | | +- Comment + | | +- Comment + | | +- Comment | | +- IfStatement | | | +- ElementGet | | | | +- Name @@ -2487,6 +2570,9 @@ | | +- FunctionNode | | +- Name | | +- Block + | | +- Comment + | | +- Comment + | | +- Comment | | +- VariableDeclaration | | | +- VariableInitializer | | | | +- Name @@ -2533,6 +2619,7 @@ | | | | +- PropertyGet | | | | +- Name | | | | +- Name + | | | +- Comment | | | +- WhileLoop | | | +- UnaryExpression | | | | +- Name @@ -2575,6 +2662,7 @@ | | | +- KeywordLiteral | | | +- Name | | | +- Name + | | +- Comment | | +- ExpressionStatement | | | +- Assignment | | | +- ElementGet @@ -2652,6 +2740,7 @@ | | +- FunctionNode | | +- Name | | +- Block + | | +- Comment | | +- IfStatement | | | +- UnaryExpression | | | | +- FunctionCall @@ -2830,6 +2919,8 @@ | | +- FunctionNode | | +- Name | | +- Block + | | +- Comment + | | +- Comment | | +- ReturnStatement | | +- InfixExpression | | +- ParenthesizedExpression @@ -2859,11 +2950,16 @@ | | +- FunctionNode | | +- Name | | +- Block + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment | | +- IfStatement | | | +- PropertyGet | | | | +- Name | | | | +- Name | | | +- Scope + | | | +- Comment | | | +- ExpressionStatement | | | +- PropertyGet | | | +- PropertyGet @@ -2881,6 +2977,10 @@ | | +- FunctionNode | | +- Name | | +- Block + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment | | +- ForLoop | | | +- Assignment | | | | +- Name @@ -3182,6 +3282,7 @@ | | +- Name | | +- Name | +- Name + +- Comment +- ForInLoop | +- Name | +- ObjectLiteral @@ -3231,6 +3332,7 @@ | +- FunctionCall | +- Name | +- Name + +- Comment +- FunctionNode | +- Name | +- Block @@ -3309,6 +3411,7 @@ | +- WhileLoop | | +- Name | | +- Scope + | | +- Comment | | +- IfStatement | | | +- InfixExpression | | | | +- UnaryExpression @@ -3325,6 +3428,7 @@ | | | +- IfStatement | | | | +- Name | | | | +- Scope + | | | | +- Comment | | | | +- ExpressionStatement | | | | +- Assignment | | | | +- Name @@ -3352,6 +3456,7 @@ | | | +- Assignment | | | +- Name | | | +- KeywordLiteral + | | +- Comment | | +- IfStatement | | | +- ParenthesizedExpression | | | | +- Assignment @@ -3468,6 +3573,9 @@ | | | +- Name | | +- Scope | | +- BreakStatement + | +- Comment + | +- Comment + | +- Comment | +- IfStatement | | +- Name | | +- Scope @@ -3606,6 +3714,7 @@ | | +- ArrayLiteral | | +- Name | | +- Name + | +- Comment | +- IfStatement | | +- Name | | +- Scope @@ -3702,6 +3811,7 @@ | | | | +- NumberLiteral | | | +- Name | | +- Scope + | | | +- Comment | | | +- ReturnStatement | | | +- ParenthesizedExpression | | | +- Assignment @@ -3712,12 +3822,14 @@ | | | +- Name | | | +- NumberLiteral | | +- Scope + | | +- Comment | | +- ExpressionStatement | | | +- Assignment | | | +- ElementGet | | | | +- Name | | | | +- Name | | | +- Name + | | +- Comment | | +- IfStatement | | +- ParenthesizedExpression | | | +- Assignment @@ -3985,6 +4097,8 @@ | +- IfStatement | | +- Name | | +- Scope + | | | +- Comment + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- Name @@ -4000,6 +4114,7 @@ | | | | | +- Name | | | | +- ArrayLiteral | | | | +- Name + | | | +- Comment | | | +- ExpressionStatement | | | +- FunctionCall | | | +- Name @@ -4012,6 +4127,7 @@ | | +- Assignment | | +- Name | | +- Name + | +- Comment | +- IfStatement | | +- Name | | +- Scope @@ -4029,6 +4145,7 @@ | | | +- ArrayLiteral | | | +- Name | | | +- Name + | | +- Comment | | +- ExpressionStatement | | | +- Assignment | | | +- Name @@ -4074,6 +4191,7 @@ | | +- IfStatement | | | +- Name | | | +- Scope + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- Name @@ -4096,6 +4214,7 @@ | | | | | +- Name | | | | | +- Name | | | | +- Scope + | | | | +- Comment | | | | +- ExpressionStatement | | | | +- FunctionCall | | | | +- PropertyGet @@ -4316,6 +4435,7 @@ | | | +- Name | | | +- Name | | | +- Name + | | +- Comment | | +- ExpressionStatement | | | +- Assignment | | | +- Name @@ -4376,11 +4496,13 @@ | | | | +- Name | | | | +- Name | | | +- Name + | | +- Comment | | +- IfStatement | | | +- ElementGet | | | | +- Name | | | | +- Name | | | +- Scope + | | | +- Comment | | | +- ExpressionStatement | | | | +- Assignment | | | | +- Name @@ -4583,6 +4705,10 @@ | | +- IfStatement | | | +- Name | | | +- Scope + | | | +- Comment + | | | +- Comment + | | | +- Comment + | | | +- Comment | | | +- ExpressionStatement | | | +- Assignment | | | +- Name @@ -4615,6 +4741,10 @@ | | | | | +- Assignment | | | | | +- Name | | | | | +- NumberLiteral + | | | | +- Comment + | | | | +- Comment + | | | | +- Comment + | | | | +- Comment | | | | +- IfStatement | | | | | +- InfixExpression | | | | | | +- UnaryExpression @@ -4670,6 +4800,7 @@ | | | +- IfStatement | | | +- Name | | | +- Scope + | | | +- Comment | | | +- IfStatement | | | | +- ParenthesizedExpression | | | | | +- Assignment @@ -4691,10 +4822,19 @@ | | | | +- Name | | | | +- Name | | | +- Name + | | +- Comment + | | +- Comment | | +- ExpressionStatement | | | +- Assignment | | | +- Name | | | +- Name + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment + | | +- Comment | | +- IfStatement | | | +- InfixExpression | | | | +- Name @@ -4725,6 +4865,7 @@ | | | +- IfStatement | | | | +- Name | | | | +- Scope + | | | | +- Comment | | | | +- IfStatement | | | | | +- InfixExpression | | | | | | +- Name @@ -4768,6 +4909,7 @@ | | | | | +- Name | | | | +- Name | | | | +- Name + | | | +- Comment | | | +- IfStatement | | | +- InfixExpression | | | | +- Name @@ -4840,6 +4982,7 @@ | | +- UnaryExpression | | | +- Name | | +- Scope + | | +- Comment | | +- IfStatement | | | +- UnaryExpression | | | | +- Name @@ -4886,6 +5029,7 @@ | | | | +- Name | | | | +- Name | | | +- Name + | | +- Comment | | +- ExpressionStatement | | | +- Assignment | | | +- Name @@ -4896,6 +5040,7 @@ | | | +- Name | | | +- Name | | | +- Name + | | +- Comment | | +- ExpressionStatement | | +- Assignment | | +- PropertyGet @@ -4904,6 +5049,7 @@ | | +- Name | +- ReturnStatement | +- Name + +- Comment +- FunctionNode | +- Name | +- Name @@ -4951,6 +5097,8 @@ | | +- InfixExpression | | +- Name | | +- ArrayLiteral + | +- Comment + | +- Comment | +- IfStatement | | +- InfixExpression | | | +- PropertyGet @@ -4958,6 +5106,7 @@ | | | | +- Name | | | +- NumberLiteral | | +- Scope + | | +- Comment | | +- ExpressionStatement | | | +- Assignment | | | +- Name @@ -5038,7 +5187,8 @@ | | | | | +- Name | | | | +- Scope | | | | | +- ReturnStatement - | | | | | +- Name + | | | | | | +- Name + | | | | | +- Comment | | | | +- IfStatement | | | | +- Name | | | | +- Scope @@ -5088,6 +5238,7 @@ | | | +- ElementGet | | | +- Name | | | +- Name + | | +- Comment | | +- IfStatement | | | +- ElementGet | | | | +- PropertyGet @@ -5111,6 +5262,7 @@ | | | | +- Name | | | +- Name | | +- Scope + | | +- Comment | | +- IfStatement | | +- ParenthesizedExpression | | | +- Assignment @@ -5145,6 +5297,7 @@ | | | | +- Name | | | +- Name | | +- Scope + | | +- Comment | | +- ExpressionStatement | | | +- FunctionCall | | | +- PropertyGet @@ -5208,6 +5361,7 @@ | | +- Name | +- ReturnStatement | +- Name + +- Comment +- ExpressionStatement | +- FunctionCall | +- Name From 1081448e5d71a28407cbc06b65be6422ef44f3ea Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 29 Apr 2021 15:13:04 +0200 Subject: [PATCH 06/80] [pmd-dist] Exclude javascript in java7 AllRules integration test --- pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java index 8ba9e0533f..4f9a38ff0f 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/AllRulesIT.java @@ -22,8 +22,8 @@ public class AllRulesIT extends AbstractBinaryDistributionTest { @Parameters public static Iterable languagesToTest() { if (PMDExecutor.isJava7Test()) { - // note: apex, scala, and visualforce require java8 - return Arrays.asList("java", "javascript", "jsp", "modelica", + // note: apex, javascript, scala, and visualforce require java8 + return Arrays.asList("java", "jsp", "modelica", "plsql", "pom", "velocitytemplate", "xml", "xsl"); } // note: scala and wsdl have no rules From 48c8654f3b754a3d918494b266c9bb44f2c2ff2f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 29 Apr 2021 17:00:57 +0200 Subject: [PATCH 07/80] [apex] Correct findBoundary when traversing AST FindBoundary was not considered yet in Apex. That means, that `node.findDescendantsOfType(ASTVariableExpression.class)` found variables usages within the current class and within any nested class. Now the nested classes are not considered by default. Rules that make use of rulechain for ASTUserClass will visit both classes. Rules, that use a standard visitor, need to call `super.visit(...)` when visiting ASTUserClass. Note that this applies for all root nodes ASTUserClass, ASTUserEnum, ASTUserInterface, ASTUserTrigger. Some rules are fixed to use rulechain. --- .../pmd/lang/apex/ast/ApexRootNode.java | 5 +++++ .../apex/rule/security/ApexBadCryptoRule.java | 1 + .../security/ApexDangerousMethodsRule.java | 2 +- .../rule/security/ApexOpenRedirectRule.java | 2 +- .../rule/security/ApexSOQLInjectionRule.java | 1 + .../ApexSuggestUsingNamedCredRule.java | 2 +- .../security/ApexXSSFromEscapeFalseRule.java | 1 + .../apex/rule/security/xml/ApexBadCrypto.xml | 19 +++++++++++++++++++ .../rule/security/xml/ApexSOQLInjection.xml | 18 ++++++++++++++++++ .../security/xml/ApexXSSFromEscapeFalse.xml | 16 ++++++++++++++++ 10 files changed, 64 insertions(+), 3 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexRootNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexRootNode.java index ffab8193bc..a14c241b44 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexRootNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ApexRootNode.java @@ -50,4 +50,9 @@ public abstract class ApexRootNode extends AbstractApexNode potentiallyStaticBlob = new HashSet<>(); public ApexBadCryptoRule() { + addRuleChainVisit(ASTUserClass.class); setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java index 9d908dcf10..f6a031ca98 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexDangerousMethodsRule.java @@ -40,7 +40,7 @@ public class ApexDangerousMethodsRule extends AbstractApexRule { private final Set whiteListedVariables = new HashSet<>(); public ApexDangerousMethodsRule() { - super.addRuleChainVisit(ASTUserClass.class); + addRuleChainVisit(ASTUserClass.class); setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java index 4cc197a0b5..be4827e185 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexOpenRedirectRule.java @@ -31,7 +31,7 @@ public class ApexOpenRedirectRule extends AbstractApexRule { private final Set listOfStringLiteralVariables = new HashSet<>(); public ApexOpenRedirectRule() { - super.addRuleChainVisit(ASTUserClass.class); + addRuleChainVisit(ASTUserClass.class); setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSOQLInjectionRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSOQLInjectionRule.java index 1e31919133..4248ce6759 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSOQLInjectionRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSOQLInjectionRule.java @@ -51,6 +51,7 @@ public class ApexSOQLInjectionRule extends AbstractApexRule { private final Map selectContainingVariables = new HashMap<>(); public ApexSOQLInjectionRule() { + addRuleChainVisit(ASTUserClass.class); setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSuggestUsingNamedCredRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSuggestUsingNamedCredRule.java index e7aa416651..5c650304e0 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSuggestUsingNamedCredRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexSuggestUsingNamedCredRule.java @@ -34,7 +34,7 @@ public class ApexSuggestUsingNamedCredRule extends AbstractApexRule { private final Set listOfAuthorizationVariables = new HashSet<>(); public ApexSuggestUsingNamedCredRule() { - super.addRuleChainVisit(ASTUserClass.class); + addRuleChainVisit(ASTUserClass.class); setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexXSSFromEscapeFalseRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexXSSFromEscapeFalseRule.java index afc190e55e..6b9c32e4ac 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexXSSFromEscapeFalseRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexXSSFromEscapeFalseRule.java @@ -23,6 +23,7 @@ public class ApexXSSFromEscapeFalseRule extends AbstractApexRule { private static final String ADD_ERROR = "addError"; public ApexXSSFromEscapeFalseRule() { + addRuleChainVisit(ASTUserClass.class); setProperty(CODECLIMATE_CATEGORIES, "Security"); setProperty(CODECLIMATE_REMEDIATION_MULTIPLIER, 100); setProperty(CODECLIMATE_BLOCK_HIGHLIGHTING, false); diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexBadCrypto.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexBadCrypto.xml index 555fd10394..42cd5e3a07 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexBadCrypto.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexBadCrypto.xml @@ -7,6 +7,7 @@ Apex Crypto hardcoded IV 1 + 6 + + + + Apex Crypto hardcoded IV in inner class + 1 + 7 + diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml index 4fdafa534c..80e244a84c 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexSOQLInjection.xml @@ -7,6 +7,7 @@ Potentially unsafe SOQL on concatenation of variables 1 1 + 5 res = Database.query('Select Id,Name From ' + (name == 'Account' ? name : 'Cases')); } +} + ]]> + + + + Potentially unsafe SOQL on concatenation of variables in nested class + 1 + 6 + diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexXSSFromEscapeFalse.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexXSSFromEscapeFalse.xml index 99cf9ac528..ca3907e778 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexXSSFromEscapeFalse.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexXSSFromEscapeFalse.xml @@ -7,6 +7,7 @@ Add error variable with escape false 1 + 3 + + + + Add error variable with escape false in nested class + 1 + 4 + From 103e5b27f53dfcf4102b7327a2383b8b0b7e94b3 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 29 Apr 2021 17:04:48 +0200 Subject: [PATCH 08/80] [doc] Update release notes, refs #3243 --- docs/pages/release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..2de24dea40 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -16,6 +16,9 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues +* apex + * [#3243](https://github.com/pmd/pmd/pull/3243): \[apex] Correct findBoundary when traversing AST + ### API Changes ### External Contributions From bdf056e96d98f2600bee95f451167d0689e24ea1 Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Thu, 29 Apr 2021 12:12:15 +0200 Subject: [PATCH 09/80] issue 3239 Implement best practice rule 'JUnit5TestShouldBePackagePrivate' Add the rule to check that JUnit5 tests methods and classes are package private. Add unit tests --- .../resources/category/java/bestpractices.xml | 70 +++++++++++++++++++ .../JUnit5TestShouldBePackagePrivateTest.java | 11 +++ .../xml/JUnit5TestShouldBePackagePrivate.xml | 63 +++++++++++++++++ 3 files changed, 144 insertions(+) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/JUnit5TestShouldBePackagePrivateTest.java create mode 100644 pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index e8c1f71d4c..880e372345 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -802,6 +802,76 @@ public class MyTest { + + + + JUnit 5 tests should be package private at the class level and for each method that uses @Test, @RepeatedTest, @TestFactory, @TestTemplate or @ParameterizedTest. + + 3 + + + + + + + + + + + + + + + + + + + Public modifier is not necessary on a test method nor on the class + 2 + + + + + Package private modifiers are what is required + 0 + + + + + Non package private modifiers on all JUnit5 test types should be rejected + 6 + + + From 345f9bfd931205a12127766b6c9f69301ed78f86 Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Tue, 4 May 2021 13:22:04 +0200 Subject: [PATCH 10/80] Fix build AFAICT the site is generated from the XML rule file, so simply add an externalInfoUrl that points at where the rule should end up. Also put in the correct since date. --- pmd-java/src/main/resources/category/java/bestpractices.xml | 6 +++--- pmd-java/src/main/resources/rulesets/java/quickstart.xml | 1 + 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 880e372345..2d28c7e7cb 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -802,13 +802,13 @@ public class MyTest { - + typeResolution="true" + externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#junit5testshouldbepackageprivate"> JUnit 5 tests should be package private at the class level and for each method that uses @Test, @RepeatedTest, @TestFactory, @TestTemplate or @ParameterizedTest. diff --git a/pmd-java/src/main/resources/rulesets/java/quickstart.xml b/pmd-java/src/main/resources/rulesets/java/quickstart.xml index b1b89a532b..23b2c85034 100644 --- a/pmd-java/src/main/resources/rulesets/java/quickstart.xml +++ b/pmd-java/src/main/resources/rulesets/java/quickstart.xml @@ -27,6 +27,7 @@ + From 6f11a61b4baa97d80766bd03240a35769fb3a9cc Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Tue, 4 May 2021 18:37:16 +0200 Subject: [PATCH 11/80] Fix example in rule --- pmd-java/src/main/resources/category/java/bestpractices.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 2d28c7e7cb..35e2cbd960 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -847,7 +847,7 @@ public class MyTest { Date: Tue, 4 May 2021 19:49:53 +0200 Subject: [PATCH 12/80] [java] AvoidFieldNameMatchingTypeName fix FN with interfaces Fields in interfaces and nested classes have not been considered. --- .../AvoidFieldNameMatchingTypeNameRule.java | 8 ++------ .../xml/AvoidFieldNameMatchingTypeName.xml | 17 +++++++++++++++++ 2 files changed, 19 insertions(+), 6 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidFieldNameMatchingTypeNameRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidFieldNameMatchingTypeNameRule.java index ca96b63ebb..85c19e99c8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidFieldNameMatchingTypeNameRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidFieldNameMatchingTypeNameRule.java @@ -10,12 +10,8 @@ import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; public class AvoidFieldNameMatchingTypeNameRule extends AbstractJavaRule { - @Override - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (node.isInterface()) { - return data; - } - return super.visit(node, data); + public AvoidFieldNameMatchingTypeNameRule() { + addRuleChainVisit(ASTFieldDeclaration.class); } @Override diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidFieldNameMatchingTypeName.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidFieldNameMatchingTypeName.xml index 3be3e6d7dc..92dcef5758 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidFieldNameMatchingTypeName.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/AvoidFieldNameMatchingTypeName.xml @@ -54,6 +54,23 @@ public class Foo { + + + + false negative with fields in interfaces and nested classes + 2 + 4,7 + { return null; }; + + class Inner { + int inner; + } } ]]> From a8da3d53473598fea0061d1a10510fe67e3c3e40 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Tue, 4 May 2021 19:58:33 +0200 Subject: [PATCH 13/80] [doc] Update release notes, rule doc for AvoidFieldNameMatchingTypeName - refs #3249 --- docs/pages/release_notes.md | 2 ++ pmd-java/src/main/resources/category/java/errorprone.xml | 5 ++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 3ffcb66921..b8a4222452 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,6 +18,8 @@ This is a {{ site.pmd.release_type }} release. * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages +* java-errorprone + * [#3249](https://github.com/pmd/pmd/pull/3249): \[java] AvoidFieldNameMatchingTypeName: False negative with interfaces ### API Changes diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index d440ac8cc6..bb35ab8ae7 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -398,7 +398,7 @@ public class Foo { class="net.sourceforge.pmd.lang.java.rule.errorprone.AvoidFieldNameMatchingTypeNameRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#avoidfieldnamematchingtypename"> -It is somewhat confusing to have a field name matching the declaring class name. +It is somewhat confusing to have a field name matching the declaring type name. This probably means that type and/or field names should be chosen more carefully. 3 @@ -407,6 +407,9 @@ This probably means that type and/or field names should be chosen more carefully public class Foo extends Bar { int foo; // There is probably a better name that can be used } +public interface Operation { + int OPERATION = 1; // There is probably a better name that can be used +} ]]> From a37f4581a8f1397849b162eaf974d8064b8f1006 Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Tue, 4 May 2021 20:52:29 +0200 Subject: [PATCH 14/80] PR review Tweak descriptions, add unit test for Junit4... --- .../resources/category/java/bestpractices.xml | 37 +++++++------------ .../xml/JUnit5TestShouldBePackagePrivate.xml | 12 ++++++ 2 files changed, 26 insertions(+), 23 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 35e2cbd960..ece9af091f 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -804,22 +804,24 @@ public class MyTest { - JUnit 5 tests should be package private at the class level and for each method that uses @Test, @RepeatedTest, @TestFactory, @TestTemplate or @ParameterizedTest. +JUnit 5 tests should be package private at the class level and for each method that uses @Test, @RepeatedTest, +@TestFactory, @TestTemplate or @ParameterizedTest. +Contrary to JUnit4 tests that required public visibility to be run by the engine, JUnit5 tests may be run +only with package private visibility. Marking them as such is a good practice to limit their visibility. 3 - - - - + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml index 93117786fc..cebcb606a5 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml @@ -57,6 +57,18 @@ protected class MyTests { @ParameterizedTest @ValueSource(strings = {"Hello", "World"}) protected void testParameterized(final String value) { } +} + ]]> + + + + Public JUnit4 tests are not flagged + 0 + From c1b537247a857ac04ed101bf4dbbb5285b428361 Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Tue, 4 May 2021 21:26:11 +0200 Subject: [PATCH 15/80] Make rule report error on the method declaration rather than the body declaration --- pmd-java/src/main/resources/category/java/bestpractices.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index ece9af091f..9c51974c92 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -834,12 +834,12 @@ only with package private visibility. Marking them as such is a good practice to | //ClassOrInterfaceDeclaration /ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration - [MethodDeclaration[@PackagePrivate=false()]] [Annotation//Name[ pmd-java:typeIs('org.junit.jupiter.api.Test') or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') ]] + /MethodDeclaration[@PackagePrivate=false()] ]]> From bee6e3a2bc6ce42e92745e14864dc95bc1831799 Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Wed, 5 May 2021 20:44:54 +0200 Subject: [PATCH 16/80] PR review: merge parts of the rule --- .../resources/category/java/bestpractices.xml | 22 +++++++++---------- 1 file changed, 11 insertions(+), 11 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 9c51974c92..5c0f172d91 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -812,8 +812,8 @@ public class MyTest { JUnit 5 tests should be package private at the class level and for each method that uses @Test, @RepeatedTest, @TestFactory, @TestTemplate or @ParameterizedTest. -Contrary to JUnit4 tests that required public visibility to be run by the engine, JUnit5 tests may be run -only with package private visibility. Marking them as such is a good practice to limit their visibility. +Contrary to JUnit4 tests that required public visibility to be run by the engine, JUnit5 tests can also be run +if they're package-private. Marking them as such is a good practice to limit their visibility. 3 @@ -822,27 +822,27 @@ only with package private visibility. Marking them as such is a good practice to - + Date: Thu, 6 May 2021 10:12:13 +0200 Subject: [PATCH 17/80] Update gems Fixes CVE-2021-28965 https://github.com/advisories/GHSA-8cr8-4vfw-mr7h --- Gemfile.lock | 4 ++-- docs/Gemfile.lock | 22 +++++++++++++--------- 2 files changed, 15 insertions(+), 11 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 3d17e8b730..d18bd6b08e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -39,14 +39,14 @@ GEM kramdown (1.17.0) liquid (5.0.1) logger-colors (1.0.0) - mini_portile2 (2.5.0) + mini_portile2 (2.5.1) multipart-post (2.1.1) nap (1.1.0) no_proxy_fix (0.1.2) nokogiri (1.11.3) mini_portile2 (~> 2.5.0) racc (~> 1.4) - octokit (4.20.0) + octokit (4.21.0) faraday (>= 0.9) sawyer (~> 0.8.0, >= 0.5.3) open4 (1.3.4) diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock index 4b36ce2dfd..1809f3590a 100644 --- a/docs/Gemfile.lock +++ b/docs/Gemfile.lock @@ -1,7 +1,7 @@ GEM remote: https://rubygems.org/ specs: - activesupport (6.0.3.6) + activesupport (6.0.3.7) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) @@ -22,15 +22,19 @@ GEM em-websocket (0.5.2) eventmachine (>= 0.12.9) http_parser.rb (~> 0.6.0) - ethon (0.12.0) - ffi (>= 1.3.0) + ethon (0.14.0) + ffi (>= 1.15.0) eventmachine (1.2.7) execjs (2.7.0) - faraday (1.3.0) + faraday (1.4.1) + faraday-excon (~> 1.1) faraday-net_http (~> 1.0) + faraday-net_http_persistent (~> 1.1) multipart-post (>= 1.2, < 3) - ruby2_keywords + ruby2_keywords (>= 0.0.4) + faraday-excon (1.1.0) faraday-net_http (1.0.1) + faraday-net_http_persistent (1.1.0) ffi (1.15.0) forwardable-extended (2.6.0) gemoji (3.0.1) @@ -205,17 +209,17 @@ GEM rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) mercenary (0.3.6) - mini_portile2 (2.5.0) + mini_portile2 (2.5.1) minima (2.5.1) jekyll (>= 3.5, < 5.0) jekyll-feed (~> 0.9) jekyll-seo-tag (~> 2.1) minitest (5.14.4) multipart-post (2.1.1) - nokogiri (1.11.2) + nokogiri (1.11.3) mini_portile2 (~> 2.5.0) racc (~> 1.4) - octokit (4.20.0) + octokit (4.21.0) faraday (>= 0.9) sawyer (~> 0.8.0, >= 0.5.3) pathutil (0.16.2) @@ -225,7 +229,7 @@ GEM rb-fsevent (0.10.4) rb-inotify (0.10.1) ffi (~> 1.0) - rexml (3.2.4) + rexml (3.2.5) rouge (3.26.0) ruby-enum (0.9.0) i18n From b73afc47f7a7f6b296f4c79fa106e1ecdc5155b5 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 10:25:28 +0200 Subject: [PATCH 18/80] Bump build-tools from 11 to 12 --- .github/workflows/build.yml | 3 ++- .github/workflows/git-repo-sync.yml | 2 +- .github/workflows/troubleshooting.yml | 2 +- pom.xml | 2 +- 4 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index d38e793524..bc43545a09 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -11,6 +11,7 @@ on: schedule: # build it monthly: At 04:00 on day-of-month 1. - cron: '0 4 1 * *' + workflow_dispatch: jobs: build: @@ -43,7 +44,7 @@ jobs: run: | echo "LANG=en_US.UTF-8" >> $GITHUB_ENV echo "MAVEN_OPTS=-Dmaven.wagon.httpconnectionManager.ttlSeconds=180 -Dmaven.wagon.http.retryHandler.count=3 -DautoReleaseAfterClose=true -DstagingProgressTimeoutMinutes=30" >> $GITHUB_ENV - echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/11/scripts" >> $GITHUB_ENV + echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/12/scripts" >> $GITHUB_ENV - name: Check Environment shell: bash run: | diff --git a/.github/workflows/git-repo-sync.yml b/.github/workflows/git-repo-sync.yml index 510f048c44..4476dcae95 100644 --- a/.github/workflows/git-repo-sync.yml +++ b/.github/workflows/git-repo-sync.yml @@ -21,7 +21,7 @@ jobs: shell: bash run: | echo "LANG=en_US.UTF-8" >> $GITHUB_ENV - echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/11/scripts" >> $GITHUB_ENV + echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/12/scripts" >> $GITHUB_ENV - name: Sync run: .ci/git-repo-sync.sh shell: bash diff --git a/.github/workflows/troubleshooting.yml b/.github/workflows/troubleshooting.yml index d5a8965dad..10a35d9785 100644 --- a/.github/workflows/troubleshooting.yml +++ b/.github/workflows/troubleshooting.yml @@ -31,7 +31,7 @@ jobs: run: | echo "LANG=en_US.UTF-8" >> $GITHUB_ENV echo "MAVEN_OPTS=-Dmaven.wagon.httpconnectionManager.ttlSeconds=180 -Dmaven.wagon.http.retryHandler.count=3 -DstagingProgressTimeoutMinutes=30" >> $GITHUB_ENV - echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/11/scripts" >> $GITHUB_ENV + echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/12/scripts" >> $GITHUB_ENV - name: Check Environment shell: bash run: | diff --git a/pom.xml b/pom.xml index 068c80b77c..3b89a6eaaf 100644 --- a/pom.xml +++ b/pom.xml @@ -105,7 +105,7 @@ -Xmx512m -Dfile.encoding=${project.build.sourceEncoding} - 11 + 12 6.27.0 From 945c030d7f97bbeae0a058be1fbd0e39aee34b8d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 10:25:55 +0200 Subject: [PATCH 19/80] Bump checkstyle from 8.30 to 8.42 --- pom.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pom.xml b/pom.xml index 3b89a6eaaf..929273b13c 100644 --- a/pom.xml +++ b/pom.xml @@ -92,8 +92,8 @@ 5.0 3.0.0-M5 - 8.30 - 3.1.1 + 8.42 + 3.1.2 3.14.0 1.10.9 3.2.0 From 9725409daabafa6fbbac9f3e963b3f0291dbfb78 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 11:08:43 +0200 Subject: [PATCH 20/80] [java] AvoidReassigningParameters reports violations on wrong line numbers Fixes #3254 --- docs/pages/release_notes.md | 2 ++ .../AvoidReassigningParametersRule.java | 6 +++--- .../xml/AvoidReassigningParameters.xml | 14 ++++++++++++++ 3 files changed, 19 insertions(+), 3 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 3ffcb66921..b3199a1a44 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,6 +18,8 @@ This is a {{ site.pmd.release_type }} release. * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages +* java-bestpractices + * [#3254](https://github.com/pmd/pmd/issues/3254): \[java] AvoidReassigningParameters reports violations on wrong line numbers ### API Changes diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java index ec06c65580..072b2a4780 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java @@ -40,9 +40,9 @@ public class AvoidReassigningParametersRule extends AbstractJavaRule { && jocc.getNameForWhichThisIsAQualifier() == null && !jocc.useThisOrSuper() && !decl.isVarargs() && (!decl.isArray() || jocc.getLocation().getParent().getParent().getNumChildren() == 1)) { - // not an array or no primary suffix to access the array - // values - addViolation(data, decl.getNode(), decl.getImage()); + // not an array or no primary suffix to access the array values + // note: this reports each assignment separately + addViolation(data, occ.getLocation(), decl.getImage()); } } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml index c639198e7e..c46370e1c3 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml @@ -231,6 +231,20 @@ public class AvoidReassigningParameters { public void a(String... s) { s[0] = ""; } +} + ]]> + + + + #3254 AvoidReassigningParameters reports wrong line numbers + 2 + 3,4 + From 4c8129316368176c7a8b8eaf8285be2effa5b07a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 13:29:33 +0200 Subject: [PATCH 21/80] Fix #3236 --- .../LiteralsFirstInComparisonsRule.java | 47 +++++++++---------- .../xml/LiteralsFirstInComparisons.xml | 39 +++++++++++++++ 2 files changed, 60 insertions(+), 26 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java index d89bcedf5e..659d02b5d8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java @@ -4,12 +4,8 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; -import java.util.List; - import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; import net.sourceforge.pmd.lang.java.ast.ASTArguments; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTConditionalAndExpression; import net.sourceforge.pmd.lang.java.ast.ASTConditionalOrExpression; import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression; @@ -21,9 +17,11 @@ import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral; 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.ASTVariableDeclarator; +import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; +import net.sourceforge.pmd.lang.symboltable.NameDeclaration; public class LiteralsFirstInComparisonsRule extends AbstractJavaRule { @@ -47,20 +45,22 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule { private boolean hasStringLiteralFirst(ASTPrimaryExpression expression) { ASTPrimaryPrefix primaryPrefix = expression.getFirstChildOfType(ASTPrimaryPrefix.class); - ASTLiteral firstLiteral = primaryPrefix.getFirstDescendantOfType(ASTLiteral.class); + ASTLiteral firstLiteral = primaryPrefix.getFirstChildOfType(ASTLiteral.class); return firstLiteral != null && firstLiteral.isStringLiteral(); } private boolean isNullableComparisonWithStringLiteral(ASTPrimaryExpression expression) { String opName = getOperationName(expression); ASTPrimarySuffix argsSuffix = getSuffixOfArguments(expression); - return opName != null && argsSuffix != null && isStringLiteralComparison(opName, argsSuffix) - && isNotWithinNullComparison(expression); + return opName != null && argsSuffix != null + && isStringLiteralComparison(opName, argsSuffix) + && isNotWithinNullComparison(expression); } private String getOperationName(ASTPrimaryExpression primaryExpression) { - return isMethodsChain(primaryExpression) ? getOperationNameBySuffix(primaryExpression) - : getOperationNameByPrefix(primaryExpression); + return isMethodsChain(primaryExpression) + ? getOperationNameBySuffix(primaryExpression) + : getOperationNameByPrefix(primaryExpression); } private boolean isMethodsChain(ASTPrimaryExpression primaryExpression) { @@ -90,12 +90,11 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule { } private ASTPrimarySuffix getPrimarySuffixAtIndexFromEnd(ASTPrimaryExpression primaryExpression, int indexFromEnd) { - List primarySuffixes = primaryExpression.findChildrenOfType(ASTPrimarySuffix.class); - if (!primarySuffixes.isEmpty()) { - int suffixIndex = primarySuffixes.size() - 1 - indexFromEnd; - return primarySuffixes.get(suffixIndex); + int index = primaryExpression.getNumChildren() - 1 - indexFromEnd; + if (index <= 0) { + return null; } - return null; + return (ASTPrimarySuffix) primaryExpression.getChild(index); } private boolean isStringLiteralComparison(String opName, ASTPrimarySuffix argsSuffix) { @@ -162,17 +161,13 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule { private boolean isConstantString(JavaNode node) { if (node instanceof ASTName) { ASTName name = (ASTName) node; - ASTClassOrInterfaceBody classBody = name.getFirstParentOfType(ASTClassOrInterfaceBody.class); - ASTClassOrInterfaceBodyDeclaration classOrInterfaceBodyDeclaration = classBody.getFirstChildOfType(ASTClassOrInterfaceBodyDeclaration.class); - List fieldDeclarations = classOrInterfaceBodyDeclaration.findChildrenOfType(ASTFieldDeclaration.class); - for (ASTFieldDeclaration fieldDeclaration : fieldDeclarations) { - ASTVariableDeclarator declaration = fieldDeclaration.getFirstChildOfType(ASTVariableDeclarator.class); - if (declaration.getName().equals(name.getImage()) - && String.class.equals(declaration.getType()) - && fieldDeclaration.isFinal() - && fieldDeclaration.isStatic()) { - return true; - } + NameDeclaration resolved = name.getNameDeclaration(); + if (resolved instanceof VariableNameDeclaration + && resolved.getNode() instanceof ASTVariableDeclaratorId) { + ASTVariableDeclaratorId resolvedNode = (ASTVariableDeclaratorId) resolved.getNode(); + return resolvedNode.isFinal() + && resolvedNode.isField() + && resolvedNode.getFirstParentOfType(ASTFieldDeclaration.class).isStatic(); } } return false; diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/LiteralsFirstInComparisons.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/LiteralsFirstInComparisons.xml index 903bb45279..31a83b3547 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/LiteralsFirstInComparisons.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/LiteralsFirstInComparisons.xml @@ -370,4 +370,43 @@ public class Foo { } ]]> + + #3236 [java] LiteralsFirstInComparisons should consider constant fields (cont'd) + 5 + 6,8,17,24,26 + + From 7d150523c3e2414a52655168955e9ed727bba248 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 13:31:01 +0200 Subject: [PATCH 22/80] Remove catch NPE --- .../LiteralsFirstInComparisonsRule.java | 32 +++++++++---------- 1 file changed, 15 insertions(+), 17 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java index 659d02b5d8..c26a8473e8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/LiteralsFirstInComparisonsRule.java @@ -125,29 +125,27 @@ public class LiteralsFirstInComparisonsRule extends AbstractJavaRule { } private boolean isStringLiteralFirstArgumentOfSuffix(ASTPrimarySuffix primarySuffix) { - try { - JavaNode firstLiteralArg = getFirstLiteralArgument(primarySuffix); - JavaNode firstNameArg = getFirstNameArgument(primarySuffix); - return isStringLiteral(firstLiteralArg) || isConstantString(firstNameArg); - } catch (NullPointerException e) { + JavaNode argumentPrimaryPrefix = getArgumentPrimaryPrefix(primarySuffix); + if (argumentPrimaryPrefix == null) { return false; } - } - - private JavaNode getFirstLiteralArgument(ASTPrimarySuffix primarySuffix) { - return getArgumentPrimaryPrefix(primarySuffix).getFirstChildOfType(ASTLiteral.class); - } - - private JavaNode getFirstNameArgument(ASTPrimarySuffix primarySuffix) { - return getArgumentPrimaryPrefix(primarySuffix).getFirstChildOfType(ASTName.class); + JavaNode firstLiteralArg = argumentPrimaryPrefix.getFirstChildOfType(ASTLiteral.class); + JavaNode firstNameArg = argumentPrimaryPrefix.getFirstChildOfType(ASTName.class); + return isStringLiteral(firstLiteralArg) || isConstantString(firstNameArg); } private JavaNode getArgumentPrimaryPrefix(ASTPrimarySuffix primarySuffix) { - ASTArguments arguments = primarySuffix.getFirstChildOfType(ASTArguments.class); - ASTArgumentList argumentList = arguments.getFirstChildOfType(ASTArgumentList.class); - ASTExpression expression = argumentList.getFirstChildOfType(ASTExpression.class); + ASTExpression expression = primarySuffix.getFirstChildOfType(ASTArguments.class) + .getFirstChildOfType(ASTArgumentList.class) + .getFirstChildOfType(ASTExpression.class); + + assert expression != null : "We checked before that we had exactly one argument, so this cannot fail"; + ASTPrimaryExpression primaryExpression = expression.getFirstChildOfType(ASTPrimaryExpression.class); - return primaryExpression.getFirstChildOfType(ASTPrimaryPrefix.class); + if (primaryExpression != null) { + return primaryExpression.getChild(0); + } + return null; } private boolean isStringLiteral(JavaNode node) { From 6beefe5580328710a8c154d1fceeeff609456fc5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 13:45:55 +0200 Subject: [PATCH 23/80] Fix #3248 - doc of SingletonClassReturningNewInstance --- pmd-java/src/main/resources/category/java/errorprone.xml | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index d440ac8cc6..28debc366d 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -2953,9 +2953,9 @@ public class Singleton { class="net.sourceforge.pmd.lang.java.rule.errorprone.SingletonClassReturningNewInstanceRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#singletonclassreturningnewinstance"> -Some classes contain overloaded getInstance. The problem with overloaded getInstance methods -is that the instance created using the overloaded method is not cached and so, -for each call and new objects will be created for every invocation. + A singleton class should only ever have one instance. Failure to check + whether an instance has already been created may result in multiple + instances being created. 2 @@ -2964,7 +2964,7 @@ class Singleton { private static Singleton instance = null; public static Singleton getInstance() { synchronized(Singleton.class) { - return new Singleton(); + return new Singleton(); // this should be assigned to the field } } } From 102636dfc2d2b27268f5bd73df04177233261ee2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 13:48:05 +0200 Subject: [PATCH 24/80] Release notes --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..acecbf21d3 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,8 @@ This is a {{ site.pmd.release_type }} release. * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports +* java-errorprone + * [#3248](https://github.com/pmd/pmd/issues/3248): \[java] Documentation is wrong for SingletonClassReturningNewInstance rule ### API Changes From 24606d59e24ebe29d7bdee17ee679a67ec92c76b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 13:49:14 +0200 Subject: [PATCH 25/80] Release notes --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..2cf28a6eed 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,6 +18,8 @@ This is a {{ site.pmd.release_type }} release. * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages +* java-bestpractices + * [#3236](https://github.com/pmd/pmd/issues/3236): \[java] LiteralsFirstInComparisons should consider constant fields (cont'd) * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports From fdf3e5cb03e362c523d077e1a73e7725a99a84d5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 13:52:00 +0200 Subject: [PATCH 26/80] Fix #2466 include all batch scripts in archive --- pmd-dist/src/main/resources/assemblies/pmd-bin.xml | 6 +----- .../java/net/sourceforge/pmd/it/BinaryDistributionIT.java | 1 + 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/pmd-dist/src/main/resources/assemblies/pmd-bin.xml b/pmd-dist/src/main/resources/assemblies/pmd-bin.xml index 8b0c0134b2..18a8b9e94e 100644 --- a/pmd-dist/src/main/resources/assemblies/pmd-bin.xml +++ b/pmd-dist/src/main/resources/assemblies/pmd-bin.xml @@ -12,11 +12,7 @@ - bgastviewer.bat - cpd.bat - cpdgui.bat - designer.bat - pmd.bat + *.bat target/extra-resources/scripts bin diff --git a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java index d5bff3f134..154ab3edcf 100644 --- a/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java +++ b/pmd-dist/src/test/java/net/sourceforge/pmd/it/BinaryDistributionIT.java @@ -48,6 +48,7 @@ public class BinaryDistributionIT extends AbstractBinaryDistributionTest { result.add(basedir + "bin/run.sh"); result.add(basedir + "bin/pmd.bat"); result.add(basedir + "bin/cpd.bat"); + result.add(basedir + "bin/ast-dump.bat"); result.add(basedir + "lib/pmd-core-" + PMDVersion.VERSION + ".jar"); result.add(basedir + "lib/pmd-java-" + PMDVersion.VERSION + ".jar"); return result; From adc962b7f29dd376a61ae4999ec0a941bff5c1e9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 13:56:18 +0200 Subject: [PATCH 27/80] Release notes --- docs/pages/release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..05b368df43 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,9 +18,12 @@ This is a {{ site.pmd.release_type }} release. * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages +* dist + * [#2466](https://github.com/pmd/pmd/issues/2466): \[dist] Distribution archive doesn't include all batch scripts * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports + ### API Changes ### External Contributions From 223b4e6efe089325e5ad1ca492828579feea56d2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 14:05:59 +0200 Subject: [PATCH 28/80] Improve doc of SwitchStmtsShouldHaveDefault, fix #2737 --- .../main/resources/category/java/bestpractices.xml | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index e8c1f71d4c..bf2711c373 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1287,12 +1287,15 @@ public class Foo { -All switch statements should include a default option to catch any unspecified values. + Switch statements should be exhaustive, to make their control flow + easier to follow. This can be achieved by addinga `default` case, or, + if the switch is on an enum type, by ensuring there is one switch branch + for each enum constant. 3 @@ -1305,14 +1308,14 @@ All switch statements should include a default option to catch any unspecified v From 07141d87c3555730b9a3c80989ae372746d56980 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 14:07:20 +0200 Subject: [PATCH 29/80] Release notes --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..0837043a34 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,6 +18,8 @@ This is a {{ site.pmd.release_type }} release. * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages +* java-bestpractices + * [#2737](https://github.com/pmd/pmd/issues/2737): \[java] Fix misleading rule message on rule SwitchStmtsShouldHaveDefault with non-exhaustive enum switch * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports From 6c9ae9b8374a157d0ef8db4706f9fca012de2dd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 14:08:39 +0200 Subject: [PATCH 30/80] Typo --- pmd-java/src/main/resources/category/java/bestpractices.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index bf2711c373..7315700fc2 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1293,7 +1293,7 @@ public class Foo { externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#switchstmtsshouldhavedefault"> Switch statements should be exhaustive, to make their control flow - easier to follow. This can be achieved by addinga `default` case, or, + easier to follow. This can be achieved by adding a `default` case, or, if the switch is on an enum type, by ensuring there is one switch branch for each enum constant. From 5b67d0ead815d32190fbbd0dc5ea985649a9a132 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 14:21:56 +0200 Subject: [PATCH 31/80] [java] Minor rulechain usage fixes These are in rules, that are not really in use, so shouldn't have any effect. --- .../pmd/lang/java/rule/GenericLiteralCheckerRule.java | 1 + .../pmd/lang/java/rule/StringConcatenationRule.java | 4 ++++ .../sourceforge/pmd/lang/java/rule/SymbolTableTestRule.java | 4 ++++ .../net/sourceforge/pmd/lang/java/rule/UselessAssignment.java | 4 ++++ 4 files changed, 13 insertions(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/GenericLiteralCheckerRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/GenericLiteralCheckerRule.java index 8407ee22fc..e9c1be4cc6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/GenericLiteralCheckerRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/GenericLiteralCheckerRule.java @@ -30,6 +30,7 @@ public class GenericLiteralCheckerRule extends AbstractJavaRule { public GenericLiteralCheckerRule() { definePropertyDescriptor(REGEX_PROPERTY); + addRuleChainVisit(ASTLiteral.class); } private void init() { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/StringConcatenationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/StringConcatenationRule.java index 11686bc535..a08a2f1a53 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/StringConcatenationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/StringConcatenationRule.java @@ -11,6 +11,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTForStatement; //FUTURE This is not referenced by any RuleSet? public class StringConcatenationRule extends AbstractJavaRule { + public StringConcatenationRule() { + addRuleChainVisit(ASTForStatement.class); + } + @Override public Object visit(ASTForStatement node, Object data) { Node forLoopStmt = null; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/SymbolTableTestRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/SymbolTableTestRule.java index ef26fb8598..530ac9509b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/SymbolTableTestRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/SymbolTableTestRule.java @@ -17,6 +17,10 @@ import net.sourceforge.pmd.lang.symboltable.NameOccurrence; @Deprecated public class SymbolTableTestRule extends AbstractJavaRule { + public SymbolTableTestRule() { + addRuleChainVisit(ASTFieldDeclaration.class); + } + @Override public Object visit(ASTFieldDeclaration node, Object data) { for (ASTVariableDeclaratorId declaration : node.findDescendantsOfType(ASTVariableDeclaratorId.class)) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/UselessAssignment.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/UselessAssignment.java index 61b7a1950b..c972f6b235 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/UselessAssignment.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/UselessAssignment.java @@ -21,6 +21,10 @@ public class UselessAssignment extends AbstractJavaRule implements Executable { private RuleContext rc; + public UselessAssignment() { + addRuleChainVisit(ASTMethodDeclaration.class); + } + @Override public Object visit(ASTMethodDeclaration node, Object data) { this.rc = (RuleContext) data; From ddde7cf48495d539ad81d11eec1d2c5c37618854 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 14:48:21 +0200 Subject: [PATCH 32/80] Fix #1175 - UnusedPrivateMethod FP with Junit 5 @MethodSource --- .../net/sourceforge/pmd/util/StringUtil.java | 21 +++++++++ .../UnusedPrivateMethodRule.java | 43 +++++++++++++++---- .../pmd/lang/java/types/TypeTestUtil.java | 8 ++++ .../bestpractices/xml/UnusedPrivateMethod.xml | 27 ++++++++++++ 4 files changed, 90 insertions(+), 9 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java index 6d463c1702..4a2b7f236c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java @@ -732,6 +732,27 @@ public final class StringUtil { return sb.toString(); } + /** + * If the string starts and ends with the delimiter, returns the substring + * within the delimiters. Otherwise returns the original string. + */ + public static String removeSurrounding(String string, char delimiter) { + if (!string.isEmpty() + && string.charAt(0) == delimiter + && string.charAt(string.length() - 1) == delimiter) { + return string.substring(1, string.length() - 1); + } + return string; + } + + /** + * Like {@link #removeSurrounding(String, char) removeSurrounding} with + * a double quote as a delimiter. + */ + public static String removeDoubleQuotes(String string) { + return removeSurrounding(string, '"'); + } + /** * Returns an empty array of string diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java index 1533e0db57..c6b294a8a4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateMethodRule.java @@ -13,15 +13,20 @@ import java.util.Map; import java.util.Set; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeBodyDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTInitializer; +import net.sourceforge.pmd.lang.java.ast.ASTLiteral; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.Annotatable; import net.sourceforge.pmd.lang.java.rule.AbstractIgnoredAnnotationRule; import net.sourceforge.pmd.lang.java.symboltable.ClassScope; import net.sourceforge.pmd.lang.java.symboltable.MethodNameDeclaration; +import net.sourceforge.pmd.lang.java.types.TypeTestUtil; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; +import net.sourceforge.pmd.util.StringUtil; /** * This rule detects private methods, that are not used and can therefore be @@ -41,25 +46,45 @@ public class UnusedPrivateMethodRule extends AbstractIgnoredAnnotationRule { } /** - * Visit each method declaration. - * - * @param node - * the method declaration - * @param data - * data - rule context - * @return data + * Return a set of method names which are considered used. Only the + * no-arg overload is considered used. */ + private static Set methodsUsedByAnnotations(ASTClassOrInterfaceDeclaration klassDecl) { + Set result = Collections.emptySet(); + for (ASTAnyTypeBodyDeclaration declaration : klassDecl.getDeclarations()) { + for (ASTAnnotation annot : declaration.findChildrenOfType(ASTAnnotation.class)) { + if (TypeTestUtil.isA("org.junit.jupiter.params.provider.MethodSource", annot)) { + // MethodSource#value() -> String[], there may be several of those methods + // todo this is not robust, revisit in pmd 7 + for (ASTLiteral literal : annot.findDescendantsOfType(ASTLiteral.class)) { + if (literal.isStringLiteral()) { + if (result.isEmpty()) { + result = new HashSet<>(); // make writable + } + result.add(StringUtil.removeDoubleQuotes(literal.getImage())); + } + } + } + } + } + return result; + } + @Override public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { if (node.isInterface()) { return data; } + Set methodsUsedByAnnotations = methodsUsedByAnnotations(node); + Map> methods = node.getScope().getEnclosingScope(ClassScope.class) - .getMethodDeclarations(); + .getMethodDeclarations(); for (MethodNameDeclaration mnd : findUnique(methods)) { List occs = methods.get(mnd); - if (!privateAndNotExcluded(mnd) || hasIgnoredAnnotation((Annotatable) mnd.getNode().getParent())) { + if (!privateAndNotExcluded(mnd) + || hasIgnoredAnnotation((Annotatable) mnd.getNode().getParent()) + || mnd.getParameterCount() == 0 && methodsUsedByAnnotations.contains(mnd.getName())) { continue; } if (occs.isEmpty()) { diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java index 3f8f437e9b..2099b8552b 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/types/TypeTestUtil.java @@ -8,6 +8,7 @@ import java.lang.reflect.Modifier; import java.util.List; import net.sourceforge.pmd.internal.util.AssertionUtil; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; @@ -15,6 +16,7 @@ import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTEnumDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTImplementsList; import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.TypeNode; import net.sourceforge.pmd.lang.java.typeresolution.TypeHelper; @@ -221,6 +223,12 @@ public final class TypeTestUtil { } private static boolean fallbackIsA(TypeNode n, String canonicalName, boolean considerSubtype) { + if (n instanceof ASTAnnotation) { + // the annotation node has no image itself + n = n.getFirstDescendantOfType(ASTName.class); + assert n != null; + } + if (n.getImage() != null && !n.getImage().contains(".") && canonicalName.contains(".")) { // simple name detected, check the imports to get the full name and use that for fallback List imports = n.getRoot().findChildrenOfType(ASTImportDeclaration.class); diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml index 1443f420bd..2db3da30a9 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateMethod.xml @@ -1697,6 +1697,33 @@ public class Outer { Inner inner = new Inner(); inner.innerUsedByOuterMethod(); } +} + ]]> + + + #1175 False positive with Junit 5 MethodSource + 0 + basenameKeyArguments() { + return Stream.of( + Arguments.of("simple", "simple"), + Arguments.of("simple", "one/two/many/simple"), + Arguments.of("simple", "//////an/////awful/key////simple") + ); + } + + @ParameterizedTest + @MethodSource("basenameKeyArguments") + void basenameKeyTest(final String expected, final String testString) { + assertEquals(expected, NetworkTable.basenameKey(testString)); + } + } ]]> From ea3522173932de3e98208e56e84910e239ce0169 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 14:51:31 +0200 Subject: [PATCH 33/80] [java] FieldDeclarationsShouldBeAtStartOfClass - FN with anon classes --- ...eclarationsShouldBeAtStartOfClassRule.java | 1 + ...ieldDeclarationsShouldBeAtStartOfClass.xml | 39 +++++++++++++++++++ 2 files changed, 40 insertions(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/FieldDeclarationsShouldBeAtStartOfClassRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/FieldDeclarationsShouldBeAtStartOfClassRule.java index 0ec9709589..dd57068ae0 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/FieldDeclarationsShouldBeAtStartOfClassRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/FieldDeclarationsShouldBeAtStartOfClassRule.java @@ -48,6 +48,7 @@ public class FieldDeclarationsShouldBeAtStartOfClassRule extends AbstractJavaRul definePropertyDescriptor(ignoreEnumDeclarations); definePropertyDescriptor(ignoreAnonymousClassDeclarations); definePropertyDescriptor(ignoreInterfaceDeclarations); + addRuleChainVisit(ASTFieldDeclaration.class); } @Override diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldDeclarationsShouldBeAtStartOfClass.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldDeclarationsShouldBeAtStartOfClass.xml index ad7b93cd55..fabc2fd19f 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldDeclarationsShouldBeAtStartOfClass.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/FieldDeclarationsShouldBeAtStartOfClass.xml @@ -129,6 +129,7 @@ public class MyClass { #1244 FieldDeclarationsShouldBeAtStartOfClass and anonymous classes, fail false 1 + 9 + + + + False negative with anon classes (1) + false + 3 + 4,5,7 + + + + + False negative with anon classes (2) + true + 2 + 4,7 + From 0211a2897cad52e38779e1b7d034407d9f9b82c5 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 14:56:06 +0200 Subject: [PATCH 34/80] [doc][skip ci] Update release notes, refs #3262 --- docs/pages/release_notes.md | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..4ec6fcd41e 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,7 @@ This is a {{ site.pmd.release_type }} release. * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports + * [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes ### API Changes From a33f5fc82bd10412dc726dee655cecaa5ee87c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 14:57:30 +0200 Subject: [PATCH 35/80] Release notes --- docs/pages/release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..e768290553 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,9 +18,12 @@ This is a {{ site.pmd.release_type }} release. * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages +* java-bestpractices + * [#1175](https://github.com/pmd/pmd/issues/1175): \[java] UnusedPrivateMethod FP with Junit 5 @MethodSource * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports + ### API Changes ### External Contributions From e73b5484a0d98aaca525476e5e940462d96ef12f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Thu, 6 May 2021 15:01:31 +0200 Subject: [PATCH 36/80] Fix #2780 DataClass example from documentation results in false negative --- docs/pages/release_notes.md | 2 ++ .../main/resources/category/java/design.xml | 4 ++++ .../lang/java/rule/design/xml/DataClass.xml | 24 +++++++++++++++++++ 3 files changed, 30 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..15df7cdc03 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,8 @@ This is a {{ site.pmd.release_type }} release. * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports +* java-design + * [#2780](https://github.com/pmd/pmd/issues/2780): \[java] DataClass example from documentation results in false-negative ### API Changes diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index de6b56cedb..9637117720 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -512,10 +512,14 @@ into the former client classes. + + Example from the documentation + 1 + + The class 'DataClass' is suspected to be a Data Class (WOC=0.000%, NOPA=3, NOAM=1, WMC=1) + + + + From d48cee2258630020123b79617ce05c0a32c5ee61 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 15:31:43 +0200 Subject: [PATCH 37/80] [java] MethodArgumentCouldBeFinal - fix FN with interfaces --- .../MethodArgumentCouldBeFinalRule.java | 9 +++-- .../xml/MethodArgumentCouldBeFinal.xml | 36 +++++++++++++++++++ 2 files changed, 43 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java index 0d11dcf72d..351d93d9b1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/MethodArgumentCouldBeFinalRule.java @@ -18,13 +18,18 @@ import net.sourceforge.pmd.lang.symboltable.Scope; public class MethodArgumentCouldBeFinalRule extends AbstractOptimizationRule { + public MethodArgumentCouldBeFinalRule() { + addRuleChainVisit(ASTConstructorDeclaration.class); + addRuleChainVisit(ASTMethodDeclaration.class); + } + @Override public Object visit(ASTMethodDeclaration meth, Object data) { if (meth.isNative() || meth.isAbstract()) { return data; } this.lookForViolation(meth.getScope(), data); - return super.visit(meth, data); + return data; } private void lookForViolation(Scope scope, Object data) { @@ -41,7 +46,7 @@ public class MethodArgumentCouldBeFinalRule extends AbstractOptimizationRule { @Override public Object visit(ASTConstructorDeclaration constructor, Object data) { this.lookForViolation(constructor.getScope(), data); - return super.visit(constructor, data); + return data; } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml index 1b3691333f..019a0c9234 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml @@ -154,6 +154,42 @@ public class Foo { public Foo(int a) { this.field = a; } +} + ]]> + + + + False negative with default methods in interface + 2 + 2,7 + + + + + False negative with classes in interfaces + 2 + 3,6 + From 150fec0f6f28047b2db08040199baef533b76566 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 15:32:09 +0200 Subject: [PATCH 38/80] [java] AvoidReassigningLoopVariables - add more test cases --- .../xml/AvoidReassigningLoopVariables.xml | 54 +++++++++++++++++++ 1 file changed, 54 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml index b39cd59870..82887e91a2 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml @@ -727,4 +727,58 @@ public class Foo { } ]]> + + + Consider also default methods in interface + 1 + 5 + + + + + Consider also classes in interface + 1 + 6 + + + + + Consider also anonymous classes + 1 + 7 + + From d3c1dbeb474f78e7141e1b65462edddf9e97e94b Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 15:38:55 +0200 Subject: [PATCH 39/80] [doc] Update release notes, refs #3265 --- docs/pages/release_notes.md | 1 + .../java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml | 4 ++-- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..05856bcad8 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,7 @@ This is a {{ site.pmd.release_type }} release. * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports + * [#3265](https://github.com/pmd/pmd/pull/3265): \[java] MethodArgumentCouldBeFinal: false negatives with interfaces and inner classes ### API Changes diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml index 019a0c9234..a875239855 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/MethodArgumentCouldBeFinal.xml @@ -159,7 +159,7 @@ public class Foo { - False negative with default methods in interface + #3265 False negative with default methods in interface 2 2,7 - False negative with classes in interfaces + #3265 False negative with classes in interfaces 2 3,6 Date: Thu, 6 May 2021 15:44:33 +0200 Subject: [PATCH 40/80] [java] LocalVariableCouldBeFinal - fix FN with interfaces, anon classes --- .../LocalVariableCouldBeFinalRule.java | 1 + .../xml/LocalVariableCouldBeFinal.xml | 68 +++++++++++++++++++ 2 files changed, 69 insertions(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java index b7ce0cab4b..444c1760e3 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/LocalVariableCouldBeFinalRule.java @@ -24,6 +24,7 @@ public class LocalVariableCouldBeFinalRule extends AbstractOptimizationRule { public LocalVariableCouldBeFinalRule() { definePropertyDescriptor(IGNORE_FOR_EACH); + addRuleChainVisit(ASTLocalVariableDeclaration.class); } @Override diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml index 3a7429e4c6..3f3735103d 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml @@ -208,6 +208,74 @@ public class Foo { System.out.println(i); } } +} + ]]> + + + + False negative with default methods in interfaces + 1 + 3 + + + + + False negative with class inside interface + 1 + 4 + + + + + False negative with anonymous classes + 2 + 5,11 + + + + + False negative with lambdas + 1 + 4 + { + int a = 0; + }; + } } ]]> From db244b0e3b731cf35437e27e0f1a2a542995a852 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 6 May 2021 15:49:29 +0200 Subject: [PATCH 41/80] [doc] Update release notes, refs #3266 --- docs/pages/release_notes.md | 1 + .../java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml | 8 ++++---- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8fbc39c87b..875ab09cde 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,7 @@ This is a {{ site.pmd.release_type }} release. * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports + * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes ### API Changes diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml index 3f3735103d..cc40e3140c 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/LocalVariableCouldBeFinal.xml @@ -213,7 +213,7 @@ public class Foo { - False negative with default methods in interfaces + #3266 False negative with default methods in interfaces 1 3 - False negative with class inside interface + #3266 False negative with class inside interface 1 4 - False negative with anonymous classes + #3266 False negative with anonymous classes 2 5,11 - False negative with lambdas + #3266 False negative with lambdas 1 4 Date: Fri, 7 May 2021 14:29:34 +0200 Subject: [PATCH 42/80] Fix JUnit5 interfaces and add more UTs --- .../resources/category/java/bestpractices.xml | 1 + .../xml/JUnit5TestShouldBePackagePrivate.xml | 28 ++++++++++++++++++- 2 files changed, 28 insertions(+), 1 deletion(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 5c0f172d91..19a15c960d 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -823,6 +823,7 @@ if they're package-private. Marking them as such is a good practice to limit the Package private modifiers are what is required - 0 + 0 + + Public modifier is allowed on an abstract test classes + 0 + + + + + Public modifier is allowed on test interfaces + 0 + + + Non package private modifiers on all JUnit5 test types should be rejected 6 From b1e7bc1c9c4db681e4a0f5c9b5bc072fa6d82108 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 7 May 2021 15:59:55 +0200 Subject: [PATCH 43/80] [java] ConstructorCallsOverridableMethod - Fix index out of bounds exception with annotations --- ...ConstructorCallsOverridableMethodRule.java | 7 +++++++ .../xml/ConstructorCallsOverridableMethod.xml | 19 +++++++++++++++++++ 2 files changed, 26 insertions(+) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java index 0589017b57..f45bfe4eb1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/errorprone/ConstructorCallsOverridableMethodRule.java @@ -14,6 +14,7 @@ import java.util.Set; import java.util.TreeMap; import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotationTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; import net.sourceforge.pmd.lang.java.ast.ASTArguments; import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral; @@ -860,6 +861,12 @@ public final class ConstructorCallsOverridableMethodRule extends AbstractJavaRul return data; } + @Override + public Object visit(ASTAnnotationTypeDeclaration node, Object data) { + // just skip Annotations + return data; + } + /** * This check must be evaluated independently for each class. Inner classes * get their own EvalPackage in order to perform independent evaluation. diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml index 204bc03c22..915a3d6326 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ConstructorCallsOverridableMethod.xml @@ -282,6 +282,25 @@ public class Foo { } public void bar(boolean b) {} +} + ]]> + + + + IndexOutOfBoundsException with annotation + 0 + From 4fcbdbb2484fb12535605e3708885abaf8056067 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 7 May 2021 16:16:15 +0200 Subject: [PATCH 44/80] [doc] Update release notes, refs #3268 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index e1aa9f5325..d1d615fb0c 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -23,6 +23,8 @@ This is a {{ site.pmd.release_type }} release. * [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes * [#3265](https://github.com/pmd/pmd/pull/3265): \[java] MethodArgumentCouldBeFinal: false negatives with interfaces and inner classes * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes +* java-errorprone + * [#3268](https://github.com/pmd/pmd/pull/3268): \[java] ConstructorCallsOverridableMethod: IndexOutOfBoundsException with annotations ### API Changes From d7b01ced47d51a86091fff628986c3f8f5970feb Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 7 May 2021 15:14:48 +0200 Subject: [PATCH 45/80] [java] Catch TypeNotPresentExceptions, fix NPE in MethodTypeResolution --- .../typeresolution/MethodTypeResolution.java | 38 ++++++++--- .../JavaTypeDefinitionSimple.java | 63 ++++++++++--------- 2 files changed, 63 insertions(+), 38 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java index 8ee0f5e347..432f42e5bc 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/MethodTypeResolution.java @@ -37,6 +37,11 @@ import net.sourceforge.pmd.lang.java.typeresolution.typeinference.Variable; @Deprecated @InternalApi public final class MethodTypeResolution { + /** + * + */ + private static final String MESSAGE_INCOMPLETE_AUXCLASSPATH = "Possible incomplete auxclasspath: Error while processing methods"; + private MethodTypeResolution() {} private static final Logger LOG = Logger.getLogger(MethodTypeResolution.class.getName()); @@ -477,7 +482,7 @@ public final class MethodTypeResolution { } } catch (final LinkageError e) { // This is an incomplete classpath, report the missing class - LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e); + LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e); } // search it's supertype @@ -496,7 +501,7 @@ public final class MethodTypeResolution { } catch (TypeNotPresentException | LinkageError e) { // might be thrown by contextClass.getGenericSuperclass() // This is an incomplete classpath, report the missing class - LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e); + LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e); } } @@ -509,7 +514,7 @@ public final class MethodTypeResolution { } catch (TypeNotPresentException | LinkageError e) { // might be thrown by contextClass.getGenericInterface() // This is an incomplete classpath, report the missing class - LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while processing methods", e); + LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e); } return result; @@ -522,15 +527,22 @@ public final class MethodTypeResolution { return MethodType.build(method); } - JavaTypeDefinition returnType = context.resolveTypeDefinition(method.getGenericReturnType(), - method, typeArguments); - List argTypes = new ArrayList<>(); + try { + JavaTypeDefinition returnType = context.resolveTypeDefinition(method.getGenericReturnType(), + method, typeArguments); + List argTypes = new ArrayList<>(); - for (Type argType : method.getGenericParameterTypes()) { - argTypes.add(context.resolveTypeDefinition(argType, method, typeArguments)); + for (Type argType : method.getGenericParameterTypes()) { + argTypes.add(context.resolveTypeDefinition(argType, method, typeArguments)); + } + return MethodType.build(returnType, argTypes, method); + } catch (TypeNotPresentException | LinkageError e) { + // might be thrown by method.getGenericReturnType() and method.getGenericParameterTypes() + // This is an incomplete classpath, report the missing class + LOG.log(Level.FINE, MESSAGE_INCOMPLETE_AUXCLASSPATH, e); + + return MethodType.build(method); } - - return MethodType.build(returnType, argTypes, method); } @@ -682,6 +694,12 @@ public final class MethodTypeResolution { // example result: List.getAsSuper(Collection) becomes Collection JavaTypeDefinition argSuper = argument.getAsSuper(parameter.getType()); // argSuper can't be null because isAssignableFrom check above returned true + // it might be null however, if the auxclasspath was not complete... + if (argSuper == null) { + // that's not really correct, because the generic type are ignored... + // be we can't compare the types + return true; + } // right now we only check if generic arguments are the same // TODO: add support for wildcard types diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java index 20beef712b..820038d271 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/typeresolution/typedefinition/JavaTypeDefinitionSimple.java @@ -182,38 +182,45 @@ import java.util.logging.Logger; return forClass(Object.class); } - if (type instanceof Class) { // Raw types take this branch as well - return forClass((Class) type); - } else if (type instanceof ParameterizedType) { - final ParameterizedType parameterizedType = (ParameterizedType) type; + try { + if (type instanceof Class) { // Raw types take this branch as well + return forClass((Class) type); + } else if (type instanceof ParameterizedType) { + final ParameterizedType parameterizedType = (ParameterizedType) type; - // recursively determine each type argument's type def. - final Type[] typeArguments = parameterizedType.getActualTypeArguments(); - final JavaTypeDefinition[] genericBounds = new JavaTypeDefinition[typeArguments.length]; - for (int i = 0; i < typeArguments.length; i++) { - genericBounds[i] = resolveTypeDefinition(typeArguments[i], method, methodTypeArgs); - } + // recursively determine each type argument's type def. + final Type[] typeArguments = parameterizedType.getActualTypeArguments(); + final JavaTypeDefinition[] genericBounds = new JavaTypeDefinition[typeArguments.length]; + for (int i = 0; i < typeArguments.length; i++) { + genericBounds[i] = resolveTypeDefinition(typeArguments[i], method, methodTypeArgs); + } - // TODO : is this cast safe? - return forClass((Class) parameterizedType.getRawType(), genericBounds); - } else if (type instanceof TypeVariable) { - return getGenericType(((TypeVariable) type).getName(), method, methodTypeArgs); - } else if (type instanceof WildcardType) { - final Type[] wildcardLowerBounds = ((WildcardType) type).getLowerBounds(); + // TODO : is this cast safe? + return forClass((Class) parameterizedType.getRawType(), genericBounds); + } else if (type instanceof TypeVariable) { + return getGenericType(((TypeVariable) type).getName(), method, methodTypeArgs); + } else if (type instanceof WildcardType) { + final Type[] wildcardLowerBounds = ((WildcardType) type).getLowerBounds(); - if (wildcardLowerBounds.length != 0) { // lower bound wildcard - return forClass(LOWER_WILDCARD, resolveTypeDefinition(wildcardLowerBounds[0], method, methodTypeArgs)); - } else { // upper bound wildcard - final Type[] wildcardUpperBounds = ((WildcardType) type).getUpperBounds(); - return forClass(UPPER_WILDCARD, resolveTypeDefinition(wildcardUpperBounds[0], method, methodTypeArgs)); - } - } else if (type instanceof GenericArrayType) { - JavaTypeDefinition component = resolveTypeDefinition(((GenericArrayType) type).getGenericComponentType(), method, methodTypeArgs); - // only if we could determine the actual type - if (component != null) { - // TODO: retain the generic types of the array component... - return forClass(Array.newInstance(component.getType(), 0).getClass()); + if (wildcardLowerBounds.length != 0) { // lower bound wildcard + return forClass(LOWER_WILDCARD, resolveTypeDefinition(wildcardLowerBounds[0], method, methodTypeArgs)); + } else { // upper bound wildcard + final Type[] wildcardUpperBounds = ((WildcardType) type).getUpperBounds(); + return forClass(UPPER_WILDCARD, resolveTypeDefinition(wildcardUpperBounds[0], method, methodTypeArgs)); + } + } else if (type instanceof GenericArrayType) { + JavaTypeDefinition component = resolveTypeDefinition(((GenericArrayType) type).getGenericComponentType(), method, methodTypeArgs); + // only if we could determine the actual type + if (component != null) { + // TODO: retain the generic types of the array component... + return forClass(Array.newInstance(component.getType(), 0).getClass()); + } } + } catch (TypeNotPresentException | LinkageError e) { + // might be thrown by parameterizedType.getActualTypeArguments(), type.getLowerBounds(), + // type.getUpperBounds(), type.getGenericComponentType() + // This is an incomplete classpath, report the missing class + LOG.log(Level.FINE, "Possible incomplete auxclasspath: Error while resolving generic types", e); } // TODO : Shall we throw here? From d444f30ee49318ada15455e383098e33b7fdd841 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 7 May 2021 16:08:30 +0200 Subject: [PATCH 46/80] [ci] Add gradle cache --- .github/workflows/build.yml | 5 +++-- .github/workflows/troubleshooting.yml | 12 +++++++----- 2 files changed, 10 insertions(+), 7 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index bc43545a09..e007d1e2f1 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -29,12 +29,13 @@ jobs: with: path: | ~/.m2/repository + ~/.gradle/caches ~/.cache ~/work/pmd/target/repositories vendor/bundle - key: ${{ runner.os }}-${{ hashFiles('**/pom.xml') }} + key: v1-${{ runner.os }}-${{ hashFiles('**/pom.xml') }} restore-keys: | - ${{ runner.os }}- + v1-${{ runner.os }}- - name: Set up Ruby 2.7 uses: actions/setup-ruby@v1 with: diff --git a/.github/workflows/troubleshooting.yml b/.github/workflows/troubleshooting.yml index 10a35d9785..23c5d3fc33 100644 --- a/.github/workflows/troubleshooting.yml +++ b/.github/workflows/troubleshooting.yml @@ -16,12 +16,14 @@ jobs: - uses: actions/cache@v2 with: path: | - ~/.m2/repository - ~/.cache - vendor/bundle - key: push-${{ runner.os }}-${{ hashFiles('**/pom.xml') }} + ~/.m2/repository + ~/.gradle/caches + ~/.cache + ~/work/pmd/target/repositories + vendor/bundle + key: v1-${{ runner.os }}-${{ hashFiles('**/pom.xml') }} restore-keys: | - push-${{ runner.os }}- + v1-${{ runner.os }}- - name: Set up Ruby 2.7 uses: actions/setup-ruby@v1 with: From d4789e17c5425731316c1c926c2c07bf1c6ec139 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 7 May 2021 16:33:49 +0200 Subject: [PATCH 47/80] [doc] Update release notes, refs #3269 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index e1aa9f5325..227b9907b0 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,6 +18,8 @@ This is a {{ site.pmd.release_type }} release. * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages +* java + * [#3269](https://github.com/pmd/pmd/pull/3269): \[java] Fix NPE in MethodTypeResolution * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports * [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes From c80b6633d7d72c8bffef695d4e5d7cf8cabd591d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 8 May 2021 18:53:48 +0200 Subject: [PATCH 48/80] [doc] Update release notes (#3237, #3110, #3205) --- docs/pages/release_notes.md | 13 +++++++++++++ 1 file changed, 13 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..cbfba82394 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,8 +14,21 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy +#### Modified rules + +* The Java rule {% rule "java/errorprone/CompareObjectsWithEquals" %} has now a new property + `typesThatCompareByReference`. With that property, you can configure types, that should be whitelisted + for comparison by reference. By default, `java.lang.Enum` and `java.lang.Class` are allowed, but + you could add custom types here. + Additionally comparisons against constants are allowed now. This makes the rule less noisy when two constants + are compared. Constants are identified by looking for an all-caps identifier. + ### Fixed Issues +* java-errorprone + * [#3110](https://github.com/pmd/pmd/issues/3110): \[java] Enhance CompareObjectsWithEquals with list of exceptions + * [#3205](https://github.com/pmd/pmd/issues/3205): \[java] Make CompareObjectWithEquals allow comparing against constants + ### API Changes ### External Contributions From df39bc068895d2e5be04cd2490df7fbc608a0c38 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Sun, 9 May 2021 13:10:26 +0200 Subject: [PATCH 49/80] Test removeSurrounding --- .../java/net/sourceforge/pmd/util/StringUtil.java | 11 +++++++++-- .../java/net/sourceforge/pmd/util/StringUtilTest.java | 10 ++++++++++ 2 files changed, 19 insertions(+), 2 deletions(-) diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java index 4a2b7f236c..7d8a36fcf5 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/StringUtil.java @@ -734,10 +734,17 @@ public final class StringUtil { /** * If the string starts and ends with the delimiter, returns the substring - * within the delimiters. Otherwise returns the original string. + * within the delimiters. Otherwise returns the original string. The + * start and end delimiter must be 2 separate instances. + * 
{@code
+     * removeSurrounding("",     _ )  = ""
+     * removeSurrounding("q",   'q')  = "q"
+     * removeSurrounding("qq",  'q')  = ""
+     * removeSurrounding("q_q", 'q')  = "_"
+     * }
*/ public static String removeSurrounding(String string, char delimiter) { - if (!string.isEmpty() + if (string.length() >= 2 && string.charAt(0) == delimiter && string.charAt(string.length() - 1) == delimiter) { return string.substring(1, string.length() - 1); diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java index 8ca15c97dc..edf70631e7 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/util/StringUtilTest.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.util; +import static org.hamcrest.CoreMatchers.equalTo; +import static org.hamcrest.MatcherAssert.assertThat; import static org.junit.Assert.assertEquals; import org.junit.Test; @@ -97,4 +99,12 @@ public class StringUtilTest { StringUtil.appendXmlEscaped(sb, test, true); assertEquals("é", sb.toString()); } + + @Test + public void testRemoveSurrounding() { + assertThat(StringUtil.removeSurrounding("", 'q'), equalTo("")); + assertThat(StringUtil.removeSurrounding("q", 'q'), equalTo("q")); + assertThat(StringUtil.removeSurrounding("qq", 'q'), equalTo("")); + assertThat(StringUtil.removeSurrounding("qqq", 'q'), equalTo("q")); + } } From 76af096687f0ac43b3626f5341fefc7989cdd8c8 Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Mon, 10 May 2021 09:41:45 +0200 Subject: [PATCH 50/80] PR review: add expected line numbers --- .../rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml | 2 ++ 1 file changed, 2 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml index de563e3cfb..afc474bab0 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml @@ -7,6 +7,7 @@ Public modifier is not necessary on a test method nor on the class 2 + 3,5 Non package private modifiers on all JUnit5 test types should be rejected 6 + 8,10,13,16,19,23 Date: Thu, 13 May 2021 15:01:10 +0200 Subject: [PATCH 51/80] [java] AvoidReassigningParameters - report only one violation per param --- .../AvoidReassigningParametersRule.java | 4 +++- .../xml/AvoidReassigningParameters.xml | 15 +++++++++++---- 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java index 072b2a4780..4972bdd188 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/AvoidReassigningParametersRule.java @@ -41,8 +41,10 @@ public class AvoidReassigningParametersRule extends AbstractJavaRule { && (!decl.isArray() || jocc.getLocation().getParent().getParent().getNumChildren() == 1)) { // not an array or no primary suffix to access the array values - // note: this reports each assignment separately addViolation(data, occ.getLocation(), decl.getImage()); + + // only the first assignment should be reported + break; } } } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml index c46370e1c3..38d0506e14 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml @@ -7,6 +7,7 @@ reassigned parameter, bad 1 + 3 instance variable and parameter have same name 1 + 4 throws a stacktrace 1 + 4 postfix increment in array dereference is bad 1 + 3 assignment to array 1 + 4 The rule should also detect parameter reassignment in constructors (at least to help young programmers still learning java basic) - 3 + 2 + 3,5 parameter name starting with "this" or "super" should still be flagged 2 + 3,4 #3254 AvoidReassigningParameters reports wrong line numbers - 2 - 3,4 + 1 + 3 From 7c2fffb3ee672e39966ccc295926d740f5b51c18 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 13 May 2021 15:19:31 +0200 Subject: [PATCH 52/80] [doc] AvoidReassigningParameters - improve rule doc Fixes #2219 --- docs/pages/release_notes.md | 1 + .../resources/category/java/bestpractices.xml | 23 +++++++++++++++---- .../xml/AvoidReassigningParameters.xml | 18 +++++++++++++++ 3 files changed, 38 insertions(+), 4 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 84829e7d69..c0611e731b 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -35,6 +35,7 @@ This is a {{ site.pmd.release_type }} release. * [#3269](https://github.com/pmd/pmd/pull/3269): \[java] Fix NPE in MethodTypeResolution * java-bestpractices * [#1175](https://github.com/pmd/pmd/issues/1175): \[java] UnusedPrivateMethod FP with Junit 5 @MethodSource + * [#2219](https://github.com/pmd/pmd/issues/2219): \[java] Document Reasons to Avoid Reassigning Parameters * [#2737](https://github.com/pmd/pmd/issues/2737): \[java] Fix misleading rule message on rule SwitchStmtsShouldHaveDefault with non-exhaustive enum switch * [#3236](https://github.com/pmd/pmd/issues/3236): \[java] LiteralsFirstInComparisons should consider constant fields (cont'd) * [#3254](https://github.com/pmd/pmd/issues/3254): \[java] AvoidReassigningParameters reports violations on wrong line numbers diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 7315700fc2..94a7c20473 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -301,14 +301,29 @@ public class Foo { class="net.sourceforge.pmd.lang.java.rule.bestpractices.AvoidReassigningParametersRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#avoidreassigningparameters"> -Reassigning values to incoming parameters is not recommended. Use temporary local variables instead. +Reassigning values to incoming parameters of a method or constructor is not recommended, as this can +make the code more difficult to understand. The code is often read with the assumption that parameter values +don't change and an assignment violates therefore the principle of least astonishment. This is especially a +problem if the parameter is documented e.g. in the method's javadoc and the new content differs from the original +documented content. + +Use temporary local variables instead. This allows you to assign a new name, which makes the code better +understandable. + +Note that this rule considers both methods and constructors. If there are multiple assignments for a formal +parameter, then only the first assignment is reported. 2 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml index 38d0506e14..5b8c6cb046 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningParameters.xml @@ -4,6 +4,24 @@ xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> + + example + 1 + 3 + + + reassigned parameter, bad 1 From ebdbec7a1b368f7ee4fc8c672c7b52d9a0839bb6 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 13 May 2021 18:56:20 +0200 Subject: [PATCH 53/80] [java] OnlyOneReturn - fix FN with anonymous class Converts this rule to use rulechain. Now all method declarations are visited regardless where they are nested. This FN has been found via #2687. --- docs/pages/release_notes.md | 1 + .../rule/codestyle/OnlyOneReturnRule.java | 13 ++---------- .../java/rule/codestyle/xml/OnlyOneReturn.xml | 21 +++++++++++++++++++ 3 files changed, 24 insertions(+), 11 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a2b26c2bc0..346e70178e 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -42,6 +42,7 @@ This is a {{ site.pmd.release_type }} release. * [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes * [#3265](https://github.com/pmd/pmd/pull/3265): \[java] MethodArgumentCouldBeFinal: false negatives with interfaces and inner classes * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes + * [#3274](https://github.com/pmd/pmd/pull/3274): \[java] OnlyOneReturn: false negative with anonymous class * java-design * [#2780](https://github.com/pmd/pmd/issues/2780): \[java] DataClass example from documentation results in false-negative * java-errorprone diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/OnlyOneReturnRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/OnlyOneReturnRule.java index 3421f616ce..394c437783 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/OnlyOneReturnRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/OnlyOneReturnRule.java @@ -8,27 +8,18 @@ import java.util.Iterator; import java.util.List; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; public class OnlyOneReturnRule extends AbstractJavaRule { - @Override - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (node.isInterface()) { - return data; - } - return super.visit(node, data); + public OnlyOneReturnRule() { + addRuleChainVisit(ASTMethodDeclaration.class); } @Override public Object visit(ASTMethodDeclaration node, Object data) { - if (node.isAbstract()) { - return data; - } - List returnNodes = node.findDescendantsOfType(ASTReturnStatement.class); if (returnNodes.size() > 1) { diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml index 41aa08a7ee..70bca8f979 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml @@ -114,6 +114,27 @@ public class OnlyOneReturn { .map(list -> list.toArray(empty)); }).orElse(Try.success(empty)); } +} + ]]> + + + + False positive with anonymous class #3274 + 2 + 6,7 + From b9550a1488a67aade463f509a7eb9240646f667a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 13 May 2021 19:16:00 +0200 Subject: [PATCH 54/80] [java] UnnecessaryLocalBeforeReturn - fix FN with lambda and anon class The rule uses rule chain now, so that each return is visited, also in lambdas and anonymous classes. This FN has been found via #2687. [skip ci] --- .../UnnecessaryLocalBeforeReturnRule.java | 12 +------ .../xml/UnnecessaryLocalBeforeReturn.xml | 35 +++++++++++++++++++ 2 files changed, 36 insertions(+), 11 deletions(-) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java index e9f43f3400..71a0e745f5 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryLocalBeforeReturnRule.java @@ -14,7 +14,6 @@ import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTMemberSelector; -import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; @@ -32,18 +31,9 @@ public class UnnecessaryLocalBeforeReturnRule extends AbstractJavaRule { private static final PropertyDescriptor STATEMENT_ORDER_MATTERS = booleanProperty("statementOrderMatters").defaultValue(true).desc("If set to false this rule no longer requires the variable declaration and return statement to be on consecutive lines. Any variable that is used solely in a return statement will be reported.").build(); - public UnnecessaryLocalBeforeReturnRule() { definePropertyDescriptor(STATEMENT_ORDER_MATTERS); - } - - @Override - public Object visit(ASTMethodDeclaration meth, Object data) { - // skip void/abstract/native method - if (meth.isVoid() || meth.isAbstract() || meth.isNative()) { - return data; - } - return super.visit(meth, data); + addRuleChainVisit(ASTReturnStatement.class); } @Override diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml index c1e5f21a5f..9944627412 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml @@ -269,6 +269,41 @@ public class ObjectCreator { final Object o = new Object(); // captured by the method ref return o::toString; } +} + ]]> + + + + FN with lambdas + 1 + 5 + c = () -> { String s = "1"; return s; }; + } +} + ]]> + + + + FN with anonymous classes + 1 + 8 + c = new Callable<>() { + public String call() { + String s = "1"; + return s; + } + }; + } } ]]> From 45098b801e4f66783457e883d8c0ae15d6a75f1e Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 13 May 2021 19:24:17 +0200 Subject: [PATCH 55/80] [doc] Update release notes (#3275) --- docs/pages/release_notes.md | 3 ++- .../java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml | 4 ++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index a2b26c2bc0..3552710f80 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -41,7 +41,8 @@ This is a {{ site.pmd.release_type }} release. * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports * [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes * [#3265](https://github.com/pmd/pmd/pull/3265): \[java] MethodArgumentCouldBeFinal: false negatives with interfaces and inner classes - * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes + * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes# + * [#3275](https://github.com/pmd/pmd/pull/3275): \[java] UnnecessaryLocalBeforeReturn: false negatives with lambda and anon class * java-design * [#2780](https://github.com/pmd/pmd/issues/2780): \[java] DataClass example from documentation results in false-negative * java-errorprone diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml index 9944627412..83cbf37e29 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryLocalBeforeReturn.xml @@ -274,7 +274,7 @@ public class ObjectCreator { - FN with lambdas + FN with lambdas #3275 1 5 - FN with anonymous classes + FN with anonymous classes #3275 1 8 Date: Thu, 13 May 2021 19:26:35 +0200 Subject: [PATCH 56/80] Update pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml --- .../pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml index 70bca8f979..850f32a2a3 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/OnlyOneReturn.xml @@ -119,7 +119,7 @@ public class OnlyOneReturn { - False positive with anonymous class #3274 + False negative with anonymous class #3274 2 6,7 Date: Fri, 14 May 2021 16:47:31 +0200 Subject: [PATCH 57/80] Fix release notes --- docs/pages/release_notes.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index cc00908567..45638eb771 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -43,7 +43,7 @@ This is a {{ site.pmd.release_type }} release. * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports * [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes * [#3265](https://github.com/pmd/pmd/pull/3265): \[java] MethodArgumentCouldBeFinal: false negatives with interfaces and inner classes - * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes# + * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes * [#3275](https://github.com/pmd/pmd/pull/3275): \[java] UnnecessaryLocalBeforeReturn: false negatives with lambda and anon class * java-design * [#2780](https://github.com/pmd/pmd/issues/2780): \[java] DataClass example from documentation results in false-negative From e54c4dc366aa0539f3af5a288f315b2353f6288f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 14 May 2021 17:39:05 +0200 Subject: [PATCH 58/80] Fix #2639 Tests are in pmd core, current cli tests are only in pmd-java unfortunately. --- .../java/net/sourceforge/pmd/util/IOUtil.java | 14 +- .../net/sourceforge/pmd/cli/CoreCliTest.java | 123 ++++++++++++++++++ .../net/sourceforge/pmd/cli/FakeRuleset.xml | 32 +++++ .../java/net/sourceforge/pmd/cli/CLITest.java | 13 +- 4 files changed, 172 insertions(+), 10 deletions(-) create mode 100644 pmd-core/src/test/java/net/sourceforge/pmd/cli/CoreCliTest.java create mode 100644 pmd-core/src/test/resources/net/sourceforge/pmd/cli/FakeRuleset.xml diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java b/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java index 1ef21f6135..50177007f5 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/util/IOUtil.java @@ -15,6 +15,7 @@ import java.nio.charset.Charset; import java.nio.charset.StandardCharsets; import java.nio.charset.UnsupportedCharsetException; import java.nio.file.Files; +import java.nio.file.Path; import java.security.AccessController; import java.security.PrivilegedAction; @@ -62,16 +63,23 @@ public final class IOUtil { /** * Creates a writer that writes to the given file or to stdout. + * The file is created if it does not exist. * *

Warning: This writer always uses the system default charset. * * @param reportFile the file name (optional) - * @return the writer, never null + * + * @return the writer, never null */ public static Writer createWriter(String reportFile) { try { - return StringUtils.isBlank(reportFile) ? createWriter() - : Files.newBufferedWriter(new File(reportFile).toPath(), getDefaultCharset()); + if (StringUtils.isBlank(reportFile)) { + return createWriter(); + } + Path path = new File(reportFile).toPath(); + Files.createDirectories(path.getParent()); // ensure parent dir exists + // this will create the file if it doesn't exist + return Files.newBufferedWriter(path, getDefaultCharset()); } catch (IOException e) { throw new IllegalArgumentException(e); } diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/cli/CoreCliTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/cli/CoreCliTest.java new file mode 100644 index 0000000000..a3bba10c4d --- /dev/null +++ b/pmd-core/src/test/java/net/sourceforge/pmd/cli/CoreCliTest.java @@ -0,0 +1,123 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.cli; + +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertTrue; + +import java.io.IOException; +import java.nio.ByteBuffer; +import java.nio.charset.StandardCharsets; +import java.nio.file.Files; +import java.nio.file.Path; + +import org.junit.Before; +import org.junit.Rule; +import org.junit.Test; +import org.junit.contrib.java.lang.system.RestoreSystemProperties; +import org.junit.rules.TemporaryFolder; + +import net.sourceforge.pmd.PMD; + +/** + * + */ +public class CoreCliTest { + + private static final String DUMMY_RULESET = "net/sourceforge/pmd/cli/FakeRuleset.xml"; + private static final String STRING_TO_REPLACE = "__should_be_replaced__"; + + @Rule + public TemporaryFolder tempDir = new TemporaryFolder(); + @Rule + public RestoreSystemProperties restoreSystemProperties = new RestoreSystemProperties(); + private Path srcDir; + + @Before + public void setup() throws IOException { + // set current directory to wd + Path root = tempRoot(); + System.setProperty("user.dir", root.toString()); + + // create a few files + srcDir = Files.createDirectories(root.resolve("src")); + writeString(srcDir.resolve("someSource.dummy"), "dummy text"); + } + + + @Test + public void testPreExistingReportFile() throws IOException { + Path reportFile = tempRoot().resolve("out/reportFile.txt"); + // now we create the file + Files.createDirectories(reportFile.getParent()); + writeString(reportFile, STRING_TO_REPLACE); + + assertTrue("Report file should exist", Files.exists(reportFile)); + + runPmdSuccessfully("-d", srcDir, "-R", DUMMY_RULESET, "-r", reportFile); + + assertNotEquals(readString(reportFile), STRING_TO_REPLACE); + } + + @Test + public void testNonExistentReportFile() { + Path reportFile = tempRoot().resolve("out/reportFile.txt"); + + assertFalse("Report file should not exist", Files.exists(reportFile)); + + runPmdSuccessfully("-d", srcDir, "-R", DUMMY_RULESET, "-r", reportFile); + + assertTrue("Report file should have been created", Files.exists(reportFile)); + } + + + + + + + // utilities + + + + private Path tempRoot() { + return tempDir.getRoot().toPath(); + } + + + private static void runPmdSuccessfully(Object... args) { + runPmd(0, args); + } + + private static String[] argsToString(Object... args) { + String[] result = new String[args.length]; + for (int i = 0; i < args.length; i++) { + result[i] = args[i].toString(); + } + return result; + } + + // available in Files on java 11+ + private static void writeString(Path path, String text) throws IOException { + ByteBuffer encoded = StandardCharsets.UTF_8.encode(text); + Files.write(path, encoded.array()); + } + + + // available in Files on java 11+ + private static String readString(Path path) throws IOException { + byte[] bytes = Files.readAllBytes(path); + ByteBuffer buf = ByteBuffer.wrap(bytes); + return StandardCharsets.UTF_8.decode(buf).toString(); + } + + private static void runPmd(int expectedExitCode, Object[] args) { + int actualExitCode = PMD.run(argsToString(args)); + assertEquals("Exit code", expectedExitCode, actualExitCode); + } + + +} diff --git a/pmd-core/src/test/resources/net/sourceforge/pmd/cli/FakeRuleset.xml b/pmd-core/src/test/resources/net/sourceforge/pmd/cli/FakeRuleset.xml new file mode 100644 index 0000000000..7b3f1a8924 --- /dev/null +++ b/pmd-core/src/test/resources/net/sourceforge/pmd/cli/FakeRuleset.xml @@ -0,0 +1,32 @@ + + + + + Ruleset used by test RuleSetFactoryTest + + + + +Just for test + + 3 + + + + + + + +Just for test + + 3 + + + + + \ No newline at end of file diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/cli/CLITest.java b/pmd-java/src/test/java/net/sourceforge/pmd/cli/CLITest.java index 3e82902345..2fc217381d 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/cli/CLITest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/cli/CLITest.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd.cli; import static org.junit.Assert.assertTrue; import java.io.File; -import java.io.IOException; import java.util.regex.Pattern; import org.junit.Assert; @@ -39,7 +38,7 @@ public class CLITest extends BaseCLITest { } @Test - public void changeJavaVersion() throws IOException { + public void changeJavaVersion() { String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", "category/java/design.xml", "-version", "1.5", "-language", "java", "-debug", }; String resultFilename = runTest(args, "chgJavaVersion"); @@ -54,14 +53,14 @@ public class CLITest extends BaseCLITest { } @Test - public void exitStatusWithViolations() throws IOException { + public void exitStatusWithViolations() { String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", "category/java/errorprone.xml", }; String resultFilename = runTest(args, "exitStatusWithViolations", 4); assertTrue(FileUtil.findPatternInFile(new File(resultFilename), "Avoid empty if")); } @Test - public void exitStatusWithViolationsAndWithoutFailOnViolations() throws IOException { + public void exitStatusWithViolationsAndWithoutFailOnViolations() { String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", "category/java/errorprone.xml", "-failOnViolation", "false", }; String resultFilename = runTest(args, "exitStatusWithViolationsAndWithoutFailOnViolations", 0); assertTrue(FileUtil.findPatternInFile(new File(resultFilename), "Avoid empty if")); @@ -71,7 +70,7 @@ public class CLITest extends BaseCLITest { * See https://sourceforge.net/p/pmd/bugs/1231/ */ @Test - public void testWrongRuleset() throws Exception { + public void testWrongRuleset() { String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", "category/java/designn.xml", }; String filename = TEST_OUPUT_DIRECTORY + "testWrongRuleset.txt"; createTestOutputFile(filename); @@ -85,7 +84,7 @@ public class CLITest extends BaseCLITest { * See https://sourceforge.net/p/pmd/bugs/1231/ */ @Test - public void testWrongRulesetWithRulename() throws Exception { + public void testWrongRulesetWithRulename() { String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", "category/java/designn.xml/UseCollectionIsEmpty", }; String filename = TEST_OUPUT_DIRECTORY + "testWrongRuleset.txt"; createTestOutputFile(filename); @@ -99,7 +98,7 @@ public class CLITest extends BaseCLITest { * See https://sourceforge.net/p/pmd/bugs/1231/ */ @Test - public void testWrongRulename() throws Exception { + public void testWrongRulename() { String[] args = { "-d", SOURCE_FOLDER, "-f", "text", "-R", "category/java/design.xml/ThisRuleDoesNotExist", }; String filename = TEST_OUPUT_DIRECTORY + "testWrongRuleset.txt"; createTestOutputFile(filename); From 23e101f62a9457bf5d8866d475d6601c625c1c18 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 14 May 2021 17:45:09 +0200 Subject: [PATCH 59/80] Update docs --- docs/pages/pmd/userdocs/cli_reference.md | 2 +- .../src/main/java/net/sourceforge/pmd/cli/PMDParameters.java | 5 ++++- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/docs/pages/pmd/userdocs/cli_reference.md b/docs/pages/pmd/userdocs/cli_reference.md index 2e6350ebc8..541f49eb25 100644 --- a/docs/pages/pmd/userdocs/cli_reference.md +++ b/docs/pages/pmd/userdocs/cli_reference.md @@ -111,7 +111,7 @@ The tool comes with a rather extensive help text, simply running with `-help`! %} {% include custom/cli_option_row.html options="-reportfile,-r" option_arg="path" - description="Path to a file in which the report output will be sent. By default the report is printed on standard output." + description="Path to a file to which report output is written. The file is created if it does not exist. If this option is not specified, the report is rendered to standard output." %} {% include custom/cli_option_row.html options="-shortnames" description="Prints shortened filenames in the report." diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDParameters.java b/pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDParameters.java index 94cd430553..73490891a2 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDParameters.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/cli/PMDParameters.java @@ -89,7 +89,10 @@ public class PMDParameters { converter = PropertyConverter.class) private List properties = new ArrayList<>(); - @Parameter(names = { "-reportfile", "-r" }, description = "Sends report output to a file; default to System.out.") + @Parameter(names = { "-reportfile", "-r" }, + description = "Path to a file to which report output is written. " + + "The file is created if it does not exist. " + + "If this option is not specified, the report is rendered to standard output.") private String reportfile = null; @Parameter(names = { "-version", "-v" }, description = "Specify version of a language PMD should use.") From 0294083874bca81dfc92905ede96282c36f8799e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 14 May 2021 18:50:08 +0200 Subject: [PATCH 60/80] Update release notes, refs #3279 --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 64db65c866..052bf30ed4 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -27,6 +27,8 @@ This is a {{ site.pmd.release_type }} release. * apex * [#3243](https://github.com/pmd/pmd/pull/3243): \[apex] Correct findBoundary when traversing AST +* core + * [#2639](https://github.com/pmd/pmd/issues/2639): \[core] PMD CLI output file is not created if directory or directories in path don't exist * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages * dist From dde7b19f970e95e1f5e0a739a1c4eefde4449c60 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Fri, 14 May 2021 19:38:13 +0200 Subject: [PATCH 61/80] Add test for #2642 Was fixed by c0eba42f2945593c4b7c1ea41d51be385dd61809 in pmd 6.27.0 (bisected) --- .../xml/InvalidLogMessageFormat.xml | 30 +++++++++++++++++++ 1 file changed, 30 insertions(+) diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml index 4f95dab5b5..94c29dc23a 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/InvalidLogMessageFormat.xml @@ -968,4 +968,34 @@ class TestInvalidLogMessageFormat { } ]]> + + #2642 [java] InvalidLogMessageFormat throws IndexOutOfBoundsException with SLF4J and a StringBuilder variable set using var + 0 + + From 9e4ea58518ada5426a37b976adc4053b26a1a3a0 Mon Sep 17 00:00:00 2001 From: Arnaud Jeansen Date: Mon, 17 May 2021 09:03:00 +0200 Subject: [PATCH 62/80] Ignore private modifiers for both methods and inner classes --- .../resources/category/java/bestpractices.xml | 4 +-- .../xml/JUnit5TestShouldBePackagePrivate.xml | 35 +++++++++++++++++-- 2 files changed, 34 insertions(+), 5 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 19a15c960d..41cfb00807 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -832,14 +832,14 @@ if they're package-private. Marking them as such is a good practice to limit the ]] [MethodDeclaration] ]/( - self::*[@Abstract=false() and @PackagePrivate=false()] + self::*[@Abstract=false() and (@Public=true() or @Protected=true())] | ClassOrInterfaceBody/ClassOrInterfaceBodyDeclaration [Annotation//Name[ pmd-java:typeIs('org.junit.jupiter.api.Test') or pmd-java:typeIs('org.junit.jupiter.api.RepeatedTest') or pmd-java:typeIs('org.junit.jupiter.api.TestFactory') or pmd-java:typeIs('org.junit.jupiter.api.TestTemplate') or pmd-java:typeIs('org.junit.jupiter.params.ParameterizedTest') ]] - /MethodDeclaration[@PackagePrivate=false()] + /MethodDeclaration[@Public=true() or @Protected=true()] ) ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml index afc474bab0..9fdea7c27d 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/JUnit5TestShouldBePackagePrivate.xml @@ -31,6 +31,35 @@ class MyTests { ]]> + + Private method modifiers should be reported by rule 'JUnit5TestNoPrivateModifier' and ignored by rule 'JUnit5TestShouldBePackagePrivate' + 0 + + + + + Private modifiers for inner classes should be reported by rule 'JUnit5TestNoPrivateModifier' and ignored by rule 'JUnit5TestShouldBePackagePrivate' + 0 + + + Public modifier is allowed on an abstract test classes 0 @@ -58,9 +87,9 @@ public interface MyTests { - Non package private modifiers on all JUnit5 test types should be rejected - 6 - 8,10,13,16,19,23 + Public and protected modifiers on all JUnit5 test types should be rejected + 5 + 8,10,13,19,23 Date: Wed, 12 May 2021 19:45:35 +1000 Subject: [PATCH 63/80] fix: check for deprecated testmethod Instead of checking methods in a test class that have system.assert, which gives false positives, check for the presence of the testmethod modifier. --- ...tMethodShouldHaveIsTestAnnotationRule.java | 42 +++++++++---------- 1 file changed, 19 insertions(+), 23 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java index 66185f9a3d..3b09f58aec 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.lang.apex.rule.bestpractices; +import static apex.jorje.semantic.symbol.type.ModifierTypeInfos.TEST_METHOD; + import java.util.HashSet; import java.util.List; import java.util.Locale; @@ -14,35 +16,29 @@ import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; import net.sourceforge.pmd.lang.apex.rule.AbstractApexUnitTestRule; public class ApexUnitTestMethodShouldHaveIsTestAnnotationRule extends AbstractApexUnitTestRule { - private static final Set ASSERT_METHODS = new HashSet<>(); - - static { - ASSERT_METHODS.add("system.assert"); - ASSERT_METHODS.add("system.assertequals"); - ASSERT_METHODS.add("system.assertnotequals"); - } @Override public Object visit(final ASTMethod node, final Object data) { - // test methods should have @isTest annotation. + // test methods should have @isTest annotation not testMethod if (isTestMethodOrClass(node)) { - return data; - } - return checkForAssertStatements(node, data); - } - - private Object checkForAssertStatements(final ASTMethod testMethod, final Object data) { - List methodCallList = testMethod.findDescendantsOfType(ASTMethodCallExpression.class); - String assertMethodName; - for (ASTMethodCallExpression assertMethodCall : methodCallList) { - assertMethodName = assertMethodCall.getFullMethodName().toLowerCase(Locale.ROOT); - if (ASSERT_METHODS.contains(assertMethodName)) { - addViolationWithMessage(data, testMethod, - "''{0}'' method should have @IsTest annotation.", - new Object[] { testMethod.getImage() }); - return data; + if (hasDeprecatedTestMethodAnnotation(node)) { + return addViolation(node, data); } } return data; } + + private boolean hasDeprecatedTestMethodAnnotation(final ASTMethod method) { + return method.getNode().getModifiers().has(TEST_METHOD); + } + + private Object addViolation(final ASTMethod testMethod, final Object data) { + addViolationWithMessage( + data, + testMethod, + "''{0}'' method should have @IsTest annotation.", + new Object[] { testMethod.getImage() } + ); + return data; + } } From 9a7933b6d737e287f4bfe2888c835f8aed0ae5ad Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Fri, 14 May 2021 00:00:56 +1000 Subject: [PATCH 64/80] fix: remove unused imports --- .../ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java | 6 ------ 1 file changed, 6 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java index 3b09f58aec..a80f6bd128 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java @@ -6,13 +6,7 @@ package net.sourceforge.pmd.lang.apex.rule.bestpractices; import static apex.jorje.semantic.symbol.type.ModifierTypeInfos.TEST_METHOD; -import java.util.HashSet; -import java.util.List; -import java.util.Locale; -import java.util.Set; - import net.sourceforge.pmd.lang.apex.ast.ASTMethod; -import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; import net.sourceforge.pmd.lang.apex.rule.AbstractApexUnitTestRule; public class ApexUnitTestMethodShouldHaveIsTestAnnotationRule extends AbstractApexUnitTestRule { From f80a53186f304aafe24def3cb73e5346e02ac016 Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Fri, 14 May 2021 00:02:43 +1000 Subject: [PATCH 65/80] chore: case sensitivity, update docs --- .../ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java | 2 +- .../src/main/resources/category/apex/bestpractices.xml | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java index a80f6bd128..0601fe8e06 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java @@ -30,7 +30,7 @@ public class ApexUnitTestMethodShouldHaveIsTestAnnotationRule extends AbstractAp addViolationWithMessage( data, testMethod, - "''{0}'' method should have @IsTest annotation.", + "''{0}'' method should have @isTest annotation.", new Object[] { testMethod.getImage() } ); return data; diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index 5c13033efc..7c98463018 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -70,8 +70,10 @@ public class Foo { class="net.sourceforge.pmd.lang.apex.rule.bestpractices.ApexUnitTestMethodShouldHaveIsTestAnnotationRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_apex_bestpractices.html#apexunittestmethodshouldhaveistestannotation"> -Apex test methods should have @isTest annotation. -As testMethod keyword is deprecated, Salesforce advices to use @isTest annotation for test class/methods. +Apex test methods should have `@isTest` annotation instead of the `testMethod` keyword, +as `testMethod` is deprecated. +Salesforce advices to use [@isTest](https://developer.salesforce.com/docs/atlas.en-us.apexcode.meta/apexcode/apex_classes_annotation_isTest.htm) +annotation for test classes and methods. 3 @@ -86,7 +88,7 @@ private class ATest { @isTest static void methodCTest() { System.assert(1==2); } - @isTest static void methodCTest() { + static testmethod void methodCTest() { System.debug('I am a debug statement'); } private void fetchData() { From 738aca9744800990d3999984c0a296ded2cc9568 Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Fri, 14 May 2021 00:03:07 +1000 Subject: [PATCH 66/80] fix: update test for false positives --- .../xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml index 77430db678..ebed385f1d 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml @@ -14,7 +14,7 @@ private class ATest { @isTest static void methodATest() { } - static void methodBTest() { + static testmethod void methodBTest() { System.assert(1==2); } @isTest static void methodCTest() { @@ -26,7 +26,7 @@ private class ATest { static void methodETest() { System.debug('I am a debug statement.'); } - static void methodFTest() { + static testmethod void methodFTest() { System.assertEquals(1,2); } private void fetchData() { From 4b2457a307ab98f19b522d445ca4b5659a6cbb4d Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Mon, 17 May 2021 15:49:30 +1000 Subject: [PATCH 67/80] feat: add isTestMethod to ASTModifierNode --- .../sourceforge/pmd/lang/apex/ast/ASTModifierNode.java | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java index 59ce9ec6d3..3bc4f3ab94 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.lang.apex.ast; +import static apex.jorje.semantic.symbol.type.ModifierTypeInfos.TEST_METHOD; + import net.sourceforge.pmd.annotation.InternalApi; import apex.jorje.semantic.ast.modifier.ModifierNode; @@ -62,10 +64,16 @@ public class ASTModifierNode extends AbstractApexNode implements A return (node.getModifiers().getJavaModifiers() & TRANSIENT) == TRANSIENT; } + // true if function has `@isTest` annotation or `testmethod` modifier public boolean isTest() { return node.getModifiers().isTest(); } + // true if function has `testmethod` modifier + public boolean isTestMethod() { + return node.getModifiers().has(TEST_METHOD); + } + public boolean isTestOrTestSetup() { return node.getModifiers().isTestOrTestSetup(); } From cf39de85b2782dc58a4de7497a106411721093d9 Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Mon, 17 May 2021 15:49:54 +1000 Subject: [PATCH 68/80] feat: rewrite rule as xpath --- ...tMethodShouldHaveIsTestAnnotationRule.java | 38 ------------------- .../resources/category/apex/bestpractices.xml | 14 ++++++- 2 files changed, 12 insertions(+), 40 deletions(-) delete mode 100644 pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java deleted file mode 100644 index 0601fe8e06..0000000000 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/bestpractices/ApexUnitTestMethodShouldHaveIsTestAnnotationRule.java +++ /dev/null @@ -1,38 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.apex.rule.bestpractices; - -import static apex.jorje.semantic.symbol.type.ModifierTypeInfos.TEST_METHOD; - -import net.sourceforge.pmd.lang.apex.ast.ASTMethod; -import net.sourceforge.pmd.lang.apex.rule.AbstractApexUnitTestRule; - -public class ApexUnitTestMethodShouldHaveIsTestAnnotationRule extends AbstractApexUnitTestRule { - - @Override - public Object visit(final ASTMethod node, final Object data) { - // test methods should have @isTest annotation not testMethod - if (isTestMethodOrClass(node)) { - if (hasDeprecatedTestMethodAnnotation(node)) { - return addViolation(node, data); - } - } - return data; - } - - private boolean hasDeprecatedTestMethodAnnotation(final ASTMethod method) { - return method.getNode().getModifiers().has(TEST_METHOD); - } - - private Object addViolation(final ASTMethod testMethod, final Object data) { - addViolationWithMessage( - data, - testMethod, - "''{0}'' method should have @isTest annotation.", - new Object[] { testMethod.getImage() } - ); - return data; - } -} diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index 7c98463018..c2070b87f8 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -64,10 +64,10 @@ public class Foo { Apex test methods should have `@isTest` annotation instead of the `testMethod` keyword, @@ -76,6 +76,15 @@ Salesforce advices to use [@isTest](https://developer.salesforce.com/docs/atlas. annotation for test classes and methods. 3 + + + + + + + + Date: Mon, 17 May 2021 15:50:17 +1000 Subject: [PATCH 69/80] fix: update a failing test due to new isTestMethod --- .../lang/apex/ast/SafeNavigationOperator.txt | 32 +++++++++---------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt index 43aa08511b..e7108922cd 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt @@ -1,11 +1,11 @@ +- UserClass[@ApexVersion = 51.0, @DefiningType = "Foo", @Image = "Foo", @InterfaceNames = null, @Location = "(4, 14, 180, 183)", @Namespace = "", @RealLoc = true, @SuperClassName = "", @TypeKind = TypeKind.CLASS] - +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- Field[@DefiningType = "Foo", @Image = "x", @Location = "(5, 13, 198, 199)", @Name = "x", @Namespace = "", @RealLoc = true, @Type = "Integer", @Value = null] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(5, 13, 198, 199)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(5, 13, 198, 199)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- Field[@DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Name = "profileUrl", @Namespace = "", @RealLoc = true, @Type = "String", @Value = null] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(8, 12, 365, 375)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(8, 12, 365, 375)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- FieldDeclarationStatements[@DefiningType = "Foo", @Location = "(5, 5, 190, 199)", @Namespace = "", @RealLoc = true, @TypeArguments = null, @TypeName = "Integer"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- FieldDeclaration[@DefiningType = "Foo", @Image = "anIntegerField", @Location = "(5, 13, 198, 199)", @Name = "anIntegerField", @Namespace = "", @RealLoc = true] | +- VariableExpression[@DefiningType = "Foo", @Image = "anIntegerField", @Location = "(5, 27, 212, 226)", @Namespace = "", @RealLoc = true] | | +- ReferenceExpression[@Context = null, @DefiningType = "Foo", @Location = "no location", @Names = null, @Namespace = "", @RealLoc = false, @ReferenceType = ReferenceType.LOAD, @SafeNav = true] @@ -14,7 +14,7 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "x", @Location = "(5, 13, 198, 199)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- FieldDeclarationStatements[@DefiningType = "Foo", @Location = "(8, 5, 358, 375)", @Namespace = "", @RealLoc = true, @TypeArguments = null, @TypeName = "String"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- FieldDeclaration[@DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Name = "profileUrl", @Namespace = "", @RealLoc = true] | +- MethodCallExpression[@DefiningType = "Foo", @FullMethodName = "toExternalForm", @InputParametersSize = 0, @Location = "(8, 47, 400, 414)", @MethodName = "toExternalForm", @Namespace = "", @RealLoc = true] | | +- ReferenceExpression[@Context = null, @DefiningType = "Foo", @Location = "no location", @Names = null, @Namespace = "", @RealLoc = false, @ReferenceType = ReferenceType.METHOD, @SafeNav = true] @@ -23,9 +23,9 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 1, @CanonicalName = "bar1", @Constructor = false, @DefiningType = "Foo", @Image = "bar1", @Location = "(10, 17, 435, 439)", @Namespace = "", @RealLoc = true, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 17, 435, 439)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 17, 435, 439)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "a", @Location = "(10, 29, 447, 448)", @Namespace = "", @RealLoc = true, @Type = "Object"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 29, 447, 448)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 29, 447, 448)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- BlockStatement[@CurlyBrace = true, @DefiningType = "Foo", @Location = "(10, 32, 450, 538)", @Namespace = "", @RealLoc = true] | +- ExpressionStatement[@DefiningType = "Foo", @Location = "(11, 12, 463, 465)", @Namespace = "", @RealLoc = true] | | +- VariableExpression[@DefiningType = "Foo", @Image = "b", @Location = "(11, 12, 463, 464)", @Namespace = "", @RealLoc = true] @@ -41,11 +41,11 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "a1", @Location = "(12, 13, 518, 520)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 2, @CanonicalName = "bar2", @Constructor = false, @DefiningType = "Foo", @Image = "bar2", @Location = "(15, 17, 556, 560)", @Namespace = "", @RealLoc = true, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 17, 556, 560)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 17, 556, 560)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "a", @Location = "(15, 31, 570, 571)", @Namespace = "", @RealLoc = true, @Type = "List"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 31, 570, 571)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 31, 570, 571)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "x", @Location = "(15, 38, 577, 578)", @Namespace = "", @RealLoc = true, @Type = "int"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 38, 577, 578)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 38, 577, 578)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- BlockStatement[@CurlyBrace = true, @DefiningType = "Foo", @Location = "(15, 41, 580, 688)", @Namespace = "", @RealLoc = true] | +- ExpressionStatement[@DefiningType = "Foo", @Location = "(16, 25, 606, 613)", @Namespace = "", @RealLoc = true] | | +- VariableExpression[@DefiningType = "Foo", @Image = "aField", @Location = "(16, 25, 606, 612)", @Namespace = "", @RealLoc = true] @@ -68,12 +68,12 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "x", @Location = "(17, 11, 661, 662)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 1, @CanonicalName = "getName", @Constructor = false, @DefiningType = "Foo", @Image = "getName", @Location = "(20, 19, 708, 715)", @Namespace = "", @RealLoc = true, @ReturnType = "String"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 19, 708, 715)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 19, 708, 715)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "accId", @Location = "(20, 31, 720, 725)", @Namespace = "", @RealLoc = true, @Type = "int"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 31, 720, 725)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 31, 720, 725)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- BlockStatement[@CurlyBrace = true, @DefiningType = "Foo", @Location = "(20, 38, 727, 905)", @Namespace = "", @RealLoc = true] | +- VariableDeclarationStatements[@DefiningType = "Foo", @Location = "(21, 9, 737, 745)", @Namespace = "", @RealLoc = true] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | | +- VariableDeclaration[@DefiningType = "Foo", @Image = "s", @Location = "(21, 16, 744, 745)", @Namespace = "", @RealLoc = true, @Type = "String"] | | +- VariableExpression[@DefiningType = "Foo", @Image = "BillingCity", @Location = "(21, 37, 765, 776)", @Namespace = "", @RealLoc = true] | | | +- ReferenceExpression[@Context = null, @DefiningType = "Foo", @Location = "no location", @Names = null, @Namespace = "", @RealLoc = false, @ReferenceType = ReferenceType.LOAD, @SafeNav = true] @@ -89,10 +89,10 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "accId", @Location = "(23, 54, 886, 891)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 0, @CanonicalName = "", @Constructor = false, @DefiningType = "Foo", @Image = "", @Location = "no location", @Namespace = "", @RealLoc = false, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 8, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = true, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 8, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = true, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- Method[@Arity = 0, @CanonicalName = "clone", @Constructor = false, @DefiningType = "Foo", @Image = "clone", @Location = "no location", @Namespace = "", @RealLoc = false, @ReturnType = "Object"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = true, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = true, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- UserClassMethods[@DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = false] | +- Method[@Arity = 0, @CanonicalName = "", @Constructor = true, @DefiningType = "Foo", @Image = "", @Location = "no location", @Namespace = "", @RealLoc = false, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = true, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = true, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- BridgeMethodCreator[@DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = false] From ecbb57cdea3c5130ae4509721e3f867f5cff2ecd Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Tue, 18 May 2021 09:47:50 +1000 Subject: [PATCH 70/80] fix: rename ambiguous isTestMethod function --- .../pmd/lang/apex/ast/ASTModifierNode.java | 10 ++++-- .../resources/category/apex/bestpractices.xml | 2 +- .../lang/apex/ast/SafeNavigationOperator.txt | 32 +++++++++---------- 3 files changed, 24 insertions(+), 20 deletions(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java index 3bc4f3ab94..08ff30bcf4 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java @@ -64,13 +64,17 @@ public class ASTModifierNode extends AbstractApexNode implements A return (node.getModifiers().getJavaModifiers() & TRANSIENT) == TRANSIENT; } - // true if function has `@isTest` annotation or `testmethod` modifier + /** + * Returns true if function has `@isTest` annotation or `testmethod` modifier + */ public boolean isTest() { return node.getModifiers().isTest(); } - // true if function has `testmethod` modifier - public boolean isTestMethod() { + /** + * Returns true if function has `testmethod` modifier + */ + public boolean isDeprecatedTestMethod() { return node.getModifiers().has(TEST_METHOD); } diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index c2070b87f8..55bd888028 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -80,7 +80,7 @@ annotation for test classes and methods. diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt index e7108922cd..4c5390e594 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/SafeNavigationOperator.txt @@ -1,11 +1,11 @@ +- UserClass[@ApexVersion = 51.0, @DefiningType = "Foo", @Image = "Foo", @InterfaceNames = null, @Location = "(4, 14, 180, 183)", @Namespace = "", @RealLoc = true, @SuperClassName = "", @TypeKind = TypeKind.CLASS] - +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- Field[@DefiningType = "Foo", @Image = "x", @Location = "(5, 13, 198, 199)", @Name = "x", @Namespace = "", @RealLoc = true, @Type = "Integer", @Value = null] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(5, 13, 198, 199)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(5, 13, 198, 199)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- Field[@DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Name = "profileUrl", @Namespace = "", @RealLoc = true, @Type = "String", @Value = null] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(8, 12, 365, 375)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(8, 12, 365, 375)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- FieldDeclarationStatements[@DefiningType = "Foo", @Location = "(5, 5, 190, 199)", @Namespace = "", @RealLoc = true, @TypeArguments = null, @TypeName = "Integer"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- FieldDeclaration[@DefiningType = "Foo", @Image = "anIntegerField", @Location = "(5, 13, 198, 199)", @Name = "anIntegerField", @Namespace = "", @RealLoc = true] | +- VariableExpression[@DefiningType = "Foo", @Image = "anIntegerField", @Location = "(5, 27, 212, 226)", @Namespace = "", @RealLoc = true] | | +- ReferenceExpression[@Context = null, @DefiningType = "Foo", @Location = "no location", @Names = null, @Namespace = "", @RealLoc = false, @ReferenceType = ReferenceType.LOAD, @SafeNav = true] @@ -14,7 +14,7 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "x", @Location = "(5, 13, 198, 199)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- FieldDeclarationStatements[@DefiningType = "Foo", @Location = "(8, 5, 358, 375)", @Namespace = "", @RealLoc = true, @TypeArguments = null, @TypeName = "String"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- FieldDeclaration[@DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Name = "profileUrl", @Namespace = "", @RealLoc = true] | +- MethodCallExpression[@DefiningType = "Foo", @FullMethodName = "toExternalForm", @InputParametersSize = 0, @Location = "(8, 47, 400, 414)", @MethodName = "toExternalForm", @Namespace = "", @RealLoc = true] | | +- ReferenceExpression[@Context = null, @DefiningType = "Foo", @Location = "no location", @Names = null, @Namespace = "", @RealLoc = false, @ReferenceType = ReferenceType.METHOD, @SafeNav = true] @@ -23,9 +23,9 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "profileUrl", @Location = "(8, 12, 365, 375)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 1, @CanonicalName = "bar1", @Constructor = false, @DefiningType = "Foo", @Image = "bar1", @Location = "(10, 17, 435, 439)", @Namespace = "", @RealLoc = true, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 17, 435, 439)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 17, 435, 439)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "a", @Location = "(10, 29, 447, 448)", @Namespace = "", @RealLoc = true, @Type = "Object"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 29, 447, 448)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(10, 29, 447, 448)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- BlockStatement[@CurlyBrace = true, @DefiningType = "Foo", @Location = "(10, 32, 450, 538)", @Namespace = "", @RealLoc = true] | +- ExpressionStatement[@DefiningType = "Foo", @Location = "(11, 12, 463, 465)", @Namespace = "", @RealLoc = true] | | +- VariableExpression[@DefiningType = "Foo", @Image = "b", @Location = "(11, 12, 463, 464)", @Namespace = "", @RealLoc = true] @@ -41,11 +41,11 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "a1", @Location = "(12, 13, 518, 520)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 2, @CanonicalName = "bar2", @Constructor = false, @DefiningType = "Foo", @Image = "bar2", @Location = "(15, 17, 556, 560)", @Namespace = "", @RealLoc = true, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 17, 556, 560)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 17, 556, 560)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "a", @Location = "(15, 31, 570, 571)", @Namespace = "", @RealLoc = true, @Type = "List"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 31, 570, 571)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 31, 570, 571)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "x", @Location = "(15, 38, 577, 578)", @Namespace = "", @RealLoc = true, @Type = "int"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 38, 577, 578)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(15, 38, 577, 578)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- BlockStatement[@CurlyBrace = true, @DefiningType = "Foo", @Location = "(15, 41, 580, 688)", @Namespace = "", @RealLoc = true] | +- ExpressionStatement[@DefiningType = "Foo", @Location = "(16, 25, 606, 613)", @Namespace = "", @RealLoc = true] | | +- VariableExpression[@DefiningType = "Foo", @Image = "aField", @Location = "(16, 25, 606, 612)", @Namespace = "", @RealLoc = true] @@ -68,12 +68,12 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "x", @Location = "(17, 11, 661, 662)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 1, @CanonicalName = "getName", @Constructor = false, @DefiningType = "Foo", @Image = "getName", @Location = "(20, 19, 708, 715)", @Namespace = "", @RealLoc = true, @ReturnType = "String"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 19, 708, 715)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 19, 708, 715)", @Modifiers = 1, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = true, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- Parameter[@DefiningType = "Foo", @Image = "accId", @Location = "(20, 31, 720, 725)", @Namespace = "", @RealLoc = true, @Type = "int"] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 31, 720, 725)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(20, 31, 720, 725)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | +- BlockStatement[@CurlyBrace = true, @DefiningType = "Foo", @Location = "(20, 38, 727, 905)", @Namespace = "", @RealLoc = true] | +- VariableDeclarationStatements[@DefiningType = "Foo", @Location = "(21, 9, 737, 745)", @Namespace = "", @RealLoc = true] - | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] | | +- VariableDeclaration[@DefiningType = "Foo", @Image = "s", @Location = "(21, 16, 744, 745)", @Namespace = "", @RealLoc = true, @Type = "String"] | | +- VariableExpression[@DefiningType = "Foo", @Image = "BillingCity", @Location = "(21, 37, 765, 776)", @Namespace = "", @RealLoc = true] | | | +- ReferenceExpression[@Context = null, @DefiningType = "Foo", @Location = "no location", @Names = null, @Namespace = "", @RealLoc = false, @ReferenceType = ReferenceType.LOAD, @SafeNav = true] @@ -89,10 +89,10 @@ | +- VariableExpression[@DefiningType = "Foo", @Image = "accId", @Location = "(23, 54, 886, 891)", @Namespace = "", @RealLoc = true] | +- EmptyReferenceExpression[@DefiningType = null, @Location = "no location", @Namespace = null, @RealLoc = false] +- Method[@Arity = 0, @CanonicalName = "", @Constructor = false, @DefiningType = "Foo", @Image = "", @Location = "no location", @Namespace = "", @RealLoc = false, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 8, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = true, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = false, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 8, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = true, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- Method[@Arity = 0, @CanonicalName = "clone", @Constructor = false, @DefiningType = "Foo", @Image = "clone", @Location = "no location", @Namespace = "", @RealLoc = false, @ReturnType = "Object"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = true, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = true, @InheritedSharing = false, @Location = "no location", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = false, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- UserClassMethods[@DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = false] | +- Method[@Arity = 0, @CanonicalName = "", @Constructor = true, @DefiningType = "Foo", @Image = "", @Location = "no location", @Namespace = "", @RealLoc = false, @ReturnType = "void"] - | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @Final = false, @Global = true, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestMethod = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] + | +- ModifierNode[@Abstract = false, @DefiningType = "Foo", @DeprecatedTestMethod = false, @Final = false, @Global = true, @InheritedSharing = false, @Location = "(4, 14, 180, 183)", @Modifiers = 0, @Namespace = "", @Override = false, @Private = false, @Protected = false, @Public = false, @RealLoc = true, @Static = false, @Test = false, @TestOrTestSetup = false, @Transient = false, @WebService = false, @WithSharing = false, @WithoutSharing = false] +- BridgeMethodCreator[@DefiningType = "Foo", @Location = "no location", @Namespace = "", @RealLoc = false] From d6dff2ee4b107d78262987d92935e97463b4cd0a Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Wed, 19 May 2021 11:58:05 +1000 Subject: [PATCH 71/80] fix: use xpath 2.0 --- pmd-apex/src/main/resources/category/apex/bestpractices.xml | 1 + 1 file changed, 1 insertion(+) diff --git a/pmd-apex/src/main/resources/category/apex/bestpractices.xml b/pmd-apex/src/main/resources/category/apex/bestpractices.xml index 55bd888028..78c89eb58f 100644 --- a/pmd-apex/src/main/resources/category/apex/bestpractices.xml +++ b/pmd-apex/src/main/resources/category/apex/bestpractices.xml @@ -77,6 +77,7 @@ annotation for test classes and methods. 3 + Date: Wed, 19 May 2021 11:58:43 +1000 Subject: [PATCH 72/80] fix: add test for false-positive on @TestSetup --- ...pexUnitTestMethodShouldHaveIsTestAnnotation.xml | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml index ebed385f1d..794fa2c374 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/bestpractices/xml/ApexUnitTestMethodShouldHaveIsTestAnnotation.xml @@ -69,6 +69,20 @@ private class A { private void fetchData() { System.assertEquals(1,1); } +} + ]]> + + + + false positive with @testsetup #3282 + 0 + From 649f62409d72e33a869aece09fa1cd2a10eaa5ff Mon Sep 17 00:00:00 2001 From: William Brockhus Date: Wed, 19 May 2021 12:04:21 +1000 Subject: [PATCH 73/80] chore: rename is->has --- .../java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java index 08ff30bcf4..d51b4cd877 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/ast/ASTModifierNode.java @@ -74,7 +74,7 @@ public class ASTModifierNode extends AbstractApexNode implements A /** * Returns true if function has `testmethod` modifier */ - public boolean isDeprecatedTestMethod() { + public boolean hasDeprecatedTestMethod() { return node.getModifiers().has(TEST_METHOD); } From 3511ef2cb7b31964910cb78d2067b2b4fa7b24c1 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 20 May 2021 13:39:51 +0200 Subject: [PATCH 74/80] [doc] Update release notes (#3183, #3272) --- docs/pages/release_notes.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 64db65c866..c743a5841c 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -26,6 +26,7 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues * apex + * [#3183](https://github.com/pmd/pmd/issues/3183): \[apex] ApexUnitTestMethodShouldHaveIsTestAnnotation false positive with helper method * [#3243](https://github.com/pmd/pmd/pull/3243): \[apex] Correct findBoundary when traversing AST * doc * [#3230](https://github.com/pmd/pmd/issues/3230): \[doc] Remove "Edit me" button for language index pages @@ -58,6 +59,7 @@ This is a {{ site.pmd.release_type }} release. ### API Changes ### External Contributions +* [#3272](https://github.com/pmd/pmd/pull/3272): \[apex] correction for ApexUnitTestMethodShouldHaveIsTestAnnotation false positives - [William Brockhus](https://github.com/YodaDaCoda) {% endtocmaker %} From 0311101d0e6a35ec7ac0a55a281606ad92bb8cc7 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 20 May 2021 14:05:01 +0200 Subject: [PATCH 75/80] [ci] Update DANGER_GITHUB_API_TOKEN --- .ci/README.md | 3 ++- .ci/files/public-env.gpg | 21 +++++++++++---------- 2 files changed, 13 insertions(+), 11 deletions(-) diff --git a/.ci/README.md b/.ci/README.md index 719d931872..bbfa516d5c 100644 --- a/.ci/README.md +++ b/.ci/README.md @@ -7,7 +7,8 @@ It uses the common scripts from [build-tools](https://github.com/pmd/build-tools This files contains the following environment variables: -* DANGER_GITHUB_API_TOKEN: Token for danger to add comments to PRs as +* DANGER_GITHUB_API_TOKEN: Token for danger to add comments to PRs as . + The token needs the scope "public_repo". * PMD_CI_CHUNK_TOKEN: Token for uploading reports to chunk.io The file is encrypted, so that the tokens are not automatically disabled when github detects them diff --git a/.ci/files/public-env.gpg b/.ci/files/public-env.gpg index 4504a0ef26..93404c7646 100644 --- a/.ci/files/public-env.gpg +++ b/.ci/files/public-env.gpg @@ -1,13 +1,14 @@ -----BEGIN PGP MESSAGE----- -jA0ECQMCkmcn6+I5p8D/0sDSAW6N6nliZko1kP0nXZ7303ywGHypHRzLyIhaiOKl -9afpeACRJ995FY66KL1NZdaeoujK3gFFl0QtTFrcde2KZTyoqrS1Nfro5RGkXYI8 -AyX/kaEd54bVlsiUbHWBjiAUvK83wJiIhClKhD13mpKtsVr5Ep+Rb6j/zeoIkJ8k -sKkS97qZjiXQVGwZzb42tO8W/K2fsahwBUfYU+pGzkqE9jPQPkVDBvWWZTczrj6p -PVWyuREFa0Tdpc2py7+/bS/ls+1JUer6GxYsNl+KiZvCUyBDVL67M42tNzU/cAcA -fdNVkzIB5TCgmUFlIrH43utl/f9O2bXC09NzK+xr3VPSp4BvO8fVA1DfDGswnMgz -OctjRQXeOJZys5Z+Ls01C37ropAC8JTzT+UHM/v0VoQmoKhBx6+ehHkqzwDeUmbV -QJ4HMFsWGyTPy64SCZbS4aOWaQx0q0NFFU7Kmdu4RQNq0u6aODZ99lEvdCODGe70 -vl5qVkzp2q8b6ayY1DUVo4kIvhu3KqFllETnVnHq4J8TCJLw -=goqq +jA0ECQMC6NJFOgCtLK790sDsAV7zf22dX5W7Ki3LdPBesQvoN+fU5xjNcu9ytrOQ +pNDQybzmGkBU7gJM5sasTEv2OKp7h+nt8xSfaE8u0i4G0+yLGZKxbCrZoHnoBiaW +udpjvvxyKdEV8wn1DPsh/W9ARmxdJezpreUwmwExamYiVEXsWOr2DWST+DPVX+4I +5vAxi/YqO1/Pn+s/wIOKM/57otuVxwzwyUFxItJF4GV3NeCKt1cDQHRT1OSn2Mlw +1LY8oUJgjKVCzI9F7xSlHYRYtvUK2icc7lvwrUliXIlcVetUB6Swe5AJmtmQ63y0 +EU52Uh7VPYjj929QgpoVpJHGTJl/Omyk63nb1EOrDWEVUMzg4fDsbAsmzvPyD/FR +R6S9OeJUCsLMXlu7MRHCQi0vDk3li25pVqJmFm9Ahk8tkY/yzgQLoWmVEOhl8xDY +oEQh0XNy9TxvzRzYlutYdU7K4ACohNsJN/MpKkRVzA3aMIBrNjVGa0dF8kd+7grg +fJ+MW8skcpIHDegDcxVAs+O4r9VO3UDAcx3E/kgdLAKSOV0sRt4ZbJZaML7sKkSV +muTtIhHzGwB41qKichY= +=fgy5 -----END PGP MESSAGE----- From 0dab8818af312a80bf691327b142c5974bcb0352 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 20 May 2021 14:26:47 +0200 Subject: [PATCH 76/80] Update gems --- Gemfile.lock | 4 ++-- docs/Gemfile.lock | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index d18bd6b08e..fa8938b390 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -43,7 +43,7 @@ GEM multipart-post (2.1.1) nap (1.1.0) no_proxy_fix (0.1.2) - nokogiri (1.11.3) + nokogiri (1.11.5) mini_portile2 (~> 2.5.0) racc (~> 1.4) octokit (4.21.0) @@ -68,7 +68,7 @@ GEM sawyer (0.8.2) addressable (>= 2.3.5) faraday (> 0.8, < 2.0) - slop (4.8.2) + slop (4.9.0) terminal-table (1.8.0) unicode-display_width (~> 1.1, >= 1.1.1) tzinfo (2.0.4) diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock index 1809f3590a..36eb9740c2 100644 --- a/docs/Gemfile.lock +++ b/docs/Gemfile.lock @@ -25,7 +25,7 @@ GEM ethon (0.14.0) ffi (>= 1.15.0) eventmachine (1.2.7) - execjs (2.7.0) + execjs (2.8.1) faraday (1.4.1) faraday-excon (~> 1.1) faraday-net_http (~> 1.0) @@ -216,7 +216,7 @@ GEM jekyll-seo-tag (~> 2.1) minitest (5.14.4) multipart-post (2.1.1) - nokogiri (1.11.3) + nokogiri (1.11.5) mini_portile2 (~> 2.5.0) racc (~> 1.4) octokit (4.21.0) @@ -226,7 +226,7 @@ GEM forwardable-extended (~> 2.6) public_suffix (4.0.6) racc (1.5.2) - rb-fsevent (0.10.4) + rb-fsevent (0.11.0) rb-inotify (0.10.1) ffi (~> 1.0) rexml (3.2.5) From ce8378b5bf30e4b4a80f97cfc847c78ae4c1ab24 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 21 May 2021 10:12:43 +0200 Subject: [PATCH 77/80] [java] Deprecate rule CloneThrowsCloneNotSupportedException Fixes #3112 --- docs/pages/release_notes.md | 13 +++++++++++++ .../src/main/resources/category/java/errorprone.xml | 7 +++++++ .../src/main/resources/rulesets/java/quickstart.xml | 2 +- 3 files changed, 21 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 2d58181c1c..3c6abfe1ad 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -23,6 +23,18 @@ This is a {{ site.pmd.release_type }} release. Additionally comparisons against constants are allowed now. This makes the rule less noisy when two constants are compared. Constants are identified by looking for an all-caps identifier. +#### Deprecated rules + +* The Java rule {% rule "java/errorprone/CloneThrowsCloneNotSupportedException" %} has been deprecated without + replacement. + + The rule has no real value as `CloneNotSupportedException` is a + checked exception and therefore you need to deal with it while implementing the `clone()` method. You either + need to declare the exception or catch it. If you catch it, then subclasses can't throw it themselves explicitly. + However, `Object.clone()` will still throw this exception if the `Cloneable` interface is not implemented. + + Note, this rule has also been removed from the Quickstart Ruleset (`rulesets/java/quickstart.xml`). + ### Fixed Issues * apex @@ -53,6 +65,7 @@ This is a {{ site.pmd.release_type }} release. * [#2780](https://github.com/pmd/pmd/issues/2780): \[java] DataClass example from documentation results in false-negative * java-errorprone * [#3110](https://github.com/pmd/pmd/issues/3110): \[java] Enhance CompareObjectsWithEquals with list of exceptions + * [#3112](https://github.com/pmd/pmd/issues/3112): \[java] Deprecate rule CloneThrowsCloneNotSupportedException * [#3205](https://github.com/pmd/pmd/issues/3205): \[java] Make CompareObjectWithEquals allow comparing against constants * [#3248](https://github.com/pmd/pmd/issues/3248): \[java] Documentation is wrong for SingletonClassReturningNewInstance rule * [#3249](https://github.com/pmd/pmd/pull/3249): \[java] AvoidFieldNameMatchingTypeName: False negative with interfaces diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 2bd70267bb..5797ddb216 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -1006,6 +1006,7 @@ public class Foo implements Cloneable { The method clone() should throw a CloneNotSupportedException. + +This rule is deprecated since PMD 6.35.0 without replacement. The rule has no real value as +`CloneNotSupportedException` is a checked exception and therefore you need to deal with it while +implementing the `clone()` method. You either need to declare the exception or catch it. If you catch it, +then subclasses can't throw it themselves explicitly. However, `Object.clone()` will still throw this +exception if the `Cloneable` interface is not implemented. 3 diff --git a/pmd-java/src/main/resources/rulesets/java/quickstart.xml b/pmd-java/src/main/resources/rulesets/java/quickstart.xml index b1b89a532b..7635f27bc5 100644 --- a/pmd-java/src/main/resources/rulesets/java/quickstart.xml +++ b/pmd-java/src/main/resources/rulesets/java/quickstart.xml @@ -205,7 +205,7 @@ - + From c2c26cbbb7026fa4f0f4b636ab92758248e07aa1 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 21 May 2021 10:46:55 +0200 Subject: [PATCH 78/80] [java] Deprecate rule DefaultPackage Fixes #3206 --- docs/pages/release_notes.md | 16 ++++++++++++++++ .../main/resources/category/java/codestyle.xml | 11 +++++++++++ .../main/resources/rulesets/java/quickstart.xml | 2 +- 3 files changed, 28 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 2d58181c1c..aa18f4bfc0 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -23,6 +23,21 @@ This is a {{ site.pmd.release_type }} release. Additionally comparisons against constants are allowed now. This makes the rule less noisy when two constants are compared. Constants are identified by looking for an all-caps identifier. +#### Deprecated rules + +* The java rule {% rule "java/codestyle/DefaultPackage" %} has been deprecated in favor of + {% rule "java/codestyle/CommentDefaultAccessModifier" %}. + + The rule "DefaultPackage" assumes that any usage of package-access is accidental, + and by doing so, prohibits using a really fundamental and useful feature of the language. + + To satisfy the rule, you have to make the member public even if it doesn't need to, or make it protected, + which muddies your intent even more if you don't intend the class to be extended, and may be at odds with + other rules like {% rule "java/codestyle/AvoidProtectedFieldInFinalClass" %}. + + The rule {% rule "java/codestyle/CommentDefaultAccessModifier" %} should be used instead. + It flags the same thing, but has an escape hatch. + ### Fixed Issues * apex @@ -44,6 +59,7 @@ This is a {{ site.pmd.release_type }} release. * [#3254](https://github.com/pmd/pmd/issues/3254): \[java] AvoidReassigningParameters reports violations on wrong line numbers * java-codestyle * [#2655](https://github.com/pmd/pmd/issues/2655): \[java] UnnecessaryImport false positive for on-demand imports + * [#3206](https://github.com/pmd/pmd/issues/3206): \[java] Deprecate rule DefaultPackage * [#3262](https://github.com/pmd/pmd/pull/3262): \[java] FieldDeclarationsShouldBeAtStartOfClass: false negative with anon classes * [#3265](https://github.com/pmd/pmd/pull/3265): \[java] MethodArgumentCouldBeFinal: false negatives with interfaces and inner classes * [#3266](https://github.com/pmd/pmd/pull/3266): \[java] LocalVariableCouldBeFinal: false negatives with interfaces, anon classes diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 5515d5ef04..226ea0acb2 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -550,6 +550,7 @@ while (true) { // preferred approach Use explicit scoping instead of accidental usage of default package private level. The rule allows methods and fields annotated with Guava's @VisibleForTesting and JUnit 5's annotations. + +This rule is deprecated since PMD 6.35.0. It assumes that any usage of package-access is accidental, +and by doing so, prohibits using a really fundamental and useful feature of the language. + +To satisfy the rule, you have to make the member public even if it doesn't need to, or make it protected, +which muddies your intent even more if you don't intend the class to be extended, and may be at odds with +other rules like {% rule "java/codestyle/AvoidProtectedFieldInFinalClass" %}. + +The rule {% rule "java/codestyle/CommentDefaultAccessModifier" %} should be used instead. This rule flags +the same thing, but has an escape hatch. 3 diff --git a/pmd-java/src/main/resources/rulesets/java/quickstart.xml b/pmd-java/src/main/resources/rulesets/java/quickstart.xml index b1b89a532b..8b8614c811 100644 --- a/pmd-java/src/main/resources/rulesets/java/quickstart.xml +++ b/pmd-java/src/main/resources/rulesets/java/quickstart.xml @@ -90,7 +90,7 @@ - + From a64077f10cc0fe14e5585acc9e12fc7d62c5d78a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 21 May 2021 11:00:48 +0200 Subject: [PATCH 79/80] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Clément Fournier --- docs/pages/release_notes.md | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 3dce1681cf..08e22c7c46 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -18,8 +18,8 @@ This is a {{ site.pmd.release_type }} release. The latest version of [Rhino](https://github.com/mozilla/rhino), the implementation of JavaScript we use for parsing JavaScript code, requires at least Java 8. Therefore we decided to upgrade the pmd-javascript -module to Java 8 as well. This means, that from now on, a Java 8 or later runtime is required in order -to analyze JavaScript code. Note, that PMD core still stays the at Java 7. +module to Java 8 as well. This means that from now on, a Java 8 or later runtime is required in order +to analyze JavaScript code. Note that PMD core still only requires Java 7. ### Fixed Issues @@ -32,4 +32,3 @@ to analyze JavaScript code. Note, that PMD core still stays the at Java 7. ### External Contributions {% endtocmaker %} - From 4048b67dd7cf7a37041e115a8186311b71fc7828 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Tue, 25 May 2021 13:12:12 +0200 Subject: [PATCH 80/80] Update release notes --- docs/pages/release_notes.md | 9 +++++++++ .../resources/category/java/bestpractices.xml | 15 +++++++++------ 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index fa4be8f336..7e967c8806 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -21,6 +21,15 @@ for parsing JavaScript code, requires at least Java 8. Therefore we decided to u module to Java 8 as well. This means that from now on, a Java 8 or later runtime is required in order to analyze JavaScript code. Note that PMD core still only requires Java 7. +#### New rules + +* The new Java rule {% rule "java/bestpractices/JUnit5TestShouldBePackagePrivate" %} + enforces the convention that JUnit 5 tests should have minimal visibility. + You can try out this rule like so: +```xml + +``` + #### Modified rules * The Java rule {% rule "java/errorprone/CompareObjectsWithEquals" %} has now a new property diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 6f1f8faf33..f18ed66aea 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -824,12 +824,15 @@ public class MyTest { class="net.sourceforge.pmd.lang.rule.XPathRule" typeResolution="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#junit5testshouldbepackageprivate"> - -JUnit 5 tests should be package private at the class level and for each method that uses @Test, @RepeatedTest, -@TestFactory, @TestTemplate or @ParameterizedTest. -Contrary to JUnit4 tests that required public visibility to be run by the engine, JUnit5 tests can also be run -if they're package-private. Marking them as such is a good practice to limit their visibility. - + 3