This document describes minor differences in the behavior of certain rules in PMD 7. Some of those items may be considered bugs that need to be fixed in pmd 7. Others are just reported here for documentation

• AvoidReassigningLoopVariables:

  • The rule might not report anymore all reassignment of the control variable in for-loops, see the test case.
  • [java] Usage resolution for java variables #3113 (comment)
  • This test case is deactivated and was working before with PMD 6.x. It will only detect now the last two assignments (15,16) but not the two before (13,14):

    pmd/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml

    Lines 676 to 704 in 9f13f12

    	<!-- Deactivated because this is hard to implement in the general case. -->
    	<test-code disabled="true">
    	<description>violation: various conditional reassignments of 'for' loop variable, skip allowed</description>
    	<rule-property name="forReassign">skip</rule-property>
    	<expected-problems>4</expected-problems>
    	<expected-linenumbers>13,14,15,16</expected-linenumbers>
    	<code><![CDATA[
    	public class Foo {
    	void foo(int bar) {
    	for (int i=0; i < 10; i++) {
    	doSomethingWith(i);
    	if (foo()) {
    	i++;
    	i--;
    	++i;
    	--i;
    	i += 1;
    	i -= 1;
    	doSomethingWith(i++);
    	i *= 1; // not OK
    	i /= 1; // not OK
    	i = 5; // not OK
    	doSomethingWith(i = 5); // not OK
    	}
    	}
    	}
    	}
    	]]></code>
    	</test-code>

  • Test case in PMD 6.55.0:

    pmd/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/bestpractices/xml/AvoidReassigningLoopVariables.xml

    Lines 580 to 607 in ef34553

    	<test-code>
    	<description>violation: various conditional reassignments of 'for' loop variable, skip allowed</description>
    	<rule-property name="forReassign">skip</rule-property>
    	<expected-problems>4</expected-problems>
    	<expected-linenumbers>13,14,15,16</expected-linenumbers>
    	<code><![CDATA[
    	public class Foo {
    	void foo(int bar) {
    	for (int i=0; i < 10; i++) {
    	doSomethingWith(i);
    	if (foo()) {
    	i++;
    	i--;
    	++i;
    	--i;
    	i += 1;
    	i -= 1;
    	doSomethingWith(i++);
    	i *= 1; // not OK
    	i /= 1; // not OK
    	i = 5; // not OK
    	doSomethingWith(i = 5); // not OK
    	}
    	}
    	}
    	}
    	]]></code>
    	</test-code>

• Reported line ranges - [java] Inconsistent line range in RuleViolation for CognitiveComplexity rule between PMD 6.54.0 and 7.0.0-rc1 #4439

• "ArrayIsStoredDirectly now reports on the assignment and not the formal parameter so line numbers will probably move." - [java] Update ArrayIsStoredDirectly and MethodReturnsInternalArray #3127

• For the statistical rules (like NcssCount, ExcessivePackageBodyLineCount, ...)
  the parameter "minimum" is now an integer and not a decimal anymore.
  See [core] Remove statistical rule (final step) #2005,
  [java] Remove java statistical rules #1772,
  [apex] Remove apex statistical rules #1750.

  • Removed properties topscore and sigma for the following rules:
    • Apex: ExcessiveClassLength 6.55.0 7.0.0-rc1
    • Apex: ExcessiveParameterList 6.55.0 7.0.0-rc1
    • Apex: ExcessivePublicCount 6.55.0 7.0.0-rc1
    • Apex: NcssConstructorCount 6.55.0 7.0.0-rc1
    • Apex: NcssMethodCount 6.55.0 7.0.0-rc1
    • Apex: NcssTypeCount 6.55.0 7.0.0-rc1
    • Java: Rules, that are anyway deprecated: ExcessiveClassLength, ExcessiveMethodLength, NcssConstructorCount, NcssMethodCount, NcssTypeCount
    • Java: ExcessiveImports 6.55.0 7.0.0-rc1
    • Java: ExcessiveParameterList 6.55.0 7.0.0-rc1
    • Java: ExcessivePublicCount 6.55.0 7.0.0-rc1
    • Java: SwitchDensity 6.55.0 7.0.0-rc1
      • ⚠️ inconsistency: minimum is still a double, not an int here
      • while we calculate the ratio between number of statements and number of labels, we shouldn't pretend that this is an exact rule - it's anyway a best guess, that high switch density is a problem. I don't think, it makes a big difference between 10.2 and 10. You e.g. want to report, when the density is bigger than 10 or bigger then 5. We don't need to report level accuracy here.
    • PLSQL: ExcessiveMethodLength 6.55.0 7.0.0-rc1
    • PLSQL: ExcessiveObjectLength 6.55.0 7.0.0-rc1
    • PLSQL: ExcessivePackageBodyLength 6.55.0 7.0.0-rc1
    • PLSQL: ExcessivePackageSpecificationLength 6.55.0 7.0.0-rc1
    • PLSQL: ExcessiveParameterList 6.55.0 7.0.0-rc1
    • PLSQL: ExcessiveTypeLength 6.55.0 7.0.0-rc1
    • PLSQL: NcssMethodCount 6.55.0 7.0.0-rc1
    • PLSQL: NcssObjectCount 6.55.0 7.0.0-rc1
    • PLSQL: NPathComplexity 6.55.0 7.0.0-rc1
    • VM: ExcessiveTemplateLength 6.55.0 7.0.0-rc1

