diff --git a/.ci/build.sh b/.ci/build.sh index 40379d57a7..0f5d26a996 100755 --- a/.ci/build.sh +++ b/.ci/build.sh @@ -85,6 +85,23 @@ function build() { pmd_ci_log_group_end 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 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 + pmd_ci_log_group_end + else + # current maven-pmd-plugin is not compatible with PMD 7 yet. + pmd_ci_log_info "Skipping PMD dogfood test with ${PMD_CI_MAVEN_PROJECT_VERSION}" + fi + pmd_ci_log_group_start "Executing build with sonar" # Note: Sonar also needs GITHUB_TOKEN (!) ./mvnw \ diff --git a/.ci/inc/regression-tester.inc b/.ci/inc/regression-tester.inc index 4fd2ceeee0..ebdbea3398 100644 --- a/.ci/inc/regression-tester.inc +++ b/.ci/inc/regression-tester.inc @@ -70,19 +70,21 @@ function regression_tester_uploadBaseline() { function regression_tester_executeDanger() { pmd_ci_log_debug "${FUNCNAME[0]}" - # Create a corresponding remote branch locally - if ! git show-ref --verify --quiet "refs/heads/${PMD_CI_BRANCH}"; then - git fetch --no-tags --depth=1 origin "+refs/heads/${PMD_CI_BRANCH}:refs/remotes/origin/${PMD_CI_BRANCH}" - git branch "${PMD_CI_BRANCH}" "origin/${PMD_CI_BRANCH}" - pmd_ci_log_debug "Created local branch ${PMD_CI_BRANCH}" - fi - # Fetch more commits of the PR for danger and regression tester - git fetch --no-tags --depth=50 origin "+$(git rev-parse HEAD^2):" - # Fetch more commits from master branch for regression tester - if [[ "${PMD_CI_BRANCH}" != "master" ]]; then - git fetch --no-tags --depth=50 origin +master: - git branch master origin/master - fi + # git clone initially only fetched with depth 2. Danger and regression tester + # need more history, so we'll fetch more here + # and create local branches as well (${PMD_CI_BRANCH} and pr-fetch) + + pmd_ci_log_info "Fetching 25 commits for ${PMD_CI_BRANCH} and pull/${PMD_CI_PULL_REQUEST_NUMBER}/head" + git fetch --no-tags --depth=25 origin "${PMD_CI_BRANCH}:${PMD_CI_BRANCH}" "pull/${PMD_CI_PULL_REQUEST_NUMBER}/head:pr-fetch" + + # if the PR is older, base might have advanced more than 25 commits... fetch more, up to 150 + for i in $(seq 1 3); do + if [ -z "$( git merge-base "${PMD_CI_BRANCH}" "pr-fetch" )" ]; then + pmd_ci_log_info "No merge-base yet - fetching more commits... (try $i)" + git fetch --no-tags --deepen=50 origin "${PMD_CI_BRANCH}:" "pull/${PMD_CI_PULL_REQUEST_NUMBER}/head:pr-fetch" + fi + done + pmd_ci_log_info "Merge base is: $( git merge-base "${PMD_CI_BRANCH}" "pr-fetch" )" pmd_ci_log_info "Running danger on branch ${PMD_CI_BRANCH}" bundle exec danger --verbose diff --git a/Gemfile.lock b/Gemfile.lock index b1ef397852..873b5a9c67 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,7 +1,7 @@ GEM remote: https://rubygems.org/ specs: - addressable (2.7.0) + addressable (2.8.0) public_suffix (>= 2.0.2, < 5.0) claide (1.0.3) claide-plugins (0.9.2) @@ -34,7 +34,7 @@ GEM fugit (1.5.0) et-orbi (~> 1.1, >= 1.1.8) raabro (~> 1.4) - git (1.8.1) + git (1.9.1) rchardet (~> 1.8) kramdown (1.17.0) liquid (5.0.1) @@ -62,7 +62,7 @@ GEM racc (1.5.2) rchardet (1.8.0) rouge (3.26.0) - rufus-scheduler (3.7.0) + rufus-scheduler (3.8.0) fugit (~> 1.1, >= 1.1.6) safe_yaml (1.0.5) sawyer (0.8.2) diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock index 36eb9740c2..396fbb24f2 100644 --- a/docs/Gemfile.lock +++ b/docs/Gemfile.lock @@ -1,13 +1,13 @@ GEM remote: https://rubygems.org/ specs: - activesupport (6.0.3.7) + activesupport (6.0.4) concurrent-ruby (~> 1.0, >= 1.0.2) i18n (>= 0.7, < 2) minitest (~> 5.1) tzinfo (~> 1.1) zeitwerk (~> 2.2, >= 2.2.2) - addressable (2.7.0) + addressable (2.8.0) public_suffix (>= 2.0.2, < 5.0) coffee-script (2.4.1) coffee-script-source @@ -16,8 +16,8 @@ GEM colorator (1.1.0) commonmarker (0.17.13) ruby-enum (~> 0.5) - concurrent-ruby (1.1.8) - dnsruby (1.61.5) + concurrent-ruby (1.1.9) + dnsruby (1.61.7) simpleidn (~> 0.1) em-websocket (0.5.2) eventmachine (>= 0.12.9) @@ -26,20 +26,28 @@ GEM ffi (>= 1.15.0) eventmachine (1.2.7) execjs (2.8.1) - faraday (1.4.1) + faraday (1.5.1) + faraday-em_http (~> 1.0) + faraday-em_synchrony (~> 1.0) faraday-excon (~> 1.1) + faraday-httpclient (~> 1.0.1) faraday-net_http (~> 1.0) faraday-net_http_persistent (~> 1.1) + faraday-patron (~> 1.0) multipart-post (>= 1.2, < 3) ruby2_keywords (>= 0.0.4) + faraday-em_http (1.0.0) + faraday-em_synchrony (1.0.0) faraday-excon (1.1.0) + faraday-httpclient (1.0.1) faraday-net_http (1.0.1) - faraday-net_http_persistent (1.1.0) - ffi (1.15.0) + faraday-net_http_persistent (1.2.0) + faraday-patron (1.0.0) + ffi (1.15.3) forwardable-extended (2.6.0) gemoji (3.0.1) - github-pages (214) - github-pages-health-check (= 1.17.0) + github-pages (215) + github-pages-health-check (= 1.17.2) jekyll (= 3.9.0) jekyll-avatar (= 0.7.0) jekyll-coffeescript (= 1.1.1) @@ -82,7 +90,7 @@ GEM nokogiri (>= 1.10.4, < 2.0) rouge (= 3.26.0) terminal-table (~> 1.4) - github-pages-health-check (1.17.0) + github-pages-health-check (1.17.2) addressable (~> 2.3) dnsruby (~> 1.60) octokit (~> 4.0) @@ -209,14 +217,14 @@ GEM rb-fsevent (~> 0.10, >= 0.10.3) rb-inotify (~> 0.9, >= 0.9.10) mercenary (0.3.6) - mini_portile2 (2.5.1) + mini_portile2 (2.5.3) minima (2.5.1) jekyll (>= 3.5, < 5.0) jekyll-feed (~> 0.9) jekyll-seo-tag (~> 2.1) minitest (5.14.4) multipart-post (2.1.1) - nokogiri (1.11.5) + nokogiri (1.11.7) mini_portile2 (~> 2.5.0) racc (~> 1.4) octokit (4.21.0) @@ -234,7 +242,7 @@ GEM ruby-enum (0.9.0) i18n ruby2_keywords (0.0.4) - rubyzip (2.3.0) + rubyzip (2.3.2) safe_yaml (1.0.5) sass (3.7.4) sass-listen (~> 4.0.0) diff --git a/docs/pages/pmd/userdocs/tools/tools.md b/docs/pages/pmd/userdocs/tools/tools.md index b3e8d550c9..daa007bba1 100644 --- a/docs/pages/pmd/userdocs/tools/tools.md +++ b/docs/pages/pmd/userdocs/tools/tools.md @@ -20,6 +20,18 @@ With Codacy you have PMDJava analysis out-of-the-box, and it is free for open so * Source code: [https://github.com/codacy/codacy-pmdjava](https://github.com/codacy/codacy-pmdjava) * Maintainer: Codacy +### Code Inspector + +[Code Inspector](https://www.code-inspector.com) automates code review, check your code quality and helps you manage your technical debt. +It is integrated with GitHub, GitLab and Bitbucket. The platform also analyzes code directly in your IDE using its integration +plugins for VS Code and IntelliJ, providing a consistent analysis along your development cycle (from the IDE to the CI/CD pipeline). + +Code Inspector uses PMD to check Java and Apex code. + +* Homepage: [https://www.code-inspector.com](https://www.code-inspector.com) +* Documentation: [https://doc.code-inspector.com](https://doc.code-inspector.com) + + ## IDE Integrations ### Summary diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 1f4595d9d0..6c62f4a564 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,22 +14,77 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy +#### New rules + +This release ships with 2 new Java rules. + +* {% rule java/bestpractices/SimplifiableTestAssertion %} suggests rewriting + some test assertions to be more readable. + +```xml + +``` + + The rule is part of the quickstart.xml ruleset. + +* {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} suggests returning empty collections / arrays + instead of null. + +```xml + +``` + + The rule is part of the quickstart.xml ruleset. + +#### Renamed rules + +* The Java rule {% rule java/errorprone/MissingBreakInSwitch %} has been renamed to + {% rule java/errorprone/ImplicitSwitchFallThrough %} (category error prone) to better reflect the rule's + purpose: The rule finds implicit fall-through cases in switch statements, which are most + likely unexpected. The old rule name described only one way how to avoid a fall-through, + namely using `break` but `continue`, `throw` and `return` avoid a fall-through + as well. This enables us to improve this rule in the future. + +#### Deprecated rules + +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 %} + +The rule {% rule java/errorprone/ReturnEmptyArrayRatherThanNull %} is deprecated and removed from +the quickstart ruleset, as the new rule {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} +supersedes it. + + ### Fixed Issues * apex + * [#3201](https://github.com/pmd/pmd/issues/3201): \[apex] ApexCRUDViolation doesn't report Database class DMLs, inline no-arg object instantiations and inline list initialization * [#3329](https://github.com/pmd/pmd/issues/3329): \[apex] ApexCRUDViolation doesn't report SOQL for loops * core + * [#1603](https://github.com/pmd/pmd/issues/1603): \[core] Language version comparison * [#3377](https://github.com/pmd/pmd/issues/3377): \[core] NPE when specifying report file in current directory in PMD CLI * [#3387](https://github.com/pmd/pmd/issues/3387): \[core] CPD should avoid unnecessary copies when running with --skip-lexical-errors * java-bestpractices + * [#2908](https://github.com/pmd/pmd/issues/2908): \[java] Merge Junit assertion simplification rules * [#3235](https://github.com/pmd/pmd/issues/3235): \[java] UseTryWithResources false positive when closeable is provided as a method argument or class field +* java-errorprone + * [#3361](https://github.com/pmd/pmd/issues/3361): \[java] Rename rule MissingBreakInSwitch to ImplicitSwitchFallThrough + * [#3382](https://github.com/pmd/pmd/pull/3382): \[java] New rule ReturnEmptyCollectionRatherThanNull ### API Changes ### External Contributions * [#3367](https://github.com/pmd/pmd/pull/3367): \[apex] Check SOQL CRUD on for loops - [Jonathan Wiesel](https://github.com/jonathanwiesel) +* [#3373](https://github.com/pmd/pmd/pull/3373): \[apex] Add ApexCRUDViolation support for database class, inline no-arg object construction DML and inline list initialization DML - [Jonathan Wiesel](https://github.com/jonathanwiesel) * [#3385](https://github.com/pmd/pmd/pull/3385): \[core] CPD: Optimize --skip-lexical-errors option - [Woongsik Choi](https://github.com/woongsikchoi) +* [#3388](https://github.com/pmd/pmd/pull/3388): \[doc] Add Code Inspector in the list of tools - [Julien Delange](https://github.com/juli1) {% endtocmaker %} diff --git a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java index 4be3586849..e8fdc1641d 100644 --- a/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java +++ b/pmd-apex/src/main/java/net/sourceforge/pmd/lang/apex/rule/internal/Helper.java @@ -20,6 +20,7 @@ import net.sourceforge.pmd.lang.apex.ast.ASTFieldDeclaration; import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; import net.sourceforge.pmd.lang.apex.ast.ASTModifierNode; import net.sourceforge.pmd.lang.apex.ast.ASTNewKeyValueObjectExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTNewObjectExpression; import net.sourceforge.pmd.lang.apex.ast.ASTParameter; import net.sourceforge.pmd.lang.apex.ast.ASTReferenceExpression; import net.sourceforge.pmd.lang.apex.ast.ASTSoqlExpression; @@ -164,6 +165,13 @@ public final class Helper { return sb.toString(); } + public static String getFQVariableName(final ASTNewObjectExpression variable) { + StringBuilder sb = new StringBuilder() + .append(variable.getDefiningType()).append(":") + .append(variable.getType()); + return sb.toString(); + } + public static boolean isSystemLevelClass(ASTUserClass node) { List interfaces = node.getInterfaceNames(); return interfaces.stream().anyMatch(Helper::isAllowed); 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 20a8bb539c..b1713937f5 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 @@ -33,6 +33,9 @@ import net.sourceforge.pmd.lang.apex.ast.ASTIfElseBlockStatement; import net.sourceforge.pmd.lang.apex.ast.ASTMethod; import net.sourceforge.pmd.lang.apex.ast.ASTMethodCallExpression; import net.sourceforge.pmd.lang.apex.ast.ASTNewKeyValueObjectExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTNewListInitExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTNewListLiteralExpression; +import net.sourceforge.pmd.lang.apex.ast.ASTNewObjectExpression; import net.sourceforge.pmd.lang.apex.ast.ASTProperty; import net.sourceforge.pmd.lang.apex.ast.ASTReferenceExpression; import net.sourceforge.pmd.lang.apex.ast.ASTReturnStatement; @@ -114,7 +117,33 @@ public class ApexCRUDViolationRule extends AbstractApexRule { @Override public Object visit(ASTMethodCallExpression node, Object data) { - collectCRUDMethodLevelChecks(node); + if (Helper.isAnyDatabaseMethodCall(node)) { + + switch (node.getMethodName().toLowerCase(Locale.ROOT)) { + case "insert": + checkForCRUD(node, data, IS_CREATEABLE); + break; + case "update": + checkForCRUD(node, data, IS_UPDATEABLE); + break; + case "delete": + checkForCRUD(node, data, IS_DELETABLE); + break; + case "upsert": + checkForCRUD(node, data, IS_CREATEABLE); + checkForCRUD(node, data, IS_UPDATEABLE); + break; + case "merge": + checkForCRUD(node, data, IS_MERGEABLE); + break; + default: + break; + } + + } else { + collectCRUDMethodLevelChecks(node); + } + return data; } @@ -353,11 +382,8 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return; } - final ASTNewKeyValueObjectExpression newObj = node.getFirstChildOfType(ASTNewKeyValueObjectExpression.class); - if (newObj != null) { - final String type = Helper.getFQVariableName(newObj); - validateCRUDCheckPresent(node, data, crudMethod, type); - } + checkInlineObject(node, data, crudMethod); + checkInlineNonArgsObject(node, data, crudMethod); final ASTVariableExpression variable = node.getFirstChildOfType(ASTVariableExpression.class); if (variable != null) { @@ -369,6 +395,36 @@ public class ApexCRUDViolationRule extends AbstractApexRule { validateCRUDCheckPresent(node, data, crudMethod, typeCheck.toString()); } } + + final ASTNewListLiteralExpression inlineListLiteral = node.getFirstChildOfType(ASTNewListLiteralExpression.class); + if (inlineListLiteral != null) { + checkInlineObject(inlineListLiteral, data, crudMethod); + checkInlineNonArgsObject(inlineListLiteral, data, crudMethod); + } + + final ASTNewListInitExpression inlineListInit = node.getFirstChildOfType(ASTNewListInitExpression.class); + if (inlineListInit != null) { + checkInlineObject(inlineListInit, data, crudMethod); + checkInlineNonArgsObject(inlineListInit, data, crudMethod); + } + } + + private void checkInlineObject(final ApexNode node, final Object data, final String crudMethod) { + + final ASTNewKeyValueObjectExpression newObj = node.getFirstChildOfType(ASTNewKeyValueObjectExpression.class); + if (newObj != null) { + final String type = Helper.getFQVariableName(newObj); + validateCRUDCheckPresent(node, data, crudMethod, type); + } + } + + private void checkInlineNonArgsObject(final ApexNode node, final Object data, final String crudMethod) { + + final ASTNewObjectExpression newEmptyObj = node.getFirstChildOfType(ASTNewObjectExpression.class); + if (newEmptyObj != null) { + final String type = Helper.getFQVariableName(newEmptyObj); + validateCRUDCheckPresent(node, data, crudMethod, type); + } } private Set getPreviousMethodCalls(final ApexNode self) { diff --git a/pmd-apex/src/main/resources/category/apex/security.xml b/pmd-apex/src/main/resources/category/apex/security.xml index de42b89592..5f14367b0d 100644 --- a/pmd-apex/src/main/resources/category/apex/security.xml +++ b/pmd-apex/src/main/resources/category/apex/security.xml @@ -58,7 +58,7 @@ public class Foo { Contact c = [SELECT Status__c FROM Contact WHERE Id=:ID WITH SECURITY_ENFORCED]; // Make sure we can update the database before even trying - if (!Schema.sObjectType.Contact.fields.Name.isUpdateable()) { + if (!Schema.sObjectType.Contact.fields.Status__c.isUpdateable()) { return null; } 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 8087ac9eb6..6cd0a03809 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 @@ -134,6 +134,32 @@ public class Foo { ]]> + + Proper CRUD,FLS via upsert with database class + 0 + + + + + No CRUD check for inline upsert with database class + 2 + + + VF built-in CRUD checks via getter, but cannot determine if is really VF 2 @@ -600,6 +626,40 @@ public class Foo { ]]> + + No CRUD,FLS check for update with database class + 2 + + + + + Proper CRUD,FLS check for update with database class + 0 + + + No CRUD check for insert 1 @@ -628,6 +688,34 @@ public class Foo { ]]> + + No CRUD check for insert with database class + 1 + + + + + Proper CRUD check for insert with database class + 0 + + + No CRUD check for delete 2 @@ -658,6 +746,36 @@ public class Foo { ]]> + + No CRUD check for delete with database class + 2 + + + + + Proper CRUD check for delete with database class + 0 + + + Proper CRUD check for a list of sObject 0 @@ -924,7 +1042,7 @@ public class Foo { public class Foo { void bar() { for (Account a : [SELECT Id FROM Account]) { - + } } } @@ -957,6 +1075,84 @@ public class Foo { } } } +} + ]]> + + + + No CRUD,FLS for inline no-args object + 1 + + + + + Proper CRUD,FLS for inline no-args object + 0 + + + + + No CRUD,FLS for inline literal list + 1 + + + + + Proper CRUD,FLS for inline literal list + 0 + + + + + No CRUD,FLS for inline initialized list + 1 + (new Account(Name = 'X')); + } +} + ]]> + + + + Proper CRUD,FLS for inline initialized list + 0 + (new Account(Name = 'X')); + } + } } ]]> diff --git a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersion.java b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersion.java index dad6165693..015f4d7f7c 100644 --- a/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersion.java +++ b/pmd-core/src/main/java/net/sourceforge/pmd/lang/LanguageVersion.java @@ -4,6 +4,8 @@ package net.sourceforge.pmd.lang; +import java.util.List; + import net.sourceforge.pmd.Rule; import net.sourceforge.pmd.annotation.InternalApi; @@ -102,27 +104,28 @@ public class LanguageVersion implements Comparable { @Override public int compareTo(LanguageVersion o) { - if (o == null) { - return 1; - } + List versions = language.getVersions(); + int thisPosition = versions.indexOf(this); + int otherPosition = versions.indexOf(o); + return Integer.compare(thisPosition, otherPosition); + } - int comp = getName().compareTo(o.getName()); - if (comp != 0) { - return comp; - } - - String[] vals1 = getName().split("\\."); - String[] vals2 = o.getName().split("\\."); - int i = 0; - while (i < vals1.length && i < vals2.length && vals1[i].equals(vals2[i])) { - i++; - } - if (i < vals1.length && i < vals2.length) { - int diff = Integer.valueOf(vals1[i]).compareTo(Integer.valueOf(vals2[i])); - return Integer.signum(diff); - } else { - return Integer.signum(vals1.length - vals2.length); + /** + * Compare this version to another version of the same language identified + * by the given version string. + * + * @param versionString The version with which to compare + * + * @throws IllegalArgumentException If the argument is not a valid version + * string for the parent language + */ + public int compareToVersion(String versionString) { + LanguageVersion otherVersion = language.getVersion(versionString); + if (otherVersion == null) { + throw new IllegalArgumentException( + "No such version '" + versionString + "' for language " + language.getName()); } + return this.compareTo(otherVersion); } @Override diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionRule.java new file mode 100644 index 0000000000..18583f425a --- /dev/null +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionRule.java @@ -0,0 +1,327 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import java.util.HashSet; +import java.util.Set; + +import net.sourceforge.pmd.lang.java.ast.ASTArgumentList; +import net.sourceforge.pmd.lang.java.ast.ASTBooleanLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; +import net.sourceforge.pmd.lang.java.ast.ASTEqualityExpression; +import net.sourceforge.pmd.lang.java.ast.ASTExpression; +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.ASTNullLiteral; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; +import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; +import net.sourceforge.pmd.lang.java.ast.ASTPrimarySuffix; +import net.sourceforge.pmd.lang.java.ast.ASTUnaryExpressionNotPlusMinus; +import net.sourceforge.pmd.lang.java.ast.JavaNode; +import net.sourceforge.pmd.lang.java.ast.TypeNode; +import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; +import net.sourceforge.pmd.lang.java.types.TypeTestUtil; + +/** + * + */ +public class SimplifiableTestAssertionRule extends AbstractJavaRule { + + private final Set importedMethodsHere = new HashSet<>(); + private boolean allAssertionsOn; + + @Override + public Object visit(ASTCompilationUnit node, Object data) { + importedMethodsHere.clear(); + allAssertionsOn = false; + for (ASTImportDeclaration importDecl : node.findChildrenOfType(ASTImportDeclaration.class)) { + if (importDecl.isStatic()) { + if (importDecl.isImportOnDemand()) { + if (isAssertionContainer(importDecl.getImportedName())) { + // import static org.junit.Assert.* + allAssertionsOn = true; + } + } else { + checkImportedAssertion(importDecl.getImportedName()); + } + } + } + + super.visit(node, data); + return null; + } + + @Override + public Object visit(ASTPrimaryExpression node, Object data) { + final boolean isAssertTrue = isAssertionCall(node, "assertTrue"); + final boolean isAssertFalse = isAssertionCall(node, "assertFalse"); + + if (isAssertTrue || isAssertFalse) { + ASTArgumentList args = getNonEmptyArgList(node); + JavaNode lastArg = getChildRev(args, -1); + ASTEqualityExpression eq = asEqualityExpr(lastArg); + if (eq != null) { + boolean isPositive = isPositiveEqualityExpr(eq) == isAssertTrue; + final String suggestion; + if (isNullLiteral(eq.getChild(0)) + || isNullLiteral(eq.getChild(1))) { + // use assertNull/assertNonNull + suggestion = isPositive ? "assertNull" : "assertNonNull"; + } else { + if (isPrimitive(eq.getChild(0)) || isPrimitive(eq.getChild(1))) { + suggestion = isPositive ? "assertEquals" : "assertNotEquals"; + } else { + suggestion = isPositive ? "assertSame" : "assertNotSame"; + } + } + addViolation(data, node, suggestion); + + } else { + JavaNode negatedExprOperand = getNegatedExprOperand(lastArg); // nullable + + if (isCall(negatedExprOperand, "equals")) { + //assertTrue(!a.equals(b)) + String suggestion = isAssertTrue ? "assertNotEquals" : "assertEquals"; + addViolation(data, node, suggestion); + + } else if (negatedExprOperand != null) { + //assertTrue(!something) + String suggestion = isAssertTrue ? "assertFalse" : "assertTrue"; + addViolation(data, node, suggestion); + + } else if (isCall(lastArg, "equals")) { + //assertTrue(a.equals(b)) + String suggestion = isAssertTrue ? "assertEquals" : "assertNotEquals"; + addViolation(data, node, suggestion); + } + } + } + + boolean isAssertEquals = isAssertionCall(node, "assertEquals"); + boolean isAssertNotEquals = isAssertionCall(node, "assertNotEquals"); + + if (isAssertEquals || isAssertNotEquals) { + ASTArgumentList argList = getNonEmptyArgList(node); + if (argList != null && argList.size() >= 2) { + JavaNode comp0 = getChildRev(argList, -1); + JavaNode comp1 = getChildRev(argList, -2); + if (isBooleanLiteral(comp0) ^ isBooleanLiteral(comp1)) { + if (isBooleanLiteral(comp1)) { + JavaNode tmp = comp0; + comp0 = comp1; + comp1 = tmp; + } + // now the literal is in comp0 and the other is some expr + if (comp1 instanceof TypeNode && TypeTestUtil.isA(boolean.class, (TypeNode) comp1)) { + ASTBooleanLiteral literal = (ASTBooleanLiteral) unwrapLiteral(comp0); + String suggestion = literal.isTrue() == isAssertEquals ? "assertTrue" : "assertFalse"; + addViolation(data, node, suggestion); + } + } + } + } + + return super.visit(node, data); + } + + private boolean isPrimitive(JavaNode node) { + if (node instanceof TypeNode) { + Class t0 = ((TypeNode) node).getType(); + return t0 != null && t0.isPrimitive(); + } + return false; + } + + /** + * Returns a child with an offset from the end. Eg {@code getChildRev(list, -1)} + * returns the last child. + */ + private static JavaNode getChildRev(JavaNode list, int i) { + assert i < 0 : "Expecting negative offset"; + return list == null ? null : list.getChild(list.getNumChildren() + i); + } + + /** + * Checks if the node is a call to a method which has the given name. + * The receiver expression may be arbitrarily complicated. + */ + private static boolean isCall(JavaNode node, String methodName) { + if (node instanceof ASTExpression) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return false; + } + } + if (!(node instanceof ASTPrimaryExpression) || node.getNumChildren() < 2) { + return false; + } + + + JavaNode prefix = getChildRev(node, -2); + JavaNode suffix = getChildRev(node, -1); + if (!(suffix instanceof ASTPrimarySuffix) || !((ASTPrimarySuffix) suffix).isArguments()) { + return false; + } + // we know it's a method call + if (prefix instanceof ASTPrimaryPrefix + && prefix.getNumChildren() > 0 + && prefix.getChild(0) instanceof ASTName) { + String image = prefix.getChild(0).getImage(); + return isPossiblyQualifiedMethodName(methodName, image); + } else if (prefix instanceof ASTPrimarySuffix) { + // call chain + return methodName.equals(prefix.getImage()); + } + + return false; + } + + private boolean isAssertionCall(JavaNode node, String methodName) { + if (node instanceof ASTExpression) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return false; + } + } + if (node.getNumChildren() != 2 || !isCall(node, methodName)) { + return false; // not a call chain + } + + ASTPrimaryPrefix prefix = (ASTPrimaryPrefix) getChildRev(node, -2); + return isAssertionMethodName(methodName, (ASTName) prefix.getChild(0)); + } + + private static boolean isPossiblyQualifiedMethodName(String methodName, String possiblyQualifiedName) { + return methodName.equals(possiblyQualifiedName) + || possiblyQualifiedName.length() > methodName.length() + && possiblyQualifiedName.endsWith(methodName) + && possiblyQualifiedName.charAt(possiblyQualifiedName.length() - methodName.length() - 1) == '.'; + } + + private boolean isAssertionMethodName(String methodName, ASTName location) { + String possiblyQualifiedName = location.getImage(); + if (methodName.equals(possiblyQualifiedName)) { + return allAssertionsOn + || importedMethodsHere.contains(methodName) + || TypeTestUtil.isA("junit.framework.TestCase", location.getFirstParentOfType(ASTClassOrInterfaceDeclaration.class)); + } + if (possiblyQualifiedName.length() > methodName.length() + && possiblyQualifiedName.endsWith(methodName) + && possiblyQualifiedName.charAt(possiblyQualifiedName.length() - methodName.length() - 1) == '.') { + return TypeTestUtil.isA("org.junit.jupiter.api.Assertions", location) + || TypeTestUtil.isA("org.junit.Assert", location); + } + return false; + } + + + private /*nullable*/ ASTArgumentList getNonEmptyArgList(ASTPrimaryExpression node) { + ASTPrimarySuffix suffix = node.getFirstChildOfType(ASTPrimarySuffix.class); + if (suffix != null && suffix.isArguments() && suffix.getArgumentCount() > 0) { + return (ASTArgumentList) suffix.getChild(0).getChild(0); + } + return null; + } + + private ASTEqualityExpression asEqualityExpr(JavaNode node) { + if (node instanceof ASTExpression) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return null; + } + } + return node instanceof ASTEqualityExpression ? (ASTEqualityExpression) node + : null; + } + + private boolean isPositiveEqualityExpr(ASTEqualityExpression node) { + return node != null && node.getOperator().equals("=="); + } + + private static JavaNode getNegatedExprOperand(JavaNode node) { + // /Expression/UnaryExpressionNotPlusMinus[@Image='!'] + // /PrimaryExpression/PrimaryPrefix + if (node instanceof ASTExpression) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return null; + } + } + if (node instanceof ASTUnaryExpressionNotPlusMinus + && "!".equals(((ASTUnaryExpressionNotPlusMinus) node).getOperator())) { + return node.getChild(0); + } + return null; + } + + private static boolean isNullLiteral(JavaNode node) { + return unwrapLiteral(node) instanceof ASTNullLiteral; + } + + private static boolean isBooleanLiteral(JavaNode node) { + return unwrapLiteral(node) instanceof ASTBooleanLiteral; + } + + private static JavaNode unwrapLiteral(JavaNode node) { + if (node instanceof ASTExpression) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return null; + } + } + if (node instanceof ASTPrimaryExpression) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return null; + } + } + if (node instanceof ASTPrimaryPrefix) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return null; + } + } + if (node instanceof ASTLiteral) { + if (node.getNumChildren() == 1) { + node = node.getChild(0); + } else { + return null; + } + } + return node; + } + + + private boolean isAssertionContainer(String importedName) { + return "org.junit.jupiter.api.Assertions".equals(importedName) + || "org.junit.Assert".equals(importedName); + } + + private void checkImportedAssertion(String importedName) { + String stripped = removePrefixOrNull(importedName, "org.junit.jupiter.api.Assertions."); + if (stripped == null) { + stripped = removePrefixOrNull(importedName, "org.junit.Assert."); + } + if (stripped != null && stripped.indexOf('.') == -1) { + importedMethodsHere.add(stripped); + } + } + + private static String removePrefixOrNull(String str, String prefix) { + if (str.startsWith(prefix)) { + return str.substring(prefix.length()); + } + return null; + } +} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BigIntegerInstantiationRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BigIntegerInstantiationRule.java index b2627c00b8..71c8a0da39 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BigIntegerInstantiationRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/performance/BigIntegerInstantiationRule.java @@ -8,9 +8,7 @@ import java.math.BigDecimal; import java.math.BigInteger; import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.JavaLanguageModule; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTArguments; import net.sourceforge.pmd.lang.java.ast.ASTArrayDimsAndInits; @@ -34,8 +32,7 @@ public class BigIntegerInstantiationRule extends AbstractJavaRule { return super.visit(node, data); } - boolean jdk15 = ((RuleContext) data).getLanguageVersion() - .compareTo(LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getVersion("1.5")) >= 0; + boolean jdk15 = ((RuleContext) data).getLanguageVersion().compareToVersion("1.5") >= 0; if ((TypeTestUtil.isA(BigInteger.class, (ASTClassOrInterfaceType) type) || jdk15 && TypeTestUtil.isA(BigDecimal.class, (ASTClassOrInterfaceType) type)) && !node.hasDescendantOfType(ASTArrayDimsAndInits.class)) { 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 index dc94d7ea03..be232bce09 100644 --- 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 @@ -7,9 +7,7 @@ package net.sourceforge.pmd.lang.java.rule.performance; import java.util.Set; import net.sourceforge.pmd.RuleContext; -import net.sourceforge.pmd.lang.LanguageRegistry; import net.sourceforge.pmd.lang.ast.Node; -import net.sourceforge.pmd.lang.java.JavaLanguageModule; import net.sourceforge.pmd.lang.java.ast.ASTName; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryExpression; import net.sourceforge.pmd.lang.java.ast.ASTPrimaryPrefix; @@ -36,8 +34,7 @@ public class UnnecessaryWrapperObjectCreationRule extends AbstractJavaRule { image = image.substring(10); } - boolean checkBoolean = ((RuleContext) data).getLanguageVersion() - .compareTo(LanguageRegistry.getLanguage(JavaLanguageModule.NAME).getVersion("1.5")) >= 0; + boolean checkBoolean = ((RuleContext) data).getLanguageVersion().compareToVersion("1.5") >= 0; if (PREFIX_SET.contains(image) || checkBoolean && "Boolean.valueOf".equals(image)) { ASTPrimaryExpression parent = (ASTPrimaryExpression) node.getParent(); diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 334dc62c30..fa1178f211 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1364,6 +1364,50 @@ 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 + + + + + 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 @@ -1740,10 +1787,13 @@ public class FooTest extends TestCase { message="Use assertNull(x) instead of assertTrue(x==null), or assertNotNull(x) vs assertFalse(x==null)" class="net.sourceforge.pmd.lang.rule.XPathRule" typeResolution="true" + deprecated="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useassertnullinsteadofasserttrue"> 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 @@ -1794,10 +1844,13 @@ public class FooTest extends TestCase { message="Use assertSame(x, y) instead of assertTrue(x==y), or assertNotSame(x,y) vs assertFalse(x==y)" class="net.sourceforge.pmd.lang.rule.XPathRule" typeResolution="true" + deprecated="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useassertsameinsteadofasserttrue"> 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 @@ -1845,9 +1898,12 @@ public class FooTest extends TestCase { since="5.0" message="Use assertTrue(x)/assertFalse(x) instead of assertEquals(true, x)/assertEquals(false, x) or assertEquals(Boolean.TRUE, x)/assertEquals(Boolean.FALSE, x)." class="net.sourceforge.pmd.lang.rule.XPathRule" + deprecated="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_bestpractices.html#useasserttrueinsteadofassertequals"> 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 diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index 7ffbe17a27..d1e326ceb3 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -1327,6 +1327,7 @@ public class Foo { message="assertTrue(!expr) can be replaced by assertFalse(expr)" class="net.sourceforge.pmd.lang.rule.XPathRule" typeResolution="true" + deprecated="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_design.html#simplifybooleanassertion"> Avoid negation in an assertTrue or assertFalse test. @@ -1339,6 +1340,7 @@ as: assertFalse(expr); +Deprecated since PMD 6.37.0, use {% rule java/bestpractices/SimplifiableTestAssertion %} instead. 3 diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 21140359c7..a92d8216e9 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -2130,6 +2130,60 @@ public class Foo { + + +Switch statements without break or return statements for each case option +may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through. + +This rule has been renamed from "MissingBreakInSwitch" with PMD 6.37.0. + + 3 + + + + + + + + + + + + + - - -Switch statements without break or return statements for each case option -may indicate problematic behaviour. Empty cases are ignored as these indicate an intentional fall-through. - - 3 - - - - - - - - - - - - + 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 @@ -2898,6 +2905,54 @@ public class Example { + + +For any method that returns an collection (such as an array, Collection or Map), it is better to return +an empty one rather than a null reference. This removes the need for null checking all results and avoids +inadvertent NullPointerExceptions. + +See Effective Java, 3rd Edition, Item 54: Return empty collections or arrays instead of null + + 1 + + + + + + + + + + + + + --> + @@ -48,10 +49,6 @@ - - - - @@ -155,7 +152,6 @@ - @@ -227,6 +223,7 @@ + @@ -235,7 +232,6 @@ - @@ -245,7 +241,7 @@ - + diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/JavaLanguageModuleTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/JavaLanguageModuleTest.java new file mode 100644 index 0000000000..12bb3b0b5c --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/JavaLanguageModuleTest.java @@ -0,0 +1,51 @@ +/* + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java; + +import java.util.List; + +import org.junit.Assert; +import org.junit.Test; + +import net.sourceforge.pmd.lang.Language; +import net.sourceforge.pmd.lang.LanguageRegistry; +import net.sourceforge.pmd.lang.LanguageVersion; + +public class JavaLanguageModuleTest { + private Language javaLanguage = LanguageRegistry.getLanguage(JavaLanguageModule.NAME); + + @Test + public void java9IsSmallerThanJava10() { + LanguageVersion java9 = javaLanguage.getVersion("9"); + LanguageVersion java10 = javaLanguage.getVersion("10"); + + Assert.assertTrue("java9 should be smaller than java10", java9.compareTo(java10) < 0); + } + + @Test + public void previewVersionShouldBeGreaterThanNonPreview() { + LanguageVersion java16 = javaLanguage.getVersion("16"); + LanguageVersion java16p = javaLanguage.getVersion("16-preview"); + + Assert.assertTrue("java16-preview should be greater than java16", java16p.compareTo(java16) > 0); + } + + @Test + public void testCompareToVersion() { + LanguageVersion java9 = javaLanguage.getVersion("9"); + Assert.assertTrue("java9 should be smaller than java10", java9.compareToVersion("10") < 0); + } + + @Test + public void allVersions() { + List versions = javaLanguage.getVersions(); + for (int i = 1; i < versions.size(); i++) { + LanguageVersion previous = versions.get(i - 1); + LanguageVersion current = versions.get(i); + Assert.assertTrue("Version " + previous + " should be smaller than " + current, + previous.compareTo(current) < 0); + } + } +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionTest.java new file mode 100644 index 0000000000..cea609a1ff --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/bestpractices/SimplifiableTestAssertionTest.java @@ -0,0 +1,11 @@ +/** + * BSD-style license; for more info see http://pmd.sourceforge.net/license.html + */ + +package net.sourceforge.pmd.lang.java.rule.bestpractices; + +import net.sourceforge.pmd.testframework.PmdRuleTst; + +public class SimplifiableTestAssertionTest extends PmdRuleTst { + // no additional unit tests +} diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingBreakInSwitchTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ImplicitSwitchFallThroughTest.java similarity index 78% rename from pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingBreakInSwitchTest.java rename to pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ImplicitSwitchFallThroughTest.java index c78f69909e..f7bf9c8ab3 100644 --- a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/MissingBreakInSwitchTest.java +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ImplicitSwitchFallThroughTest.java @@ -6,6 +6,6 @@ package net.sourceforge.pmd.lang.java.rule.errorprone; import net.sourceforge.pmd.testframework.PmdRuleTst; -public class MissingBreakInSwitchTest extends PmdRuleTst { +public class ImplicitSwitchFallThroughTest extends PmdRuleTst { // no additional unit tests } diff --git a/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnEmptyCollectionRatherThanNullTest.java b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnEmptyCollectionRatherThanNullTest.java new file mode 100644 index 0000000000..87265d8934 --- /dev/null +++ b/pmd-java/src/test/java/net/sourceforge/pmd/lang/java/rule/errorprone/ReturnEmptyCollectionRatherThanNullTest.java @@ -0,0 +1,12 @@ +/** + * 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; + +public class ReturnEmptyCollectionRatherThanNullTest extends PmdRuleTst { + + // no additional unit tests +} diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SimplifiableTestAssertion.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SimplifiableTestAssertion.xml new file mode 100644 index 0000000000..85966d420b --- /dev/null +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/SimplifiableTestAssertion.xml @@ -0,0 +1,662 @@ + + + + + UseAssertNullInsteadOfAssertTrue: assertTrue with null + 1 + + Assertion may be simplified using assertNull + + + + + + UseAssertNullInsteadOfAssertTrue: assertFalse with != null + 1 + + Assertion may be simplified using assertNull + + + + + + UseAssertNullInsteadOfAssertTrue: assertTrue with x == y + 1 + + Assertion may be simplified using assertSame + + + + + + UseAssertNullInsteadOfAssertTrue: Not a JUnit test - assertTrue with null + 0 + + + + + UseAssertNullInsteadOfAssertTrue: JUnit 4 - assertTrue with null + 1 + + + + + UseAssertNullInsteadOfAssertTrue: JUnit 5 - assertTrue with null - @Test + 1 + + + + + UseAssertNullInsteadOfAssertTrue: JUnit 5 - Assertions.assertTrue + 1 + + + + + + + UseAssertSameInsteadOfAssertTrue: assert true a == b + 1 + + + + + UseAssertSameInsteadOfAssertTrue: assert true a != b + 1 + + + + + UseAssertSameInsteadOfAssertTrue: assert false a == b + 1 + + + + + UseAssertSameInsteadOfAssertTrue: assert false a != b + 1 + + + + + UseAssertSameInsteadOfAssertTrue: bug 1626715, the null check in the rule shouldn't match the null outside the assert method + 1 + + Assertion may be simplified using assertNotSame + + + + + + UseAssertSameInsteadOfAssertTrue: assert true a == b BUT not a Junit test + 0 + + + + + UseAssertSameInsteadOfAssertTrue: JUnit 4 - assert true a == b + 1 + + + + Do not FP if we don't know where the method is coming from + 0 + + + + + UseAssertSameInsteadOfAssertTrue: JUnit 5 - assert true a == b - @Test + 1 + + + + UseAssertSameInsteadOfAssertTrue: JUnit 5 - assert true a == b - with qualifier + 1 + + + + + + + assertFalse(!) + 1 + + + + + assertTrue(!) + 1 + + + + + ok + 0 + + + + + not a JUnit test - assertFalse(!) + 0 + + + + + JUnit 4 - assertFalse(!) + 1 + + + + + JUnit 5 - assertFalse(!) + 1 + + + + + Use assert equals: positive test 1 + 1 + + Assertion may be simplified using assertEquals + + + + + + Use assert equals: positive test 2 + 1 + + Assertion may be simplified using assertNotEquals + + + + + + Use assert equals: positive test 3 + 1 + + Assertion may be simplified using assertEquals + + + + + Use assert equals: positive test 4 + 1 + + Assertion may be simplified using assertNotEquals + + + + + + Use assert equals: negative test + 0 + + + + + Use assert equals: Not a JUnit test + 0 + + + + + Use assert equals: JUnit4 + 1 + + + + + Use assert equals: JUnit5 - @Test + 1 + + Assertion may be simplified using assertEquals + + + + + + + JUnit Test contains assertEquals on other than boolean literal + 0 + + + + + JUnit Test contains assertEquals on boolean literal + 5 + + + + JUnit Test contains assertEquals on boolean with other thing not bool + 0 + + + + + #1323 False positive case of UseAssertTrueInsteadOfAssertEquals + 0 + + + + + JUnit Test contains assertEquals with Boxed booleans + 0 + + + + + + JUnit Test contains assertEquals with Boxed booleans as param + 0 + + + + + AssertEquals with call chain and not operator + 2 + + Assertion may be simplified using assertNotEquals + Assertion may be simplified using assertEquals + + + + + assertSame is not applicable on primitives + 2 + + Assertion may be simplified using assertNotSame + Assertion may be simplified using assertNotEquals + + + + + + 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 index 992f5403b5..5d14d01ce6 100644 --- 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 @@ -5,7 +5,7 @@ xsi:schemaLocation="http://pmd.sourceforge.net/rule-tests http://pmd.sourceforge.net/rule-tests_1_0_0.xsd"> - TEST1 + use assert equals 0 + + + Returning null array + 1 + + + + Nonnull empty array + 0 + + + + Returning null instead of collection (List) + 1 + 5 + bar() + { + // ... + return null; + } +} + ]]> + + + Returning proper empty collection + 0 + bar() + { + // ... + return Collections.emptyList(); + } +} + ]]> + + + Returning null instead of collection (Set) + 1 + 5 + bar() + { + // ... + return null; + } +} + ]]> + + + Returning null instead of collection (Map) + 1 + 5 + bar() + { + // ... + return null; + } +} + ]]> + + diff --git a/pom.xml b/pom.xml index b8db4fc7f7..cd97aac5cb 100644 --- a/pom.xml +++ b/pom.xml @@ -105,7 +105,7 @@ -Xmx512m -Dfile.encoding=${project.build.sourceEncoding} - 14 + 15-SNAPSHOT 6.27.0 @@ -1048,6 +1048,33 @@ + + + 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} + + + + + +