From 0253929cc5c98691efbffcf809b0951680f45767 Mon Sep 17 00:00:00 2001 From: Tarush Singh Date: Tue, 29 Nov 2022 17:05:33 +0530 Subject: [PATCH 01/31] user mode and System mode with test cases added --- .../rule/security/ApexCRUDViolationRule.java | 21 ++++++++++ .../rule/security/xml/ApexCRUDViolation.xml | 39 +++++++++++++++++++ 2 files changed, 60 insertions(+) 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 9891cc52e5..810d297b0c 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 @@ -94,6 +94,11 @@ public class ApexCRUDViolationRule extends AbstractApexRule { private static final Pattern WITH_SECURITY_ENFORCED = Pattern.compile("(?is).*[^']\\s*WITH\\s+SECURITY_ENFORCED\\s*[^']*"); + //Added For USER MODE + private static final Pattern WITH_USER_MODE = Pattern.compile("(?is).*[^']\\s+USER_MODE\\s*[^']*"); + //Added For SYSTEM MODE + private static final Pattern WITH_SYSTEM_MODE = Pattern.compile("(?is).*[^']\\s+SYSTEM_MODE\\s*[^']*"); + // AuthMethodPattern config properties; these are string properties instead of regex properties to help // ensure that the compiled patterns are case-insensitive vs. requiring the pattern author to use "(?i)" private static final PropertyDescriptor CREATE_AUTH_METHOD_PATTERN_DESCRIPTOR = authMethodPatternProperty("create"); @@ -435,6 +440,22 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return false; } + //For USER_MODE + private boolean isWithUserMode(final ApexNode node) { + if (node instanceof ASTSoqlExpression) { + return WITH_USER_MODE.matcher(((ASTSoqlExpression) node).getQuery()).matches(); + } + return false; + } + + //For System Mode + private boolean isWithSystemMode(final ApexNode node) { + if (node instanceof ASTSoqlExpression) { + return WITH_SYSTEM_MODE.matcher(((ASTSoqlExpression) node).getQuery()).matches(); + } + return false; + } + private String getType(final ASTMethodCallExpression methodNode) { final ASTReferenceExpression reference = methodNode.getFirstChildOfType(ASTReferenceExpression.class); if (reference.getNames().size() > 0) { 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 fbf529e30f..db3f48a392 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 @@ -376,6 +376,45 @@ public class Foo { ]]> + + Flagged as Critical With No Mode + 0 + + + + + User Mode + 0 + + + + + System Mode, gives warning because it ignores CRUD but explicitly + 0 + + + Proper accessibility CRUD,FLS 0 From 1ce646a535b02b2ea524a9163c3e77c5a3e92a5b Mon Sep 17 00:00:00 2001 From: Tarush Singh Date: Thu, 1 Dec 2022 16:17:11 +0530 Subject: [PATCH 02/31] check added for User and system mode in validateCRUDCheckPresent --- .../lang/apex/rule/security/ApexCRUDViolationRule.java | 10 ++++++++++ .../lang/apex/rule/security/xml/ApexCRUDViolation.xml | 7 +++++-- 2 files changed, 15 insertions(+), 2 deletions(-) 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 810d297b0c..336facdb90 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 @@ -657,12 +657,22 @@ public class ApexCRUDViolationRule extends AbstractApexRule { boolean isImproperDMLCheck = !isProperESAPICheckForDML(typeCheck, crudMethod) && !isProperAuthPatternBasedCheckForDML(typeCheck, crudMethod); boolean noSecurityEnforced = !isWithSecurityEnforced(node); + boolean userMode = isWithUserMode(node); + boolean systemMode = isWithSystemMode(node); if (missingKey) { //if condition returns true, add violation, otherwise return. if (isImproperDMLCheck && noSecurityEnforced) { addViolation(data, node); return true; } + if (isImproperDMLCheck && userMode) { + addViolation(data, node); + return true; + } + if (isImproperDMLCheck && systemMode) { + addViolation(data, node); + return true; + } } else { boolean properChecksHappened = false; 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 db3f48a392..9058b4140f 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 @@ -378,7 +378,7 @@ public class Foo { Flagged as Critical With No Mode - 0 + 1 System Mode, gives warning because it ignores CRUD but explicitly - 0 + 1 + + This CRUD statement uses explicit system mode + Date: Thu, 1 Dec 2022 16:49:42 +0530 Subject: [PATCH 03/31] check added for User and system mode in validateCRUDCheckPresent --- .../pmd/lang/apex/rule/security/ApexCRUDViolationRule.java | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) 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 336facdb90..99d45f2082 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 @@ -665,11 +665,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { addViolation(data, node); return true; } - if (isImproperDMLCheck && userMode) { - addViolation(data, node); - return true; - } - if (isImproperDMLCheck && systemMode) { + if (isImproperDMLCheck && !userMode && !systemMode) { addViolation(data, node); return true; } From bce0331fe789b63c815f9edbc9971651ea4a1504 Mon Sep 17 00:00:00 2001 From: Tarush Singh Date: Thu, 1 Dec 2022 16:51:43 +0530 Subject: [PATCH 04/31] check added for User and system mode in validateCRUDCheckPresent --- .../pmd/lang/apex/rule/security/ApexCRUDViolationRule.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) 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 99d45f2082..e172265e43 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 @@ -666,7 +666,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return true; } if (isImproperDMLCheck && !userMode && !systemMode) { - addViolation(data, node); + addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode"); return true; } } else { From efd83665dc36f99201ab3f2559b1af7bedae5a31 Mon Sep 17 00:00:00 2001 From: Tarush Singh Date: Thu, 1 Dec 2022 17:52:48 +0530 Subject: [PATCH 05/31] Simple change in voilation check --- .../pmd/lang/apex/rule/security/ApexCRUDViolationRule.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) 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 e172265e43..eb15790534 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 @@ -666,7 +666,8 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return true; } if (isImproperDMLCheck && !userMode && !systemMode) { - addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode"); + //addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode"); + addViolation(data, node); return true; } } else { From a3b95bec47d4d8e447f554e2896db50370bad811 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 1 Dec 2022 17:31:00 +0100 Subject: [PATCH 06/31] [java] DoNotUseThreads: Fix duplicated violations reporting Fixes #4210 --- docs/pages/release_notes.md | 2 ++ .../category/java/multithreading.xml | 8 +++++++- .../multithreading/xml/DoNotUseThreads.xml | 20 +++++++++++++++++++ 3 files changed, 29 insertions(+), 1 deletion(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..8f2e90f453 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -15,6 +15,8 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy ### Fixed Issues +* java-multithreading + * [#4210](https://github.com/pmd/pmd/issues/4210): \[java] DoNotUseThreads report duplicate warnings ### API Changes diff --git a/pmd-java/src/main/resources/category/java/multithreading.xml b/pmd-java/src/main/resources/category/java/multithreading.xml index c01e70971a..81cd0ab632 100644 --- a/pmd-java/src/main/resources/category/java/multithreading.xml +++ b/pmd-java/src/main/resources/category/java/multithreading.xml @@ -148,7 +148,7 @@ public class ThrDeux { The J2EE specification explicitly forbids the use of threads. Threads are resources, that should be managed and monitored by the J2EE server. If the application creates threads on its own or uses own custom thread pools, then these threads are not managed, which could lead to resource exhaustion. -Also EJB's might be moved between machines in a cluster and only managed resources can be moved along. +Also, EJBs might be moved between machines in a cluster and only managed resources can be moved along. 3 @@ -157,6 +157,12 @@ Also EJB's might be moved between machines in a cluster and only managed resourc diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/multithreading/xml/DoNotUseThreads.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/multithreading/xml/DoNotUseThreads.xml index 60dd425d0a..0018cb0d7b 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/multithreading/xml/DoNotUseThreads.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/multithreading/xml/DoNotUseThreads.xml @@ -16,6 +16,7 @@ public class UsingThread { extending threads is not allowed 1 + 2 Implementing ExecutorService is not allowed 1 + 3 Extending AbstractExecutorService is not allowed 1 + 3 Using ExecutorService is not allowed 2 + 6,7 Using Executors directly is not allowed 2 + 5,10 + + + [java] DoNotUseThreads report duplicate warnings #4210 + 2 + 2,4 + + From eb7c2e5468af5f926240b3c97992e86373e4d109 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 1 Dec 2022 21:45:54 +0100 Subject: [PATCH 07/31] [java] DoNotUseThreads: Fix false negatives with field declarations --- .../category/java/multithreading.xml | 4 +-- .../multithreading/xml/DoNotUseThreads.xml | 32 +++++++++++++++++++ 2 files changed, 34 insertions(+), 2 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/multithreading.xml b/pmd-java/src/main/resources/category/java/multithreading.xml index 81cd0ab632..41f199bf69 100644 --- a/pmd-java/src/main/resources/category/java/multithreading.xml +++ b/pmd-java/src/main/resources/category/java/multithreading.xml @@ -157,8 +157,8 @@ Also, EJBs might be moved between machines in a cluster and only managed resourc + + + + False negatives with field declarations + 10 + 5,6,7,8,11,15,16,19,20,23 + System.out.println("foo")).start(); // violation expected + Thread t2 = new Thread(); // one violation expected + } + + public Thread getThread() { // violation expected + return new Thread(); // violation expected + } + + private static class MyThread extends Thread { } // violation expected +} ]]> From a377f07b9c1da5139e986b7f7940650f00820d0c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Cl=C3=A9ment=20Fournier?= Date: Wed, 7 Dec 2022 11:14:40 +0100 Subject: [PATCH 08/31] Fix doc showing deprecated CLI switches --- docs/pages/pmd/languages/java.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/docs/pages/pmd/languages/java.md b/docs/pages/pmd/languages/java.md index 29f24c02a7..477406fa08 100644 --- a/docs/pages/pmd/languages/java.md +++ b/docs/pages/pmd/languages/java.md @@ -35,6 +35,10 @@ In order to analyze a project with PMD that uses preview language features, you' it via the environment variable `PMD_JAVA_OPTS` and select the new language version, e.g. `19-preview`: export PMD_JAVA_OPTS=--enable-preview - ./run.sh pmd -language java -version 19-preview ... + ./run.sh pmd --use-version java-19-preview ... Note: we only support preview language features for the latest two java versions. + +Note: `--use-version` is only supported since PMD 6.52.0. Older versions of PMD use two CLI options that have to be specified together: `-language java -version 19-preview`. + + From 9f5bd42d439a8872f74041ca450f986c13b4b298 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 8 Dec 2022 11:41:02 +0100 Subject: [PATCH 09/31] Update gems Fixes https://github.com/pmd/pmd/security/dependabot/31 Fixes CVE-2022-23476 Fixes https://github.com/advisories/GHSA-qv4q-mr5r-qprj --- Gemfile.lock | 8 ++++---- docs/Gemfile.lock | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/Gemfile.lock b/Gemfile.lock index 5591103f89..b073c8cb4b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -53,7 +53,7 @@ GEM faraday-patron (1.0.0) faraday-rack (1.0.0) faraday-retry (1.0.3) - fugit (1.7.2) + fugit (1.8.0) et-orbi (~> 1, >= 1.2.7) raabro (~> 1.4) git (1.12.0) @@ -69,7 +69,7 @@ GEM multipart-post (2.2.3) nap (1.1.0) no_proxy_fix (0.1.2) - nokogiri (1.13.9) + nokogiri (1.13.10) mini_portile2 (~> 2.8.0) racc (~> 1.4) octokit (5.6.1) @@ -83,9 +83,9 @@ GEM nokogiri (~> 1.13) rufus-scheduler (~> 3.8) slop (~> 4.9) - public_suffix (5.0.0) + public_suffix (5.0.1) raabro (1.4.0) - racc (1.6.0) + racc (1.6.1) rchardet (1.8.0) rexml (3.2.5) rouge (4.0.0) diff --git a/docs/Gemfile.lock b/docs/Gemfile.lock index 0e2dca88c4..51dcd4846c 100644 --- a/docs/Gemfile.lock +++ b/docs/Gemfile.lock @@ -212,7 +212,7 @@ GEM jekyll-feed (~> 0.9) jekyll-seo-tag (~> 2.1) minitest (5.16.3) - nokogiri (1.13.9) + nokogiri (1.13.10) mini_portile2 (~> 2.8.0) racc (~> 1.4) octokit (4.25.1) @@ -221,7 +221,7 @@ GEM pathutil (0.16.2) forwardable-extended (~> 2.6) public_suffix (4.0.7) - racc (1.6.0) + racc (1.6.1) rb-fsevent (0.11.2) rb-inotify (0.10.1) ffi (~> 1.0) From 5a3ff840aa8c71936edc65b1948a5f4b780f82ff Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 8 Dec 2022 18:59:58 +0100 Subject: [PATCH 10/31] [java] UnusedPrivateField - add new property "annotations" Fixes #4166 --- docs/pages/release_notes.md | 9 +++++ .../bestpractices/UnusedPrivateFieldRule.java | 21 ++++++++++ .../bestpractices/xml/UnusedPrivateField.xml | 39 +++++++++++++++++++ 3 files changed, 69 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..8f93d11cb6 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,7 +14,16 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy +#### Modified rules + +* The Java rule {% rule java/bestpractices/UnusedPrivateField %} has a new property `annotations`. + This is a list of fully qualified names of the annotation types that should be reported anyway. If an unused field + has any of these annotations, then it is reported. If it has any other annotation, then it is still considered + to be used and is not reported. + ### Fixed Issues +* java-bestpractices + * [#4166](https://github.com/pmd/pmd/issues/4166): \[java] UnusedPrivateField doesn't find annotated unused private fields anymore ### API Changes diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java index 009bc3f486..41fc0ebc13 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java @@ -7,11 +7,13 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; import static net.sourceforge.pmd.properties.PropertyFactory.stringListProperty; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.logging.Logger; import net.sourceforge.pmd.PMDVersion; +import net.sourceforge.pmd.lang.java.ast.ASTAnnotation; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBody; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceBodyDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; @@ -28,6 +30,7 @@ import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractJavaRule; import net.sourceforge.pmd.lang.java.symboltable.JavaNameOccurrence; import net.sourceforge.pmd.lang.java.symboltable.VariableNameDeclaration; +import net.sourceforge.pmd.lang.java.types.TypeTestUtil; import net.sourceforge.pmd.lang.symboltable.NameDeclaration; import net.sourceforge.pmd.lang.symboltable.NameOccurrence; import net.sourceforge.pmd.properties.PropertyDescriptor; @@ -43,6 +46,7 @@ public class UnusedPrivateFieldRule extends AbstractJavaRule { + "This property has been deprecated since PMD 6.50.0 and will be completely ignored.") .defaultValue(new ArrayList()) .build(); + private static boolean warnedAboutDeprecatedIgnoredAnnotationsProperty = false; private static final PropertyDescriptor> IGNORED_FIELD_NAMES = PropertyFactory.stringListProperty("ignoredFieldNames") @@ -50,9 +54,18 @@ public class UnusedPrivateFieldRule extends AbstractJavaRule { .desc("Field Names that are ignored from the unused check") .build(); + private static final PropertyDescriptor> ANNOTATIONS_DESCRIPTOR + = stringListProperty("annotations") + .desc("Fully qualified names of the annotation types that should be reported anyway. If an unused field " + + "has any of these annotations, then it is reported. If it has any other annotation, then " + + "it is still considered to used and is not reported.") + .defaultValue(Arrays.asList("org.openqa.selenium.support.FindBy")) + .build(); + public UnusedPrivateFieldRule() { definePropertyDescriptor(IGNORED_ANNOTATIONS_DESCRIPTOR); definePropertyDescriptor(IGNORED_FIELD_NAMES); + definePropertyDescriptor(ANNOTATIONS_DESCRIPTOR); } @Override @@ -93,6 +106,14 @@ public class UnusedPrivateFieldRule extends AbstractJavaRule { } private boolean hasAnyAnnotation(Annotatable node) { + List annotations = node.getDeclaredAnnotations(); + for (String reportAnnotation : getProperty(ANNOTATIONS_DESCRIPTOR)) { + for (ASTAnnotation annotation : annotations) { + if (TypeTestUtil.isA(reportAnnotation, annotation)) { + return false; + } + } + } return !node.getDeclaredAnnotations().isEmpty(); } diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml index b773a97ba6..e51f15799a 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml @@ -767,6 +767,45 @@ public class C { @ToString.Include private int a; // Should not report a warning in this line } +]]> + + + + [java] UnusedPrivateField doesn't find annotated unused private fields anymore #4166 (default) + 1 + 6 + + + + + [java] UnusedPrivateField doesn't find annotated unused private fields anymore #4166 (default, other annotation) + 0 + + + + + [java] UnusedPrivateField doesn't find annotated unused private fields anymore #4166 (configuration) + java.lang.Deprecated + 1 + 3 + From 330594d51d38a50ec6c42881f3c46e0c4301f2e1 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 9 Dec 2022 15:23:56 +0100 Subject: [PATCH 11/31] [java] Deprecate rules ExcessiveClassLength and ExcessiveMethodLength Fixes #2127 --- docs/pages/release_notes.md | 9 ++++++++ .../rule/design/ExcessiveClassLengthRule.java | 3 +++ .../java/rule/design/ExcessiveLengthRule.java | 3 +++ .../design/ExcessiveMethodLengthRule.java | 3 +++ .../main/resources/category/java/design.xml | 22 +++++++++++++++++++ 5 files changed, 40 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..e03494d88d 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -14,8 +14,17 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy +#### Deprecated rules + +* The Java rules {% rule java/design/ExcessiveClassLength %} and {% rule java/design/ExcessiveMethodLength %} + have been deprecated. The rule {% rule java/design/NcssCount %} can be used instead. + The deprecated rules will be removed with PMD 7.0.0. + ### Fixed Issues +* java-design + * [#2127](https://github.com/pmd/pmd/issues/2127): \[java] Deprecate rules ExcessiveClassLength and ExcessiveMethodLength + ### API Changes ### External Contributions diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveClassLengthRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveClassLengthRule.java index 766dc252c6..fc53ab39ba 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveClassLengthRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveClassLengthRule.java @@ -9,7 +9,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; /** * This rule detects when a class exceeds a certain threshold. i.e. if a class * has more than 1000 lines of code. + * + * @deprecated Use {@link NcssCountRule} instead. */ +@Deprecated public class ExcessiveClassLengthRule extends ExcessiveLengthRule { public ExcessiveClassLengthRule() { super(ASTAnyTypeDeclaration.class); diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java index 7b20aedb6d..a012ab5c51 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java @@ -15,7 +15,10 @@ import net.sourceforge.pmd.stat.DataPoint; * *

To implement an ExcessiveLength rule, you pass in the Class of node you want * to check, and this does the rest for you.

+ * + * @deprecated This base class is not needed anymore and will be removed with PMD 7. */ +@Deprecated public class ExcessiveLengthRule extends AbstractStatisticalJavaRule { private Class nodeClass; diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveMethodLengthRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveMethodLengthRule.java index 86c654ec70..82a54d4053 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveMethodLengthRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveMethodLengthRule.java @@ -10,7 +10,10 @@ import net.sourceforge.pmd.lang.java.ast.ASTMethodOrConstructorDeclaration; /** * This rule detects when a method exceeds a certain threshold. i.e. if a method * has more than x lines of code. + * + * @deprecated Use {@link NcssCountRule} instead. */ +@Deprecated public class ExcessiveMethodLengthRule extends ExcessiveLengthRule { public ExcessiveMethodLengthRule() { super(ASTMethodOrConstructorDeclaration.class); diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index bf5a1289d8..4def9bbb52 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -675,6 +675,7 @@ public void bar() { @@ -682,6 +683,16 @@ public void bar() { Excessive class file lengths are usually indications that the class may be burdened with excessive responsibilities that could be provided by external classes or functions. In breaking these methods apart the code becomes more manageable and ripe for reuse. + +This rule is deprecated since PMD 6.53.0 and will be removed with PMD 7.0.0. + +The rule is based on the simple metric lines of code (LoC). The reasons for deprecation are: +* LoC is a noisy metric, NCSS (non-commenting source statements) is a more solid metric +(results are code-style independent, comment-insensitive) +* LoC is easily circumvented by bad code style (e.g. stuffing several assignments into one, concatenating code lines) +* Enforcing length limits with LoC is not very meaningful, could even be called a bad practice + +In order to find "big" classes, the rule {% rule NcssCount %} can be used instead. 3 @@ -732,6 +743,7 @@ public class Foo { @@ -740,6 +752,16 @@ When methods are excessively long this usually indicates that the method is doin name/signature might suggest. They also become challenging for others to digest since excessive scrolling causes readers to lose focus. Try to reduce the method length by creating helper methods and removing any copy/pasted code. + +This rule is deprecated since PMD 6.53.0 and will be removed with PMD 7.0.0. + +The rule is based on the simple metric lines of code (LoC). The reasons for deprecation are: +* LoC is a noisy metric, NCSS (non-commenting source statements) is a more solid metric +(results are code-style independent, comment-insensitive) +* LoC is easily circumvented by bad code style (e.g. stuffing several assignments into one, concatenating code lines) +* Enforcing length limits with LoC is not very meaningful, could even be called a bad practice + +In order to find "big" methods, the rule {% rule NcssCount %} can be used instead. 3 From e8510fa7daf8a91529a1a0a51432a7779ead1e8d Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 9 Dec 2022 20:32:58 +0100 Subject: [PATCH 12/31] [java] Deprecate base class ExcessiveLengthRule --- docs/pages/release_notes.md | 9 +++++++++ .../pmd/lang/java/rule/design/ExcessiveLengthRule.java | 2 ++ 2 files changed, 11 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index e03494d88d..d1410e99e7 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -27,6 +27,15 @@ This is a {{ site.pmd.release_type }} release. ### API Changes +#### Deprecated APIs + +##### Internal API + +Those APIs are not intended to be used by clients, and will be hidden or removed with PMD 7.0.0. You can identify +them with the `@InternalApi` annotation. You'll also get a deprecation warning. + +* {% jdoc java::lang.java.rule.design.ExcessiveLengthRule %} (Java) + ### External Contributions {% endtocmaker %} diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java index a012ab5c51..08734e63e2 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java @@ -4,6 +4,7 @@ package net.sourceforge.pmd.lang.java.rule.design; +import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractStatisticalJavaRule; import net.sourceforge.pmd.stat.DataPoint; @@ -19,6 +20,7 @@ import net.sourceforge.pmd.stat.DataPoint; * @deprecated This base class is not needed anymore and will be removed with PMD 7. */ @Deprecated +@InternalApi public class ExcessiveLengthRule extends AbstractStatisticalJavaRule { private Class nodeClass; From 7237d0c076b6923e5e3e7eac41d170514b80e3c4 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 9 Dec 2022 20:37:23 +0100 Subject: [PATCH 13/31] [doc] Consistently document deprecated and renamed rules --- docs/pages/next_major_development.md | 184 +++++++++++------- docs/pages/release_notes.md | 5 + .../resources/category/apex/codestyle.xml | 2 +- .../resources/category/apex/performance.xml | 6 +- .../resources/category/java/bestpractices.xml | 22 ++- .../resources/category/java/codestyle.xml | 31 +-- .../main/resources/category/java/design.xml | 13 +- .../resources/category/java/errorprone.xml | 59 +++--- .../category/java/multithreading.xml | 3 +- .../resources/category/java/performance.xml | 25 ++- 10 files changed, 211 insertions(+), 139 deletions(-) diff --git a/docs/pages/next_major_development.md b/docs/pages/next_major_development.md index 35f95c478f..3c2d594192 100644 --- a/docs/pages/next_major_development.md +++ b/docs/pages/next_major_development.md @@ -1497,96 +1497,132 @@ large projects, with many duplications, it was causing `OutOfMemoryError`s (see ### List of currently deprecated rules -* The Java rules {% rule java/codestyle/VariableNamingConventions %}, {% rule java/codestyle/MIsLeadingVariableName %}, - {% rule java/codestyle/SuspiciousConstantFieldName %}, and {% rule java/codestyle/AvoidPrefixingMethodParameters %} are - now deprecated, and will be removed with version 7.0.0. They are replaced by the more general - {% rule java/codestyle/FieldNamingConventions %}, {% rule java/codestyle/FormalParameterNamingConventions %}, and - {% rule java/codestyle/LocalVariableNamingConventions %}. +These rules will be removed with PMD 7.0.0. -* The Java rule {% rule java/codestyle/AbstractNaming %} is deprecated - in favour of {% rule java/codestyle/ClassNamingConventions %}. +* Since 6.0.0: The Java rules {% rule java/design/NcssConstructorCount %}, {% rule java/design/NcssMethodCount %}, + and {% rule java/design/NcssTypeCount %} have been deprecated. They will be replaced by the new rule + {% rule java/design/NcssCount %}. -* The Java rules {% rule java/codestyle/WhileLoopsMustUseBraces %}, {% rule java/codestyle/ForLoopsMustUseBraces %}, {% rule java/codestyle/IfStmtsMustUseBraces %}, and {% rule java/codestyle/IfElseStmtsMustUseBraces %} - are deprecated. They will be replaced by the new rule {% rule java/codestyle/ControlStatementBraces %}. +* Since 6.0.0: The Java rule `LooseCoupling` in ruleset `java-typeresolution` is deprecated. Use the rule with the + same name from category `bestpractices` instead: {% rule java/bestpractices/LooseCoupling %}. -* The Java rules {% rule java/design/NcssConstructorCount %}, {% rule java/design/NcssMethodCount %}, and {% rule java/design/NcssTypeCount %} have been - deprecated. They will be replaced by the new rule {% rule java/design/NcssCount %} in the category `design`. +* Since 6.0.0: The Java rule `CloneMethodMustImplementCloneable` in ruleset `java-typeresolution` is deprecated. + Use the rule with the same name from category `errorprone` instead: + {% rule java/errorprone/CloneMethodMustImplementCloneable %}. -* The Java rule `LooseCoupling` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from category `bestpractices` instead. +* Since 6.0.0: The Java rule `UnusedImports` in ruleset `java-typeresolution` is deprecated. Use the rule with + the same name from category `bestpractices` instead: {% rule java/bestpractices/UnusedImports %}. -* The Java rule `CloneMethodMustImplementCloneable` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from category `errorprone` instead. +* Since 6.0.0: The Java rule `SignatureDeclareThrowsException` in ruleset `java-typeresolution` is deprecated. + Use the rule with the same name from category `design` instead: + {% rule java/design/SignatureDeclareThrowsException %}. -* The Java rule `UnusedImports` in ruleset `java-typeresolution` is deprecated. Use the rule with - the same name from category `bestpractices` instead. +* Since 6.0.0: The Java rule `EmptyStaticInitializer` in ruleset `java-empty` is deprecated. + Use the rule {% rule java/errorprone/EmptyInitializer %} instead, which covers both static and non-static + empty initializers. -* The Java rule `SignatureDeclareThrowsException` in ruleset `java-typeresolution` is deprecated. Use the rule with the same name from category `design` instead. +* Since 6.0.0: The Java rules `GuardDebugLogging` (ruleset `java-logging-jakarta-commons`) and + `GuardLogStatementJavaUtil` (ruleset `java-logging-java`) have been deprecated. Use the rule + {% rule java/bestpractices/GuardLogStatement %} instead, which covers all cases regardless of the logging framework. -* The Java rule `EmptyStaticInitializer` in ruleset `java-empty` is deprecated. Use the rule {% rule java/errorprone/EmptyInitializer %}, which covers both static and non-static empty initializers.` +* Since 6.2.0: The Java rules {% rule java/codestyle/WhileLoopsMustUseBraces %}, + {% rule java/codestyle/ForLoopsMustUseBraces %}, {% rule java/codestyle/IfStmtsMustUseBraces %}, and + {% rule java/codestyle/IfElseStmtsMustUseBraces %} are deprecated. They will be replaced by the new rule + {% rule java/codestyle/ControlStatementBraces %}. -* The Java rules `GuardDebugLogging` (ruleset `java-logging-jakarta-commons`) and `GuardLogStatementJavaUtil` - (ruleset `java-logging-java`) have been deprecated. Use the rule {% rule java/bestpractices/GuardLogStatement %}, which covers all cases regardless of the logging framework. +* Since 6.3.0: The Java rule {% rule java/codestyle/AbstractNaming %} is deprecated + in favour of {% rule java/codestyle/ClassNamingConventions %}. -* The Java rule {% rule "java/multithreading/UnsynchronizedStaticDateFormatter" %} has been deprecated and - will be removed with PMD 7.0.0. The rule is replaced by the more general - {% rule "java/multithreading/UnsynchronizedStaticFormatter" %}. +* Since 6.7.0: The Java rules {% rule java/codestyle/VariableNamingConventions %}, + {% rule java/codestyle/MIsLeadingVariableName %}, {% rule java/codestyle/SuspiciousConstantFieldName %}, and + {% rule java/codestyle/AvoidPrefixingMethodParameters %} are now deprecated. They are replaced by the more general + {% rule java/codestyle/FieldNamingConventions %}, {% rule java/codestyle/FormalParameterNamingConventions %}, and + {% rule java/codestyle/LocalVariableNamingConventions %}. -* The two Java rules {% rule "java/bestpractices/PositionLiteralsFirstInComparisons" %} - and {% rule "java/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisons" %} (ruleset `java-bestpractices`) - have been deprecated in favor of the new rule {% rule "java/bestpractices/LiteralsFirstInComparisons" %}. +* Since 6.11.0: The Java rule {% rule java/multithreading/UnsynchronizedStaticDateFormatter %} has been deprecated. + The rule is replaced by the more general {% rule java/multithreading/UnsynchronizedStaticFormatter %}. -* The Java rule [`AvoidFinalLocalVariable`](https://pmd.github.io/pmd-6.16.0/pmd_rules_java_codestyle.html#avoidfinallocalvariable) (`java-codestyle`) has been deprecated - and will be removed with PMD 7.0.0. The rule is controversial and also contradicts other existing - rules such as [`LocalVariableCouldBeFinal`](https://pmd.github.io/pmd-6.16.0/pmd_rules_java_codestyle.html#localvariablecouldbefinal). If the goal is to avoid defining - constants in a scope smaller than the class, then the rule [`AvoidDuplicateLiterals`](https://pmd.github.io/pmd-6.16.0/pmd_rules_java_errorprone.html#avoidduplicateliterals) - should be used instead. +* Since 6.15.0: The Apex rule {% rule apex/codestyle/VariableNamingConventions %} has been deprecated. The rule is + replaced by the more general rules {% rule apex/codestyle/FieldNamingConventions %}, + {% rule apex/codestyle/FormalParameterNamingConventions %}, {% rule apex/codestyle/LocalVariableNamingConventions %}, + and {% rule apex/codestyle/PropertyNamingConventions %}. -* The Apex rule [`VariableNamingConventions`](https://pmd.github.io/pmd-6.15.0/pmd_rules_apex_codestyle.html#variablenamingconventions) (`apex-codestyle`) has been deprecated and - will be removed with PMD 7.0.0. The rule is replaced by the more general rules - [`FieldNamingConventions`](https://pmd.github.io/pmd-6.15.0/pmd_rules_apex_codestyle.html#fieldnamingconventions), - [`FormalParameterNamingConventions`](https://pmd.github.io/pmd-6.15.0/pmd_rules_apex_codestyle.html#formalparameternamingconventions), - [`LocalVariableNamingConventions`](https://pmd.github.io/pmd-6.15.0/pmd_rules_apex_codestyle.html#localvariablenamingconventions), and - [`PropertyNamingConventions`](https://pmd.github.io/pmd-6.15.0/pmd_rules_apex_codestyle.html#propertynamingconventions). +* Since 6.15.0: The Java rule {% rule java/errorprone/LoggerIsNotStaticFinal %} has been deprecated. + The rule is replaced by {% rule java/errorprone/ProperLogger %}. -* The Java rule [`LoggerIsNotStaticFinal`](https://pmd.github.io/pmd-6.15.0/pmd_rules_java_errorprone.html#loggerisnotstaticfinal) (`java-errorprone`) has been deprecated - and will be removed with PMD 7.0.0. The rule is replaced by [`ProperLogger`](https://pmd.github.io/pmd-6.15.0/pmd_rules_java_errorprone.html#properlogger). +* Since 6.16.0: The Java rule {% rule java/codestyle/AvoidFinalLocalVariable %} has been deprecated. + The rule is controversial and also contradicts other existing rules such as + {% rule java/codestyle/LocalVariableCouldBeFinal %}. If the goal is to avoid defining + constants in a scope smaller than the class, then the rule {% rule java/errorprone/AvoidDuplicateLiterals %} + should be used instead. -* The Java rule {% rule "java/errorprone/DataflowAnomalyAnalysis" %} (`java-errorprone`) - is deprecated in favour of {% rule "java/bestpractices/UnusedAssignment" %} (`java-bestpractices`), - which was introduced in PMD 6.26.0. +* Since 6.19.0: The Java rule {% rule java/errorprone/InvalidSlf4jMessageFormat %} has been renamed to + {% rule java/errorprone/InvalidLogMessageFormat %}. -* The java rule {% rule "java/codestyle/DefaultPackage" %} has been deprecated in favor of - {% rule "java/codestyle/CommentDefaultAccessModifier" %}. +* Since 6.24.0: The two Java rules {% rule java/bestpractices/PositionLiteralsFirstInComparisons %} + and {% rule java/bestpractices/PositionLiteralsFirstInCaseInsensitiveComparisons %} + have been deprecated in favor of the new rule {% rule java/bestpractices/LiteralsFirstInComparisons %}. -* The Java rule {% rule "java/errorprone/CloneThrowsCloneNotSupportedException" %} has been deprecated without - replacement. +* Since 6.27.0: The Java rule {% rule java/errorprone/DataflowAnomalyAnalysis %} + is deprecated in favour of {% rule java/bestpractices/UnusedAssignment %}, + which was introduced in PMD 6.26.0. -* 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 %} +* Since 6.29.0: The Apex rules {% rule apex/performance/AvoidDmlStatementsInLoops %}, + {% rule apex/performance/AvoidSoqlInLoops %}, and {% rule apex/performance/AvoidSoslInLoops %} are deprecated + in favor of the new rule {% rule apex/performance/OperationWithLimitsInLoop %}. -* The Java rule {% rule java/errorprone/ReturnEmptyArrayRatherThanNull %} is deprecated and removed from - the quickstart ruleset, as the new rule {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} - supersedes it. +* Since 6.29.0: The Java rule {% rule java/errorprone/DoNotCallSystemExit %} has been renamed to + {% rule/java/errorprone/DoNotTerminateVM %}. -* 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 %} +* Since 6.31:0: The Java rule {% rule java/performance/AvoidUsingShortType %} is deprecated + for removal without replacement. -* The Java rule {% 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 - new rule `UnnecessaryBoxing`. +* Since 6.31.0: The Java rule {% rule java/performance/SimplifyStartsWith %} is deprecated + for removal without replacement. + +* Since 6.34.0: The Java rules {% rule java/bestpractices/UnusedImports %}, {% rule java/codestyle/DuplicateImports %}, + {% rule java/codestyle/DontImportJavaLang %}, and {% rule java/errorprone/ImportFromSamePackage %} are + deprecated. These rules are replaced by {% rule java/codestyle/UnnecessaryImport %}. + +* Since 6.35.0: The Java rule {% rule java/codestyle/DefaultPackage %} has been deprecated in favor of + {% rule java/codestyle/CommentDefaultAccessModifier %}. + +* Since 6.35.0: The Java rule {% rule java/errorprone/CloneThrowsCloneNotSupportedException %} has been + deprecated without replacement. + +* Since 6.36.0: The Java rule {% rule java/errorprone/BadComparison %} has been renamed to + {% rule java/errorprone/ComparisonWithNaN %}. + +* Since 6.37.0: 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 %} + +* Since 6.37.0: The Java rule {% rule java/errorprone/ReturnEmptyArrayRatherThanNull %} is deprecated and removed from + the quickstart ruleset, as the new rule {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} + supersedes it. + +* Since 6.37.0: 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 %} + +* Since 6.37.0: The Java rule {% rule java/performance/UnnecessaryWrapperObjectCreation %} is deprecated + with no planned replacement before PMD 7. In its 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 + new rule `UnnecessaryBoxing`. + +* Since 6.37.0: The Java rule {% rule java/errorprone/MissingBreakInSwitch %} has been renamed to + {% rule java/errorprone/ImplicitSwitchFallThrough %}. * Since 6.46.0: The following Java rules are deprecated and removed from the quickstart ruleset, as the new rule {% rule java/codestyle/EmptyControlStatement %} merges their functionality: @@ -1599,8 +1635,12 @@ large projects, with many duplications, it was causing `OutOfMemoryError`s (see * {% rule java/errorprone/EmptyTryBlock %} * {% rule java/errorprone/EmptyWhileStmt %} -* Since 6.46.0: The Java rule {% rule java/errorprone/EmptyStatementNotInLoop %} is deprecated and removed from the quickstart - ruleset. Use the new rule {% rule java/codestyle/UnnecessarySemicolon %} instead. - * Since 6.52.0: The Java rule {% rule java/errorprone/BeanMembersShouldSerialize %} has been renamed to {% rule java/errorprone/NonSerializableClass %}. + +* Since 6.53.0: The Java rules {% rule java/design/ExcessiveClassLength %} and + {% rule java/design/ExcessiveMethodLength %} have been deprecated. The rule + {% rule java/design/NcssCount %} can be used instead. + +* Since 6.53.0: The Java rule {% rule java/errorprone/EmptyStatementNotInLoop %} is deprecated. + Use the rule {% rule java/codestyle/UnnecessarySemicolon %} instead. diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index d1410e99e7..1d093a5e44 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -20,6 +20,11 @@ This is a {{ site.pmd.release_type }} release. have been deprecated. The rule {% rule java/design/NcssCount %} can be used instead. The deprecated rules will be removed with PMD 7.0.0. +* The Java rule {% rule java/errorprone/EmptyStatementNotInLoop %} is deprecated. + Use the rule {% rule java/codestyle/UnnecessarySemicolon %} instead. + Note: Actually it was announced to be deprecated since 6.46.0 but the rule was not marked as deprecated yet. + This has been done now. + ### Fixed Issues * java-design diff --git a/pmd-apex/src/main/resources/category/apex/codestyle.xml b/pmd-apex/src/main/resources/category/apex/codestyle.xml index d694d99dc7..7cb205528c 100644 --- a/pmd-apex/src/main/resources/category/apex/codestyle.xml +++ b/pmd-apex/src/main/resources/category/apex/codestyle.xml @@ -351,7 +351,7 @@ A variable naming conventions rule - customize this to your liking. Currently, checks for final variables that should be fully capitalized and non-final variables that should not include underscores. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.15.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rules {% rule "apex/codestyle/FieldNamingConventions" %}, {% rule "apex/codestyle/FormalParameterNamingConventions" %}, {% rule "apex/codestyle/LocalVariableNamingConventions" %}, and diff --git a/pmd-apex/src/main/resources/category/apex/performance.xml b/pmd-apex/src/main/resources/category/apex/performance.xml index 6d714c8e37..1e679081f4 100644 --- a/pmd-apex/src/main/resources/category/apex/performance.xml +++ b/pmd-apex/src/main/resources/category/apex/performance.xml @@ -64,7 +64,7 @@ public class Foo { Avoid DML statements inside loops to avoid hitting the DML governor limit. Instead, try to batch up the data into a list and invoke your DML once on that list of data outside the loop. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.29.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rule {% rule "apex/performance/OperationWithLimitsInLoop" %}. 3 @@ -93,7 +93,7 @@ public class Something { New objects created within loops should be checked to see if they can created outside them and reused. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.29.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rule {% rule "apex/performance/OperationWithLimitsInLoop" %}. 3 @@ -120,7 +120,7 @@ public class Something { Sosl calls within loops can cause governor limit exceptions. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.29.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rule {% rule "apex/performance/OperationWithLimitsInLoop" %}. 3 diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 31a6d776f9..c6c18c5aaf 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -1205,7 +1205,8 @@ String name, Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. -This rule is replaced by the more general rule {% rule "LiteralsFirstInComparisons" %}. +_Note:_ This rule is deprecated since PMD 6.24.0 and will be removed with PMD 7.0.0. The rule is replaced +by the more general rule {% rule "LiteralsFirstInComparisons" %}. 3 @@ -1230,7 +1231,8 @@ class Foo { Position literals first in comparisons, if the second argument is null then NullPointerExceptions can be avoided, they will just return false. -This rule is replaced by the more general rule {% rule "LiteralsFirstInComparisons" %}. +_Note:_ This rule is deprecated since PMD 6.24.0 and will be removed with PMD 7.0.0. The rule is replaced +by the more general rule {% rule "LiteralsFirstInComparisons" %}. 3 @@ -1687,8 +1689,8 @@ Reports import statements that are not used within the file. This also reports duplicate imports, and imports from the same package. The simplest fix is just to delete those imports. -This rule is deprecated since PMD 6.34.0. Use the rule {% rule "java/codestyle/UnnecessaryImport" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.34.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/UnnecessaryImport" %} from category codestyle instead. 4 @@ -1783,7 +1785,8 @@ 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule SimplifiableTestAssertion %} instead. 3 @@ -1837,7 +1840,8 @@ public class FooTest extends TestCase { 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule SimplifiableTestAssertion %} instead. 3 @@ -1894,7 +1898,8 @@ public class FooTest extends TestCase { 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule SimplifiableTestAssertion %} instead. 3 @@ -1947,7 +1952,8 @@ public class FooTest extends TestCase { 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule SimplifiableTestAssertion %} instead. 3 diff --git a/pmd-java/src/main/resources/category/java/codestyle.xml b/pmd-java/src/main/resources/category/java/codestyle.xml index 6f918c76f2..cd91f1e825 100644 --- a/pmd-java/src/main/resources/category/java/codestyle.xml +++ b/pmd-java/src/main/resources/category/java/codestyle.xml @@ -19,7 +19,7 @@ Abstract classes should be named 'AbstractXXX'. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.3.0 and will be removed with PMD 7.0.0. The rule is replaced by {% rule java/codestyle/ClassNamingConventions %}. 3 @@ -108,7 +108,7 @@ avoid local literals being declared in a scope smaller than the class. Also note, that this rule is the opposite of {% rule "java/codestyle/LocalVariableCouldBeFinal" %}. Having both rules enabled results in contradictory violations being reported. -This rule is deprecated and will be removed with PMD 7.0.0. There is no replacement planned. +_Note:_ This rule is deprecated since PMD 6.16.0 and will be removed with PMD 7.0.0. There is no replacement planned. If the goal is to avoid defining constants in a scope smaller than the class, then the rule {% rule "java/errorprone/AvoidDuplicateLiterals" %} should be used instead. @@ -155,7 +155,7 @@ Prefixing parameters by 'in' or 'out' pollutes the name of the parameters and re To indicate whether or not a parameter will be modify in a method, its better to document method behavior with Javadoc. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.7.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rule {% rule java/codestyle/FormalParameterNamingConventions %}. 4 @@ -573,7 +573,8 @@ while (true) { // preferred approach Use explicit scoping instead of accidental usage of default package private level. The rule allows methods and fields annotated with Guava's @VisibleForTesting and JUnit 5's annotations. -This rule is deprecated since PMD 6.35.0. It assumes that any usage of package-access is accidental, +_Note:_ This rule is deprecated since PMD 6.35.0 and will be removed with PMD 7.0.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, @@ -623,8 +624,8 @@ or MethodDeclaration[@PackagePrivate= true()] Avoid importing anything from the package 'java.lang'. These classes are automatically imported (JLS 7.5.3). -This rule is deprecated since PMD 6.34.0. Use the rule {% rule "java/codestyle/UnnecessaryImport" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.34.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/UnnecessaryImport" %} from category codestyle instead. 4 @@ -652,8 +653,8 @@ public class Foo {} Duplicate or overlapping import statements should be avoided. -This rule is deprecated since PMD 6.34.0. Use the rule {% rule "java/codestyle/UnnecessaryImport" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.34.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/UnnecessaryImport" %} from category codestyle instead. 4 @@ -934,7 +935,7 @@ Avoid using 'for' statements without using curly braces. If the code formatting indentation is lost then it becomes difficult to separate the code being controlled from the rest. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.2.0 and will be removed with PMD 7.0.0. The rule is replaced by the rule {% rule java/codestyle/ControlStatementBraces %}. 3 @@ -1082,7 +1083,7 @@ Avoid using if..else statements without using surrounding braces. If the code fo or indentation is lost then it becomes difficult to separate the code being controlled from the rest. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.2.0 and will be removed with PMD 7.0.0. The rule is replaced by the rule {% rule java/codestyle/ControlStatementBraces %}. 3 @@ -1125,7 +1126,7 @@ Avoid using if statements without using braces to surround the code block. If th formatting or indentation is lost then it becomes difficult to separate the code being controlled from the rest. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.2.0 and will be removed with PMD 7.0.0. The rule is replaced by the rule {% rule java/codestyle/ControlStatementBraces %}. 3 @@ -1487,7 +1488,7 @@ public class Foo { Detects when a non-field has a name starting with 'm_'. This usually denotes a field and could be confusing. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.7.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rule {% rule java/codestyle/LocalVariableNamingConventions %}. @@ -1881,7 +1882,7 @@ public class Something { Field names using all uppercase characters - Sun's Java naming conventions indicating constants - should be declared as final. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.7.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rule {% rule java/codestyle/FieldNamingConventions %}. 3 @@ -2464,7 +2465,7 @@ A variable naming conventions rule - customize this to your liking. Currently, checks for final variables that should be fully capitalized and non-final variables that should not include underscores. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.7.0 and will be removed with PMD 7.0.0. The rule is replaced by the more general rules {% rule java/codestyle/FieldNamingConventions %}, {% rule java/codestyle/FormalParameterNamingConventions %}, and {% rule java/codestyle/LocalVariableNamingConventions %}. @@ -2493,7 +2494,7 @@ Avoid using 'while' statements without using braces to surround the code block. formatting or indentation is lost then it becomes difficult to separate the code being controlled from the rest. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.2.0 and will be removed with PMD 7.0.0. The rule is replaced by the rule {% rule java/codestyle/ControlStatementBraces %}. 3 diff --git a/pmd-java/src/main/resources/category/java/design.xml b/pmd-java/src/main/resources/category/java/design.xml index 4def9bbb52..55e7d6e1c7 100644 --- a/pmd-java/src/main/resources/category/java/design.xml +++ b/pmd-java/src/main/resources/category/java/design.xml @@ -684,7 +684,7 @@ Excessive class file lengths are usually indications that the class may be burde responsibilities that could be provided by external classes or functions. In breaking these methods apart the code becomes more manageable and ripe for reuse. -This rule is deprecated since PMD 6.53.0 and will be removed with PMD 7.0.0. +_Note:_ This rule is deprecated since PMD 6.53.0 and will be removed with PMD 7.0.0. The rule is based on the simple metric lines of code (LoC). The reasons for deprecation are: * LoC is a noisy metric, NCSS (non-commenting source statements) is a more solid metric @@ -753,7 +753,7 @@ name/signature might suggest. They also become challenging for others to digest scrolling causes readers to lose focus. Try to reduce the method length by creating helper methods and removing any copy/pasted code. -This rule is deprecated since PMD 6.53.0 and will be removed with PMD 7.0.0. +_Note:_ This rule is deprecated since PMD 6.53.0 and will be removed with PMD 7.0.0. The rule is based on the simple metric lines of code (LoC). The reasons for deprecation are: * LoC is a noisy metric, NCSS (non-commenting source statements) is a more solid metric @@ -1164,7 +1164,7 @@ This rule uses the NCSS (Non-Commenting Source Statements) algorithm to determin of code for a given constructor. NCSS ignores comments, and counts actual statements. Using this algorithm, lines of code that are split are counted as one. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.0.0 and will be removed with PMD 7.0.0. The rule is replaced by the rule {% rule java/design/NcssCount %}. 3 @@ -1242,7 +1242,7 @@ This rule uses the NCSS (Non-Commenting Source Statements) algorithm to determin of code for a given method. NCSS ignores comments, and counts actual statements. Using this algorithm, lines of code that are split are counted as one. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.0.0 and will be removed with PMD 7.0.0. The rule is replaced by the rule {% rule java/design/NcssCount %}. 3 @@ -1277,7 +1277,7 @@ This rule uses the NCSS (Non-Commenting Source Statements) algorithm to determin of code for a given type. NCSS ignores comments, and counts actual statements. Using this algorithm, lines of code that are split are counted as one. -This rule is deprecated and will be removed with PMD 7.0.0. The rule is replaced +_Note:_ This rule is deprecated since PMD 6.0.0 and will be removed with PMD 7.0.0. The rule is replaced by the rule {% rule java/design/NcssCount %}. 3 @@ -1455,7 +1455,8 @@ as: assertFalse(expr); -Deprecated since PMD 6.37.0, use {% rule java/bestpractices/SimplifiableTestAssertion %} instead. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule 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 8ba16182a0..86a3bc2d1d 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -957,7 +957,8 @@ public class Foo implements Cloneable { The method clone() should throw a CloneNotSupportedException. -This rule is deprecated since PMD 6.35.0 without replacement. The rule has no real value as +_Note:_ This rule is deprecated since PMD 6.35.0 and will be removed with PMD 7.0.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 @@ -1117,7 +1118,7 @@ class Foo { Finally, comparisons like `someDouble <= Double.NaN` are nonsensical and will always evaluate to false. - This rule has been renamed from "BadComparison" with PMD 6.36.0. + This rule has been renamed from "BadComparison" in PMD 6.36.0. ]]> 3 @@ -1192,7 +1193,8 @@ From those informations there can be found various problems. 1. DU - Anomaly: A recently defined variable is undefined. These anomalies may appear in normal source text. 2. DD - Anomaly: A recently defined variable is redefined. This is ominous but don't have to be a bug. -This rule is deprecated. Use {% rule "java/bestpractices/UnusedAssignment" %} in category bestpractices instead. +_Note:_ This rule is deprecated since PMD 6.27.0 and will be removed with PMD 7.0.0. The rule is replaced +by the rule {% rule "java/bestpractices/UnusedAssignment" %}. 5 @@ -1391,7 +1393,7 @@ running on the same application server. This rule also checks for the equivalent calls `Runtime.getRuntime().exit()` and `Runtime.getRuntime().halt()`. -This rule was called *DoNotCallSystemExit* until PMD 6.29.0. +This rule has been renamed from "DoNotCallSystemExit" in PMD 6.29.0. 3 @@ -1597,8 +1599,8 @@ public class Foo { Empty finally blocks serve no purpose and should be removed. -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -1636,8 +1638,8 @@ public class Foo { Empty If Statement finds instances where a condition is checked but nothing is done about it. -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -1674,8 +1676,8 @@ public class Foo { Empty initializers serve no purpose and should be removed. -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -1707,8 +1709,8 @@ public class Foo { Empty block statements serve no purpose and should be removed. -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -1736,6 +1738,7 @@ public class Foo { @@ -1743,6 +1746,9 @@ public class Foo { An empty statement (or a semicolon by itself) that is not used as the sole body of a 'for' or 'while' loop is probably a bug. It could also be a double semicolon, which has no purpose and should be removed. + +_Note:_ This rule is deprecated since PMD 6.53.0 and will be removed with PMD 7.0.0. +Use the rule {% rule java/codestyle/UnnecessarySemicolon %} instead. 3 @@ -1786,8 +1792,8 @@ public void doit() { Empty switch statements serve no purpose and should be removed.# -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -1819,8 +1825,8 @@ public void bar() { Empty synchronized blocks serve no purpose and should be removed. -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -1852,8 +1858,8 @@ public class Foo { Avoid empty try blocks - what's the point? -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -1892,8 +1898,8 @@ Empty While Statement finds all instances where a while statement does nothing. If it is a timing loop, then you should use Thread.sleep() for it; if it is a while loop that does a lot in the exit expression, rewrite it to make it clearer. -This rule is deprecated since PMD 6.46.0. Use the rule {% rule "java/codestyle/EmptyControlStatement" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.46.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/EmptyControlStatement" %} from category codestyle instead. 3 @@ -2142,7 +2148,7 @@ 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. +This rule has been renamed from "MissingBreakInSwitch" in PMD 6.37.0. 3 @@ -2196,8 +2202,8 @@ public void bar(int status) { There is no need to import a type that lives in the same package. -This rule is deprecated since PMD 6.34.0. Use the rule {% rule "java/codestyle/UnnecessaryImport" %} -from category codestyle instead. +_Note:_ This rule is deprecated since PMD 6.34.0 and will be removed with PMD 7.0.0. +Use the rule {% rule "java/codestyle/UnnecessaryImport" %} from category codestyle instead. 3 @@ -2263,6 +2269,8 @@ Check for messages in slf4j and log4j2 (since 6.19.0) loggers with non matching Since 6.32.0 in addition to parameterized message placeholders (`{}`) also format specifiers of string formatted messages are supported (`%s`). + +This rule has been renamed from "InvalidSlf4jMessageFormat" in PMD 6.19.0. 5 @@ -2371,7 +2379,7 @@ public class Foo extends TestCase { In most cases, the Logger reference can be declared as static and final. -This rule is deprecated and will be removed with PMD 7.0.0. +_Note:_ This rule is deprecated since PMD 6.15.0 and will be removed with PMD 7.0.0. The rule is replaced by {% rule java/errorprone/ProperLogger %}. 2 @@ -2929,7 +2937,8 @@ For any method that returns an array, it is a better to return an empty array ra 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule java/errorprone/ReturnEmptyCollectionRatherThanNull %} instead. 1 diff --git a/pmd-java/src/main/resources/category/java/multithreading.xml b/pmd-java/src/main/resources/category/java/multithreading.xml index c01e70971a..ab0378af0e 100644 --- a/pmd-java/src/main/resources/category/java/multithreading.xml +++ b/pmd-java/src/main/resources/category/java/multithreading.xml @@ -322,7 +322,8 @@ SimpleDateFormat instances are not synchronized. Sun recommends using separate f for each thread. If multiple threads must access a static formatter, the formatter must be synchronized on block level. -This rule has been deprecated in favor of the rule {% rule UnsynchronizedStaticFormatter %}. +_Note:_ This rule has been deprecated since PMD 6.11.0 and will be removed with PMD 7.0.0. The rule is replaced +by the more general rule {% rule UnsynchronizedStaticFormatter %}. 3 diff --git a/pmd-java/src/main/resources/category/java/performance.xml b/pmd-java/src/main/resources/category/java/performance.xml index ebcae5cae6..a42715732e 100644 --- a/pmd-java/src/main/resources/category/java/performance.xml +++ b/pmd-java/src/main/resources/category/java/performance.xml @@ -317,7 +317,8 @@ public class Something { deprecated="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#avoidusingshorttype"> -Note: this rule is deprecated, as its rationale does not hold. +_Note:_ This rule is deprecated since PMD 6.31.0 and will be removed with PMD 7.0.0. +There is no planned replacement. 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 @@ -392,7 +393,8 @@ bi4 = new BigInteger(0); // reference BigInteger.ZERO instead 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. 2 @@ -416,7 +418,8 @@ Calling new Byte() causes memory allocation that can be avoided by the static By 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. 2 @@ -597,7 +600,8 @@ Calling new Integer() causes memory allocation that can be avoided by the static 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. 2 @@ -634,7 +638,8 @@ Calling new Long() causes memory allocation that can be avoided by the static Lo 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. 2 @@ -753,7 +758,8 @@ public class C { deprecated="true" externalInfoUrl="${pmd.website.baseurl}/pmd_rules_java_performance.html#simplifystartswith"> -Note: this rule is deprecated for removal, as the optimization is insignificant. +_Note:_ This rule is deprecated since PMD 6.31.0 and will be removed with PMD 7.0.0. There is no planned replacement. +The 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, @@ -808,7 +814,8 @@ Calling new Short() causes memory allocation that can be avoided by the static S 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +Use {% rule java/bestpractices/PrimitiveWrapperInstantiation %} instead. 2 @@ -924,7 +931,9 @@ Most wrapper classes provide static conversion methods that avoid the need to cr 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. +_Note:_ This rule is deprecated since PMD 6.37.0 and will be removed with PMD 7.0.0. +The planned replacement is not expected before PMD 7.0.0. In PMD 7 the new rule +`UnnecessaryBoxing` can be used instead. 3 From 0f9a0f4876757affa170b32838348cc0875a836e Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 9 Dec 2022 20:37:44 +0100 Subject: [PATCH 14/31] [java] Update quickstart.xml - remove deprecated Excessive*Length rules --- pmd-java/src/main/resources/rulesets/java/quickstart.xml | 2 -- 1 file changed, 2 deletions(-) diff --git a/pmd-java/src/main/resources/rulesets/java/quickstart.xml b/pmd-java/src/main/resources/rulesets/java/quickstart.xml index bb8d951dbf..bd8e53a284 100644 --- a/pmd-java/src/main/resources/rulesets/java/quickstart.xml +++ b/pmd-java/src/main/resources/rulesets/java/quickstart.xml @@ -133,9 +133,7 @@ - - From 1ce1a2fcef383ef44af1b8569b585160fe15c60d Mon Sep 17 00:00:00 2001 From: Tarush Singh Date: Thu, 15 Dec 2022 00:58:59 +0530 Subject: [PATCH 15/31] Simple change in voilation check --- .../pmd/lang/apex/rule/security/ApexCRUDViolationRule.java | 3 +-- .../pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml | 2 +- 2 files changed, 2 insertions(+), 3 deletions(-) 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 eb15790534..e172265e43 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 @@ -666,8 +666,7 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return true; } if (isImproperDMLCheck && !userMode && !systemMode) { - //addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode"); - addViolation(data, node); + addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode"); return true; } } else { 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 9058b4140f..5ef3a72645 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 @@ -404,7 +404,7 @@ public class User { System Mode, gives warning because it ignores CRUD but explicitly - 1 + 0 This CRUD statement uses explicit system mode From 47128c8b99ff2a7b17f5a4f266d359fa6fea514f Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Thu, 15 Dec 2022 18:45:04 +0100 Subject: [PATCH 16/31] Bump build-tools from 18 to 19 --- .github/workflows/build.yml | 2 +- .github/workflows/git-repo-sync.yml | 2 +- .github/workflows/troubleshooting.yml | 2 +- pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/.github/workflows/build.yml b/.github/workflows/build.yml index a2e4d53c90..4c618ee94d 100644 --- a/.github/workflows/build.yml +++ b/.github/workflows/build.yml @@ -54,7 +54,7 @@ jobs: run: | echo "LANG=en_US.UTF-8" >> $GITHUB_ENV echo "MAVEN_OPTS=-Dmaven.wagon.httpconnectionManager.ttlSeconds=180 -Dmaven.wagon.http.retryHandler.count=3 -DautoReleaseAfterClose=true -DstagingProgressTimeoutMinutes=30" >> $GITHUB_ENV - echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/18/scripts" >> $GITHUB_ENV + echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/19/scripts" >> $GITHUB_ENV - name: Check Environment shell: bash run: | diff --git a/.github/workflows/git-repo-sync.yml b/.github/workflows/git-repo-sync.yml index 790922db40..ce128eaaac 100644 --- a/.github/workflows/git-repo-sync.yml +++ b/.github/workflows/git-repo-sync.yml @@ -24,7 +24,7 @@ jobs: shell: bash run: | echo "LANG=en_US.UTF-8" >> $GITHUB_ENV - echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/18/scripts" >> $GITHUB_ENV + echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/19/scripts" >> $GITHUB_ENV - name: Sync run: .ci/git-repo-sync.sh shell: bash diff --git a/.github/workflows/troubleshooting.yml b/.github/workflows/troubleshooting.yml index ea4bd54fd9..45e41023aa 100644 --- a/.github/workflows/troubleshooting.yml +++ b/.github/workflows/troubleshooting.yml @@ -36,7 +36,7 @@ jobs: run: | echo "LANG=en_US.UTF-8" >> $GITHUB_ENV echo "MAVEN_OPTS=-Dmaven.wagon.httpconnectionManager.ttlSeconds=180 -Dmaven.wagon.http.retryHandler.count=3 -DstagingProgressTimeoutMinutes=30" >> $GITHUB_ENV - echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/18/scripts" >> $GITHUB_ENV + echo "PMD_CI_SCRIPTS_URL=https://raw.githubusercontent.com/pmd/build-tools/19/scripts" >> $GITHUB_ENV - name: Check Environment shell: bash run: | diff --git a/pom.xml b/pom.xml index 6d38930913..894ca46375 100644 --- a/pom.xml +++ b/pom.xml @@ -107,7 +107,7 @@ -Xmx512m -Dfile.encoding=${project.build.sourceEncoding} ${extraArgLine} - 18 + 19 6.49.0 From 637663212e9a77c5c38dfb8732105b96a78d2f65 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 16 Dec 2022 16:06:11 +0100 Subject: [PATCH 17/31] [apex] ApexCRUDViolation - fix rule tests --- .../rule/security/ApexCRUDViolationRule.java | 20 +- .../main/resources/category/apex/security.xml | 8 +- .../rule/security/ApexCRUDViolationTest.java | 1 - .../rule/security/xml/ApexCRUDViolation.xml | 277 ++++++++++++++---- 4 files changed, 241 insertions(+), 65 deletions(-) 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 e172265e43..d0f04f99b1 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 @@ -657,17 +657,17 @@ public class ApexCRUDViolationRule extends AbstractApexRule { boolean isImproperDMLCheck = !isProperESAPICheckForDML(typeCheck, crudMethod) && !isProperAuthPatternBasedCheckForDML(typeCheck, crudMethod); boolean noSecurityEnforced = !isWithSecurityEnforced(node); - boolean userMode = isWithUserMode(node); - boolean systemMode = isWithSystemMode(node); + boolean noUserMode = !isWithUserMode(node); + boolean noSystemMode = !isWithSystemMode(node); if (missingKey) { - //if condition returns true, add violation, otherwise return. - if (isImproperDMLCheck && noSecurityEnforced) { - addViolation(data, node); - return true; - } - if (isImproperDMLCheck && !userMode && !systemMode) { - addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode"); - return true; + if (isImproperDMLCheck) { + if (noSecurityEnforced && noUserMode && noSystemMode) { + addViolation(data, node); + return true; + } else if (!noSystemMode) { + addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode without validating CRUD permissions"); + return true; + } } } else { boolean properChecksHappened = false; diff --git a/pmd-apex/src/main/resources/category/apex/security.xml b/pmd-apex/src/main/resources/category/apex/security.xml index 423cfc35b2..d53437dd92 100644 --- a/pmd-apex/src/main/resources/category/apex/security.xml +++ b/pmd-apex/src/main/resources/category/apex/security.xml @@ -35,14 +35,18 @@ public without sharing class Foo { - Scheduled classes don't need crud, run as system - + Scheduled classes don't need crud, run as system 0 , Database.Stateful { @@ -79,8 +78,7 @@ public class Foo { - Proper CRUD checks for Aggregate Result return - + Proper CRUD checks for Aggregate Result return 0 Not a getter 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + @@ -125,6 +126,11 @@ public class Foo { No CRUD check for inline upsert 2 + 3,3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD check for inline upsert with database class 2 + 3,3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + VF built-in CRUD checks via getter, but cannot determine if is really VF 2 + 4,10 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + @@ -198,19 +214,23 @@ public class Foo { No CRUD,FLS via sObject property 1 + 5 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + - Proper FLS check + Proper FLS check 0 - Proper FLS fall-through check + Proper FLS fall-through check 0 - Improper accessibility CRUD + Improper accessibility CRUD 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + - Accepts Closure SECURITY ENFORCED + Accepts Closure SECURITY ENFORCED 0 - Accepts Closure SECURITY ENFORCED Line Break + Accepts Closure SECURITY ENFORCED Line Break 0 - Accepts Closure SECURITY ENFORCED in a List + Accepts Closure SECURITY ENFORCED in a List 0 - Accepts Closure SECURITY ENFORCED with Case Insensitivity + Accepts Closure SECURITY ENFORCED with Case Insensitivity 0 - Accepts Closure SECURITY ENFORCED with Case Insensitivity Line Break + Accepts Closure SECURITY ENFORCED with Case Insensitivity Line Break 0 - Accepts Closure SECURITY ENFORCED Not Secured + Accepts Closure SECURITY ENFORCED Not Secured 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + - Accepts Closure SECURITY ENFORCED Secured + Accepts Closure SECURITY ENFORCED Secured 0 - Accepts Closure SECURITY ENFORCED Secured Line Break + Accepts Closure SECURITY ENFORCED Secured Line Break 0 Flagged as Critical With No Mode 1 + 4 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + @@ -396,7 +428,7 @@ public class User { public class User { public void coverAllCasesWithTest() { Contact c; - c = [SELECT Name FROM Contact WITH USER_MODE]; + c = [SELECT Name FROM Contact WITH USER_MODE]; } } ]]> @@ -404,22 +436,38 @@ public class User { System Mode, gives warning because it ignores CRUD but explicitly - 0 + 1 + 4 - This CRUD statement uses explicit system mode + This CRUD statement uses explicit system mode without validating CRUD permissions - Proper accessibility CRUD,FLS + Explicit system Mode, no warning, because CRUD permissions are checked + 0 + + + + + Proper accessibility CRUD,FLS 0 - No CRUD,FLS via sObject property, write is not protected - + No CRUD,FLS via sObject property, write is not protected 1 + 6 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + - No CRUD,FLS for List insertion + No CRUD,FLS for List insertion 1 + 6 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + MyWriteOnlyProp { get; set; } @@ -484,17 +538,17 @@ public class Foo { - Proper CRUD,FLS for List insertion + Proper CRUD,FLS for List insertion 0 MyWriteOnlyProp { get; set; } public void foo(String newName, String tempID) { - MyWriteOnlyProp.add(new Contact(FirstName = 'First', LastName = 'Last', Phone = '414-414-4414')); - if(Contact.sObjectType.getDescribe().isCreateable()) { - insert MyWriteOnlyProp; - } + MyWriteOnlyProp.add(new Contact(FirstName = 'First', LastName = 'Last', Phone = '414-414-4414')); + if(Contact.sObjectType.getDescribe().isCreateable()) { + insert MyWriteOnlyProp; + } } } ]]> @@ -503,13 +557,17 @@ public class Foo { No CRUD,FLS via String property 1 + 5 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + @@ -533,6 +591,10 @@ public class Foo { Partial CRUD,FLS via upsert 1 + 5 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + - No CRUD,FLS via upsert + No CRUD,FLS via upsert 2 + 4,4 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Improper CRUD,FLS via ESAPI 1 2 + 4,9 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Improper CRUD,FLS via ESAPI 2 2 + 4,9 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD,FLS check for update 2 + 3,5 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD,FLS check for update with database class 2 + 3,5 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD check for insert 1 + 4 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD check for insert with database class 1 + 4 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD check for delete 2 + 3,4 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + @@ -791,11 +891,16 @@ public class Foo { No CRUD check for delete with database class 2 + 3,4 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + @@ -893,8 +998,7 @@ public class Foo { - Control flow constructor based CRUD,FLS check - + Control flow constructor based CRUD,FLS check 0 Forgot to call the CRUD check 1 + 4 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Control flow substitute CRUD check should fail when check follows SOQL 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Count does expose data and CRUD checks are necessary 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + - Count does leak data and CRUD checks are necessary - + Count does leak data and CRUD checks are necessary 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Field detection 1 + 5 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD check in SOQL for-loop 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD check inside for-each loop 1 + 5 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD,FLS for inline no-args object 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD,FLS for inline literal list 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + No CRUD,FLS for inline initialized list 1 + 3 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + (new Account(Name = 'X')); } } @@ -1237,6 +1380,10 @@ public class Foo { #3576 - Verify use of createAuthMethodPattern, negative test 2 4,8 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + #3576 - Verify use of readAuthMethodPattern, negative test 2 4,9 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + #3576 - Verify use of updateAuthMethodPattern, negative test 2 4,8 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + #3576 - Verify use of createAuthMethodPattern and updateAuthMethodPattern for upsert, negative test 4 4,4,8,8 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + #3576 - Verify use of deleteAuthMethodPattern, negative test 2 4,8 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + #3576 - Verify use of undeleteAuthMethodPattern, negative test 2 4,8 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + #3576 - Verify use of mergeAuthMethodPattern, negative test 2 7,11 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Date: Fri, 16 Dec 2022 17:45:58 +0100 Subject: [PATCH 18/31] [apex] ApexCRUDViolation - no violation for explicit system mode --- .../lang/apex/rule/security/ApexCRUDViolationRule.java | 3 --- .../pmd/lang/apex/rule/security/xml/ApexCRUDViolation.xml | 8 ++------ 2 files changed, 2 insertions(+), 9 deletions(-) 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 d0f04f99b1..9eee370188 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 @@ -664,9 +664,6 @@ public class ApexCRUDViolationRule extends AbstractApexRule { if (noSecurityEnforced && noUserMode && noSystemMode) { addViolation(data, node); return true; - } else if (!noSystemMode) { - addViolationWithMessage(data, node, "This CRUD statement uses explicit system mode without validating CRUD permissions"); - return true; } } } else { 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 e82cfc7cee..867cb4a3d4 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 @@ -435,12 +435,8 @@ public class User { - System Mode, gives warning because it ignores CRUD but explicitly - 1 - 4 - - This CRUD statement uses explicit system mode without validating CRUD permissions - + Explicit System Mode, gives no warning because it ignores CRUD but explicitly + 0 Date: Fri, 16 Dec 2022 18:58:31 +0100 Subject: [PATCH 19/31] [apex] ApexCRUDViolation - support AccessLevel Also support more DML methods: *async and *immediate --- .../rule/security/ApexCRUDViolationRule.java | 48 ++++++++++- .../rule/security/xml/ApexCRUDViolation.xml | 84 +++++++++++++++++++ 2 files changed, 130 insertions(+), 2 deletions(-) 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 9eee370188..03ba104288 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 @@ -43,6 +43,7 @@ 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.ASTParameter; import net.sourceforge.pmd.lang.apex.ast.ASTProperty; import net.sourceforge.pmd.lang.apex.ast.ASTReferenceExpression; import net.sourceforge.pmd.lang.apex.ast.ASTReturnStatement; @@ -79,6 +80,8 @@ public class ApexCRUDViolationRule extends AbstractApexRule { private static final String S_OBJECT_TYPE = "sObjectType"; private static final String GET_DESCRIBE = "getDescribe"; + private static final String ACCESS_LEVEL = "AccessLevel"; + // ESAPI.accessController().isAuthorizedToView(Lead.sObject, fields) private static final String[] ESAPI_ISAUTHORIZED_TO_VIEW = new String[] { "ESAPI", "accessController", "isAuthorizedToView", }; @@ -95,9 +98,9 @@ public class ApexCRUDViolationRule extends AbstractApexRule { private static final Pattern WITH_SECURITY_ENFORCED = Pattern.compile("(?is).*[^']\\s*WITH\\s+SECURITY_ENFORCED\\s*[^']*"); //Added For USER MODE - private static final Pattern WITH_USER_MODE = Pattern.compile("(?is).*[^']\\s+USER_MODE\\s*[^']*"); + private static final Pattern WITH_USER_MODE = Pattern.compile("(?is).*[^']\\s*WITH\\s+USER_MODE\\s*[^']*"); //Added For SYSTEM MODE - private static final Pattern WITH_SYSTEM_MODE = Pattern.compile("(?is).*[^']\\s+SYSTEM_MODE\\s*[^']*"); + private static final Pattern WITH_SYSTEM_MODE = Pattern.compile("(?is).*[^']\\s*WITH\\s+SYSTEM_MODE\\s*[^']*"); // AuthMethodPattern config properties; these are string properties instead of regex properties to help // ensure that the compiled patterns are case-insensitive vs. requiring the pattern author to use "(?i)" @@ -197,14 +200,24 @@ public class ApexCRUDViolationRule extends AbstractApexRule { public Object visit(ASTMethodCallExpression node, Object data) { if (Helper.isAnyDatabaseMethodCall(node)) { + if (hasAccessLevelArgument(node)) { + return data; + } + switch (node.getMethodName().toLowerCase(Locale.ROOT)) { case "insert": + case "insertasync": + case "insertimmediate": checkForCRUD(node, data, IS_CREATEABLE); break; case "update": + case "updateasync": + case "updateimmediate": checkForCRUD(node, data, IS_UPDATEABLE); break; case "delete": + case "deleteasync": + case "deleteimmediate": checkForCRUD(node, data, IS_DELETABLE); break; case "undelete": @@ -228,6 +241,30 @@ public class ApexCRUDViolationRule extends AbstractApexRule { return data; } + /** + * Checks whether any parameter is of type "AccessLevel". It doesn't check + * whether it is "USER_MODE" or "SYSTEM_MODE", because this rule doesn't + * report a violation for neither. + * + * @param node the Database DML method call + */ + private boolean hasAccessLevelArgument(ASTMethodCallExpression node) { + for (int i = 0; i < node.getNumChildren(); i++) { + ApexNode argument = node.getChild(i); + if (argument instanceof ASTVariableExpression + && argument.getFirstChildOfType(ASTReferenceExpression.class) != null) { + ASTReferenceExpression ref = argument.getFirstChildOfType(ASTReferenceExpression.class); + List names = ref.getNames(); + if (names.size() == 1 && ACCESS_LEVEL.equalsIgnoreCase(names.get(0))) { + return true; + } else if (names.size() == 2 && "System".equalsIgnoreCase(names.get(0)) && ACCESS_LEVEL.equalsIgnoreCase(names.get(1))) { + return true; + } + } + } + return false; + } + @Override public Object visit(ASTDmlInsertStatement node, Object data) { checkForCRUD(node, data, IS_CREATEABLE); @@ -289,6 +326,13 @@ public class ApexCRUDViolationRule extends AbstractApexRule { } + @Override + public Object visit(ASTParameter node, Object data) { + String type = node.getType(); + addVariableToMapping(Helper.getFQVariableName(node), type); + return data; + } + @Override public Object visit(final ASTFieldDeclaration node, Object data) { ASTFieldDeclarationStatements field = node.getFirstParentOfType(ASTFieldDeclarationStatements.class); 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 867cb4a3d4..12a11a5729 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 @@ -1738,4 +1738,88 @@ public class Foo { } ]]> + + + Consider Database DML methods with AccessLevel parameter + 7 + 4,12,19,19,26,33,39 + + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + Validate CRUD permission before SOQL/DML operation or enforce user mode + + + + + + + Consider "insert as" + 1 + 7 + + From 4efbbdd41ab0b0b2b2d859cd2bf9a2157792c559 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 16 Dec 2022 19:19:36 +0100 Subject: [PATCH 20/31] Add @Tarush-Singh35 as a contributor --- .all-contributorsrc | 9 ++++ docs/pages/pmd/projectdocs/credits.md | 69 ++++++++++++++------------- 2 files changed, 44 insertions(+), 34 deletions(-) diff --git a/.all-contributorsrc b/.all-contributorsrc index 194e0a8055..0c0c976aa3 100644 --- a/.all-contributorsrc +++ b/.all-contributorsrc @@ -7042,6 +7042,15 @@ "contributions": [ "bug" ] + }, + { + "login": "Tarush-Singh35", + "name": "Tarush Singh", + "avatar_url": "https://avatars.githubusercontent.com/u/86368099?v=4", + "profile": "https://www.linkedin.com/in/tarush-singh-46763819b", + "contributions": [ + "code" + ] } ], "contributorsPerLine": 7, diff --git a/docs/pages/pmd/projectdocs/credits.md b/docs/pages/pmd/projectdocs/credits.md index 8fabcb6a0d..06f8d1feb9 100644 --- a/docs/pages/pmd/projectdocs/credits.md +++ b/docs/pages/pmd/projectdocs/credits.md @@ -690,311 +690,312 @@ Thanks goes to these wonderful people ([emoji key](https://allcontributors.org/d
TIOBE Software
💻 🐛 +
Tarush Singh
💻
Taylor Smock
🐛
Techeira Damián
💻 🐛
Ted Husted
🐛
TehBakker
🐛
The Gitter Badger
🐛 -
Theodoor
🐛 +
Theodoor
🐛
Thiago Henrique Hüpner
🐛
Thibault Meyer
🐛
Thomas Güttler
🐛
Thomas Jones-Low
🐛
Thomas Smith
💻 🐛
ThrawnCA
🐛 -
Thunderforge
💻 🐛 +
Thunderforge
💻 🐛
Tim van der Lippe
🐛
Tobias Weimer
💻 🐛
Tom Copeland
🐛 💻 📖
Tom Daly
🐛
Tomer Figenblat
🐛
Tomi De Lucca
💻 🐛 -
Torsten Kleiber
🐛 +
Torsten Kleiber
🐛
TrackerSB
🐛
Tyson Stewart
🐛
Ullrich Hafner
🐛
Utku Cuhadaroglu
💻 🐛
Valentin Brandl
🐛
Valeria
🐛 -
Valery Yatsynovich
📖 +
Valery Yatsynovich
📖
Vasily Anisimov
🐛
Vibhor Goyal
🐛
Vickenty Fesunov
🐛
Victor Noël
🐛
Vincent Galloy
💻
Vincent HUYNH
🐛 -
Vincent Maurin
🐛 +
Vincent Maurin
🐛
Vincent Privat
🐛
Vishhwas
🐛
Vitaly
🐛
Vitaly Polonetsky
🐛
Vojtech Polivka
🐛
Vsevolod Zholobov
🐛 -
Vyom Yadav
💻 +
Vyom Yadav
💻
Wang Shidong
🐛
Waqas Ahmed
🐛
Wayne J. Earl
🐛
Wchenghui
🐛
Will Winder
🐛
William Brockhus
💻 🐛 -
Wilson Kurniawan
🐛 +
Wilson Kurniawan
🐛
Wim Deblauwe
🐛
Woongsik Choi
🐛
XenoAmess
💻 🐛
Yang
💻
YaroslavTER
🐛
Yasar Shaikh
💻 -
Young Chan
💻 🐛 +
Young Chan
💻 🐛
YuJin Kim
🐛
Yuri Dolzhenko
🐛
Yurii Dubinka
🐛
Zoltan Farkas
🐛
Zustin
🐛
aaronhurst-google
🐛 💻 -
alexmodis
🐛 +
alexmodis
🐛
andreoss
🐛
andrey81inmd
💻 🐛
anicoara
🐛
arunprasathav
🐛
asiercamara
🐛
astillich-igniti
💻 -
avesolovksyy
🐛 +
avesolovksyy
🐛
avishvat
🐛
avivmu
🐛
axelbarfod1
🐛
b-3-n
🐛
balbhadra9
🐛
base23de
🐛 -
bergander
🐛 +
bergander
🐛
berkam
💻 🐛
breizh31
🐛
caesarkim
🐛
carolyujing
🐛
cbfiddle
🐛
cesares-basilico
🐛 -
chrite
🐛 +
chrite
🐛
cobratbq
🐛
coladict
🐛
cosmoJFH
🐛
cristalp
🐛
crunsk
🐛
cwholmes
🐛 -
cyberjj999
🐛 +
cyberjj999
🐛
cyw3
🐛
d1ss0nanz
🐛
dalizi007
💻
danbrycefairsailcom
🐛
dariansanity
🐛
darrenmiliband
🐛 -
davidburstrom
🐛 +
davidburstrom
🐛
dbirkman-paloalto
🐛
deepak-patra
🐛
dependabot[bot]
💻 🐛
dinesh150
🐛
diziaq
🐛
dreaminpast123
🐛 -
duanyanan
🐛 +
duanyanan
🐛
dutt-sanjay
🐛
dylanleung
🐛
dzeigler
🐛
ekkirala
🐛
emersonmoura
🐛
fairy
🐛 -
filiprafalowicz
💻 +
filiprafalowicz
💻
foxmason
🐛
frankegabor
🐛
frankl
🐛
freafrea
🐛
fsapatin
🐛
gracia19
🐛 -
guo fei
🐛 +
guo fei
🐛
gurmsc5
🐛
gwilymatgearset
💻 🐛
haigsn
🐛
hemanshu070
🐛
henrik242
🐛
hongpuwu
🐛 -
hvbtup
💻 🐛 +
hvbtup
💻 🐛
igniti GmbH
🐛
ilovezfs
🐛
itaigilo
🐛
jakivey32
🐛
jbennett2091
🐛
jcamerin
🐛 -
jkeener1
🐛 +
jkeener1
🐛
jmetertea
🐛
johnra2
💻
josemanuelrolon
💻 🐛
kabroxiko
💻 🐛
karwer
🐛
kaulonline
🐛 -
kdaemonv
🐛 +
kdaemonv
🐛
kenji21
💻 🐛
kfranic
🐛
khalidkh
🐛
koalalam
🐛
krzyk
🐛
lasselindqvist
🐛 -
lgemeinhardt
🐛 +
lgemeinhardt
🐛
lihuaib
🐛
lonelyma1021
🐛
lpeddy
🐛
lujiefsi
💻
lukelukes
💻
lyriccoder
🐛 -
marcelmore
🐛 +
marcelmore
🐛
matchbox
🐛
matthiaskraaz
🐛
meandonlyme
🐛
mikesive
🐛
milossesic
🐛
mohan-chinnappan-n
💻 -
mriddell95
🐛 +
mriddell95
🐛
mrlzh
🐛
msloan
🐛
mucharlaravalika
🐛
mvenneman
🐛
nareshl119
🐛
nicolas-harraudeau-sonarsource
🐛 -
noerremark
🐛 +
noerremark
🐛
novsirion
🐛
oggboy
🐛
oinume
🐛
orimarko
💻 🐛
pacvz
💻
pallavi agarwal
🐛 -
parksungrin
🐛 +
parksungrin
🐛
patpatpat123
🐛
patriksevallius
🐛
pbrajesh1
🐛
phoenix384
🐛
piotrszymanski-sc
💻
plan3d
🐛 -
poojasix
🐛 +
poojasix
🐛
prabhushrikant
🐛
pujitha8783
🐛
r-r-a-j
🐛
raghujayjunk
🐛
rajeshveera
🐛
rajeswarreddy88
🐛 -
recdevs
🐛 +
recdevs
🐛
reudismam
💻 🐛
rijkt
🐛
rillig-tk
🐛
rmohan20
💻 🐛
rnveach
🐛
rxmicro
🐛 -
ryan-gustafson
💻 🐛 +
ryan-gustafson
💻 🐛
sabi0
🐛
scais
🐛
sebbASF
🐛
sergeygorbaty
💻
shilko2013
🐛
shiomiyan
📖 -
simeonKondr
🐛 +
simeonKondr
🐛
snajberk
🐛
sniperrifle2004
🐛
snuyanzin
🐛 💻
sratz
🐛
stonio
🐛
sturton
💻 🐛 -
sudharmohan
🐛 +
sudharmohan
🐛
suruchidawar
🐛
svenfinitiv
🐛
tashiscool
🐛
test-git-hook
🐛
testation21
💻 🐛
thanosa
🐛 -
tiandiyixian
🐛 +
tiandiyixian
🐛
tobwoerk
🐛
tprouvot
🐛 💻
trentchilders
🐛
triandicAnt
🐛
trishul14
🐛
tsui
🐛 -
winhkey
🐛 +
winhkey
🐛
witherspore
🐛
wjljack
🐛
wuchiuwong
🐛
xingsong
🐛
xioayuge
🐛
xnYi9wRezm
💻 🐛 -
xuanuy
🐛 +
xuanuy
🐛
xyf0921
🐛
yalechen-cyw3
🐛
yasuharu-sato
🐛
zenglian
🐛
zgrzyt93
💻 🐛
zh3ng
🐛 -
zt_soft
🐛 +
zt_soft
🐛
ztt79
🐛
zzzzfeng
🐛
Árpád Magosányi
🐛 From 03db70602f507e3b0a7180c7498b0e9c9a485dd8 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Fri, 16 Dec 2022 19:19:59 +0100 Subject: [PATCH 21/31] [doc] Update release notes (#4146, #4244) --- docs/pages/release_notes.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..ea7e8e435e 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -15,10 +15,13 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy ### Fixed Issues +* apex-security + * [#4146](https://github.com/pmd/pmd/issues/4146): \[apex] ApexCRUDViolation: Recognize User Mode in SOQL + DML ### API Changes ### External Contributions +* [#4244](https://github.com/pmd/pmd/pull/4244): \[apex] ApexCRUDViolation: user mode and system mode with test cases added - [Tarush Singh](https://github.com/Tarush-Singh35) (@Tarush-Singh35) {% endtocmaker %} From bef608058bba84dba777119fd0c1d248191638e2 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 17 Dec 2022 18:14:14 +0100 Subject: [PATCH 22/31] [java] UnusedPrivateField - rename property "reportForAnnotations" And other PR review fixups --- docs/pages/release_notes.md | 2 +- .../bestpractices/UnusedPrivateFieldRule.java | 11 +++++------ .../rule/bestpractices/xml/UnusedPrivateField.xml | 15 ++++++++++----- 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 8f93d11cb6..0be2255786 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -16,7 +16,7 @@ This is a {{ site.pmd.release_type }} release. #### Modified rules -* The Java rule {% rule java/bestpractices/UnusedPrivateField %} has a new property `annotations`. +* The Java rule {% rule java/bestpractices/UnusedPrivateField %} has a new property `reportForAnnotations`. This is a list of fully qualified names of the annotation types that should be reported anyway. If an unused field has any of these annotations, then it is reported. If it has any other annotation, then it is still considered to be used and is not reported. diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java index 41fc0ebc13..6a89593ad6 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/bestpractices/UnusedPrivateFieldRule.java @@ -7,7 +7,6 @@ package net.sourceforge.pmd.lang.java.rule.bestpractices; import static net.sourceforge.pmd.properties.PropertyFactory.stringListProperty; import java.util.ArrayList; -import java.util.Arrays; import java.util.List; import java.util.Map; import java.util.logging.Logger; @@ -54,18 +53,18 @@ public class UnusedPrivateFieldRule extends AbstractJavaRule { .desc("Field Names that are ignored from the unused check") .build(); - private static final PropertyDescriptor> ANNOTATIONS_DESCRIPTOR - = stringListProperty("annotations") + private static final PropertyDescriptor> REPORT_FOR_ANNOTATIONS_DESCRIPTOR + = stringListProperty("reportForAnnotations") .desc("Fully qualified names of the annotation types that should be reported anyway. If an unused field " + "has any of these annotations, then it is reported. If it has any other annotation, then " + "it is still considered to used and is not reported.") - .defaultValue(Arrays.asList("org.openqa.selenium.support.FindBy")) + .defaultValue(new ArrayList()) .build(); public UnusedPrivateFieldRule() { definePropertyDescriptor(IGNORED_ANNOTATIONS_DESCRIPTOR); definePropertyDescriptor(IGNORED_FIELD_NAMES); - definePropertyDescriptor(ANNOTATIONS_DESCRIPTOR); + definePropertyDescriptor(REPORT_FOR_ANNOTATIONS_DESCRIPTOR); } @Override @@ -107,7 +106,7 @@ public class UnusedPrivateFieldRule extends AbstractJavaRule { private boolean hasAnyAnnotation(Annotatable node) { List annotations = node.getDeclaredAnnotations(); - for (String reportAnnotation : getProperty(ANNOTATIONS_DESCRIPTOR)) { + for (String reportAnnotation : getProperty(REPORT_FOR_ANNOTATIONS_DESCRIPTOR)) { for (ASTAnnotation annotation : annotations) { if (TypeTestUtil.isA(reportAnnotation, annotation)) { return false; diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml index e51f15799a..ddc22494b2 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/UnusedPrivateField.xml @@ -772,8 +772,7 @@ public class C { [java] UnusedPrivateField doesn't find annotated unused private fields anymore #4166 (default) - 1 - 6 + 0 [java] UnusedPrivateField doesn't find annotated unused private fields anymore #4166 (configuration) - java.lang.Deprecated - 1 - 3 + java.lang.Deprecated|org.openqa.selenium.support.FindBy + 2 + 6,9 From 8e72aaf9ab312f97a5f9dcd98774f5f965f77c6a Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 17 Dec 2022 18:38:20 +0100 Subject: [PATCH 23/31] PR review fixups --- docs/pages/release_notes.md | 5 ++--- .../pmd/lang/java/rule/design/ExcessiveLengthRule.java | 2 -- 2 files changed, 2 insertions(+), 5 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index 1d093a5e44..c09af054f8 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -34,10 +34,9 @@ This is a {{ site.pmd.release_type }} release. #### Deprecated APIs -##### Internal API +##### For removal -Those APIs are not intended to be used by clients, and will be hidden or removed with PMD 7.0.0. You can identify -them with the `@InternalApi` annotation. You'll also get a deprecation warning. +These classes / APIs have been deprecated and will be removed with PMD 7.0.0. * {% jdoc java::lang.java.rule.design.ExcessiveLengthRule %} (Java) diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java index 08734e63e2..a012ab5c51 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/rule/design/ExcessiveLengthRule.java @@ -4,7 +4,6 @@ package net.sourceforge.pmd.lang.java.rule.design; -import net.sourceforge.pmd.annotation.InternalApi; import net.sourceforge.pmd.lang.java.ast.JavaNode; import net.sourceforge.pmd.lang.java.rule.AbstractStatisticalJavaRule; import net.sourceforge.pmd.stat.DataPoint; @@ -20,7 +19,6 @@ import net.sourceforge.pmd.stat.DataPoint; * @deprecated This base class is not needed anymore and will be removed with PMD 7. */ @Deprecated -@InternalApi public class ExcessiveLengthRule extends AbstractStatisticalJavaRule { private Class nodeClass; From d7bab260b3fdf9a7fe32dd72233b4f80b4bfcc54 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Sat, 17 Dec 2022 19:58:47 +0100 Subject: [PATCH 24/31] [java] WhileLoopWithLiteralBoolean - don't limit to two bool literals Fixes #4250 --- docs/pages/release_notes.md | 2 + .../resources/category/java/bestpractices.xml | 18 ++- .../xml/WhileLoopWithLiteralBoolean.xml | 121 ++++++++++++++++-- 3 files changed, 123 insertions(+), 18 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..d26a83b97f 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -15,6 +15,8 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy ### Fixed Issues +* java-bestpractices + * [#4250](https://github.com/pmd/pmd/issues/4250): \[java] WhileLoopWithLiteralBoolean - false negative with complex expressions still occurs in PMD 6.52.0 ### API Changes diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index 31a6d776f9..dac68f9290 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -2187,15 +2187,16 @@ a block `{}` is sufficient. = 1]] | @@ -2206,14 +2207,19 @@ a block `{}` is sufficient. | (: do-while loops with conditional or'ed boolean literals, maybe parenthesized :) //DoStatement[Expression/(InclusiveOrExpression|ConditionalOrExpression|(PrimaryExpression/PrimaryPrefix/Expression/(InclusiveOrExpression|ConditionalOrExpression))) - [count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral) = 2]] + (: at least one true literal :) + [count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral[@True = true()]) >= 1 + (: or no var access - that means, we have only boolean literals here :) + or count(PrimaryExpression/PrimaryPrefix/Name) = 0 + ]] | (: do-while loops with conditional and'ed boolean literals, maybe parenthesized :) //DoStatement[Expression/(AndExpression|ConditionalAndExpression|(PrimaryExpression/PrimaryPrefix/Expression/(AndExpression|ConditionalAndExpression))) (: at least one false literal :) [count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral[@True = false()]) >= 1 - (: or two true literals (e.g. true & true) :) - or count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral[@True = true()]) = 2]] + (: or no var access - that means, we have only boolean literals here :) + or count(PrimaryExpression/PrimaryPrefix/Name) = 0 + ]] ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml index 36b063fd31..e919b7c6d5 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml @@ -22,8 +22,8 @@ class Foo { do while true | true - 4 - 3,4,5,6 + 8 + 3,4,5,6,9,10,11,12 @@ -54,8 +60,8 @@ class Foo { do while false | false #3455 - 2 - 3,5 + 4 + 3,5,8,10 - do while false || false #3455 - 2 - 3,6 + do while false & false + 8 + 3,5,7,9,13,15,17,19 + + do while true && true + 8 + 3,5,7,9,12,14,16,18 + + + do while call 0 @@ -129,8 +194,8 @@ class Foo { while false | false - 4 - 3,4,5,6 + 8 + 3,4,5,6,8,9,10,11 @@ -231,6 +301,33 @@ class Foo { do {} while ((true && true)); } } +]]> + + + + [java] WhileLoopWithLiteralBoolean - false negative with complex expressions still occurs in PMD 6.52.0 #4250 + 4 + 3,7,11,15 + From c2de15e4c5095eff19a04d303cd8fb1575c1b2a9 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 19 Dec 2022 15:26:43 +0100 Subject: [PATCH 25/31] [java] WhileLoopWithLiteralBoolean - fix false positives --- .../main/resources/category/java/bestpractices.xml | 10 ++++++---- .../xml/WhileLoopWithLiteralBoolean.xml | 14 ++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/bestpractices.xml b/pmd-java/src/main/resources/category/java/bestpractices.xml index dac68f9290..d1c140cb6c 100644 --- a/pmd-java/src/main/resources/category/java/bestpractices.xml +++ b/pmd-java/src/main/resources/category/java/bestpractices.xml @@ -2209,16 +2209,18 @@ a block `{}` is sufficient. //DoStatement[Expression/(InclusiveOrExpression|ConditionalOrExpression|(PrimaryExpression/PrimaryPrefix/Expression/(InclusiveOrExpression|ConditionalOrExpression))) (: at least one true literal :) [count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral[@True = true()]) >= 1 - (: or no var access - that means, we have only boolean literals here :) - or count(PrimaryExpression/PrimaryPrefix/Name) = 0 + (: or only boolean literal and no no var access :) + or count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral) >= 1 + and count(PrimaryExpression/PrimaryPrefix/Name) = 0 ]] | (: do-while loops with conditional and'ed boolean literals, maybe parenthesized :) //DoStatement[Expression/(AndExpression|ConditionalAndExpression|(PrimaryExpression/PrimaryPrefix/Expression/(AndExpression|ConditionalAndExpression))) (: at least one false literal :) [count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral[@True = false()]) >= 1 - (: or no var access - that means, we have only boolean literals here :) - or count(PrimaryExpression/PrimaryPrefix/Name) = 0 + (: or only boolean literal and no no var access :) + or count(PrimaryExpression/PrimaryPrefix/Literal/BooleanLiteral) >= 1 + and count(PrimaryExpression/PrimaryPrefix/Name) = 0 ]] ]]> diff --git a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml index e919b7c6d5..763543045a 100644 --- a/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml +++ b/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/WhileLoopWithLiteralBoolean.xml @@ -328,6 +328,20 @@ public class Foo { } while (false & false & false & false & false); } } +]]> + + + + False positives without literal booleans + 0 + From 87c991f31d4806033f5f55f1b37e36f576a6cce7 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 19 Dec 2022 15:55:22 +0100 Subject: [PATCH 26/31] [java] DoNotUseThreads: Fix false negatives with field declarations in anonymous classes --- .../category/java/multithreading.xml | 8 +-- .../multithreading/xml/DoNotUseThreads.xml | 52 ++++++++++++++----- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/pmd-java/src/main/resources/category/java/multithreading.xml b/pmd-java/src/main/resources/category/java/multithreading.xml index 41f199bf69..8d15f634d3 100644 --- a/pmd-java/src/main/resources/category/java/multithreading.xml +++ b/pmd-java/src/main/resources/category/java/multithreading.xml @@ -157,10 +157,12 @@ Also, EJBs might be moved between machines in a cluster and only managed resourc False negatives with field declarations - 10 - 5,6,7,8,11,15,16,19,20,23 + 19 + 6,7,8,9,13,14,20,26,30,31,33,34,37,38,42,43,47,48,51 System.out.println("foo")).start(); // violation expected - Thread t2 = new Thread(); // one violation expected + new Thread(() -> System.out.println("foo")).start(); // 30: violation expected + Thread t2 = new Thread(); // 31: one violation expected + // report two violations, if on two separate lines + Thread t3 = // 33: violation expected + new Thread(); // 34: violation expected } - public Thread getThread() { // violation expected - return new Thread(); // violation expected + public Thread getThread() { // 37: violation expected + return new Thread(); // 38: violation expected } - private static class MyThread extends Thread { } // violation expected + @Override + public Thread getThread1() { // 42: violation expected + return new Thread(); // 43: violation expected + } + + @Override + public Thread getThread2(final Runnable r) { // 47: violation expected + return new Thread(r); // 48: violation expected + } + + private static class MyThread extends Thread { } // 51: violation expected } ]]> From 2cdef852c5a5f3fedb1be63dfe05bb1be2636539 Mon Sep 17 00:00:00 2001 From: Andreas Dangel Date: Mon, 19 Dec 2022 16:13:11 +0100 Subject: [PATCH 27/31] [java][doc] AvoidAssertAsIdentifier and AvoidEnumAsIdentifier - clarify use case Fixes #4164 --- docs/pages/release_notes.md | 2 ++ .../main/resources/category/java/errorprone.xml | 14 ++++++++++++-- 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/docs/pages/release_notes.md b/docs/pages/release_notes.md index b8f8783555..72f1a36bba 100644 --- a/docs/pages/release_notes.md +++ b/docs/pages/release_notes.md @@ -15,6 +15,8 @@ This is a {{ site.pmd.release_type }} release. ### New and noteworthy ### Fixed Issues +* java-errorprone + * [#4164](https://github.com/pmd/pmd/issues/4164): \[java]\[doc] AvoidAssertAsIdentifier and AvoidEnumAsIdentifier - clarify use case ### API Changes diff --git a/pmd-java/src/main/resources/category/java/errorprone.xml b/pmd-java/src/main/resources/category/java/errorprone.xml index 8ba16182a0..c685142b91 100644 --- a/pmd-java/src/main/resources/category/java/errorprone.xml +++ b/pmd-java/src/main/resources/category/java/errorprone.xml @@ -130,12 +130,17 @@ public class Violation { -Use of the term 'assert' will conflict with newer versions of Java since it is a reserved word. +Use of the term `assert` will conflict with newer versions of Java since it is a reserved word. + +Since Java 1.4, the token `assert` became a reserved word and using it as an identifier will +result in a compilation failure for Java 1.4 and later. This rule is therefore only useful +for old Java code before Java 1.4. It can be used to identify problematic code prior to a Java update. 2 @@ -341,12 +346,17 @@ private void buz(String x) {} -Use of the term 'enum' will conflict with newer versions of Java since it is a reserved word. +Use of the term `enum` will conflict with newer versions of Java since it is a reserved word. + +Since Java 1.5, the token `enum` became a reserved word and using it as an identifier will +result in a compilation failure for Java 1.5 and later. This rule is therefore only useful +for old Java code before Java 1.5. It can be used to identify problematic code prior to a Java update. 2 From c7148064474ae5e0639bac7dc506d7eebb74e0d8 Mon Sep 17 00:00:00 2001 From: Krzysztof Debski Date: Wed, 28 Dec 2022 10:37:58 +0100 Subject: [PATCH 28/31] [java] Fix finding lambda scope in record compact constructor --- .../java/qname/QualifiedNameResolver.java | 9 ++++--- .../ASTCompactConstructorDeclarationTest.java | 24 +++++++++++++++++++ 2 files changed, 30 insertions(+), 3 deletions(-) create mode 100644 pmd-java/src/test/java/net/sourceforge/pmd/lang/java/ast/ASTCompactConstructorDeclarationTest.java diff --git a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java index 44351217c8..6920519ae8 100644 --- a/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java +++ b/pmd-java/src/main/java/net/sourceforge/pmd/lang/java/qname/QualifiedNameResolver.java @@ -18,6 +18,7 @@ import net.sourceforge.pmd.lang.ast.Node; import net.sourceforge.pmd.lang.java.ast.ASTAllocationExpression; import net.sourceforge.pmd.lang.java.ast.ASTAnyTypeDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTClassOrInterfaceDeclaration; +import net.sourceforge.pmd.lang.java.ast.ASTCompactConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTCompilationUnit; import net.sourceforge.pmd.lang.java.ast.ASTConstructorDeclaration; import net.sourceforge.pmd.lang.java.ast.ASTEnumConstant; @@ -287,7 +288,8 @@ public class QualifiedNameResolver extends JavaParserVisitorReducedAdapter { * was declared in. It can be: *