• violationSuppressXPath now requires XPath 2.0 or 3.1 - in PMD6 it was XPath 1.0. This is a breaking change. See also [core] Select XPath version in violationSuppressXPath #1676.

• Changed/removed properties for rules - already documented in the release notes, but incomplete

  • Java: NPathComplexity: property minimum removed (was deprecated with 6.0.0 already)
  • Apex: code climate properties removed - see [apex,vf] Remove CodeClimate dependency #1648. Properties: cc_categories, cc_remediation_points_multiplier, cc_block_highlighting
    • many rules
  • Java: MethodNamingConventions: property checkNativeMethods removed (deprecated since 6.3.0)
  • Java: CyclomaticComplexity: property reportLevel removed (deprecated since 6.0.0)
  • Java: ImmutableField: property ignoredAnnotations, see [java] ImmutableField: Remove deprecated property "ignoredAnnotations" #4489
  • Java: AvoidDuplicateLiterals: property exceptionfile removed (deprecated since 6.10.0)

• Other disabled test cases in PMD 7, that were enabled before in PMD 6.x

  • PreserveStackTrace: 3 test cases disabled, but these test cases are new for PMD 7.
  • UnusedAssignment: 1 test case disabled. New test case only present in PMD 7.
  • UnusedPrivateMethod: 2 test cases disabled. Both are disabled in PMD 6 as well.
  • UseVarArgs: 1 test case disabled. Disabled in PMD 6 as well.
  • MethodNamingConventions: 1 test case disabled. Disabled in PMD 6 as well.
  • UnnecessaryFullyQualifiedName: 1 test case disabled. New test case only present in PMD 7.
  • UnnecessaryImport: 1 test case disabled.
    • "Used static import" - will produce a false positive with incomplete auxclasspath:

      pmd/pmd-java/src/test/resources/net/sourceforge/pmd/lang/java/rule/codestyle/xml/UnnecessaryImport.xml

      Lines 161 to 173 in 5eee9f6

      	<test-code disabled="true">
      	<!-- This doesn't work anymore since the import is unresolved -->
      	<description>Used static import</description>
      	<expected-problems>0</expected-problems>
      	<code><![CDATA[
      	import static com.foo.MyClass.staticMethod;
      	public class Foo {
      	public Foo(){
      	staticMethod("This is OK");
      	}
      	}
      	]]></code>
      	</test-code>

  • UnnecessaryReturn: 1 test case disabled. New test case only present in PMD 7.
  • InvalidLogMessageFormat: 1 test case disabled. Disabled in PMD 6 as well
  • ✔️ Conclusion - nothing to do here. No special differences visible.

