Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[core] Ruleset loading processes commented rules #4378

Closed
blacelle opened this issue Feb 4, 2023 · 3 comments · Fixed by #4795
Closed

[core] Ruleset loading processes commented rules #4378

blacelle opened this issue Feb 4, 2023 · 3 comments · Fixed by #4795
Labels
a:bug PMD crashes or fails to analyse a file. in:ruleset-xml About the ruleset schema/parser
Milestone
7.0.0

Comments

@blacelle
Copy link

blacelle commented Feb 4, 2023 

[INFO] --- maven-pmd-plugin:3.20.0:pmd (pmd) @ pepper-agent ---
[INFO] PMD version: 6.54.0

I consider a ruleset with a commented rule:

<!-- <rule ref="rulesets/java/unnecessary.xml"> -->
<!-- <exclude name="UnnecessaryFinalModifier" /> -->
<!-- </rule> -->

This lead to warnings referring to this commented block:

[WARNING] Applying rule set filter for exclusions: The rule "UnnecessaryFinalModifier" has been renamed to "UnnecessaryModifier". Please change your ruleset!
[WARNING] Applying rule set filter for exclusions: The rule "UnnecessaryFinalModifier" has been renamed to "UnnecessaryModifier". Please change your ruleset!

This can be reproduced with a ruleset like:

<?xml version="1.0" encoding="UTF-8"?>
<ruleset xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" name="Pepper" xmlns="http://pmd.sf.net/ruleset/1.0.0" xsi:noNamespaceSchemaLocation="http://pmd.sf.net/ruleset_xml_schema.xsd" xsi:schemaLocation="http://pmd.sf.net/ruleset/1.0.0 http://pmd.sf.net/ruleset_xml_schema.xsd">

	<!-- https://pmd.github.io/pmd-6.8.0/pmd_rules_java.html -->
	<description>Nearly all PMD rules, except those considered not relevant by Pepper</description>

	<rule ref="category/java/multithreading.xml">
		<exclude name="DoNotUseThreads" />
		<exclude name="UseConcurrentHashMap" />
	</rule>

	<!-- <rule ref="rulesets/java/unnecessary.xml"> -->
	<!-- MAT introduced many final classes with final methods: keep final methods in case the final on class is removed -->
	<!-- <exclude name="UnnecessaryFinalModifier" /> -->
	<!-- </rule> -->

</ruleset>

This feel very strange to get reports related to a commented block. Happily, it seems the commented rule is not iuncluded for processing (just in the WARN, which is then a false-positive).

Here is a debug log, if it may help:

