diff --git a/.ci/build.sh b/.ci/build.sh index 9268888029..7bd31fb01c 100755 --- a/.ci/build.sh +++ b/.ci/build.sh @@ -87,15 +87,15 @@ function build() { if pmd_ci_maven_isSnapshotBuild; then if [ "${PMD_CI_MAVEN_PROJECT_VERSION}" != "7.0.0-SNAPSHOT" ]; then pmd_ci_log_group_start "Executing PMD dogfood test with ${PMD_CI_MAVEN_PROJECT_VERSION}" - ./mvnw versions:set -DnewVersion=${PMD_CI_MAVEN_PROJECT_VERSION}-dogfood -DgenerateBackupPoms=false + ./mvnw versions:set -DnewVersion="${PMD_CI_MAVEN_PROJECT_VERSION}-dogfood" -DgenerateBackupPoms=false + sed -i 's/[0-9]\{1,\}\.[0-9]\{1,\}\.[0-9]\{1,\}.*<\/version>\( *\)/'"${PMD_CI_MAVEN_PROJECT_VERSION}"'<\/version>\1/' pom.xml ./mvnw verify --show-version --errors --batch-mode --no-transfer-progress "${PMD_MAVEN_EXTRA_OPTS[@]}" \ -DskipTests \ -Dmaven.javadoc.skip=true \ -Dmaven.source.skip=true \ - -Dcheckstyle.skip=true \ - -Ppmd-dogfood \ - -Dpmd.dogfood.version=${PMD_CI_MAVEN_PROJECT_VERSION} - ./mvnw versions:set -DnewVersion=${PMD_CI_MAVEN_PROJECT_VERSION} -DgenerateBackupPoms=false + -Dcheckstyle.skip=true + ./mvnw versions:set -DnewVersion="${PMD_CI_MAVEN_PROJECT_VERSION}" -DgenerateBackupPoms=false + git checkout -- pom.xml pmd_ci_log_group_end else # current maven-pmd-plugin is not compatible with PMD 7 yet. diff --git a/.ci/files/all-java.xml b/.ci/files/all-java.xml index bd0946ee66..e3b25d645e 100644 --- a/.ci/files/all-java.xml +++ b/.ci/files/all-java.xml @@ -54,10 +54,6 @@ - - - - @@ -77,7 +73,6 @@ - @@ -130,7 +125,7 @@ - + @@ -153,17 +148,16 @@ - - + - + - + - + @@ -210,7 +204,6 @@ - @@ -228,31 +221,31 @@ - - - - + + + + - + - + - + - + @@ -268,7 +261,7 @@ - + @@ -294,25 +287,17 @@ - - - - - - - - diff --git a/.mvn/wrapper/maven-wrapper.properties b/.mvn/wrapper/maven-wrapper.properties index ffdc10e59f..abd303b673 100644 --- a/.mvn/wrapper/maven-wrapper.properties +++ b/.mvn/wrapper/maven-wrapper.properties @@ -1,2 +1,2 @@ -distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.8.1/apache-maven-3.8.1-bin.zip +distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.8.2/apache-maven-3.8.2-bin.zip wrapperUrl=https://repo.maven.apache.org/maven2/io/takari/maven-wrapper/0.5.6/maven-wrapper-0.5.6.jar diff --git a/docs/_config.yml b/docs/_config.yml index 8faa80876b..ca829f79b7 100644 --- a/docs/_config.yml +++ b/docs/_config.yml @@ -2,7 +2,7 @@ repository: pmd/pmd pmd: version: 7.0.0-SNAPSHOT - previous_version: 6.37.0 + previous_version: 6.38.0 date: ??-?????-2021 release_type: major diff --git a/docs/_data/sidebars/pmd_sidebar.yml b/docs/_data/sidebars/pmd_sidebar.yml index c743d80699..c4d8aad28c 100644 --- a/docs/_data/sidebars/pmd_sidebar.yml +++ b/docs/_data/sidebars/pmd_sidebar.yml @@ -397,8 +397,14 @@ entries: - title: Major contributions output: web, pdf subfolderitems: - - title: Adding a new language - url: /pmd_devdocs_major_adding_new_language.html + - title: Rule Guidelines + url: /pmd_devdocs_major_rule_guidelines.html + output: web, pdf + - title: Adding a new language (JavaCC) + url: /pmd_devdocs_major_adding_new_language_javacc.html + output: web, pdf + - title: Adding a new language (Antlr) + url: /pmd_devdocs_major_adding_new_language_antlr.html output: web, pdf - title: Adding a new CPD language url: /pmd_devdocs_major_adding_new_cpd_language.html diff --git a/docs/_data/xpath_funs.yml b/docs/_data/xpath_funs.yml index b2a7beb5f5..830217ae71 100644 --- a/docs/_data/xpath_funs.yml +++ b/docs/_data/xpath_funs.yml @@ -10,6 +10,22 @@ aliases: - &needs_typenode "The context node must be a {% jdoc jast::TypeNode %}" langs: + - name: "Any language" + ns: "pmd" + funs: + - name: fileName + returnType: "xs:string" + shortDescription: "Returns the current filename" + description: "Returns the current simple filename without path but including the extension. + This can be used to write rules that check filename naming conventions. + +

This function is available since PMD 6.38.0.

" + notes: "The function can be called on any node." + examples: + - code: "//b[pmd:fileName() = 'Foo.xml']" + outcome: "Matches any `<b>` tags in files called `Foo.xml`." + + - name: "Java" ns: "pmd-java" funs: diff --git a/docs/pages/7_0_0_release_notes.md b/docs/pages/7_0_0_release_notes.md index 2b4c261c86..d01bca241a 100644 --- a/docs/pages/7_0_0_release_notes.md +++ b/docs/pages/7_0_0_release_notes.md @@ -100,34 +100,53 @@ conversions that may be made implicit. * {% rule "java/codestyle/UseDiamondOperator" %}: the property `java7Compatibility` is removed. The rule now handles Java 7 properly without a property. * {% rule "java/design/SingularField" %}: Properties `checkInnerClasses` and `disallowNotAssignment` are removed. The rule is now more precise and will check these cases properly. +* {% rule "java/design/UseUtilityClass" %}: The property `ignoredAnnotations` has been removed. #### Deprecated Rules -* {% rule "java/performance/UnnecessaryWrapperObjectCreation" %} has been deprecated. - It is replaced by the more general rule {% rule java/codestyle/UnnecessaryBoxing %}. #### Removed Rules The following previously deprecated rules have been finally removed: -* AbstractNaming (java-codestyle) -* AvoidFinalLocalVariable (java-codestyle) -* AvoidPrefixingMethodParameters (java-codestyle) -* DataflowAnomalyAnalysis (java-errorprone) -* ForLoopsMustUseBraces (java-codestyle) -* IfElseStmtsMustUseBraces (java-codestyle) -* IfStmtsMustUseBraces (java-codestyle) +* AbstractNaming (java-codestyle) -> use {% rule "java/codestyle/ClassNamingConventions" %} +* AvoidFinalLocalVariable (java-codestyle) -> not replaced +* AvoidPrefixingMethodParameters (java-codestyle) -> use {% rule "java/codestyle/FormalParameterNamingConventions" %} +* AvoidUsingShortType (java-performance) -> not replaced +* BadComparison (java-errorprone) -> use {% rule "java/errorprone/ComparisonWithNaN" %} +* BooleanInstantiation (java-performance) -> use {% rule "java/codestyle/UnnecessaryBoxing" %} and {% rule "java/bestpractices/PrimitiveWrapperInstantiation" %} +* ByteInstantiation (java-performance) -> use {% rule "java/codestyle/UnnecessaryBoxing" %} and {% rule "java/bestpractices/PrimitiveWrapperInstantiation" %} +* CloneThrowsCloneNotSupportedException (java-errorprone) -> not replaced +* DataflowAnomalyAnalysis (java-errorprone) -> not replaced +* DefaultPackage (java-codestyle) -> use {% rule "java/codestyle/CommentDefaultAccessModifier" %} +* DoNotCallSystemExit (java-errorprone) -> use {% rule "java/errorprone/DoNotTerminateVM" %} +* ForLoopsMustUseBraces (java-codestyle) -> use {% rule "java/codestyle/ControlStatementBraces" %} +* IfElseStmtsMustUseBraces (java-codestyle) -> use {% rule "java/codestyle/ControlStatementBraces" %} +* IfStmtsMustUseBraces (java-codestyle) -> use {% rule "java/codestyle/ControlStatementBraces" %} +* IntegerInstantiation (java-performance) -> use {% rule "java/codestyle/UnnecessaryBoxing" %} and {% rule "java/bestpractices/PrimitiveWrapperInstantiation" %} +* InvalidSlf4jMessageFormat (java-errorprone) -> use {% rule "java/errorprone/InvalidLogMessageFormat" %} * LoggerIsNotStaticFinal (java-errorprone) -* MIsLeadingVariableName (java-codestyle) -* ModifiedCyclomaticComplexity (java-design) -* PositionLiteralsFirstInCaseInsensitiveComparisons (java-bestpractices) -* PositionLiteralsFirstInComparisons (java-bestpractices) -* StdCyclomaticComplexity (java-design) +* LongInstantiation (java-performance) -> use {% rule "java/codestyle/UnnecessaryBoxing" %} and {% rule "java/bestpractices/PrimitiveWrapperInstantiation" %} +* MIsLeadingVariableName (java-codestyle) -> use {% rule "java/codestyle/FieldNamingConventions" %} +* MissingBreakInSwitch (java-errorprone) -> use {% rule "java/errorprone/ImplicitSwitchFallThrough" %} +* ModifiedCyclomaticComplexity (java-design) -> use {% rule "java/design/CyclomaticComplexity" %} +* PositionLiteralsFirstInCaseInsensitiveComparisons (java-bestpractices) -> use {% rule "java/bestpractices/LiteralsFirstInComparisons" %} +* PositionLiteralsFirstInComparisons (java-bestpractices) -> use {% rule "java/bestpractices/LiteralsFirstInComparisons" %} +* ReturnEmptyArrayRatherThanNull (java-errorprone) -> use {% rule "java/errorprone/ReturnEmptyCollectionRatherThanNull" %} +* ShortInstantiation (java-performance) -> use {% rule "java/codestyle/UnnecessaryBoxing" %} and {% rule "java/bestpractices/PrimitiveWrapperInstantiation" %} +* SimplifyBooleanAssertion (java-design) -> use {% rule "java/bestpractices/SimplifiableTestAssertion" %} +* SimplifyStartsWith (java-performance) -> not replaced +* StdCyclomaticComplexity (java-design) -> use {% rule "java/design/CyclomaticComplexity" %} * SuspiciousConstantFieldName (java-codestyle) +* UnnecessaryWrapperObjectCreation (java-performance) -> use the new rule {% rule "java/codestyle/UnnecessaryBoxing" %} * UnsynchronizedStaticDateFormatter (java-multithreading) +* UseAssertEqualsInsteadOfAssertTrue (java-bestpractices) -> use {% rule "java/bestpractices/SimplifiableTestAssertion" %} +* UseAssertNullInsteadOfAssertEquals (java-bestpractices) -> use {% rule "java/bestpractices/SimplifiableTestAssertion" %} +* UseAssertSameInsteadOfAssertEquals (java-bestpractices) -> use {% rule "java/bestpractices/SimplifiableTestAssertion" %} +* UseAssertTrueInsteadOfAssertEquals (java-bestpractices) -> use {% rule "java/bestpractices/SimplifiableTestAssertion" %} * VariableNamingConventions (apex-codestyle) -* VariableNamingConventions (java-codestyle) -* WhileLoopsMustUseBraces (java-codestyle) +* VariableNamingConventions (java-codestyle) -> use {% rule "java/codestyle/FieldNamingConventions" %} and such +* WhileLoopsMustUseBraces (java-codestyle) -> use {% rule "java/codestyle/ControlStatementBraces" %} ### Fixed Issues @@ -178,6 +197,8 @@ The following previously deprecated rules have been finally removed: * [#3218](https://github.com/pmd/pmd/pull/3218): \[java] Generalize UnnecessaryCast to flag all unnecessary casts * [#3221](https://github.com/pmd/pmd/issues/3221): \[java] PrematureDeclaration false positive for unused variables * [#3238](https://github.com/pmd/pmd/issues/3238): \[java] Improve ExprContext, fix FNs of UnnecessaryCast +* java-design + * [#2536](https://github.com/pmd/pmd/issues/2536): \[java] ClassWithOnlyPrivateConstructorsShouldBeFinal can't detect inner class * java-errorprone * [#659](https://github.com/pmd/pmd/issues/659): \[java] MissingBreakInSwitch - last default case does not contain a break * [#1005](https://github.com/pmd/pmd/issues/1005): \[java] CloneMethodMustImplementCloneable triggers for interfaces @@ -238,7 +259,8 @@ The metrics framework has been made simpler and more general. * [#1658](https://github.com/pmd/pmd/pull/1658): \[core] Node support for Antlr-based languages - [Matías Fraga](https://github.com/matifraga) * [#1698](https://github.com/pmd/pmd/pull/1698): \[core] [swift] Antlr Base Parser adapter and Swift Implementation - [Lucas Soncini](https://github.com/lsoncini) * [#1774](https://github.com/pmd/pmd/pull/1774): \[core] Antlr visitor rules - [Lucas Soncini](https://github.com/lsoncini) -* [#1877](https://github.com/pmd/pmd/pull/1877): \[swift] Feature/swift rules - [Matias Fraga](https://github.com/matifraga) +* [#1877](https://github.com/pmd/pmd/pull/1877): \[swift] Feature/swift rules - [Matías Fraga](https://github.com/matifraga) +* [#1881](https://github.com/pmd/pmd/pull/1881): \[doc] Add ANTLR documentation - [Matías Fraga](https://github.com/matifraga) * [#1882](https://github.com/pmd/pmd/pull/1882): \[swift] UnavailableFunction Swift rule - [Tomás de Lucca](https://github.com/tomidelucca) * [#2830](https://github.com/pmd/pmd/pull/2830): \[apex] Apexlink POC - [Kevin Jones](https://github.com/nawforce) diff --git a/docs/pages/next_major_development.md b/docs/pages/next_major_development.md index a6dc241374..97d9c9329d 100644 --- a/docs/pages/next_major_development.md +++ b/docs/pages/next_major_development.md @@ -246,6 +246,10 @@ the breaking API changes will be performed in 7.0.0. an API is tagged as `@Deprecated` or not in the latest minor release. During the development of 7.0.0, we may decide to remove some APIs that were not tagged as deprecated, though we'll try to avoid it." %} +#### 6.38.0 + +No changes. + #### 6.37.0 ##### PMD CLI @@ -1352,35 +1356,35 @@ large projects, with many duplications, it was causing `OutOfMemoryError`s (see is deprecated in favour of {% rule "java/bestpractices/UnusedAssignment" %} (`java-bestpractices`), which was introduced in PMD 6.26.0. -* The java rule {% rule "java/codestyle/DefaultPackage" %} has been deprecated in favor of +* The java rule `DefaultPackage` (java-codestyle) has been deprecated in favor of {% rule "java/codestyle/CommentDefaultAccessModifier" %}. -* The Java rule {% rule "java/errorprone/CloneThrowsCloneNotSupportedException" %} has been deprecated without +* The Java rule `CloneThrowsCloneNotSupportedException` (java-errorprone) has been deprecated without replacement. * The following Java rules are deprecated and removed from the quickstart ruleset, as the new rule {% rule java/bestpractices/SimplifiableTestAssertion %} merges their functionality: - * {% rule java/bestpractices/UseAssertEqualsInsteadOfAssertTrue %} - * {% rule java/bestpractices/UseAssertNullInsteadOfAssertTrue %} - * {% rule java/bestpractices/UseAssertSameInsteadOfAssertTrue %} - * {% rule java/bestpractices/UseAssertTrueInsteadOfAssertEquals %} - * {% rule java/design/SimplifyBooleanAssertion %} + * `UseAssertEqualsInsteadOfAssertTrue` (java-bestpractices) + * `UseAssertNullInsteadOfAssertTrue` (java-bestpractices) + * `UseAssertSameInsteadOfAssertTrue` (java-bestpractices) + * `UseAssertTrueInsteadOfAssertEquals` (java-bestpractices) + * `SimplifyBooleanAssertion` (java-design) -* The Java rule {% rule java/errorprone/ReturnEmptyArrayRatherThanNull %} is deprecated and removed from +* The Java rule `ReturnEmptyArrayRatherThanNull` (java-errorprone) is deprecated and removed from the quickstart ruleset, as the new rule {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} supersedes it. * The following Java rules are deprecated and removed from the quickstart ruleset, as the new rule {% rule java/bestpractices/PrimitiveWrapperInstantiation %} merges their functionality: - * {% rule java/performance/BooleanInstantiation %} - * {% rule java/performance/ByteInstantiation %} - * {% rule java/performance/IntegerInstantiation %} - * {% rule java/performance/LongInstantiation %} - * {% rule java/performance/ShortInstantiation %} + * java/performance/BooleanInstantiation + * java/performance/ByteInstantiation + * java/performance/IntegerInstantiation + * java/performance/LongInstantiation + * java/performance/ShortInstantiation -* The Java rule {% rule java/performance/UnnecessaryWrapperObjectCreation %} is deprecated +* The Java rule java/performance/UnnecessaryWrapperObjectCreation is deprecated with no planned replacement before PMD 7. In it's current state, the rule is not useful as it finds only contrived cases of creating a primitive wrapper and unboxing it explicitly in the same expression. In PMD 7 this and more cases will be covered by a diff --git a/docs/pages/pmd/devdocs/major_contributions/adding_a_new_antlr_based_language.md b/docs/pages/pmd/devdocs/major_contributions/adding_a_new_antlr_based_language.md new file mode 100644 index 0000000000..7f156054d7 --- /dev/null +++ b/docs/pages/pmd/devdocs/major_contributions/adding_a_new_antlr_based_language.md @@ -0,0 +1,156 @@ +--- +title: Adding PMD support for a new ANTLR grammar based language +short_title: Adding a new language with ANTLR +tags: [devdocs, extending] +summary: "How to add a new language to PMD using ANTLR grammar." +last_updated: July 21, 2019 +sidebar: pmd_sidebar +permalink: pmd_devdocs_major_adding_new_language_antlr.html +folder: pmd/devdocs + +# needs to be changed to branch master instead of pmd/7.0.x +# https://github.com/pmd/pmd/blob/pmd/7.0.x -> https://github.com/pmd/pmd/blob/master +--- + + +## 1. Start with a new sub-module. +* See pmd-swift for examples. + +## 2. Implement an AST parser for your language +* ANTLR will generate the parser for you based on the grammar file. The grammar file needs to be placed in the + folder `src/main/antlr4` in the appropriate sub package `ast` of the language. E.g. for swift, the grammar + file is [Swift.g4](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/antlr4/net/sourceforge/pmd/lang/swift/ast/Swift.g4) + and is placed in the package `net.sourceforge.pmd.lang.swift.ast`. + +## 3. Create AST node classes +* The individual AST nodes are generated, but you need to define the common interface for them. +* You need a need to define the supertype interface for all nodes of the language. For that, we provide + [`AntlrNode`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/AntlrNode.java). +* See [`SwiftNode`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/ast/SwiftNode.java) + as an example. +* Additionally, you need several base classes: + * a language specific inner node - these nodes represent the production rules from the grammar. + In Antlr, they are called "ParserRuleContext". We call them "InnerNode". Use the + base class from pmd-core + [`BaseAntlrInnerNode`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/BaseAntlrInnerNode.java) + . And example is [`SwiftInnerNode`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/ast/SwiftInnerNode.java). + * a language specific root node - this provides the root of the AST and our parser will return + subtypes of this node. The root node itself is a "InnerNode". + See [`SwiftRootNode`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/ast/SwiftRootNode.java). + * a language specific terminal node. + See [`SwiftTerminalNode`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/ast/SwiftTerminalNode.java). + * a language specific error node. + See [`SwiftErrorNode`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/ast/SwiftErrorNode.java). +* In order for the generated code to match and use our custom classes, we have a common ant script, that fiddles with + the generated code. The ant script is [`antlr4-wrapper.xml`](https://github.com/pmd/pmd/blob/pmd/7.0.x/antlr4-wrapper.xml) and + does not need to be adjusted - it has plenty of parameters to set. The ant script is added in the + language module's `pom.xml` where the parameters are set (e.g. name of root name class). Have a look at + Swift's example: [`pmd-swift/pom.xml`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/pom.xml). +* You can add additional methods in your "InnerNode" (e.g. `SwiftInnerNode`) that are available on all nodes. + But on most cases you won't need to do anything. + +## 4. Generate your parser +* Make sure, you have the property `true` in your `pom.xml` file. +* This is just a matter of building the language module. ANTLR is called via ant, and this step is added + to the phase `generate-sources`. So you can just call e.g. `./mvnw generate-source -pl pmd-swift` to + have the parser generated. +* The generated code will be placed under `target/generated-sources/antlr4` and will not be committed to + source control. +* You should review the [swift pom](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/pom.xml). + +## 5. Create a TokenManager +* This is needed to support CPD (copy paste detection) +* We provide a default implementation using [`AntlrTokenManager`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/cpd/internal/AntlrTokenizer.java). +* You must create your own "AntlrTokenizer" such as we do with + [`SwiftTokenizer`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/cpd/SwiftTokenizer.java). +* If you wish to filter specific tokens (e.g. comments to support CPD suppression via "CPD-OFF" and "CPD-ON") + you can create your own implementation of + [`AntlrTokenFilter`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/cpd/token/AntlrTokenFilter.java). + You'll need to override then the protected method `getTokenFilter(AntlrTokenManager)` + and return your custom filter. See the tokenizer for C# as an exmaple: + [`CsTokenizer`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-cs/src/main/java/net/sourceforge/pmd/cpd/CsTokenizer.java). + + If you don't need a custom token filter, you don't need to override the method. It returns the default + `AntlrTokenFilter` which doesn't filter anything. + +## 6. Create a PMD parser “adapter” +* Create your own parser, that adapts the ANLTR interface to PMD's parser interface. +* We provide a [`AntlrBaseParser`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/AntlrBaseParser.java) + implementation that you need to extend to create your own adapter as we do with + [`PmdSwiftParser`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/ast/PmdSwiftParser.java). + +## 7. Create a rule violation factory +* This is an optional step. Most like, the default implementation will do what you need. + The default implementation is [`DefaultRuleViolationFactory`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/impl/DefaultRuleViolationFactory.java). +* The purpose of a rule violation factory is to create a rule violation instance for your handler (spoiler). + In case you want to provide additional data in your rule violation, you can create a custom one. However, + adding additional date here is discouraged, as you would need a custom renderer to actually use this + additional data. Such extensions are not language agnostic. + +## 8. Create a version handler +* Now you need to create your version handler, as we did with [`SwiftHandler`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/SwiftHandler.java). +* This class is sort of a gateway between PMD and all parsing logic specific to your language. It has 2 purposes: + * `getRuleViolationFactory` method returns an instance of your rule violation factory *(see step #7)*. + By default, this returns the default rule violation factory. + * `getParser` returns an instance of your parser adapter *(see step #6)*. + That's the only method, that needs to be implemented here. + +## 9. Create a parser visitor adapter +* A parser visitor adapter is not needed anymore with PMD 7. The visitor interface now provides a default + implementation. +* The visitor for ANTLR based AST is generated along the parser from the ANTLR grammar file. The + base interface for a visitor is [`AstVisitor`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/AstVisitor.java). +* The generated visitor class for Swift is called `SwiftVisitor`. +* In order to help use this visitor later on, a base visitor class should be created. + See [`SwiftVisitorBase`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/ast/SwiftVisitorBase.java) + as an example. + +## 10. Create a rule chain visitor +* This step is not needed anymore. For using rule chain, there is no additional adjustment necessary anymore + in the languages. +* This feature has been merged into AbstractRule via the overridable method + {% jdoc !!core::lang.rule.AbstractRule#buildTargetSelector() %}. Individual rules can make use of this optimization + by overriding this method and return an appropriate RuleTargetSelector. + +## 11. Make PMD recognize your language +* Create your own subclass of `net.sourceforge.pmd.lang.BaseLanguageModule`, see Swift as an example: + [`SwiftLanguageModule`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/SwiftLanguageModule.java). +* Add your default version with `addDefaultVersion` in your language module's constructor. +* Add for each additional version of your language a call to `addVersion` as well. +* Create the service registration via the text file `src/main/resources/META-INF/services/net.sourceforge.pmd.lang.Language`. Add your fully qualified class name as a single line into it. + +## 12. Create an abstract rule class for the language +* You need to create your own `AbstractRule` in order to interface your language with PMD's generic rule + execution. +* See [`AbstractSwiftRule`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/AbstractSwiftRule.java) as an example. +* While the rule basically just extends + [`AntlrBaseRule`](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/impl/antlr4/AntlrBaseRule.java) without adding anything, every language should have its own base class for rule. + This helps to organize the code. +* All other rules for your language should extend this class. The purpose of this class is to provide a visitor + via the method `buildVisitor()` for analyzing the AST. The provided visitor only implements the visit methods + for specific AST nodes. The other node types use the default behavior and you don't need to care about them. + +## 13. Create rules +* Creating rules is already pretty well documented in PMD - and it’s no different for a new language, except you + may have different AST nodes. +* PMD supports 2 types of rules, through visitors or XPath. +* To add a visitor rule: + * You need to extend the abstract rule you created on the previous step, you can use the swift + rule [UnavailableFunctionRule](https://github.com/pmd/pmd/blob/pmd/7.0.x/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/rule/bestpractices/UnavailableFunctionRule.java) + as an example. Note, that all rule classes should be suffixed with `Rule` and should be placed + in a package the corresponds to their category. +* To add an XPath rule you can follow our guide [Writing XPath Rules](pmd_userdocs_extending_writing_xpath_rules.html). + +## 14. Test the rules +* See UnavailableFunctionRuleTest for example. Each rule has it's own test class. +* You have to create the category rule set for your language *(see pmd-swift/src/main/resources/bestpractices.xml for example)* +* When executing the test class + * this triggers the unit test to read the corresponding XML file with the rule test data + *(see `UnavailableFunctionRule.xml` for example)* + * This test XML file contains sample pieces of code which should trigger a specified number of + violations of this rule. The unit test will execute the rule on this piece of code, and verify + that the number of violations matches. +* To verify the validity of all the created rulesets, create a subclass of `AbstractRuleSetFactoryTest` (*see `RuleSetFactoryTest` in pmd-swift for example)*. + This will load all rulesets and verify, that all required attributes are provided. + + *Note:* You'll need to add your ruleset to `categories.properties`, so that it can be found. diff --git a/docs/pages/pmd/devdocs/major_contributions/adding_new_language.md b/docs/pages/pmd/devdocs/major_contributions/adding_a_new_javacc_based_language.md similarity index 96% rename from docs/pages/pmd/devdocs/major_contributions/adding_new_language.md rename to docs/pages/pmd/devdocs/major_contributions/adding_a_new_javacc_based_language.md index 1ec23a6dd9..c3b470247c 100644 --- a/docs/pages/pmd/devdocs/major_contributions/adding_new_language.md +++ b/docs/pages/pmd/devdocs/major_contributions/adding_a_new_javacc_based_language.md @@ -1,11 +1,11 @@ --- -title: Adding PMD support for a new language -short_title: Adding a new language +title: Adding PMD support for a new JAVACC grammar based language +short_title: Adding a new language with JAVACC tags: [devdocs, extending] -summary: "How to add a new language to PMD." +summary: "How to add a new language to PMD using JAVACC grammar." last_updated: October 5, 2019 sidebar: pmd_sidebar -permalink: pmd_devdocs_major_adding_new_language.html +permalink: pmd_devdocs_major_adding_new_language_javacc.html folder: pmd/devdocs --- @@ -47,10 +47,9 @@ folder: pmd/devdocs ## 8. Create a version handler * Extend `AbstractLanguageVersionHandler` *(see VmHandler for example)* -* This class is sort of a gateway between PMD and all parsing logic specific to your language. It has 3 purposes: +* This class is sort of a gateway between PMD and all parsing logic specific to your language. It has 2 purposes: * `getRuleViolationFactory` method returns an instance of your rule violation factory *(see step #7)* * `getParser` returns an instance of your parser adapter *(see step #6)* - * `getDumpFacade` returns a `VisitorStarter` that allows to dump a text representation of the AST into a writer *(likely for debugging purposes)* ## 9. Create a parser visitor adapter * If you use JJT to generate your parser, it should also generate an interface for a parser visitor *(see VmParserVisitor for example)* diff --git a/docs/pages/pmd/devdocs/major_contributions/rule_guidelines.md b/docs/pages/pmd/devdocs/major_contributions/rule_guidelines.md new file mode 100644 index 0000000000..2b252ba996 --- /dev/null +++ b/docs/pages/pmd/devdocs/major_contributions/rule_guidelines.md @@ -0,0 +1,77 @@ +--- +title: Guidelines for standard rules +short_title: Rule guidelines +tags: [devdocs, extending] +summary: "Guidelines for rules that are included in the standard distribution" +last_updated: August, 2021 +sidebar: pmd_sidebar +permalink: pmd_devdocs_major_rule_guidelines.html +--- + +{% include note.html content=" +These guidelines are new and most rules don't follow these guidelines yet. +The goal is, that eventually all rules are updated. +" %} + +## Why do we need these guidelines? + +* To prevent low quality contributions +* To reduce time reviewing rules + +They just apply to rules included in the standard distribution. + +## Requirements for standard rules + +To be included in stock PMD, a rule needs + +* Broad applicability. It may be specific to a framework, but then, this framework should be widely used +* Solid documentation. See below +* If it's a performance rule: solid benchmarks. No micro-optimization rules +* No overlap with other rules + +## Dos/Don'ts (rule rules) + +* Rule naming + * **Don't** put the implementation of the rule in the name, because it will be awkward + if the scope of the rule changes + * Eg. *SwitchStmtShouldHaveDefault* -> since enums are a thing they don't necessarily + need to have a default anymore, they should be exhaustive. So the rule name lies now... + * Eg. *MissingBreakInSwitch* -> it's obvious that this is supposed to find fall-through + switches. Counting breaks is not a clever way to do it, but since it's in the name + we can't change it without renaming the rule. + * **Do** use rule names that name the underlying problem that violations exhibit + * Eg. instead of *SwitchStmtShouldHaveDefault*, use *NonExhaustiveSwitchStatement* -> this + is the problem, the description of the rule will clarify why it is a problem and how + to fix it (add a default, or add branches, or something else in the future) + * Eg. instead of *MissingBreakInSwitch*, use *SwitchCaseFallsThrough* + * **Don't** create several rules for instances of the same problem + * *EmptyIfStmt* and *EmptyWhileStmt* are actually the same problem, namely, + that there's useless syntax in the tree. + * **Don't** limit the rule name to strictly what the rule can do today + * Eg. *UnusedPrivateField* is a bad name. The problem is that there is an unused field, + not that it is private as well. If we had the ability to find unused package-private + fields, we would report them too. So if one day we get that ability, + using a name like *UnusedField* would allow us to keep the name. +* Rule messages + * **Do** write rule messages that neutrally point out a problem or construct that should + be reviewed ("Unnecessary parentheses") + * **Don't** write rule messages that give an order ("Avoid unnecessary parentheses") + especially without explaining why, like here + * **Don't** write rule messages that are tautological ("Unnecessary parentheses should be removed"). + The answer to this would be an annoyed "yes I know, so what?". +* **Do** use Markdown in rule descriptions and break lines at a reasonable 80 chars +* **Do** thoroughly comment rule examples. It must be obvious where to look +* **Do** comment your xpath expressions too + +## Rule description template + +* What the rule reports (1 summary line) +* Why the rule exists and where it might be useful (including, since which language version, etc) +* Blank line +* Explain all assumptions that the rule makes and keywords used in the previous paragraph. + ("overridden methods are ignored", "for the purposes of this rule, a 'visible' field is + non-private"). +* Describe known limitations if any +* Blank line +* For each property, explain how it modifies the assumptions and why you would want to use it. + **If you can't explain why it's there then it shouldn’t be there!** diff --git a/docs/pages/pmd/projectdocs/credits.md b/docs/pages/pmd/projectdocs/credits.md index d1a771e9ec..41b5a8a3d6 100644 --- a/docs/pages/pmd/projectdocs/credits.md +++ b/docs/pages/pmd/projectdocs/credits.md @@ -428,7 +428,7 @@ author: Tom Copeland * Mat Booth - #1109 Patch to build with Javacc 5.0 * Stuart Turton - for PLSQL support. See also [pldoc](http://pldoc.sourceforge.net/) * Andrey Utis - for adding Apache Velocity as a new language and writing up a - [howto for adding new languages](pmd_devdocs_major_adding_new_language.html). + [howto for adding new languages JavaCC](pmd_devdocs_major_adding_new_language_javacc.html). * Alan Hohn - for adding Standard and modified cyclomatic complexity rules * Jan van Nunen - for adding CPD support for Matlab, Objective-C, Python, Scala and various bug fixes * Juan Martín Sotuyo Dodero - for many bugfixes/pull requests improving Java grammar and performance diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index f2b41c6432..1922dce49d 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -21,14 +21,14 @@ This is a {{ site.pmd.release_type }} release. ### Fixed Issues -* java-bestpractices - * [#3403](https://github.com/pmd/pmd/issues/3403): \[java] MethodNamingConventions junit5TestPattern does not detect parameterized tests +* core + * [#3499](https://github.com/pmd/pmd/pull/3499): \[core] Fix XPath rulechain with combined node tests +* java-errorprone + * [#3493](https://github.com/pmd/pmd/pull/3493): \[java] AvoidAccessibilityAlteration: add tests and fix rule ### API Changes ### External Contributions -* [#3445](https://github.com/pmd/pmd/pull/3445): \[java] Fix #3403 about MethodNamingConventions and JUnit5 parameterized tests - [Cyril Sicard](https://github.com/CyrilSicard) - {% endtocmaker %} diff --git a/docs/pages/release_notes_old.md b/docs/pages/release_notes_old.md index 8f725ea160..bb4f14fc50 100644 --- a/docs/pages/release_notes_old.md +++ b/docs/pages/release_notes_old.md @@ -5,6 +5,38 @@ permalink: pmd_release_notes_old.html Previous versions of PMD can be downloaded here: https://github.com/pmd/pmd/releases +## 28-August-2021 - 6.38.0 + +The PMD team is pleased to announce PMD 6.38.0. + +This is a minor release. + +### Table Of Contents + +* [Fixed Issues](#fixed-issues) +* [External Contributions](#external-contributions) +* [Stats](#stats) + +### Fixed Issues + +* apex + * [#3462](https://github.com/pmd/pmd/issues/3462): \[apex] SOQL performed in a for-each loop doesn't trigger ApexCRUDViolationRule + * [#3484](https://github.com/pmd/pmd/issues/3484): \[apex] ApexCRUDViolationRule maintains state across files +* core + * [#3446](https://github.com/pmd/pmd/issues/3446): \[core] Allow XPath rules to access the current file name +* java-bestpractices + * [#3403](https://github.com/pmd/pmd/issues/3403): \[java] MethodNamingConventions junit5TestPattern does not detect parameterized tests + +### External Contributions + +* [#3445](https://github.com/pmd/pmd/pull/3445): \[java] Fix #3403 about MethodNamingConventions and JUnit5 parameterized tests - [Cyril Sicard](https://github.com/CyrilSicard) +* [#3470](https://github.com/pmd/pmd/pull/3470): \[apex] Fix ApexCRUDViolationRule - add super call - [Josh Feingold](https://github.com/jfeingold35) + +### Stats +* 32 commits +* 8 closed tickets & PRs +* Days since last release: 27 + ## 28-August-2021 - 6.38.0-SNAPSHOT The PMD team is pleased to announce PMD 6.38.0-SNAPSHOT. diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java index bee47a8bcd..a50b071fb5 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/security/ApexCRUDViolationRule.java @@ -18,6 +18,7 @@ import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; +import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.apex.ast.ASTAssignmentExpression; import net.sourceforge.pmd.lang.apex.ast.ASTBlockStatement; import net.sourceforge.pmd.lang.apex.ast.ASTDmlDeleteStatement; @@ -85,12 +86,25 @@ public class ApexCRUDViolationRule extends AbstractApexRule { private static final Pattern WITH_SECURITY_ENFORCED = Pattern.compile("(?is).*[^']\\s*WITH\\s+SECURITY_ENFORCED\\s*[^']*"); - private final Map varToTypeMapping = new HashMap<>(); - private final ListMultimap typeToDMLOperationMapping = ArrayListMultimap.create(); - private final Map checkedTypeToDMLOperationViaESAPI = new HashMap<>(); - private final Map classMethods = new WeakHashMap<>(); + private Map varToTypeMapping; + private ListMultimap typeToDMLOperationMapping; + private Map checkedTypeToDMLOperationViaESAPI; + private Map classMethods; private String className; + @Override + public void start(RuleContext ctx) { + // At the start of each rule execution, these member variables need to be fresh. So they're initialized in the + // .start() method instead of the constructor, since .start() is called before every execution. + varToTypeMapping = new HashMap<>(); + typeToDMLOperationMapping = ArrayListMultimap.create(); + checkedTypeToDMLOperationViaESAPI = new HashMap<>(); + classMethods = new WeakHashMap<>(); + className = null; + super.start(ctx); + } + + @Override public Object visit(ASTUserClass node, Object data) { if (Helper.isTestMethodOrClass(node) || Helper.isSystemLevelClass(node)) { @@ -240,7 +254,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { checkForAccessibility(soql, data); } - return data; + return super.visit(node, data); } private void addVariableToMapping(final String variableName, final String type) { @@ -533,7 +547,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } - private void validateCRUDCheckPresent(final ApexNode node, final Object data, final String crudMethod, + private boolean validateCRUDCheckPresent(final ApexNode node, final Object data, final String crudMethod, final String typeCheck) { boolean missingKey = !typeToDMLOperationMapping.containsKey(typeCheck); boolean isImproperDMLCheck = !isProperESAPICheckForDML(typeCheck, crudMethod); @@ -542,6 +556,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { //if condition returns true, add violation, otherwise return. if (isImproperDMLCheck && noSecurityEnforced) { addViolation(data, node); + return true; } } else { boolean properChecksHappened = false; @@ -560,8 +575,10 @@ public class ApexCRUDViolationRule extends AbstractApexRule { if (!properChecksHappened) { addViolation(data, node); + return true; } } + return false; } private void checkForAccessibility(final ASTSoqlExpression node, Object data) { @@ -585,7 +602,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { if (wrappingMethod != null) { returnType = getReturnType(wrappingMethod); } - + boolean violationAdded = false; final ASTVariableDeclaration variableDecl = node.getFirstParentOfType(ASTVariableDeclaration.class); if (variableDecl != null) { String type = variableDecl.getType(); @@ -594,15 +611,20 @@ public class ApexCRUDViolationRule extends AbstractApexRule { .append(":").append(type); if (typesFromSOQL.isEmpty()) { - validateCRUDCheckPresent(node, data, ANY, typeCheck.toString()); + violationAdded = validateCRUDCheckPresent(node, data, ANY, typeCheck.toString()); } else { for (String typeFromSOQL : typesFromSOQL) { - validateCRUDCheckPresent(node, data, ANY, typeFromSOQL); + violationAdded |= validateCRUDCheckPresent(node, data, ANY, typeFromSOQL); } } } + // If the node's already in violation, we don't need to keep checking. + if (violationAdded) { + return; + } + final ASTAssignmentExpression assignment = node.getFirstParentOfType(ASTAssignmentExpression.class); if (assignment != null) { final ASTVariableExpression variable = assignment.getFirstChildOfType(ASTVariableExpression.class); @@ -611,10 +633,10 @@ public class ApexCRUDViolationRule extends AbstractApexRule { if (varToTypeMapping.containsKey(variableWithClass)) { String type = varToTypeMapping.get(variableWithClass); if (typesFromSOQL.isEmpty()) { - validateCRUDCheckPresent(node, data, ANY, type); + violationAdded = validateCRUDCheckPresent(node, data, ANY, type); } else { for (String typeFromSOQL : typesFromSOQL) { - validateCRUDCheckPresent(node, data, ANY, typeFromSOQL); + violationAdded |= validateCRUDCheckPresent(node, data, ANY, typeFromSOQL); } } } @@ -622,17 +644,27 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } + // If the node's already in violation, we don't need to keep checking. + if (violationAdded) { + return; + } + final ASTReturnStatement returnStatement = node.getFirstParentOfType(ASTReturnStatement.class); if (returnStatement != null) { if (typesFromSOQL.isEmpty()) { - validateCRUDCheckPresent(node, data, ANY, returnType); + violationAdded = validateCRUDCheckPresent(node, data, ANY, returnType); } else { for (String typeFromSOQL : typesFromSOQL) { - validateCRUDCheckPresent(node, data, ANY, typeFromSOQL); + violationAdded |= validateCRUDCheckPresent(node, data, ANY, typeFromSOQL); } } } + // If the node's already in violation, we don't need to keep checking. + if (violationAdded) { + return; + } + final ASTForEachStatement forEachStatement = node.getFirstParentOfType(ASTForEachStatement.class); if (forEachStatement != null) { if (typesFromSOQL.isEmpty()) { diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTest.java index fa10da7d26..bfd713a34d 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTest.java @@ -43,6 +43,15 @@ public class ApexParserTest extends ApexParserTestBase { assertEquals(4, methods.size()); } + @Test + public void fileName() { + String code = "class Outer { class Inner {}}"; + + ASTUserClass rootNode = (ASTUserClass) parse(code, "src/filename.cls"); + + assertEquals("src/filename.cls", rootNode.getAstInfo().getFileName()); + } + private final String testCodeForLineNumbers = "public class SimpleClass {\n" // line 1 + " public void method1() {\n" // line 2 diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTestBase.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTestBase.java index 71c58ae95f..6ad7de0e66 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTestBase.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserTestBase.java @@ -13,6 +13,10 @@ public class ApexParserTestBase { return apex.parse(code).getMainNode(); } + protected ASTUserClassOrInterface parse(String code, String fileName) { + return apex.parse(code, null, fileName).getMainNode(); + } + protected ASTUserClassOrInterface parseResource(String code) { return apex.parseResource(code).getMainNode(); } diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserXPathTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserXPathTest.java deleted file mode 100644 index af43119509..0000000000 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexParserXPathTest.java +++ /dev/null @@ -1,31 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.apex.ast; - -import java.nio.charset.StandardCharsets; -import java.util.List; - -import org.apache.commons.io.IOUtils; -import org.junit.Assert; -import org.junit.Test; - -import net.sourceforge.pmd.lang.ast.Node; - -public class ApexParserXPathTest extends ApexParserTestBase { - - @Test - public void testBooleanExpressions() throws Exception { - ApexNode node = parse(IOUtils.toString(ApexParserXPathTest.class.getResourceAsStream("BooleanExpressions.cls"), - StandardCharsets.UTF_8)); - List booleanExpressions = node.descendants(ASTBooleanExpression.class).toList(); - Assert.assertEquals(2, booleanExpressions.size()); - Assert.assertEquals("&&", booleanExpressions.get(0).getOperator().toString()); - Assert.assertEquals("!=", booleanExpressions.get(1).getOperator().toString()); - - List xpathResult = node.findChildNodesWithXPath("//BooleanExpression[@Operator='&&']"); - Assert.assertEquals(1, xpathResult.size()); - Assert.assertSame(booleanExpressions.get(0), xpathResult.get(0)); - } -} diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeDumpTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeDumpTest.java index dc7c66b6f1..6ce49d352e 100644 --- a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeDumpTest.java +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/ast/ApexTreeDumpTest.java @@ -23,7 +23,7 @@ public class ApexTreeDumpTest extends BaseTreeDumpTest { } @Test - public void safeNavigationOperator() throws Exception { + public void safeNavigationOperator() { doTest("SafeNavigationOperator"); } } diff --git a/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXPathRuleTest.java b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXPathRuleTest.java new file mode 100644 index 0000000000..b90d4a26ce --- /dev/null +++ b/pmd-apex/src/test/java/net/sourceforge/pmd/lang/apex/rule/ApexXPathRuleTest.java @@ -0,0 +1,48 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.apex.rule; + +import static net.sourceforge.pmd.lang.ast.test.TestUtilsKt.assertSize; + +import org.junit.Test; + +import net.sourceforge.pmd.Report; +import net.sourceforge.pmd.lang.LanguageRegistry; +import net.sourceforge.pmd.lang.apex.ApexLanguageModule; +import net.sourceforge.pmd.lang.apex.ast.ApexParserTestBase; +import net.sourceforge.pmd.lang.rule.XPathRule; +import net.sourceforge.pmd.lang.rule.xpath.XPathVersion; + +/** + * @author daniels + */ +public class ApexXPathRuleTest extends ApexParserTestBase { + + private XPathRule makeXPath(String expression) { + XPathRule rule = new XPathRule(XPathVersion.XPATH_2_0, expression); + rule.setLanguage(LanguageRegistry.getLanguage(ApexLanguageModule.NAME)); + rule.setMessage("XPath Rule Failed"); + return rule; + } + + + @Test + public void testFileNameInXpath() { + Report report = apex.executeRule(makeXPath("/UserClass[pmd:fileName() = 'Foo.cls']"), + "class Foo {}", + "src/Foo.cls"); + + assertSize(report, 1); + } + + @Test + public void testBooleanExpressions() { + Report report = apex.executeRuleOnResource(makeXPath("//BooleanExpression[@Operator='&&']"), + "BooleanExpressions.cls"); + assertSize(report, 1); + } + + +} diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/BooleanExpressions.cls b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/BooleanExpressions.cls similarity index 100% rename from pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/ast/BooleanExpressions.cls rename to pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/BooleanExpressions.cls diff --git a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml index 6cd0a03809..697ec2b31b 100644 --- a/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml +++ b/pmd-apex/src/test/resources/net/sourceforge/pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml @@ -1049,6 +1049,38 @@ public class Foo { ]]> + + No CRUD check inside for-each loop + 1 + (); + for (Id accId : accIds) { + Account acc = [SELECT Id FROM Account WHERE Id = :accId]; + } + } +} + ]]> + + + + Proper CRUD check inside for-each loop + 0 + (); + if (Account.sObjectType.getDescribe().isAccessible()) { + for (Id accId : accIds) { + Account a = [SELECT Id FROM Account WHERE Id = :accId]; + } + } + } +} + ]]> + + Proper CRUD check in SOQL for-loop with security enforced 0 diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/NodeStream.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/NodeStream.java index d12c91f76c..75b2f90b3f 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/NodeStream.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/ast/NodeStream.java @@ -14,6 +14,7 @@ import java.util.function.BiFunction; import java.util.function.Consumer; import java.util.function.Function; import java.util.function.Predicate; +import java.util.function.ToIntFunction; import java.util.stream.Collector; import java.util.stream.Collectors; import java.util.stream.Stream; @@ -620,6 +621,23 @@ public interface NodeStream<@NonNull T extends Node> extends Iterable<@NonNull T return result; } + /** + * Sum the elements of this stream by associating them to an integer. + * + * @param toInt Map an element to an integer, which will be added + * to the running sum + * returns the next intermediate result + * + * @return The sum, zero if the stream is empty. + */ + default int sumBy(ToIntFunction toInt) { + int result = 0; + for (T node : this) { + result += toInt.applyAsInt(node); + } + return result; + } + /** * Returns the number of nodes in this stream. diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AbstractXPathFunctionDef.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AbstractXPathFunctionDef.java index 47a2d86815..7a466ea25c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AbstractXPathFunctionDef.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/AbstractXPathFunctionDef.java @@ -19,12 +19,18 @@ public abstract class AbstractXPathFunctionDef extends ExtensionFunctionDefiniti private static final String PMD_URI_PREFIX = "http://pmd.sourceforge.net/"; private final StructuredQName qname; - protected AbstractXPathFunctionDef(String localName, String languageTerseName) { - String namespacePrefix = "pmd-" + languageTerseName; - String uri = PMD_URI_PREFIX + namespacePrefix; + private AbstractXPathFunctionDef(String localName, String namespacePrefix, String uri) { this.qname = new StructuredQName(namespacePrefix, uri, localName); } + protected AbstractXPathFunctionDef(String localName) { + this(localName, "pmd", PMD_URI_PREFIX + "pmd-core"); + } + + protected AbstractXPathFunctionDef(String localName, String languageTerseName) { + this(localName, "pmd-" + languageTerseName, PMD_URI_PREFIX + "pmd-" + languageTerseName); + } + @Override public final StructuredQName getFunctionQName() { return qname; diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/XPathHandler.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/XPathHandler.java index 3e048f3adb..7ca9d5893c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/XPathHandler.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/impl/XPathHandler.java @@ -5,8 +5,10 @@ package net.sourceforge.pmd.lang.rule.xpath.impl; import java.util.Collections; +import java.util.HashSet; import java.util.Set; +import net.sourceforge.pmd.lang.rule.xpath.internal.DefaultXPathFunctions; import net.sourceforge.pmd.util.CollectionUtil; import net.sf.saxon.lib.ExtensionFunctionDefinition; @@ -26,15 +28,16 @@ public interface XPathHandler { static XPathHandler noFunctionDefinitions() { - return Collections::emptySet; + return () -> DefaultXPathFunctions.getDefaultFunctions(); } - /** * Returns a default XPath handler. */ static XPathHandler getHandlerForFunctionDefs(ExtensionFunctionDefinition first, ExtensionFunctionDefinition... defs) { - Set set = CollectionUtil.setOf(first, defs); - return () -> set; + Set set = new HashSet<>(CollectionUtil.setOf(first, defs)); + set.addAll(DefaultXPathFunctions.getDefaultFunctions()); + + return () -> Collections.unmodifiableSet(set); } } diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/DefaultXPathFunctions.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/DefaultXPathFunctions.java new file mode 100644 index 0000000000..bf0a074f70 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/DefaultXPathFunctions.java @@ -0,0 +1,26 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + + +package net.sourceforge.pmd.lang.rule.xpath.internal; + +import java.util.Set; + +import net.sourceforge.pmd.util.CollectionUtil; + +import net.sf.saxon.lib.ExtensionFunctionDefinition; + +/** + * Default XPath functions provided by pmd-core. + */ +public final class DefaultXPathFunctions { + + private DefaultXPathFunctions() { + // utility class + } + + public static Set getDefaultFunctions() { + return CollectionUtil.setOf(FileNameXPathFunction.INSTANCE); + } +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/FileNameXPathFunction.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/FileNameXPathFunction.java new file mode 100644 index 0000000000..7605b11856 --- /dev/null +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/FileNameXPathFunction.java @@ -0,0 +1,62 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.rule.xpath.internal; + +import java.nio.file.Paths; +import java.util.Objects; + +import net.sourceforge.pmd.lang.ast.Node; +import net.sourceforge.pmd.lang.ast.RootNode; +import net.sourceforge.pmd.lang.rule.xpath.impl.AbstractXPathFunctionDef; + +import net.sf.saxon.expr.XPathContext; +import net.sf.saxon.lib.ExtensionFunctionCall; +import net.sf.saxon.om.Sequence; +import net.sf.saxon.trans.XPathException; +import net.sf.saxon.value.SequenceType; +import net.sf.saxon.value.StringValue; + +/** + * A function that returns the current file name. + * + * @author Clément Fournier + */ +public final class FileNameXPathFunction extends AbstractXPathFunctionDef { + + public static final FileNameXPathFunction INSTANCE = new FileNameXPathFunction(); + + private FileNameXPathFunction() { + super("fileName"); + } + + @Override + public SequenceType[] getArgumentTypes() { + return new SequenceType[0]; + } + + @Override + public SequenceType getResultType(SequenceType[] suppliedArgumentTypes) { + return SequenceType.STRING_SEQUENCE; + } + + @Override + public ExtensionFunctionCall makeCallExpression() { + return new ExtensionFunctionCall() { + + @Override + public Sequence call(XPathContext context, Sequence[] arguments) throws XPathException { + Node node = ((AstElementNode) context.getContextItem()).getUnderlyingNode(); + RootNode root = node.getRoot(); + Objects.requireNonNull(root, "No root node in tree?"); + + String fileName = root.getAstInfo().getFileName(); + Objects.requireNonNull(fileName, "File name was not set"); + String simpleFilename = Paths.get(fileName).getFileName().toString(); + + return new StringValue(simpleFilename); + } + }; + } +} diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java index fe04a55145..afaf60ff6b 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/rule/xpath/internal/RuleChainAnalyzer.java @@ -6,8 +6,10 @@ package net.sourceforge.pmd.lang.rule.xpath.internal; import static net.sourceforge.pmd.util.CollectionUtil.listOf; +import java.util.ArrayDeque; import java.util.Collections; import java.util.Comparator; +import java.util.Deque; import java.util.List; import net.sourceforge.pmd.lang.ast.Node; @@ -22,6 +24,7 @@ import net.sf.saxon.expr.SlashExpression; import net.sf.saxon.expr.VennExpression; import net.sf.saxon.expr.sort.DocumentSorter; import net.sf.saxon.om.AxisInfo; +import net.sf.saxon.pattern.CombinedNodeTest; import net.sf.saxon.pattern.NameTest; import net.sf.saxon.type.Type; @@ -45,6 +48,7 @@ public class RuleChainAnalyzer extends SaxonExprVisitor { private boolean rootElementReplaced; private boolean insideExpensiveExpr; private boolean foundPathInsideExpensive; + private boolean foundCombinedNodeTest; public RuleChainAnalyzer(Configuration currentConfiguration) { this.configuration = currentConfiguration; @@ -92,7 +96,17 @@ public class RuleChainAnalyzer extends SaxonExprVisitor { Expression step = newPath.getStep(); if (step instanceof FilterExpression) { FilterExpression filterExpression = (FilterExpression) step; - result = new FilterExpression(new AxisExpression(AxisInfo.SELF, null), filterExpression.getFilter()); + + Deque filters = new ArrayDeque<>(); + Expression walker = filterExpression; + while (walker instanceof FilterExpression) { + filters.push(((FilterExpression) walker).getFilter()); + walker = ((FilterExpression) walker).getBase(); + } + result = new FilterExpression(new AxisExpression(AxisInfo.SELF, null), filters.pop()); + while (!filters.isEmpty()) { + result = new FilterExpression(result, filters.pop()); + } rootElementReplaced = true; } else if (step instanceof AxisExpression) { Expression start = newPath.getStart(); @@ -124,13 +138,15 @@ public class RuleChainAnalyzer extends SaxonExprVisitor { @Override public Expression visit(AxisExpression e) { - if (rootElement == null && e.getNodeTest() instanceof NameTest) { + if (rootElement == null && e.getNodeTest() instanceof NameTest && !foundCombinedNodeTest) { NameTest test = (NameTest) e.getNodeTest(); if (test.getPrimitiveType() == Type.ELEMENT && e.getAxis() == AxisInfo.DESCENDANT) { rootElement = listOf(configuration.getNamePool().getClarkName(test.getFingerprint())); } else if (test.getPrimitiveType() == Type.ELEMENT && e.getAxis() == AxisInfo.CHILD) { rootElement = listOf(configuration.getNamePool().getClarkName(test.getFingerprint())); } + } else if (e.getNodeTest() instanceof CombinedNodeTest) { + foundCombinedNodeTest = true; } return super.visit(e); } diff --git a/pmd-core/src/main/resources/rulesets/releases/34.xml b/pmd-core/src/main/resources/rulesets/releases/34.xml index a3328515ae..6d2924711e 100644 --- a/pmd-core/src/main/resources/rulesets/releases/34.xml +++ b/pmd-core/src/main/resources/rulesets/releases/34.xml @@ -15,7 +15,7 @@ This ruleset contains links to rules that are new in PMD v3.4 - + diff --git a/pmd-core/src/main/resources/rulesets/releases/35.xml b/pmd-core/src/main/resources/rulesets/releases/35.xml index 7753b454c6..debffe92c2 100644 --- a/pmd-core/src/main/resources/rulesets/releases/35.xml +++ b/pmd-core/src/main/resources/rulesets/releases/35.xml @@ -16,9 +16,9 @@ This ruleset contains links to rules that are new in PMD v3.5 - + - + diff --git a/pmd-core/src/main/resources/rulesets/releases/36.xml b/pmd-core/src/main/resources/rulesets/releases/36.xml index 16a32f41f4..8a5cdf594f 100644 --- a/pmd-core/src/main/resources/rulesets/releases/36.xml +++ b/pmd-core/src/main/resources/rulesets/releases/36.xml @@ -11,7 +11,7 @@ This ruleset contains links to rules that are new in PMD v3.6 - + diff --git a/pmd-core/src/main/resources/rulesets/releases/38.xml b/pmd-core/src/main/resources/rulesets/releases/38.xml index b0528c8caa..9dacc12123 100644 --- a/pmd-core/src/main/resources/rulesets/releases/38.xml +++ b/pmd-core/src/main/resources/rulesets/releases/38.xml @@ -10,7 +10,7 @@ This ruleset contains links to rules that are new in PMD v3.8 - + diff --git a/pmd-core/src/main/resources/rulesets/releases/40rc1.xml b/pmd-core/src/main/resources/rulesets/releases/40rc1.xml index 132f866adf..0ef4736820 100644 --- a/pmd-core/src/main/resources/rulesets/releases/40rc1.xml +++ b/pmd-core/src/main/resources/rulesets/releases/40rc1.xml @@ -8,9 +8,9 @@ This ruleset contains links to rules that are new in PMD v4.0rc1 - - - + + + @@ -26,4 +26,3 @@ This ruleset contains links to rules that are new in PMD v4.0rc1 - \ No newline at end of file diff --git a/pmd-core/src/main/resources/rulesets/releases/41.xml b/pmd-core/src/main/resources/rulesets/releases/41.xml index 031efb5aee..cbd7252332 100644 --- a/pmd-core/src/main/resources/rulesets/releases/41.xml +++ b/pmd-core/src/main/resources/rulesets/releases/41.xml @@ -13,7 +13,7 @@ This ruleset contains links to rules that are new in PMD v4.1 - + diff --git a/pmd-core/src/main/resources/rulesets/releases/42.xml b/pmd-core/src/main/resources/rulesets/releases/42.xml index 01fe98bc02..aa5f42de36 100644 --- a/pmd-core/src/main/resources/rulesets/releases/42.xml +++ b/pmd-core/src/main/resources/rulesets/releases/42.xml @@ -11,7 +11,7 @@ This ruleset contains links to rules that are new in PMD v4.2 - + diff --git a/pmd-core/src/main/resources/rulesets/releases/50.xml b/pmd-core/src/main/resources/rulesets/releases/50.xml index d612ceac00..7406926431 100644 --- a/pmd-core/src/main/resources/rulesets/releases/50.xml +++ b/pmd-core/src/main/resources/rulesets/releases/50.xml @@ -38,7 +38,7 @@ This ruleset contains links to rules that are new in PMD v5.0 - + diff --git a/pmd-core/src/main/resources/rulesets/releases/550.xml b/pmd-core/src/main/resources/rulesets/releases/550.xml index 5fb17c056b..82a29edb22 100644 --- a/pmd-core/src/main/resources/rulesets/releases/550.xml +++ b/pmd-core/src/main/resources/rulesets/releases/550.xml @@ -9,7 +9,7 @@ This ruleset contains links to rules that are new in PMD v5.5.0 - + diff --git a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonXPathRuleQueryTest.java b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonXPathRuleQueryTest.java index 454ccc12a5..eab4f0f9f6 100644 --- a/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonXPathRuleQueryTest.java +++ b/pmd-core/src/test/java/net/sourceforge/pmd/lang/rule/xpath/internal/SaxonXPathRuleQueryTest.java @@ -322,6 +322,38 @@ public class SaxonXPathRuleQueryTest { assertExpression("docOrder((((/)/descendant::element(Q{}dummyNode))[matches(convertUntyped(data(@SimpleName)), \"a\", \"\")])/child::element(Q{}foo))", query.getFallbackExpr()); } + + @Test + public void ruleChainWithUnionsCustomFunctionsVariant1() { + SaxonXPathRuleQuery query = createQuery("(//ForStatement | //WhileStatement | //DoStatement)//dummyNode[@Image != '']"); + List ruleChainVisits = query.getRuleChainVisits(); + Assert.assertEquals(0, ruleChainVisits.size()); + } + + @Test + public void ruleChainWithUnionsCustomFunctionsVariant2() { + SaxonXPathRuleQuery query = createQuery("//(ForStatement | WhileStatement | DoStatement)//dummyNode[@Image != '']"); + List ruleChainVisits = query.getRuleChainVisits(); + Assert.assertEquals(0, ruleChainVisits.size()); + } + + @Test + public void ruleChainWithUnionsCustomFunctionsVariant3() { + SaxonXPathRuleQuery query = createQuery("//ForStatement//dummyNode[@Image != '']" + + " | //WhileStatement//dummyNode[@Image != '']" + + " | //DoStatement//dummyNode[@Image != '']"); + List ruleChainVisits = query.getRuleChainVisits(); + Assert.assertEquals(3, ruleChainVisits.size()); + Assert.assertTrue(ruleChainVisits.contains("ForStatement")); + Assert.assertTrue(ruleChainVisits.contains("WhileStatement")); + Assert.assertTrue(ruleChainVisits.contains("DoStatement")); + + final String expectedSubexpression = "(self::node()/descendant::element(dummyNode))[(string(data(@Image))) ne \"\"]"; + assertExpression(expectedSubexpression, query.nodeNameToXPaths.get("ForStatement").get(0)); + assertExpression(expectedSubexpression, query.nodeNameToXPaths.get("WhileStatement").get(0)); + assertExpression(expectedSubexpression, query.nodeNameToXPaths.get("DoStatement").get(0)); + } + private static void assertExpression(String expected, Expression actual) { assertEquals(normalizeExprDump(expected), normalizeExprDump(actual.toString())); diff --git a/pmd-dist/pom.xml b/pmd-dist/pom.xml index 1663f9d031..7ba243789b 100644 --- a/pmd-dist/pom.xml +++ b/pmd-dist/pom.xml @@ -252,7 +252,7 @@ org.apache.commons commons-compress - 1.19 + 1.21 test diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnonymousClassDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnonymousClassDeclaration.java index 1969d8e16c..de7b83edb4 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnonymousClassDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnonymousClassDeclaration.java @@ -4,13 +4,9 @@ package net.sourceforge.pmd.lang.java.ast; -import static net.sourceforge.pmd.util.CollectionUtil.listOf; - -import java.util.Collections; -import java.util.List; - import org.checkerframework.checker.nullness.qual.NonNull; +import net.sourceforge.pmd.lang.ast.NodeStream; import net.sourceforge.pmd.lang.java.types.JTypeMirror; /** @@ -44,15 +40,15 @@ public final class ASTAnonymousClassDeclaration extends AbstractAnyTypeDeclarati } @Override - public @NonNull List getSuperInterfaceTypeNodes() { + public @NonNull NodeStream getSuperInterfaceTypeNodes() { if (getParent() instanceof ASTConstructorCall) { ASTConstructorCall ctor = (ASTConstructorCall) getParent(); @NonNull JTypeMirror type = ctor.getTypeMirror(); if (type.isInterface()) { - return listOf(ctor.getTypeNode()); + return NodeStream.of(ctor.getTypeNode()); } } - return Collections.emptyList(); + return NodeStream.empty(); } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java index 799249e581..5a51c7b109 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/ast/ASTAnyTypeDeclaration.java @@ -6,8 +6,6 @@ package net.sourceforge.pmd.lang.java.ast; import static net.sourceforge.pmd.lang.java.ast.JModifier.ABSTRACT; -import java.util.List; - import org.checkerframework.checker.nullness.qual.NonNull; import org.checkerframework.checker.nullness.qual.Nullable; @@ -279,9 +277,8 @@ public interface ASTAnyTypeDeclaration * Returns the list of interfaces implemented by this class, or * extended by this interface. Returns null if no such list is declared. */ - @NonNull - default List getSuperInterfaceTypeNodes() { - return ASTList.orEmpty(isInterface() ? getFirstChildOfType(ASTExtendsList.class) - : getFirstChildOfType(ASTImplementsList.class)); + default @NonNull NodeStream getSuperInterfaceTypeNodes() { + return ASTList.orEmptyStream(isInterface() ? firstChild(ASTExtendsList.class) + : firstChild(ASTImplementsList.class)); } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryBoxingRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryBoxingRule.java index 3ac1554cac..33f56b41b8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryBoxingRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/codestyle/UnnecessaryBoxingRule.java @@ -8,6 +8,8 @@ import static net.sourceforge.pmd.util.CollectionUtil.setOf; import java.util.Set; +import org.apache.commons.lang3.StringUtils; + import net.sourceforge.pmd.RuleContext; import net.sourceforge.pmd.lang.java.ast.ASTConstructorCall; import net.sourceforge.pmd.lang.java.ast.ASTExpression; @@ -19,6 +21,7 @@ import net.sourceforge.pmd.lang.java.types.JMethodSig; import net.sourceforge.pmd.lang.java.types.JTypeMirror; import net.sourceforge.pmd.lang.java.types.OverloadSelectionResult; import net.sourceforge.pmd.lang.java.types.TypePrettyPrint; +import net.sourceforge.pmd.lang.java.types.TypeTestUtil; import net.sourceforge.pmd.lang.java.types.ast.ExprContext; /** @@ -71,6 +74,8 @@ public class UnnecessaryBoxingRule extends AbstractJavaRulechainRule { if (isValueOf && isWrapperValueOf(m)) { checkBox((RuleContext) data, "boxing", node, node.getArguments().get(0), m.getFormalParameters().get(0)); + } else if (isValueOf && isStringValueOf(m) && qualifier != null) { + checkUnboxing((RuleContext) data, node, qualifier.getTypeMirror()); } else if (!isValueOf && isUnboxingCall(m) && qualifier != null) { checkBox((RuleContext) data, "unboxing", node, qualifier, qualifier.getTypeMirror()); } @@ -89,6 +94,13 @@ public class UnnecessaryBoxingRule extends AbstractJavaRulechainRule { && m.getFormalParameters().get(0).isPrimitive(); } + private boolean isStringValueOf(JMethodSig m) { + return m.isStatic() + && (m.getArity() == 1 || m.getArity() == 2) + && m.getDeclaringType().isBoxedPrimitive() + && TypeTestUtil.isA(String.class, m.getFormalParameters().get(0)); + } + private void checkBox( RuleContext rctx, String opKind, @@ -150,6 +162,28 @@ public class UnnecessaryBoxingRule extends AbstractJavaRulechainRule { } } + private void checkUnboxing( + RuleContext rctx, + ASTMethodCall methodCall, + JTypeMirror conversionOutput + ) { + // methodCall is e.g. Integer.valueOf("42") + // this checks, whether the resulting type "Integer" is e.g. assigned to an "int" + // which triggers implicit unboxing. + ExprContext ctx = methodCall.getConversionContext(); + JTypeMirror ctxType = ctx.getTargetType(); + + if (ctxType != null) { + if (isImplicitlyConvertible(conversionOutput, ctxType)) { + if (conversionOutput.unbox().equals(ctxType)) { + addViolation(rctx, methodCall, "implicit unboxing. Use " + + conversionOutput.getSymbol().getSimpleName() + ".parse" + + StringUtils.capitalize(ctxType.getSymbol().getSimpleName()) + "(...) instead"); + } + } + } + } + private boolean isImplicitlyConvertible(JTypeMirror i, JTypeMirror o) { return i.box().isSubtypeOf(o.box()) || i.unbox().isSubtypeOf(o.unbox()); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ClassWithOnlyPrivateConstructorsShouldBeFinalRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ClassWithOnlyPrivateConstructorsShouldBeFinalRule.java new file mode 100644 index 0000000000..85720dedc4 --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ClassWithOnlyPrivateConstructorsShouldBeFinalRule.java @@ -0,0 +1,49 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.design; + +import static net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility.V_PRIVATE; + +import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.types.TypeTestUtil; + +public class ClassWithOnlyPrivateConstructorsShouldBeFinalRule extends AbstractJavaRulechainRule { + + public ClassWithOnlyPrivateConstructorsShouldBeFinalRule() { + super(ASTClassOrInterfaceDeclaration.class); + } + + @Override + public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { + if (node.isRegularClass() + && !node.isFinal() + && hasOnlyPrivateCtors(node) + && hasNoSubclasses(node)) { + addViolation(data, node); + } + return null; + } + + private boolean hasNoSubclasses(ASTClassOrInterfaceDeclaration klass) { + return klass.getRoot() + .descendants(ASTAnyTypeDeclaration.class) + .crossFindBoundaries() + .none(it -> doesExtend(it, klass)); + } + + private boolean doesExtend(ASTAnyTypeDeclaration sub, ASTClassOrInterfaceDeclaration superClass) { + return sub != superClass && TypeTestUtil.isA(superClass.getTypeMirror(), sub); + } + + private boolean hasOnlyPrivateCtors(ASTClassOrInterfaceDeclaration node) { + return node.getDeclarations(ASTConstructorDeclaration.class).all(it -> it.getVisibility() == V_PRIVATE) + && (node.getVisibility() == V_PRIVATE // then the default ctor is private + || node.getDeclarations(ASTConstructorDeclaration.class).nonEmpty()); + } + +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyConditionalRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyConditionalRule.java new file mode 100644 index 0000000000..a5312867cf --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyConditionalRule.java @@ -0,0 +1,63 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.design; + +import static net.sourceforge.pmd.lang.java.ast.BinaryOp.CONDITIONAL_AND; +import static net.sourceforge.pmd.lang.java.ast.BinaryOp.CONDITIONAL_OR; +import static net.sourceforge.pmd.lang.java.ast.BinaryOp.INSTANCEOF; +import static net.sourceforge.pmd.lang.java.ast.BinaryOp.NE; +import static net.sourceforge.pmd.lang.java.ast.BinaryOp.isInfixExprWithOperator; +import static net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil.getOtherOperandIfInInfixExpr; +import static net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil.isBooleanNegation; +import static net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil.isNullCheck; + +import net.sourceforge.pmd.lang.java.ast.ASTExpression; +import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.rule.internal.StablePathMatcher; + + +public class SimplifyConditionalRule extends AbstractJavaRulechainRule { + + public SimplifyConditionalRule() { + super(ASTInfixExpression.class); + } + + + @Override + public Object visit(ASTInfixExpression node, Object data) { + if (node.getOperator() == INSTANCEOF) { + + StablePathMatcher instanceOfSubject = StablePathMatcher.matching(node.getLeftOperand()); + if (instanceOfSubject == null) { + return null; + } + + ASTExpression nullCheckExpr; + boolean negated; + if (isInfixExprWithOperator(node.getParent(), CONDITIONAL_AND)) { + // a != null && a instanceof T + negated = false; + nullCheckExpr = getOtherOperandIfInInfixExpr(node); + } else if (isBooleanNegation(node.getParent()) + && isInfixExprWithOperator(node.getParent().getParent(), CONDITIONAL_OR)) { + // a == null || a instanceof T + negated = true; + nullCheckExpr = getOtherOperandIfInInfixExpr(node.getParent()); + } else { + return null; + } + + if (!isNullCheck(nullCheckExpr, instanceOfSubject)) { + return null; + } + + if (negated != isInfixExprWithOperator(nullCheckExpr, NE)) { + addViolation(data, nullCheckExpr); + } + } + return null; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java index 97c3db3541..21f8d2bcb1 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityRule.java @@ -6,11 +6,11 @@ package net.sourceforge.pmd.lang.java.rule.design; import static net.sourceforge.pmd.properties.constraints.NumericConstraints.positive; -import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTStatement; -import net.sourceforge.pmd.lang.java.ast.ASTSwitchLabel; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchBranch; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchExpression; +import net.sourceforge.pmd.lang.java.ast.ASTSwitchLike; import net.sourceforge.pmd.lang.java.ast.ASTSwitchStatement; -import net.sourceforge.pmd.lang.java.ast.JavaParserVisitorAdapter; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; import net.sourceforge.pmd.properties.PropertyDescriptor; import net.sourceforge.pmd.properties.PropertyFactory; @@ -24,6 +24,7 @@ import net.sourceforge.pmd.properties.PropertyFactory; * looking at Subclasses or State Pattern to alleviate the problem.

* * @author David Dixon-Peugh + * @author Clément Fournier */ public class SwitchDensityRule extends AbstractJavaRulechainRule { @@ -36,51 +37,32 @@ public class SwitchDensityRule extends AbstractJavaRulechainRule { .build(); public SwitchDensityRule() { - super(ASTSwitchStatement.class); + super(ASTSwitchStatement.class, ASTSwitchExpression.class); definePropertyDescriptor(REPORT_LEVEL); } @Override public Object visit(ASTSwitchStatement node, Object data) { - double density = new SwitchDensityVisitor().compute(node); + return visitSwitchLike(node, data); + } + + @Override + public Object visit(ASTSwitchExpression node, Object data) { + return visitSwitchLike(node, data); + } + + public Void visitSwitchLike(ASTSwitchLike node, Object data) { + // note: this does not cross find boundaries. + int stmtCount = node.descendants(ASTStatement.class).count(); + int labelCount = node.getBranches() + .map(ASTSwitchBranch::getLabel) + .sumBy(label -> label.isDefault() ? 1 : label.getExprList().count()); + + // note: if labelCount is zero, double division will produce +Infinity or NaN, not ArithmeticException + double density = stmtCount / (double) labelCount; if (density >= getProperty(REPORT_LEVEL)) { addViolation(data, node); } - return super.visit(node, data); - } - - private static class SwitchDensityVisitor extends JavaParserVisitorAdapter { - - private int labels = 0; - private int stmts = 0; - private ASTSwitchStatement root; - - - double compute(ASTSwitchStatement root) { - this.root = root; - root.jjtAccept(this, null); - return labels == 0 ? 0 : ((double) stmts) / labels; - } - - - @Override - public Object visitStatement(ASTStatement statement, Object data) { - stmts++; - return super.visitStatement(statement, data); - } - - @Override - public Object visit(ASTExpression node, Object data) { - // don't recurse on anonymous class, etc - return data; - } - - @Override - public Object visit(ASTSwitchLabel switchLabel, Object data) { - if (switchLabel.getParent() == root) { - labels++; - } - return super.visit(switchLabel, data); - } + return null; } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java index 672c6b56b8..abeeb9d7b7 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassRule.java @@ -6,98 +6,85 @@ package net.sourceforge.pmd.lang.java.rule.design; import static net.sourceforge.pmd.util.CollectionUtil.setOf; -import java.util.Collection; +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.ASTClassOrInterfaceBody; +import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr; +import net.sourceforge.pmd.lang.java.ast.ASTBodyDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTMemberValuePair; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTResultType; -import net.sourceforge.pmd.lang.java.rule.AbstractLombokAwareRule; +import net.sourceforge.pmd.lang.java.ast.AccessNode.Visibility; +import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; +import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil; import net.sourceforge.pmd.lang.java.types.TypeTestUtil; -public class UseUtilityClassRule extends AbstractLombokAwareRule { +public class UseUtilityClassRule extends AbstractJavaRulechainRule { - @Override - protected Collection defaultSuppressionAnnotations() { - return setOf("lombok.experimental.UtilityClass"); + private static final Set IGNORED_CLASS_ANNOT = setOf( + "lombok.experimental.UtilityClass", + "org.junit.runner.RunWith" // for suites and such + ); + + public UseUtilityClassRule() { + super(ASTClassOrInterfaceDeclaration.class); } @Override - public Object visit(ASTClassOrInterfaceDeclaration node, Object data) { - if (hasIgnoredAnnotation(node)) { + public Object visit(ASTClassOrInterfaceDeclaration klass, Object data) { + if (JavaRuleUtil.hasAnyAnnotation(klass, IGNORED_CLASS_ANNOT) + || TypeTestUtil.isA("junit.framework.TestSuite", klass) // suite method is ok + || klass.isInterface() + || klass.isAbstract() + || klass.getSuperClassTypeNode() != null + || klass.getSuperInterfaceTypeNodes().nonEmpty() + ) { return data; } - return super.visit(node, data); - } - @Override - public Object visit(ASTClassOrInterfaceBody decl, Object data) { - Object result = super.visit(decl, data); - - if (decl.getParent() instanceof ASTClassOrInterfaceDeclaration) { - ASTClassOrInterfaceDeclaration parent = (ASTClassOrInterfaceDeclaration) decl.getParent(); - if (parent.isAbstract() || parent.isInterface() || parent.getSuperClassTypeNode() != null) { - return result; + boolean hasAnyMethods = false; + boolean hasNonPrivateCtor = false; + boolean hasAnyCtor = false; + for (ASTBodyDeclaration declaration : klass.getDeclarations()) { + if (declaration instanceof ASTFieldDeclaration + && !((ASTFieldDeclaration) declaration).isStatic()) { + return null; } - - if (hasLombokNoArgsConstructor(parent)) { - return result; - } - - int i = decl.getNumChildren(); - int methodCount = 0; - boolean isOK = false; - while (i > 0) { - Node p = decl.getChild(--i); - if (p.getNumChildren() == 0) { - continue; - } - Node n = skipAnnotations(p); - if (n instanceof ASTFieldDeclaration) { - if (!((ASTFieldDeclaration) n).isStatic()) { - isOK = true; - break; - } - } else if (n instanceof ASTConstructorDeclaration) { - if (((ASTConstructorDeclaration) n).isPrivate()) { - isOK = true; - break; - } - } else if (n instanceof ASTMethodDeclaration) { - ASTMethodDeclaration m = (ASTMethodDeclaration) n; - if (!m.isPrivate()) { - methodCount++; - } - if (!m.isStatic()) { - isOK = true; - break; - } - - // TODO use symbol table - if ("suite".equals(m.getName())) { - ASTResultType res = m.getResultType(); - ASTClassOrInterfaceType c = res.getFirstDescendantOfType(ASTClassOrInterfaceType.class); - if (c != null && c.hasImageEqualTo("Test")) { - isOK = true; - break; - } - } + if (declaration instanceof ASTConstructorDeclaration) { + hasAnyCtor = true; + if (((ASTConstructorDeclaration) declaration).getVisibility() != Visibility.V_PRIVATE) { + hasNonPrivateCtor = true; } } - if (!isOK && methodCount > 0) { - addViolation(data, decl); + + if (declaration instanceof ASTMethodDeclaration) { + if (((ASTMethodDeclaration) declaration).getVisibility() != Visibility.V_PRIVATE) { + hasAnyMethods = true; + } + if (!((ASTMethodDeclaration) declaration).isStatic()) { + return null; + } } } - return result; + + // account for default ctor + hasNonPrivateCtor |= !hasAnyCtor + && klass.getVisibility() != Visibility.V_PRIVATE + && !hasLombokPrivateCtor(klass); + + + String message; + if (hasAnyMethods && hasNonPrivateCtor) { + message = "This utility class has a non-private constructor"; + addViolationWithMessage(data, klass, message); + } + return null; } - private boolean hasLombokNoArgsConstructor(ASTClassOrInterfaceDeclaration parent) { + private boolean hasLombokPrivateCtor(ASTClassOrInterfaceDeclaration parent) { // check if there's a lombok no arg private constructor, if so skip the rest of the rules return parent.getDeclaredAnnotations() @@ -105,18 +92,16 @@ public class UseUtilityClassRule extends AbstractLombokAwareRule { .flatMap(ASTAnnotation::getMembers) // to set the access level of a constructor in lombok, you set the access property on the annotation .filterMatching(ASTMemberValuePair::getName, "access") - .map(ASTMemberValuePair::getValue) // This is from the AccessLevel enum in Lombok // if the constructor is found and the accesslevel is private no need to check anything else - .any(it -> "PRIVATE".equals(it.getImage())); + .any(it -> isAccessToVarWithName(it.getValue(), "PRIVATE")); } - private Node skipAnnotations(Node p) { - int index = 0; - Node n = p.getChild(index++); - while (n instanceof ASTAnnotation && index < p.getNumChildren()) { - n = p.getChild(index++); + private static boolean isAccessToVarWithName(JavaNode node, String name) { + if (node instanceof ASTNamedReferenceExpr) { + return ((ASTNamedReferenceExpr) node).getName().equals(name); } - return n; + return false; } + } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java index 5cfd743803..72d81182ae 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/JavaRuleUtil.java @@ -659,7 +659,7 @@ public final class JavaRuleUtil { return e instanceof ASTBooleanLiteral; } - public static boolean isBooleanNegation(ASTExpression e) { + public static boolean isBooleanNegation(JavaNode e) { return e instanceof ASTUnaryExpression && ((ASTUnaryExpression) e).getOperator() == UnaryOp.NEGATION; } @@ -963,4 +963,24 @@ public final class JavaRuleUtil { public static boolean hasLombokAnnotation(Annotatable node) { return LOMBOK_ANNOTATIONS.stream().anyMatch(node::isAnnotationPresent); } + + /** + * Returns true if the expression is a null check on the given variable. + */ + public static boolean isNullCheck(ASTExpression expr, JVariableSymbol var) { + return isNullCheck(expr, StablePathMatcher.matching(var)); + } + + public static boolean isNullCheck(ASTExpression expr, StablePathMatcher matcher) { + if (expr instanceof ASTInfixExpression) { + ASTInfixExpression condition = (ASTInfixExpression) expr; + if (condition.getOperator().hasSamePrecedenceAs(BinaryOp.EQ)) { + ASTNullLiteral nullLit = condition.firstChild(ASTNullLiteral.class); + if (nullLit != null) { + return matcher.matches(getOtherOperandIfInInfixExpr(nullLit)); + } + } + } + return false; + } } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/StablePathMatcher.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/StablePathMatcher.java index f3c43de9f7..c6df092758 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/StablePathMatcher.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/internal/StablePathMatcher.java @@ -4,7 +4,9 @@ package net.sourceforge.pmd.lang.java.rule.internal; -import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collections; +import java.util.List; import java.util.Objects; import org.checkerframework.checker.nullness.qual.Nullable; @@ -31,9 +33,9 @@ public final class StablePathMatcher { // if owner == null, then the owner is `this`. private final @Nullable JVariableSymbol owner; - private final ArrayDeque path; + private final List path; - private StablePathMatcher(@Nullable JVariableSymbol owner, ArrayDeque path) { + private StablePathMatcher(@Nullable JVariableSymbol owner, List path) { this.owner = owner; this.path = path; } @@ -88,18 +90,21 @@ public final class StablePathMatcher { * Otherwise returns null. */ public static @Nullable StablePathMatcher matching(ASTExpression e) { + if (e == null) { + return null; + } JVariableSymbol owner = null; - ArrayDeque segments = new ArrayDeque<>(); + List segments = new ArrayList<>(); while (e != null) { if (e instanceof ASTFieldAccess) { ASTFieldAccess access = (ASTFieldAccess) e; - segments.addLast(new Segment(access.getName(), true)); + segments.add(new Segment(access.getName(), true)); e = access.getQualifier(); } else if (e instanceof ASTMethodCall) { ASTMethodCall call = (ASTMethodCall) e; if (JavaRuleUtil.isGetterCall(call)) { - segments.addLast(new Segment(call.getMethodName(), false)); + segments.add(new Segment(call.getMethodName(), false)); e = call.getQualifier(); } else { return null; @@ -128,6 +133,10 @@ public final class StablePathMatcher { return new StablePathMatcher(owner, segments); } + public static StablePathMatcher matching(JVariableSymbol e) { + return new StablePathMatcher(e, Collections.emptyList()); + } + private static final class Segment { final String name; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/multithreading/DoubleCheckedLockingRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/multithreading/DoubleCheckedLockingRule.java index 04427bf008..a944309044 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/multithreading/DoubleCheckedLockingRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/multithreading/DoubleCheckedLockingRule.java @@ -14,15 +14,13 @@ import net.sourceforge.pmd.lang.java.ast.ASTAssignableExpr.ASTNamedReferenceExpr import net.sourceforge.pmd.lang.java.ast.ASTAssignmentExpression; import net.sourceforge.pmd.lang.java.ast.ASTExpression; import net.sourceforge.pmd.lang.java.ast.ASTIfStatement; -import net.sourceforge.pmd.lang.java.ast.ASTInfixExpression; import net.sourceforge.pmd.lang.java.ast.ASTMethodDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTNullLiteral; import net.sourceforge.pmd.lang.java.ast.ASTPrimitiveType; import net.sourceforge.pmd.lang.java.ast.ASTReturnStatement; import net.sourceforge.pmd.lang.java.ast.ASTSynchronizedStatement; import net.sourceforge.pmd.lang.java.ast.ASTVariableDeclaratorId; -import net.sourceforge.pmd.lang.java.ast.BinaryOp; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.rule.internal.JavaRuleUtil; import net.sourceforge.pmd.lang.java.symbols.JFieldSymbol; import net.sourceforge.pmd.lang.java.symbols.JLocalVariableSymbol; import net.sourceforge.pmd.lang.java.symbols.JVariableSymbol; @@ -93,15 +91,15 @@ public class DoubleCheckedLockingRule extends AbstractJavaRule { List isl = node.findDescendantsOfType(ASTIfStatement.class); if (isl.size() == 2) { ASTIfStatement outerIf = isl.get(0); - if (isNullCheck(outerIf.getCondition(), returnVariable)) { + if (JavaRuleUtil.isNullCheck(outerIf.getCondition(), returnVariable)) { // find synchronized List ssl = outerIf.findDescendantsOfType(ASTSynchronizedStatement.class); if (ssl.size() == 1 && ssl.get(0).ancestors().any(it -> it == outerIf)) { ASTIfStatement is2 = isl.get(1); - if (isNullCheck(is2.getCondition(), returnVariable)) { + if (JavaRuleUtil.isNullCheck(is2.getCondition(), returnVariable)) { List assignments = is2.findDescendantsOfType(ASTAssignmentExpression.class); if (assignments.size() == 1 - && isReferenceTo(assignments.get(0).getLeftOperand(), returnVariable)) { + && JavaRuleUtil.isReferenceToVar(assignments.get(0).getLeftOperand(), returnVariable)) { addViolation(data, node); } @@ -127,7 +125,7 @@ public class DoubleCheckedLockingRule extends AbstractJavaRule { return (initializer == null || isVolatileFieldReference(initializer)) && method.descendants(ASTAssignmentExpression.class) - .filter(it -> isReferenceTo(it.getLeftOperand(), local)) + .filter(it -> JavaRuleUtil.isReferenceToVar(it.getLeftOperand(), local)) .all(it -> isVolatileFieldReference(it.getRightOperand())); } @@ -140,26 +138,4 @@ public class DoubleCheckedLockingRule extends AbstractJavaRule { } } - private boolean isReferenceTo(@Nullable ASTExpression expr, JVariableSymbol symbol) { - if (expr instanceof ASTNamedReferenceExpr) { - return symbol != null && symbol.equals(((ASTNamedReferenceExpr) expr).getReferencedSym()); - } else { - return false; - } - } - - private boolean isNullCheck(ASTExpression expr, JVariableSymbol var) { - if (expr instanceof ASTInfixExpression) { - ASTInfixExpression condition = (ASTInfixExpression) expr; - if (condition.getOperator().hasSamePrecedenceAs(BinaryOp.EQ)) { - ASTNullLiteral nullLit = condition.getFirstChildOfType(ASTNullLiteral.class); - if (nullLit != null) { - ASTExpression otherChild = (ASTExpression) condition.getChild(1 - nullLit.getIndexInParent()); - return isReferenceTo(otherChild, var); - } - } - } - return false; - } - } diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BooleanInstantiationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BooleanInstantiationRule.java deleted file mode 100644 index b8b345c071..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BooleanInstantiationRule.java +++ /dev/null @@ -1,112 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; -import net.sourceforge.pmd.lang.java.ast.ASTArrayDimsAndInits; -import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral; -import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceType; -import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; -import net.sourceforge.pmd.lang.java.ast.ASTImportDeclaration; -import net.sourceforge.pmd.lang.java.ast.ASTLiteral; -import net.sourceforge.pmd.lang.java.ast.ASTName; -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.rule.AbstractJavaRule; -import net.sourceforge.pmd.lang.java.types.TypeTestUtil; - -/** - * Avoid instantiating Boolean objects; you can reference Boolean.TRUE, - * Boolean.FALSE, or call Boolean.valueOf() instead. - * - * 
- *  public class Foo {
- *       Boolean bar = new Boolean("true");    // just do a Boolean
- *       bar = Boolean.TRUE;                   //ok
- *       Boolean buz = Boolean.valueOf(false); // just do a Boolean buz = Boolean.FALSE;
- *  }
- *
- */ -public class BooleanInstantiationRule extends AbstractJavaRule { - - /* - * see bug 1744065 : If somebody create it owns Boolean, the rule should not - * be triggered Therefore, we use this boolean to flag if the source code - * contains such an import - * - */ - private boolean customBoolean; - - @Override - public Object visit(ASTCompilationUnit decl, Object data) { - // customBoolean needs to be reset for each new file - customBoolean = false; - - return super.visit(decl, data); - } - - @Override - public Object visit(ASTImportDeclaration decl, Object data) { - // If the import actually import a Boolean class that overrides - // java.lang.Boolean - if (decl.getImportedName().endsWith("Boolean") && !"java.lang".equals(decl.getImportedName())) { - customBoolean = true; - } - return super.visit(decl, data); - } - - @Override - public Object visit(ASTAllocationExpression node, Object data) { - - if (!customBoolean) { - if (node.hasDescendantOfType(ASTArrayDimsAndInits.class)) { - return super.visit(node, data); - } - - ASTClassOrInterfaceType n1 = node.getFirstChildOfType(ASTClassOrInterfaceType.class); - if (TypeTestUtil.isA(Boolean.class, n1)) { - super.addViolation(data, node); - return data; - } - } - return super.visit(node, data); - } - - @Override - public Object visit(ASTPrimaryPrefix node, Object data) { - - if (!customBoolean) { - if (node.getNumChildren() == 0 || !(node.getChild(0) instanceof ASTName)) { - return super.visit(node, data); - } - - if ("Boolean.valueOf".equals(((ASTName) node.getChild(0)).getImage()) - || "java.lang.Boolean.valueOf".equals(((ASTName) node.getChild(0)).getImage())) { - ASTPrimaryExpression parent = (ASTPrimaryExpression) node.getParent(); - ASTPrimarySuffix suffix = parent.getFirstDescendantOfType(ASTPrimarySuffix.class); - if (suffix == null) { - return super.visit(node, data); - } - ASTPrimaryPrefix prefix = suffix.getFirstDescendantOfType(ASTPrimaryPrefix.class); - if (prefix == null) { - return super.visit(node, data); - } - - if (prefix.hasDescendantOfType(ASTBooleanLiteral.class)) { - super.addViolation(data, node); - return data; - } - ASTLiteral literal = prefix.getFirstDescendantOfType(ASTLiteral.class); - if (literal != null - && ("\"true\"".equals(literal.getImage()) || "\"false\"".equals(literal.getImage()))) { - super.addViolation(data, node); - return data; - } - } - } - return super.visit(node, data); - } -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UnnecessaryWrapperObjectCreationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UnnecessaryWrapperObjectCreationRule.java deleted file mode 100644 index 41521dda36..0000000000 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/UnnecessaryWrapperObjectCreationRule.java +++ /dev/null @@ -1,43 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import static net.sourceforge.pmd.util.CollectionUtil.setOf; - -import java.util.Set; - -import net.sourceforge.pmd.lang.java.ast.ASTMethodCall; -import net.sourceforge.pmd.lang.java.rule.AbstractJavaRulechainRule; -import net.sourceforge.pmd.lang.java.rule.codestyle.UnnecessaryBoxingRule; - -/** - * @deprecated Replaced by {@link UnnecessaryBoxingRule}. - */ -@Deprecated -public class UnnecessaryWrapperObjectCreationRule extends AbstractJavaRulechainRule { - private static final Set SUFFIX_SET = setOf("toString", "byteValue", - "shortValue", "intValue", "longValue", "floatValue", "doubleValue", "charValue", "booleanValue"); - - public UnnecessaryWrapperObjectCreationRule() { - super(ASTMethodCall.class); - } - - @Override - public Object visit(ASTMethodCall node, Object data) { - if (!"valueOf".equals(node.getMethodName()) || node.getQualifier() == null - || !node.getQualifier().getTypeMirror().isBoxedPrimitive() - || !(node.getParent() instanceof ASTMethodCall)) { - return data; - } - - ASTMethodCall nextMethodCall = (ASTMethodCall) node.getParent(); - String methodName = nextMethodCall.getMethodName(); - if (SUFFIX_SET.contains(methodName)) { - addViolation(data, node); - } - - return data; - } -} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/JClassSymbol.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/JClassSymbol.java index 933646f934..d5a018fdcf 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/JClassSymbol.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/symbols/JClassSymbol.java @@ -166,6 +166,7 @@ public interface JClassSymbol extends JTypeDeclSymbol, * Returns a set with all enum constant names. If this symbol does * not represent an enum, returns null. */ + @SuppressWarnings("PMD.ReturnEmptyCollectionRatherThanNull") default @Nullable Set getEnumConstantNames() { return null; } 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 4b7017df95..98150320be 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 @@ -95,7 +95,7 @@ public final class TypeTestUtil { return isExactlyA(clazz, type.getSymbol()); } - return isA(type, otherType); + return isA(otherType, type); } @@ -134,26 +134,35 @@ public final class TypeTestUtil { return isA(canonicalName, thisType, null); } + public static boolean isA(@NonNull JTypeMirror t1, @Nullable TypeNode t2) { + return t2 != null && isA(t1, t2.getTypeMirror()); + } + /** - * This is the subtyping routine we use, which prunes some behavior - * of isSubtypeOf that we don't want (eg, that unresolved types are - * subtypes of everything). + * Checks whether the first type is a subtype of the second. This + * removes some behavior of isSubtypeOf that we don't want (eg, that + * unresolved types are subtypes of everything). + * + * @param t1 A supertype + * @param t2 A type + * + * @return Whether t1 is a subtype of t2 */ - private static boolean isA(JTypeMirror t1, JTypeMirror t2) { - if (t1 == null || t2 == null) { + private static boolean isA(@Nullable JTypeMirror t1, @NonNull JTypeMirror t2) { + if (t1 == null) { return false; - } else if (t1.isPrimitive() || t2.isPrimitive()) { - return t1.equals(t2); // isSubtypeOf considers primitive widening like subtyping - } else if (TypeOps.isUnresolved(t1)) { + } else if (t2.isPrimitive() || t1.isPrimitive()) { + return t2.equals(t1); // isSubtypeOf considers primitive widening like subtyping + } else if (TypeOps.isUnresolved(t2)) { // we can't get any useful info from this, isSubtypeOf would return true return false; - } else if (t2.isClassOrInterface() && ((JClassType) t2).getSymbol().isAnonymousClass()) { + } else if (t1.isClassOrInterface() && ((JClassType) t1).getSymbol().isAnonymousClass()) { return false; // conventionally - } else if (t1 instanceof JTypeVar) { - return t2.isTop() || isA(((JTypeVar) t1).getUpperBound(), t2); + } else if (t2 instanceof JTypeVar) { + return t1.isTop() || isA(t1, ((JTypeVar) t2).getUpperBound()); } - return t1.isSubtypeOf(t2); + return t2.isSubtypeOf(t1); } private static boolean isA(@NonNull String canonicalName, @NonNull JTypeMirror thisType, @Nullable UnresolvedClassStore unresolvedStore) { @@ -173,7 +182,7 @@ public final class TypeTestUtil { TypeSystem ts = thisType.getTypeSystem(); @Nullable JTypeMirror otherType = TypesFromReflection.loadType(ts, canonicalName, unresolvedStore); - return isA(thisType, otherType); + return isA(otherType, thisType); } /** diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index e3a667f011..3a2e29b77a 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1326,11 +1326,6 @@ public class Foo { Reports test assertions that may be simplified using a more specific assertion method. This enables better error messages, and makes the assertions more readable. - - The rule only applies within test classes for the moment. It replaces - the deprecated rules {% rule UseAssertEqualsInsteadOfAssertTrue %}, - {% rule UseAssertNullInsteadOfAssertTrue %}, {% rule UseAssertSameInsteadOfAssertTrue %}, - {% rule UseAssertTrueInsteadOfAssertEquals %}, and {% rule java/design/SimplifyBooleanAssertion %}. 3 @@ -1647,176 +1642,6 @@ public class Something { - - -This rule detects JUnit assertions in object equality. These assertions should be made by more specific methods, like assertEquals. - -Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead. - - 3 - - - - - - - - - - - - - - -This rule detects JUnit assertions in object references equality. These assertions should be made by -more specific methods, like assertNull, assertNotNull. - -Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead. - - 3 - - - - - - - - - - - - - - -This rule detects JUnit assertions in object references equality. These assertions should be made -by more specific methods, like assertSame, assertNotSame. - -Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead. - - 3 - - - - - - - - - - - - - - -When asserting a value is the same as a literal or Boxed boolean, use assertTrue/assertFalse, instead of assertEquals. - -Deprecated since PMD 6.37.0, use {% rule SimplifiableTestAssertion %} instead. - - 3 - - - - - - - - - - - - - - -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 - - - - - - - - - 3 diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index c3c579c464..964bd92fae 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -312,27 +312,15 @@ public void foo() throws RuntimeException { -A class with only private constructors should be final, unless the private constructor -is invoked by a inner class. +Reports classes that may be made final because they cannot be extended from outside +their compilation unit anyway. This is because all their constructors are private, +so a subclass could not call the super constructor. 1 - - - - - - - @@ -1162,64 +1148,6 @@ public class Foo { - - -Avoid negation in an assertTrue or assertFalse test. - -For example, rephrase: - - assertTrue(!expr); - -as: - - assertFalse(expr); - -Deprecated since PMD 6.37.0, use {% rule java/bestpractices/SimplifiableTestAssertion %} instead. - - 3 - - - - - - - - - - - No need to check for null before an instanceof; the instanceof keyword returns false when given a null argument. 3 - - - - - - - -Methods such as getDeclaredConstructors(), getDeclaredConstructor(Class[]) and setAccessible(), -as the interface PrivilegedAction, allow for the runtime alteration of variable, class, or -method visibility, even if they are private. This violates the principle of encapsulation. +Methods such as `getDeclaredConstructors()`, `getDeclaredMethods()`, and `getDeclaredFields()` also +return private constructors, methods and fields. These can be made accessible by calling `setAccessible(true)`. +This gives access to normally protected data which violates the principle of encapsulation. + +This rule detects calls to `setAccessible` and finds possible accessibility alterations. +If the call to `setAccessible` is wrapped within a `PrivilegedAction`, then the access alteration +is assumed to be deliberate and is not reported. + +Note that with Java 17 the Security Manager, which is used for `PrivilegedAction` execution, +is deprecated: [JEP 411: Deprecate the Security Manager for Removal](https://openjdk.java.net/jeps/411). +For future-proof code, deliberate access alteration should be suppressed using the usual +suppression methods (e.g. by using `@SuppressWarnings` annotation). 3 constructor = this.getClass().getDeclaredConstructor(String.class); + // call to forbidden setAccessible + constructor.setAccessible(true); - // Possible call to forbidden PrivilegedAction - PrivilegedAction priv = (PrivilegedAction) new Object(); priv.run(); - } + Method privateMethod = this.getClass().getDeclaredMethod("aPrivateMethod"); + // call to forbidden setAccessible + privateMethod.setAccessible(true); + + // deliberate accessibility alteration + String privateField = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public String run() { + try { + Field field = Violation.class.getDeclaredField("aPrivateField"); + field.setAccessible(true); + return (String) field.get(null); + } catch (ReflectiveOperationException | SecurityException e) { + throw new RuntimeException(e); + } + } + }); + } } ]]> @@ -637,8 +636,6 @@ k = i * j; // set k with 80 not 120 - - - - -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 - - - - - - - - - - - - @@ -1963,12 +1904,7 @@ If the finalize() is implemented, it should do something besides just calling su @@ -2032,7 +1968,7 @@ Note that Oracle has declared Object.finalize() as deprecated since JDK 9. @@ -2123,12 +2059,9 @@ Avoid instantiating an object just to call getClass() on it; use the .class publ @@ -2144,10 +2077,6 @@ Class c = String.class; - - - - @@ -2546,7 +2472,7 @@ confusing. @@ -2706,54 +2632,6 @@ public class Foo { - - -For any method that returns an array, it is a better to return an empty array rather than a -null reference. This removes the need for null checking all results and avoids inadvertent -NullPointerExceptions. - -Deprecated since PMD 6.37.0, use {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} instead. - - 1 - - - - - - - - - - - - 3 - //FinallyStatement//ReturnStatement except //FinallyStatement//(MethodDeclaration|LambdaExpression)//ReturnStatement + //FinallyClause//ReturnStatement except //FinallyClause//(MethodDeclaration|LambdaExpression)//ReturnStatement @@ -3422,16 +3300,18 @@ public class Main { class="net.sourceforge.pmd.lang.rule.XPathRule" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_errorprone.html#useequalstocomparestrings"> -Using '==' or '!=' to compare strings only works if intern version is used on both sides. -Use the equals() method instead. +Using '==' or '!=' to compare strings is only reliable if the interned string (`String#intern()`) +is used on both sides. + +Use the `equals()` method instead. 3 diff --git a/pmd-java/src/main/resources/category/java/performance.xml b/pmd-java/src/main/resources/category/java/performance.xml index f265b4d939..ffe1823903 100644 --- a/pmd-java/src/main/resources/category/java/performance.xml +++ b/pmd-java/src/main/resources/category/java/performance.xml @@ -275,51 +275,6 @@ public class Something { - - -Note: this rule is deprecated, as its rationale does not hold. - -Java uses the 'short' type to reduce memory usage, not to optimize calculation. In fact, the JVM does not have any -arithmetic capabilities for the short type: the JVM must convert the short into an int, do the proper calculation -and convert the int back to a short. Thus any storage gains found through use of the 'short' type may be offset by -adverse impacts on performance. - - 1 - - - - - - - - - - - - - - -Avoid instantiating Boolean objects; you can reference Boolean.TRUE, Boolean.FALSE, or call Boolean.valueOf() instead. -Note that new Boolean() is deprecated since JDK 9 for that reason. - -Deprecated since PMD 6.37.0, use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. - - 2 - - - - - - - -Calling new Byte() causes memory allocation that can be avoided by the static Byte.valueOf(). -It makes use of an internal cache that recycles earlier instances making it more memory efficient. -Note that new Byte() is deprecated since JDK 9 for that reason. - -Deprecated since PMD 6.37.0, use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. - - 2 - - - - - - - - - - - - - - -Calling new Integer() causes memory allocation that can be avoided by the static Integer.valueOf(). -It makes use of an internal cache that recycles earlier instances making it more memory efficient. -Note that new Integer() is deprecated since JDK 9 for that reason. - -Deprecated since PMD 6.37.0, use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. - - 2 - - - - - - - - - - - - - - -Calling new Long() causes memory allocation that can be avoided by the static Long.valueOf(). -It makes use of an internal cache that recycles earlier instances making it more memory efficient. -Note that new Long() is deprecated since JDK 9 for that reason. - -Deprecated since PMD 6.37.0, use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. - - 2 - - - - - - - - - - - - - - -Note: this rule is deprecated for removal, as the optimization is insignificant. - -Calls to `string.startsWith("x")` with a string literal of length 1 can be rewritten using `string.charAt(0)`, -at the expense of some readability. To prevent `IndexOutOfBoundsException` being thrown by the `charAt` method, -ensure that the string is not empty by making an additional check first. - - 3 - - - - - - - - - - - - - - -Calling new Short() causes memory allocation that can be avoided by the static Short.valueOf(). -It makes use of an internal cache that recycles earlier instances making it more memory efficient. -Note that new Short() is deprecated since JDK 9 for that reason. - -Deprecated since PMD 6.37.0, use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. - - 2 - - - - - - - - - - - - - - -Most wrapper classes provide static conversion methods that avoid the need to create intermediate objects -just to create the primitive forms. Using these avoids the cost of creating objects that also need to be -garbage-collected later. - -Deprecated since PMD 6.37.0. The planned replacement is not expected before PMD 7.0.0. - - 3 - - - - - - + diff --git a/pmd-java/src/main/resources/rulesets/java/clone.xml b/pmd-java/src/main/resources/rulesets/java/clone.xml index f662ff5473..ec7dc50558 100644 --- a/pmd-java/src/main/resources/rulesets/java/clone.xml +++ b/pmd-java/src/main/resources/rulesets/java/clone.xml @@ -12,7 +12,7 @@ The Clone Implementation ruleset contains a collection of rules that find questi - + - \ No newline at end of file + diff --git a/pmd-java/src/main/resources/rulesets/java/controversial.xml b/pmd-java/src/main/resources/rulesets/java/controversial.xml index 5b2be135bd..1efb0be7c9 100644 --- a/pmd-java/src/main/resources/rulesets/java/controversial.xml +++ b/pmd-java/src/main/resources/rulesets/java/controversial.xml @@ -24,7 +24,7 @@ They are held here to allow people to include them as they see fit within their - + @@ -33,7 +33,7 @@ They are held here to allow people to include them as they see fit within their - + diff --git a/pmd-java/src/main/resources/rulesets/java/design.xml b/pmd-java/src/main/resources/rulesets/java/design.xml index 9ac4ae2fda..e2bde4ae93 100644 --- a/pmd-java/src/main/resources/rulesets/java/design.xml +++ b/pmd-java/src/main/resources/rulesets/java/design.xml @@ -28,18 +28,18 @@ are suggested. - + - + - + diff --git a/pmd-java/src/main/resources/rulesets/java/junit.xml b/pmd-java/src/main/resources/rulesets/java/junit.xml index d122e26f81..057cd7f536 100644 --- a/pmd-java/src/main/resources/rulesets/java/junit.xml +++ b/pmd-java/src/main/resources/rulesets/java/junit.xml @@ -17,10 +17,10 @@ These rules deal with different problems that can occur with JUnit tests. - - - - + + + + - + diff --git a/pmd-java/src/main/resources/rulesets/java/logging-java.xml b/pmd-java/src/main/resources/rulesets/java/logging-java.xml index e4882f5f14..65858b65d8 100644 --- a/pmd-java/src/main/resources/rulesets/java/logging-java.xml +++ b/pmd-java/src/main/resources/rulesets/java/logging-java.xml @@ -10,7 +10,7 @@ The Java Logging ruleset contains a collection of rules that find questionable u - + diff --git a/pmd-java/src/main/resources/rulesets/java/migrating.xml b/pmd-java/src/main/resources/rulesets/java/migrating.xml index bc7681960a..9046281651 100644 --- a/pmd-java/src/main/resources/rulesets/java/migrating.xml +++ b/pmd-java/src/main/resources/rulesets/java/migrating.xml @@ -24,9 +24,9 @@ rather, use a wrapper ruleset such as migrating_to_13.xml. - - - - + + + + diff --git a/pmd-java/src/main/resources/rulesets/java/migrating_to_15.xml b/pmd-java/src/main/resources/rulesets/java/migrating_to_15.xml index 58f833176f..a1514ce199 100644 --- a/pmd-java/src/main/resources/rulesets/java/migrating_to_15.xml +++ b/pmd-java/src/main/resources/rulesets/java/migrating_to_15.xml @@ -9,9 +9,9 @@ Contains rules for migrating to JDK 1.5 - - - - + + + + diff --git a/pmd-java/src/main/resources/rulesets/java/optimizations.xml b/pmd-java/src/main/resources/rulesets/java/optimizations.xml index 4439c5385e..ec6d63d4f0 100644 --- a/pmd-java/src/main/resources/rulesets/java/optimizations.xml +++ b/pmd-java/src/main/resources/rulesets/java/optimizations.xml @@ -17,10 +17,10 @@ These rules deal with different optimizations that generally apply to best pract - - + + - \ No newline at end of file + diff --git a/pmd-java/src/main/resources/rulesets/java/quickstart.xml b/pmd-java/src/main/resources/rulesets/java/quickstart.xml index 8c0ed47cc6..eca04a16fe 100644 --- a/pmd-java/src/main/resources/rulesets/java/quickstart.xml +++ b/pmd-java/src/main/resources/rulesets/java/quickstart.xml @@ -88,7 +88,6 @@ - @@ -295,7 +294,6 @@ - @@ -304,7 +302,6 @@ - diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java index c8043d4f0b..f337873ede 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ParserCornersTest.java @@ -40,8 +40,8 @@ public class ParserCornersTest extends BaseJavaTreeDumpTest { @Test public void testInvalidUnicodeEscape() { expect.expect(TokenMgrError.class); // previously Error - expect.expectMessage("Lexical error at line 1, column 2. Encountered: Invalid unicode escape"); - java.parse("\\u00k0"); + expect.expectMessage("Lexical error in file x/filename.java at line 1, column 2. Encountered: Invalid unicode escape"); + java.parse("\\u00k0", null, "x/filename.java"); } /** diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertEqualsInsteadOfAssertTrueTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertEqualsInsteadOfAssertTrueTest.java deleted file mode 100644 index 4df42d6789..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertEqualsInsteadOfAssertTrueTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.bestpractices; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class UseAssertEqualsInsteadOfAssertTrueTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertNullInsteadOfAssertTrueTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertNullInsteadOfAssertTrueTest.java deleted file mode 100644 index a593a9e0ff..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertNullInsteadOfAssertTrueTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.bestpractices; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class UseAssertNullInsteadOfAssertTrueTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertSameInsteadOfAssertTrueTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertSameInsteadOfAssertTrueTest.java deleted file mode 100644 index e874a309b1..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertSameInsteadOfAssertTrueTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.bestpractices; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class UseAssertSameInsteadOfAssertTrueTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertTrueInsteadOfAssertEqualsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertTrueInsteadOfAssertEqualsTest.java deleted file mode 100644 index cb8ded71bf..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UseAssertTrueInsteadOfAssertEqualsTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.bestpractices; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class UseAssertTrueInsteadOfAssertEqualsTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/DefaultPackageTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/DefaultPackageTest.java deleted file mode 100644 index 19c6c714fd..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/codestyle/DefaultPackageTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.codestyle; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class DefaultPackageTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/ClassWithOnlyPrivateConstructorsShouldBeFinalTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/ClassWithOnlyPrivateConstructorsShouldBeFinalTest.java index 929c188965..1687261131 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/ClassWithOnlyPrivateConstructorsShouldBeFinalTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/ClassWithOnlyPrivateConstructorsShouldBeFinalTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class ClassWithOnlyPrivateConstructorsShouldBeFinalTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifiedTernaryTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifiedTernaryTest.java index b57ae4b5b8..c381684027 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifiedTernaryTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifiedTernaryTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class SimplifiedTernaryTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanAssertionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanAssertionTest.java deleted file mode 100644 index 365bb56a0d..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyBooleanAssertionTest.java +++ /dev/null @@ -1,12 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.design; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -@org.junit.Ignore("Rule has not been updated yet") -public class SimplifyBooleanAssertionTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyConditionalTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyConditionalTest.java index 43ed3e5754..927ae8d8f8 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyConditionalTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SimplifyConditionalTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class SimplifyConditionalTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityTest.java index c19a339f55..0da92cecac 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/SwitchDensityTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class SwitchDensityTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassTest.java index b9275c8101..36e7942a09 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/design/UseUtilityClassTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.design; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class UseUtilityClassTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnEmptyArrayRatherThanNullTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidAccessibilityAlterationTest.java similarity index 64% rename from pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnEmptyArrayRatherThanNullTest.java rename to pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidAccessibilityAlterationTest.java index 428d6737bf..cd5c41a995 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnEmptyArrayRatherThanNullTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/AvoidAccessibilityAlterationTest.java @@ -1,4 +1,4 @@ -/** +/* * BSD-style license; for more info see http://pmd.sourceforge.net/license.html */ @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") -public class ReturnEmptyArrayRatherThanNullTest extends PmdRuleTst { +public class AvoidAccessibilityAlterationTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloneThrowsCloneNotSupportedExceptionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloneThrowsCloneNotSupportedExceptionTest.java deleted file mode 100644 index 2dfbc243d2..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/CloneThrowsCloneNotSupportedExceptionTest.java +++ /dev/null @@ -1,12 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.errorprone; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -@org.junit.Ignore("Rule has not been updated yet") -public class CloneThrowsCloneNotSupportedExceptionTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeDoesNotCallSuperFinalizeTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeDoesNotCallSuperFinalizeTest.java index f74194a4d5..97017d2b87 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeDoesNotCallSuperFinalizeTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeDoesNotCallSuperFinalizeTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class FinalizeDoesNotCallSuperFinalizeTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOnlyCallsSuperFinalizeTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOnlyCallsSuperFinalizeTest.java index 929b7ebe7b..af0ab11739 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOnlyCallsSuperFinalizeTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOnlyCallsSuperFinalizeTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class FinalizeOnlyCallsSuperFinalizeTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOverloadedTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOverloadedTest.java index 5efb414193..e123e98a1c 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOverloadedTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeOverloadedTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class FinalizeOverloadedTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeShouldBeProtectedTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeShouldBeProtectedTest.java index 5fb2be61f2..73b0ee58fc 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeShouldBeProtectedTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/FinalizeShouldBeProtectedTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class FinalizeShouldBeProtectedTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/InstantiationToGetClassTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/InstantiationToGetClassTest.java index 002a4b5e05..f27f820f6f 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/InstantiationToGetClassTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/InstantiationToGetClassTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class InstantiationToGetClassTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingSerialVersionUIDTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingSerialVersionUIDTest.java index 608f84996c..b2437fbaf8 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingSerialVersionUIDTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingSerialVersionUIDTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class MissingSerialVersionUIDTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/NonStaticInitializerTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/NonStaticInitializerTest.java index 8f3c4fa311..f009ce5ab2 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/NonStaticInitializerTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/NonStaticInitializerTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class NonStaticInitializerTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnFromFinallyBlockTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnFromFinallyBlockTest.java index 1d83115c71..5945a9133f 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnFromFinallyBlockTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnFromFinallyBlockTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class ReturnFromFinallyBlockTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UseEqualsToCompareStringsTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UseEqualsToCompareStringsTest.java index 2ac4f0f2dc..903c0d420d 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UseEqualsToCompareStringsTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/UseEqualsToCompareStringsTest.java @@ -6,7 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -@org.junit.Ignore("Rule has not been updated yet") public class UseEqualsToCompareStringsTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidUsingShortTypeTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidUsingShortTypeTest.java deleted file mode 100644 index 5a639a8a74..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/AvoidUsingShortTypeTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class AvoidUsingShortTypeTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/BooleanInstantiationTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/BooleanInstantiationTest.java deleted file mode 100644 index 57510e54c8..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/BooleanInstantiationTest.java +++ /dev/null @@ -1,12 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -@org.junit.Ignore("Rule has not been updated yet") -public class BooleanInstantiationTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ByteInstantiationTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ByteInstantiationTest.java deleted file mode 100644 index 2db2199779..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ByteInstantiationTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class ByteInstantiationTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/IntegerInstantiationTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/IntegerInstantiationTest.java deleted file mode 100644 index a951c8eda7..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/IntegerInstantiationTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class IntegerInstantiationTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/LongInstantiationTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/LongInstantiationTest.java deleted file mode 100644 index a2b951cb00..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/LongInstantiationTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class LongInstantiationTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ShortInstantiationTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ShortInstantiationTest.java deleted file mode 100644 index 564c94b6fd..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/ShortInstantiationTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class ShortInstantiationTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/SimplifyStartsWithTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/SimplifyStartsWithTest.java deleted file mode 100644 index e2a3fbcd15..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/SimplifyStartsWithTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class SimplifyStartsWithTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/UnnecessaryWrapperObjectCreationTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/UnnecessaryWrapperObjectCreationTest.java deleted file mode 100644 index 08da381245..0000000000 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/performance/UnnecessaryWrapperObjectCreationTest.java +++ /dev/null @@ -1,11 +0,0 @@ -/** - * BSD-style license; for more info see http://pmd.sourceforge.net/license.html - */ - -package net.sourceforge.pmd.lang.java.rule.performance; - -import net.sourceforge.pmd.testframework.PmdRuleTst; - -public class UnnecessaryWrapperObjectCreationTest extends PmdRuleTst { - // no additional unit tests -} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertEqualsInsteadOfAssertTrue.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertEqualsInsteadOfAssertTrue.xml deleted file mode 100644 index b604960de4..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertEqualsInsteadOfAssertTrue.xml +++ /dev/null @@ -1,99 +0,0 @@ - - - - - Identity test doesn't match - 0 - - - - - Junit 3 problem - 1 - - - - - Overload of equal doesn't match - 0 - - - - - JUnit 4, even outside of @Test method - 1 - - - - - JUnit4 - match - 1 - - - - - JUnit5 - @Test - 1 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertNullInsteadOfAssertTrue.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertNullInsteadOfAssertTrue.xml deleted file mode 100644 index fdd79d4e26..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertNullInsteadOfAssertTrue.xml +++ /dev/null @@ -1,92 +0,0 @@ - - - - - assertTrue with null - 1 - - - - - assertFalse with != null - 1 - - - - - assertTrue with x == y - 0 - - - - - Not a JUnit test - assertTrue with null - 0 - - - - - JUnit 4 - assertTrue with null - 1 - - - - - JUnit 5 - assertTrue with null - @Test - 1 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertSameInsteadOfAssertTrue.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertSameInsteadOfAssertTrue.xml deleted file mode 100644 index 5a0a74559e..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertSameInsteadOfAssertTrue.xml +++ /dev/null @@ -1,127 +0,0 @@ - - - - - assert true a == b - 1 - - - - - assert true a != b - 1 - - - - - assert false a == b - 1 - - - - - assert false a != b - 1 - - - - - skip assertTrue(x == null), UseAssertNullInsteadOfAssertTrue will pick those up - 0 - - - - - bug 1626715, the null check in the rule shouldn't match the null outside the assert method - 1 - - - - - assert true a == b BUT not a Junit test - 0 - - - - - JUnit 4 - assert true a == b - 1 - - - - - JUnit 5 - assert true a == b - @Test - 1 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertTrueInsteadOfAssertEquals.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertTrueInsteadOfAssertEquals.xml deleted file mode 100644 index 13358b5119..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UseAssertTrueInsteadOfAssertEquals.xml +++ /dev/null @@ -1,96 +0,0 @@ - - - - - JUnit Test contains assertEquals on other than boolean literal - 0 - - - - - JUnit Test contains assertEquals on boolean literal - 5 - - - - - #1323 False positive case of UseAssertTrueInsteadOfAssertEquals - 0 - - - - - JUnit Test contains assertEquals with Boxed booleans - 8 - - - - - JUnit Test contains assertEquals with Boxed booleans as param - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/DefaultPackage.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/DefaultPackage.xml deleted file mode 100644 index 5882f7e174..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/DefaultPackage.xml +++ /dev/null @@ -1,176 +0,0 @@ - - - - - ok - 0 - - - - - bad - 1 - - - - - interface methods are always public - 0 - - - - - interface field are always public - 0 - - - - - bad - 1 - - - - - #1410 DefaultPackage triggers on field annotated with @VisibleForTesting - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @Test - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @RepeatedTest - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @ParameterizedTest - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @TestFactory - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @TestTemplate - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @BeforeAll - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @AfterAll - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @BeforeEach - 0 - - - - - #2573 DefaultPackage triggers on field annotated with JUnit 5 @AfterEach - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryBoxing.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryBoxing.xml index a4b5e0ea2d..d891877022 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryBoxing.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryBoxing.xml @@ -200,4 +200,137 @@ public class Foo { } } ]]> - + + + + Uses of Integer.valueOf(someString) where an int is expected + 4 + 3,4,5,6 + + Unnecessary implicit unboxing. Use Integer.parseInt(...) instead + Unnecessary implicit unboxing. Use Integer.parseInt(...) instead + Unnecessary implicit unboxing. Use Integer.parseInt(...) instead + Unnecessary implicit unboxing. Use Integer.parseInt(...) instead + + + + + + Uses of Long.valueOf(someString) where an long is expected + 4 + 3,4,5,6 + + Unnecessary implicit unboxing. Use Long.parseLong(...) instead + Unnecessary implicit unboxing. Use Long.parseLong(...) instead + Unnecessary implicit unboxing. Use Long.parseLong(...) instead + Unnecessary implicit unboxing. Use Long.parseLong(...) instead + + + + + + Uses of Double.valueOf(someString) where an double is expected + 2 + 3,4 + + Unnecessary implicit unboxing. Use Double.parseDouble(...) instead + Unnecessary implicit unboxing. Use Double.parseDouble(...) instead + + + + + + Uses of Float.valueOf(someString) where an float is expected + 2 + 3,4 + + Unnecessary implicit unboxing. Use Float.parseFloat(...) instead + Unnecessary implicit unboxing. Use Float.parseFloat(...) instead + + + + + + Uses of Short.valueOf(someString) where an short is expected + 4 + 3,4,5,6 + + Unnecessary implicit unboxing. Use Short.parseShort(...) instead + Unnecessary implicit unboxing. Use Short.parseShort(...) instead + Unnecessary implicit unboxing. Use Short.parseShort(...) instead + Unnecessary implicit unboxing. Use Short.parseShort(...) instead + + + + + + Uses of Boolean.valueOf(someString) where an boolean is expected + 2 + 3,4 + + Unnecessary implicit unboxing. Use Boolean.parseBoolean(...) instead + Unnecessary implicit unboxing. Use Boolean.parseBoolean(...) instead + + + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ClassWithOnlyPrivateConstructorsShouldBeFinal.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ClassWithOnlyPrivateConstructorsShouldBeFinal.xml index 538aca8368..fe66fa1313 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ClassWithOnlyPrivateConstructorsShouldBeFinal.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/ClassWithOnlyPrivateConstructorsShouldBeFinal.xml @@ -135,4 +135,28 @@ class ClassWithOnlyPrivateConstructorsShouldBeFinal { } ]]> + + #2536 [java] ClassWithOnlyPrivateConstructorsShouldBeFinal can't detect inner class with only private constructor + 1 + 2 + + + + Private inner class with no ctor + 1 + 2 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifiedTernary.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifiedTernary.xml index 2002135780..e3305ee2ff 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifiedTernary.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifiedTernary.xml @@ -9,7 +9,7 @@ 1 1 1 1 condition ? true : false - 0 + 1 @@ -69,8 +69,8 @@ public class Foo { 0 @@ -81,7 +81,7 @@ public class SimplifiedTernary { 0 - - - - assertFalse(!) - 1 - - - - - assertTrue(!) - 1 - - - - - ok - 0 - - - - - not a JUnit test - assertFalse(!) - 0 - - - - - JUnit 4 - assertFalse(!) - 1 - - - - - JUnit 5 - assertFalse(!) - 1 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifyConditional.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifyConditional.xml index 32846c87f2..ad2f14df16 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifyConditional.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/SimplifyConditional.xml @@ -98,11 +98,10 @@ public class Bug1843273 { Bug 2317099 : False + in SimplifyConditional 0 + + Switch expr + 4 + 1 + + + + Switch expr, composite label + 4 + 0 + + One stmt in one switch case, ok @@ -69,6 +113,44 @@ public class SwitchDensity3 { } } } +} + ]]> + + + + False positive with default label + 10 + 0 + + + + Empty switch + 0 + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml index 81ab0468f8..32af58ff8f 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/design/xml/UseUtilityClass.xml @@ -7,6 +7,9 @@ should be utility class since all static, public constructor 1 + + This utility class has a non-private constructor + - junit 'suite' method is OK + junit 3 'suite' method is OK 0 + + junit 4 'suite' method is OK + 0 + + Reproducing bug [ 2315599 ] False +: UseSingleton with class containing constructor: Although there is a static method, the class also has a non-private constructor. This is a common design for custom exceptions which contain a private static method to format error message strings. @@ -206,19 +226,17 @@ public class MyException extends RuntimeException { #1467 UseUtilityClass can't correctly check functions with multiple annotations 0 @@ -257,21 +275,6 @@ public class Foo { ]]> - - Lombok NoArgsConstructor no import- ok - 0 - - - Lombok NoArgsConstructor with no access level- should be a utility class 1 @@ -352,9 +355,21 @@ public class MyUtil { Inner class in abstract class false-negative 1 + 2 + + + Private inner class in abstract class + 0 + 1 1 Inner class in sub-class false-negative 1 + 2 + + + + Example code snippet + 2 + 11,15 + constructor = this.getClass().getDeclaredConstructor(String.class); + // call to forbidden setAccessible + constructor.setAccessible(true); + + Method privateMethod = this.getClass().getDeclaredMethod("aPrivateMethod"); + // call to forbidden setAccessible + privateMethod.setAccessible(true); + + // deliberate accessibility alteration + String privateField = AccessController.doPrivileged(new PrivilegedAction() { + @Override + public String run() { + try { + Field field = Violation.class.getDeclaredField("aPrivateField"); + field.setAccessible(true); + return (String) field.get(null); + } catch (ReflectiveOperationException | SecurityException e) { + throw new RuntimeException(e); + } + } + }); + } +} + ]]> + + + + Detect calls to setAccessible + 9 + 9,12,13,16,19,20,23,26,27 + constructor : this.getClass().getConstructors()) { + constructor.setAccessible(true); + } + Constructor[] constructors = this.getClass().getConstructors(); + AccessibleObject.setAccessible(constructors, true); + Constructor.setAccessible(constructors, true); + + for (Method method : this.getClass().getMethods()) { + method.setAccessible(true); + } + Method[] methods = this.getClass().getMethods(); + AccessibleObject.setAccessible(methods, true); + Method.setAccessible(methods, true); + + for (Field field : this.getClass().getFields()) { + field.setAccessible(true); + } + Field[] fields = this.getClass().getFields(); + AccessibleObject.setAccessible(fields, true); + Field.setAccessible(fields, true); + } +} + ]]> + + + + + Make sure to detect method call chains + 6 + 3,4,5,6,7,8 + + + + + Anonymous privileged action is OK + 0 + () { + @Override + public Method[] run() { + Method[] declaredMethods = Violation.class.getDeclaredMethods(); + AccessibleObject.setAccessible(declaredMethods, true); + return declaredMethods; + } + }); + try { + methods[0].invoke(null); + } catch (ReflectiveOperationException e) { + e.printStackTrace(); + } + } +} + ]]> + + + + Inner class privileged action is OK + 0 + { + @Override + public Field[] run() { + Field[] declaredFields = Violation.class.getDeclaredFields(); + AccessibleObject.setAccessible(declaredFields, true); + return declaredFields; + } + } +} + ]]> + + + + false positive when accessible object is used as primary prefix + 0 + > list = new ArrayList<>(); + Constructor ctor = NoViolation.class.getConstructor(); + list.add(ctor); +} } + ]]> + + + + setAccessible(false) is ok + 0 + constructor = this.getClass().getDeclaredConstructor(String.class); + // call to setAccessible with false - that's ok + constructor.setAccessible(false); + + Constructor[] constructors = this.getClass().getConstructors(); + AccessibleObject.setAccessible(constructors, false); + Constructor.setAccessible(constructors, false); + } +} + ]]> + + \ No newline at end of file diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloneThrowsCloneNotSupportedException.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloneThrowsCloneNotSupportedException.xml deleted file mode 100644 index f4e4da306f..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/CloneThrowsCloneNotSupportedException.xml +++ /dev/null @@ -1,51 +0,0 @@ - - - - - ok, throws CloneNotSupportedException - 0 - - - - - bad - 1 - - - - - final class, rule does not apply - 0 - - - - - testing with multiple methods - 1 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/MissingSerialVersionUID.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/MissingSerialVersionUID.xml index e88e57ea14..3da6fa9c53 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/MissingSerialVersionUID.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/MissingSerialVersionUID.xml @@ -17,6 +17,8 @@ public class Foo { Simple failure case 1 diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ReturnEmptyArrayRatherThanNull.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ReturnEmptyArrayRatherThanNull.xml deleted file mode 100644 index dbdbc79bb5..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/ReturnEmptyArrayRatherThanNull.xml +++ /dev/null @@ -1,36 +0,0 @@ - - - - - Basic test case - 1 - - - - - good behavior should not trigger violation - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseEqualsToCompareStrings.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseEqualsToCompareStrings.xml index 934090bf33..d535631744 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseEqualsToCompareStrings.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/errorprone/xml/UseEqualsToCompareStrings.xml @@ -144,6 +144,8 @@ public class O { #2979 UseEqualsToCompareStrings: FP with "var" variables 0 #2979 UseEqualsToCompareStrings: FP with "var" variables (control, types are explicit) 0 + + + False positive with string concatentation + 0 + + diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidUsingShortType.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidUsingShortType.xml deleted file mode 100644 index 95eecb27da..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/AvoidUsingShortType.xml +++ /dev/null @@ -1,144 +0,0 @@ - - - - - Short as field - 1 - - - - - Short as local variable - 1 - - - - - Short as method return type - 2 - - - - - Short as method return type - 0 - - - - - #1449 false positive when casting a variable to short - 0 - - - - - short as method parameter - 1 - - - - - short as method parameter with @Override - 0 - - - - - [java] AvoidUsingShortType erroneously triggered on overrides of 3rd party methods (anon. class) #586 - 0 - - - - - [java] AvoidUsingShortType erroneously triggered on overrides of 3rd party methods #586 - 0 - - - - - short as annotation property - 1 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/BooleanInstantiation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/BooleanInstantiation.xml deleted file mode 100644 index 4d38c36a64..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/BooleanInstantiation.xml +++ /dev/null @@ -1,163 +0,0 @@ - - - - - simple failure case - 1 - - - - - new java.lang.Boolean - 1 - - - - - ok - 0 - - - - - don't use Boolean.valueOf() with literal - 2 - - - - - valueOf() with variable is fine - 0 - - - - - don't use Boolean.valueOf() with string literal - 1 - - - - - don't use Boolean.valueOf() in method call - 1 - - - - - don't use new Boolean() in method call - 1 - - - - - ok - 0 - - - - - ok - 0 - - - - - don't use new Boolean() in static block - 1 - - - - - Bug 1744065, should be ok - 0 - - - - - Test for failure after rule with custom Boolean, should report failure if rule reset done correctly - 1 - - - - - #1533 [java] BooleanInstantiation: ClassCastException with Annotation - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/ByteInstantiation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/ByteInstantiation.xml deleted file mode 100644 index 2cbd7a2b44..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/ByteInstantiation.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - - new Byte(), bad - 1 - - - - - Byte.valueOf(), ok - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/IntegerInstantiation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/IntegerInstantiation.xml deleted file mode 100644 index 208c229141..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/IntegerInstantiation.xml +++ /dev/null @@ -1,26 +0,0 @@ - - - - - new Integer(), bad - 1 - - - - - Integer.valueOf(), ok - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/LongInstantiation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/LongInstantiation.xml deleted file mode 100644 index 154dc5c2ab..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/LongInstantiation.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - - new Long(), bad - 1 - - - - - Long.valueOf(), ok - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/ShortInstantiation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/ShortInstantiation.xml deleted file mode 100644 index 22f52ea6a1..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/ShortInstantiation.xml +++ /dev/null @@ -1,30 +0,0 @@ - - - - - new Short(), bad - 1 - - - - - Short.valueOf(), ok - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/SimplifyStartsWith.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/SimplifyStartsWith.xml deleted file mode 100644 index 3a67cfa8b9..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/SimplifyStartsWith.xml +++ /dev/null @@ -1,90 +0,0 @@ - - - - - failure case - 1 - - - - - startsWith multiple chars - 0 - - - - - startsWith defined on some other class, doesn't take a String - 0 - - - - - Document Jaxen exception parsing Unicode chars in startsWith - 0 - - - - - #1392 SimplifyStartsWith false-negative - 1 - 5 - - - - #2712 SimplifyStartsWith false-positive on receiver != String - 0 - - - diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UnnecessaryWrapperObjectCreation.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UnnecessaryWrapperObjectCreation.xml deleted file mode 100644 index 1ca86628de..0000000000 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/performance/xml/UnnecessaryWrapperObjectCreation.xml +++ /dev/null @@ -1,87 +0,0 @@ - - - - - failure case - 1 - 3 - - - - - calling valueOf is OK - 0 - - - - - failure case for 1.5+ - 1 - 3 - - - - - Patch 2075906: Add toString() to the rule UnnecessaryWrapperObjectCreation - 1 - 3 - - - - - #1057 False positive for UnnecessaryWrapperObjectCreation - 1 - 3 - - - - - NPE with static method call - 0 - - - diff --git a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/cpd/test/CpdTextComparisonTest.kt b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/cpd/test/CpdTextComparisonTest.kt index 9ea953c086..2a09f7874f 100644 --- a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/cpd/test/CpdTextComparisonTest.kt +++ b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/cpd/test/CpdTextComparisonTest.kt @@ -46,8 +46,8 @@ abstract class CpdTextComparisonTest( */ @JvmOverloads fun doTest(fileBaseName: String, expectedSuffix: String = "", properties: Properties = defaultProperties()) { - super.doTest(fileBaseName, expectedSuffix) { sourceText -> - val sourceCode = SourceCode(SourceCode.StringCodeLoader(sourceText, "$fileBaseName$extensionIncludingDot")) + super.doTest(fileBaseName, expectedSuffix) { fileData -> + val sourceCode = SourceCode(SourceCode.StringCodeLoader(fileData.fileText, fileData.fileName)) val tokens = Tokens().also { val tokenizer = newTokenizer(properties) tokenizer.tokenize(sourceCode, it) @@ -58,10 +58,18 @@ abstract class CpdTextComparisonTest( } @JvmOverloads - fun expectTokenMgrError(source: String, properties: Properties = defaultProperties()): TokenMgrError = - shouldThrow { - newTokenizer(properties).tokenize(sourceCodeOf(source), Tokens()) - } + fun expectTokenMgrError( + source: String, + fileName: String = SourceCode.StringCodeLoader.DEFAULT_NAME, + properties: Properties = defaultProperties() + ): TokenMgrError = + expectTokenMgrError(FileData(fileName, source), properties) + + @JvmOverloads + fun expectTokenMgrError(fileData: FileData, properties: Properties = defaultProperties()): TokenMgrError = + shouldThrow { + newTokenizer(properties).tokenize(sourceCodeOf(fileData), Tokens()) + } private fun StringBuilder.format(tokens: Tokens) { @@ -140,11 +148,13 @@ abstract class CpdTextComparisonTest( fun sourceCodeOf(str: String): SourceCode = SourceCode(SourceCode.StringCodeLoader(str)) + fun sourceCodeOf(fileData: FileData): SourceCode = + SourceCode(SourceCode.StringCodeLoader(fileData.fileText, fileData.fileName)) fun tokenize(tokenizer: Tokenizer, str: String): Tokens = - Tokens().also { - tokenizer.tokenize(sourceCodeOf(str), it) - } + Tokens().also { + tokenizer.tokenize(sourceCodeOf(str), it) + } private companion object { const val Indent = " " diff --git a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt index 6e4453d7e1..f5d70c0767 100644 --- a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt +++ b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseParsingHelper.kt @@ -60,7 +60,8 @@ abstract class BaseParsingHelper, T : RootNode fun getVersion(version: String?): LanguageVersion { val language = language return if (version == null) language.defaultVersion - else language.getVersion(version) ?: throw AssertionError("Unsupported version $version for language $language") + else language.getVersion(version) + ?: throw AssertionError("Unsupported version $version for language $language") } val language: Language @@ -116,13 +117,17 @@ abstract class BaseParsingHelper, T : RootNode * so. */ @JvmOverloads - fun parse(sourceCode: String, version: String? = null, filename: String = FileAnalysisException.NO_FILE_NAME): T { + open fun parse( + sourceCode: String, + version: String? = null, + fileName: String = "src/a/test-file-name.${language.extensions[0]}" + ): T { val lversion = if (version == null) defaultVersion else getVersion(version) val handler = lversion.languageVersionHandler val parser = handler.parser - val source = DataSource.forString(sourceCode, filename) + val source = DataSource.forString(sourceCode, fileName) val toString = DataSource.readToString(source, StandardCharsets.UTF_8) // this removed the BOM - val task = Parser.ParserTask(lversion, filename, toString, SemanticErrorReporter.noop()) + val task = Parser.ParserTask(lversion, fileName, toString, SemanticErrorReporter.noop()) task.properties.also { handler.declareParserTaskProperties(it) it.setProperty(Parser.ParserTask.COMMENT_MARKER, params.suppressMarker) @@ -165,14 +170,14 @@ abstract class BaseParsingHelper, T : RootNode */ @JvmOverloads open fun parseResource(resource: String, version: String? = null): T = - parse(readResource(resource), version) + parse(readResource(resource), version, fileName = resource) /** * Fetches and [parse]s the [path]. */ @JvmOverloads open fun parseFile(path: Path, version: String? = null): T = - parse(IOUtils.toString(Files.newBufferedReader(path)), version, filename = path.toAbsolutePath().toString()) + parse(IOUtils.toString(Files.newBufferedReader(path)), version, fileName = path.toAbsolutePath().toString()) /** * Fetches the source of the given [clazz]. @@ -220,7 +225,11 @@ abstract class BaseParsingHelper, T : RootNode * found by the rule. The language version of the piece of code is determined by the [params]. */ @JvmOverloads - fun executeRule(rule: Rule, code: String, filename: String = "testfile.${language.extensions[0]}"): Report { + fun executeRule( + rule: Rule, + code: String, + fileName: String = "testfile.${language.extensions[0]}" + ): Report { val config = PMDConfiguration().apply { suppressMarker = params.suppressMarker } @@ -228,7 +237,7 @@ abstract class BaseParsingHelper, T : RootNode val ctx = RuleContext() val report = Report() ctx.report = report - ctx.sourceCodeFile = File(filename) + ctx.sourceCodeFile = File(fileName) ctx.isIgnoreExceptions = false val rules = RuleSet.forSingleRule(rule) @@ -242,6 +251,4 @@ abstract class BaseParsingHelper, T : RootNode fun executeRuleOnResource(rule: Rule, resourcePath: String): Report = executeRule(rule, readResource(resourcePath)) - - } diff --git a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseTreeDumpTest.kt b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseTreeDumpTest.kt index 2201b384a8..49af1e58ac 100644 --- a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseTreeDumpTest.kt +++ b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/lang/ast/test/BaseTreeDumpTest.kt @@ -32,9 +32,13 @@ abstract class BaseTreeDumpTest( */ @JvmOverloads fun doTest(fileBaseName: String, parser: BaseParsingHelper<*, *> = this.parser) { - super.doTest(fileBaseName, "") { sourceText -> + super.doTest(fileBaseName, "") { fileData -> buildString { - printer.renderSubtree(parser.parse(sourceText), this) + val ast = parser.parse( + sourceCode = fileData.fileText, + fileName = fileData.fileName + ) + printer.renderSubtree(ast, this) } } } diff --git a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/test/BaseTextComparisonTest.kt b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/test/BaseTextComparisonTest.kt index b7a0cc9816..cb45c4ed41 100644 --- a/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/test/BaseTextComparisonTest.kt +++ b/pmd-lang-test/src/main/kotlin/net/sourceforge/pmd/test/BaseTextComparisonTest.kt @@ -27,6 +27,8 @@ abstract class BaseTextComparisonTest { /** Extension that the unparsed source file is supposed to have. */ protected abstract val extensionIncludingDot: String + data class FileData(val fileName:String, val fileText:String) + /** * Executes the test. The test files are looked up using the [parser]. * The reference test file must be named [fileBaseName] + [ExpectedExt]. @@ -37,7 +39,7 @@ abstract class BaseTextComparisonTest { */ internal fun doTest(fileBaseName: String, expectedSuffix: String = "", - transformTextContent: (String) -> String) { + transformTextContent: (FileData) -> String) { val expectedFile = findTestFile(resourceLoader, "${resourcePrefix}/$fileBaseName$expectedSuffix$ExpectedExt").toFile() val actual = transformTextContent(sourceText(fileBaseName)) @@ -56,7 +58,7 @@ abstract class BaseTextComparisonTest { assertEquals(expected.normalize(), actual.normalize(), "File comparison failed, see the reference: $expectedFile") } - protected fun sourceText(fileBaseName: String): String { + protected fun sourceText(fileBaseName: String): FileData { val sourceFile = findTestFile(resourceLoader, "${resourcePrefix}/$fileBaseName$extensionIncludingDot").toFile() assert(sourceFile.isFile) { @@ -64,7 +66,7 @@ abstract class BaseTextComparisonTest { } val sourceText = sourceFile.readText(Charsets.UTF_8).normalize() - return sourceText + return FileData(fileName = sourceFile.toString(), fileText = sourceText) } protected open fun String.normalize() = replace( diff --git a/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/SwiftLanguageModule.java b/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/SwiftLanguageModule.java index e36a3c0af7..6dff8fbf56 100644 --- a/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/SwiftLanguageModule.java +++ b/pmd-swift/src/main/java/net/sourceforge/pmd/lang/swift/SwiftLanguageModule.java @@ -21,6 +21,6 @@ public class SwiftLanguageModule extends BaseLanguageModule { */ public SwiftLanguageModule() { super(NAME, null, TERSE_NAME, "swift"); - addVersion("", new SwiftHandler(), true); + addDefaultVersion("", new SwiftHandler()); } } diff --git a/pmd-xml/src/test/java/net/sourceforge/pmd/lang/xml/rule/XmlXPathRuleTest.java b/pmd-xml/src/test/java/net/sourceforge/pmd/lang/xml/rule/XmlXPathRuleTest.java new file mode 100644 index 0000000000..ef22aa88d8 --- /dev/null +++ b/pmd-xml/src/test/java/net/sourceforge/pmd/lang/xml/rule/XmlXPathRuleTest.java @@ -0,0 +1,39 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.xml.rule; + +import static net.sourceforge.pmd.lang.ast.test.TestUtilsKt.assertSize; + +import org.junit.Test; + +import net.sourceforge.pmd.Report; +import net.sourceforge.pmd.lang.LanguageRegistry; +import net.sourceforge.pmd.lang.rule.XPathRule; +import net.sourceforge.pmd.lang.rule.xpath.XPathVersion; +import net.sourceforge.pmd.lang.xml.XmlLanguageModule; +import net.sourceforge.pmd.lang.xml.XmlParsingHelper; + +public class XmlXPathRuleTest { + + final XmlParsingHelper xml = XmlParsingHelper.XML; + + private XPathRule makeXPath(String expression) { + XPathRule rule = new XPathRule(XPathVersion.XPATH_2_0, expression); + rule.setLanguage(LanguageRegistry.getLanguage(XmlLanguageModule.NAME)); + rule.setMessage("XPath Rule Failed"); + return rule; + } + + + @Test + public void testFileNameInXpath() { + Report report = xml.executeRule(makeXPath("//b[pmd:fileName() = 'Foo.xml']"), + "", + "src/Foo.xml"); + + assertSize(report, 1); + } + +} diff --git a/pom.xml b/pom.xml index 8a4ac7022f..83fa78759f 100644 --- a/pom.xml +++ b/pom.xml @@ -76,7 +76,7 @@ - 2021-07-31T17:02:07Z + 2021-08-28T15:27:18Z 8 @@ -95,7 +95,7 @@ 8.42 3.1.2 3.14.0 - 1.10.9 + 1.10.11 3.2.0 4.8 @@ -105,7 +105,7 @@ -Xmx512m -Dfile.encoding=${project.build.sourceEncoding} - 15 + 16 6.37.0 ${settings.localRepository}/net/java/dev/javacc/javacc/${javacc.version}/javacc-${javacc.version}.jar @@ -412,15 +412,16 @@ + net.sourceforge.pmd pmd-core - 6.37.0 + 6.38.0 net.sourceforge.pmd pmd-java - 6.37.0 + 6.38.0 @@ -1055,33 +1056,6 @@ - - - pmd-dogfood - - ${project.version} - - - - - org.apache.maven.plugins - maven-pmd-plugin - - - net.sourceforge.pmd - pmd-core - ${pmd.dogfood.version} - - - net.sourceforge.pmd - pmd-java - ${pmd.dogfood.version} - - - - - -