• Check diff regression report:

  • https://chunk.io/pmd/9ba3a41020364534a828c66631585431/diff/index.html
  • Created with the following command: bundle exec pmdtester --local-git-repo ../pmd --base-branch pmd_releases/6.55.0 --patch-branch master --config ../pmd/.ci/files/all-regression-rules.xml --list-of-project ../pmd/.ci/files/project-list.xml --mode online --error-recovery --baseline-download-url https://pmd-code.org/pmd-regression-tester/
  • Apex: ExcessivePublicCount: in PMD 6, it reported it per file. Now it is reported per class. This makes a difference when inner classes are used. That is actually a implicit bug fix done with PMD 7 - see https://chunk.io/pmd/9ba3a41020364534a828c66631585431/diff/fflib-apex-common/index.html#section-violations . E.g. fflib_Application.cls
    • see [apex] Improve ExcessivePublicCount message #4497
    • [apex] ExcessivePublicCount doesn't consider inner classes correctly #4509
  • Java: LocalVariableCouldBeFinal - seems to have now fixed a false positive (kind of) for lambdas but also introduced a false negative for local variables declared with "var". But not only for var. See https://chunk.io/pmd/9ba3a41020364534a828c66631585431/diff/java-regression-tests/index.html#section-violations
    • It's actually now, that we don't report unused variables anymore. The removed violations are all unused vars.
    • [java] LocalVariableCouldBeFinal shouldn't report unused variables #4511
  • Java: MethodArgumentCouldBeFinal - false negative. Maybe only for varargs or array arguments. No, also for int params. Maybe we just don't report unused params anymore? see https://chunk.io/pmd/9ba3a41020364534a828c66631585431/diff/java-regression-tests/index.html#section-violations
    • Yes, unused params are not reported anymore.
    • [java] MethodArgumentCouldBeFinal shouldn't report unused parameters #4512
  • Java: ShortVariable seems to find now also short enum constant names. Maybe update rule doc?
    • Yes, that's due to the grammar change: The enum constant now produces also a VariableDeclarationId node.
  • Java: CommentSize doesn't count the leading comment markers like // or /* or * anymore... seems like. TODO, verify.
    • Yes, that's the case: see [java] Improve CommentSize #4369
  • Java: ImplicitySwitchFallthrough - now reports on the case statements rather than on the switch statement. This results in more violations (maybe multiple per switch). But it is more accurate that way.
  • Java: ClassWithOnlyPrivateConstructorsShouldBeFinal
    • ✔️ now finds inner classes reliably. This is due to fix [java] ClassWithOnlyPrivateConstructorsShouldBeFinal can't detect inner class #2536 probably
  • Java: PrematureDeclaration
    • ✔️ Doesn't report unused variables anymore, see [java] PrematureDeclaration false positive for unused variables #3221
  • Java: UselessOperationOnImmutable
    • finds more cases now. But on first glance, they are all legit.
    • [java] UselessOperationOnImmutable various false negatives with String #4513
  • Java: AvoidLiteralsInIfCondition
    • possible false negative when a nested condition is used, e.g. https://github.com/checkstyle/checkstyle/blob/checkstyle-9.1/src/main/java/com/puppycrawl/tools/checkstyle/checks/design/ThrowsCountCheck.java#L285 is not detected anymore
    • behavior of the rule depends on the property ignoreExpressions which is by default true: So we have actually one fix for false positives (expressions are correctly now ignored) and one fix for false negatives for String literals, which weren't reported before PMD 7.
    • [java] AvoidLiteralsInIfCondition false positive and negative for String literals when ignoreExpressions=true #4514
  • Java: CommentRequired seems to have changed a lot. Many new violations but also many removed. see https://chunk.io/pmd/9ba3a41020364534a828c66631585431/diff/checkstyle/index.html#section-violations
    • Some difference in behavior: for class comments, when the class is annotated, then the class comment must be before the annotation. e.g. https://github.com/checkstyle/checkstyle/blob/checkstyle-9.1/src/it/resources/com/google/checkstyle/test/chapter7javadoc/rule734nonrequiredjavadoc/InputInvalidJavadocPosition.java#L48 is now correctly detected to have no javadoc. Actually, that's "just" a bugfix.
    • Annotations are considered. Maybe they weren't with PMD 6?
      • no they weren't. The node type changed from AnnotationMethodDeclaration to MethodDeclaration - now these are considered methods and are reported
      • But the annotation type itself is missing, as well as records.
      • Created for that: [java] CommentRequired doesn't consider annotations and records #4515
    • There are really some false negatives now: https://github.com/checkstyle/checkstyle/blob/checkstyle-9.1/src/it/resources/org/checkstyle/suppressionxpathfilter/noclone/SuppressionXpathRegressionNoCloneOne.java#L5
      • No, these are overridden methods. PMD now can detect overridden methods even without the @Override annotations. See [java] Improve ASTMethodDeclaration::isOverridden #3757.
  • Java: UnusedLocalVariable
    • ⚠️ seems to have a lot new false positives. E.g. i is said to be unused in https://github.com/checkstyle/checkstyle/blob/checkstyle-9.1/src/it/resources/com/google/checkstyle/test/chapter4formatting/rule451wheretobreak/InputMethodParamPad.java#L202 but it is read when assigning a new value to itself. Or do we ignore the expressions, if the left side also contains the variable? While this example is very contrived, I think, this is a false positive.
      • actually, these new cases are correct. Yes, compound assignment don't count as usage anymore, so all the new cases do something like i++, but otherwise don't use the vars.
      • See [java] UnusedLocalVariable: false-negative with compound assignments #4517
    • ✔️ now also finds unused closeables in try-with-resources. While the closeable will be implicitly used to close it, I think it's good to highlight such cases, as it might be just unnecessary code. Example: https://github.com/checkstyle/checkstyle/blob/checkstyle-9.1/src/test/resources-noncompilable/com/puppycrawl/tools/checkstyle/checks/coding/fallthrough/InputFallThrough.java#L29
      • See [java] UnusedLocalVariable: false-negative with try-with-resources #4516
    • Overall, we have 3 false negative fixed, which might lead to a lot of new violations when upgrading to PMD 7.
    • And one false positive: [java] UnusedLocalVariable: false-positive with multiple for-loop indices #4518
  • Java: UnnecessaryImport
    • possible false positive: https://github.com/checkstyle/checkstyle/blob/checkstyle-9.1/src/test/java/com/puppycrawl/tools/checkstyle/AbstractXmlTestSupport.java#L22 - it's used in line 230. Did I have a wrong auxclasspath here? Hm... I had /com/google/truth/truth/1.1.3/truth-1.1.3.jar on the classpath though
    • This has already been reported here: [java] [7.0-rc1] UnnecessaryImport - Unused static import is being used #4432