[INFO] PMD version: 6.54.0
[DEBUG] Configured jul-to-slf4j bridge for net.sourceforge.pmd
[DEBUG] Using language java+version:1.8
[DEBUG] Adding classpath entry: </Users/blacelle/workspace3/pepper/agent/target/classes>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/com/google/guava/guava/31.1-jre/guava-31.1-jre.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/com/google/guava/failureaccess/1.0.1/failureaccess-1.0.1.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/com/google/guava/listenablefuture/9999.0-empty-to-avoid-conflict-with-guava/listenablefuture-9999.0-empty-to-avoid-conflict-with-guava.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/com/google/code/findbugs/jsr305/3.0.2/jsr305-3.0.2.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/com/google/errorprone/error_prone_annotations/2.11.0/error_prone_annotations-2.11.0.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/com/google/j2objc/j2objc-annotations/1.3/j2objc-annotations-1.3.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/org/ehcache/sizeof/0.4.3/sizeof-0.4.3.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/net/bytebuddy/byte-buddy-agent/1.12.22/byte-buddy-agent-1.12.22.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/org/checkerframework/checker-qual/3.29.0/checker-qual-3.29.0.jar>
[DEBUG] Adding classpath entry: </Users/blacelle/.m2/repository/org/slf4j/slf4j-api/1.7.36/slf4j-api-1.7.36.jar>
[WARNING] Applying rule set filter for exclusions: The rule "UnnecessaryFinalModifier" has been renamed to "UnnecessaryModifier". Please change your ruleset!
[WARNING] Applying rule set filter for exclusions: The rule "UnnecessaryFinalModifier" has been renamed to "UnnecessaryModifier". Please change your ruleset!
[DEBUG] Rules loaded from /Users/blacelle/workspace3/pepper/agent/target/pmd/rulesets/pepper.pmd.rulesets.xml:
[DEBUG] - AvoidSynchronizedAtMethodLevel (Java)
[DEBUG] - AvoidThreadGroup (Java)
[DEBUG] - AvoidUsingVolatile (Java)
[DEBUG] - DontCallThreadRun (Java)
[DEBUG] - DoubleCheckedLocking (Java)
[DEBUG] - NonThreadSafeSingleton (Java)
[DEBUG] - UnsynchronizedStaticFormatter (Java)
[DEBUG] - UseNotifyAllInsteadOfNotify (Java)
[DEBUG] Executing PMD...
[DEBUG] Processing /Users/blacelle/workspace3/pepper/agent/src/main/java/eu/solven/pepper/agent/InstrumentationAgent.java
[DEBUG] Processing /Users/blacelle/workspace3/pepper/agent/src/main/java/eu/solven/pepper/agent/PepperAgentHelper.java
[DEBUG] Processing /Users/blacelle/workspace3/pepper/agent/src/main/java/eu/solven/pepper/agent/VirtualMachineWithoutToolsJar.java
[DEBUG] Processing /Users/blacelle/workspace3/pepper/agent/src/main/java/org/ehcache/sizeof/impl/AgentLoaderPepperSpy.java
[DEBUG] No type information for node Expression
[DEBUG] No type information for node Expression
[DEBUG] No type information for node Expression
[DEBUG] Method MethodType [method=public abstract void org.slf4j.Logger.warn(org.slf4j.Marker,java.lang.String,java.lang.Object,java.lang.Object), returnType=JavaTypeDefinition [clazz=void, definitionType=EXACT, genericArgs=[], isGeneric=false]
, argTypes=[JavaTypeDefinition [clazz=interface org.slf4j.Marker, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.String, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
]] couldnt be resolved
[DEBUG] Method MethodType [method=public abstract void org.slf4j.Logger.trace(org.slf4j.Marker,java.lang.String,java.lang.Object,java.lang.Object), returnType=JavaTypeDefinition [clazz=void, definitionType=EXACT, genericArgs=[], isGeneric=false]
, argTypes=[JavaTypeDefinition [clazz=interface org.slf4j.Marker, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.String, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
]] couldnt be resolved
[DEBUG] No type information for node Expression
[DEBUG] Method MethodType [method=public abstract void org.slf4j.Logger.warn(org.slf4j.Marker,java.lang.String,java.lang.Object,java.lang.Object), returnType=JavaTypeDefinition [clazz=void, definitionType=EXACT, genericArgs=[], isGeneric=false]
, argTypes=[JavaTypeDefinition [clazz=interface org.slf4j.Marker, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.String, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
]] couldnt be resolved
[DEBUG] No type information for node Expression
[DEBUG] No type information for node Expression
[DEBUG] No type information for node Expression
[DEBUG] Method MethodType [method=public abstract void org.slf4j.Logger.warn(org.slf4j.Marker,java.lang.String,java.lang.Object,java.lang.Object), returnType=JavaTypeDefinition [clazz=void, definitionType=EXACT, genericArgs=[], isGeneric=false]
, argTypes=[JavaTypeDefinition [clazz=interface org.slf4j.Marker, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.String, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
, JavaTypeDefinition [clazz=class java.lang.Object, definitionType=EXACT, genericArgs=[], isGeneric=false]
]] couldnt be resolved
[DEBUG] No type information for node Expression
[DEBUG] No type information for node Expression
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidSynchronizedAtMethodLevel (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidSynchronizedAtMethodLevel (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidSynchronizedAtMethodLevel (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidSynchronizedAtMethodLevel (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidThreadGroup (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidThreadGroup (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidThreadGroup (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidThreadGroup (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidUsingVolatile (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidUsingVolatile (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidUsingVolatile (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: AvoidUsingVolatile (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: DontCallThreadRun (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: DontCallThreadRun (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: DontCallThreadRun (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: DontCallThreadRun (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: UseNotifyAllInsteadOfNotify (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: UseNotifyAllInsteadOfNotify (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: UseNotifyAllInsteadOfNotify (Multithreading)
[DEBUG] Using rule chain for XPath 2.0 rule: UseNotifyAllInsteadOfNotify (Multithreading)
[DEBUG] PMD finished. Found 0 violations.
[DEBUG] Removing excluded violations. Using 0 configured exclusions.
[DEBUG] Excluded 0 violations.
[DEBUG] Target PMD output file: /Users/blacelle/workspace3/pepper/agent/target/pmd.xml
@blacelle blacelle added the a:bug PMD crashes or fails to analyse a file. label Feb 4, 2023
blacelle added a commit to solven-eu/pepper that referenced this issue Feb 4, 2023
@blacelle
Remove deprecated commented rule exclusions pmd/pmd#4378
a686110
@jsotuyod
Copy link
Member

This is down to how ruleset compatibility works. It simply applies Regular expressions on the ruleset to update it's contents before actually parsing it. Disabling ruleset compatibility is possible (ie: using --no-ruleset-compatibility in the CLI), but that would force you to manually keep your rulesets up to date as rules are renamed / moved / replaced.

We are going to remove this from PMD 7 however (see #4314), and PMD 6 is no longer getting updates after today's release.

@blacelle
Copy link
Author

PMD 6 is no longer getting updates after today's release.

No more PMD 6 updates, while PMD7 is not yet released ? OK

@oowekyala
Copy link
Member

PMD 7 currently hasn't removed the flag as described in #4314, but it also does not have this bug. Instead of applying the regex on the entire file, it does so only on the ref attribute of the rule references we parse. So comments are naturally ignored.

@oowekyala oowekyala added this to the 7.0.0 milestone Feb 26, 2023
@adangel adangel changed the title Ruleset loading processes commented rules [core] Ruleset loading processes commented rules Feb 28, 2023
@adangel adangel added the in:ruleset-xml About the ruleset schema/parser label Jan 11, 2024
This was referenced Jan 11, 2024
Merged
Closed
@adangel adangel closed this as completed in ba602d1 Feb 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
a:bug PMD crashes or fails to analyse a file. in:ruleset-xml About the ruleset schema/parser
Projects
None yet
Milestone
7.0.0
Development

Successfully merging a pull request may close this issue.

[core] Remove ruleset compatibility filter (RuleSetFactoryCompatibili… adangel/pmd
4 participants
@jsotuyod @adangel @blacelle @oowekyala