[core] Deprecate some syntax for ruleset references #3958
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some undocumented ruleset reference forms are deprecated: - java-basic (resolves to rulesets/java/basic.xml), refs pmd#2352 - 641 (resolves to rulesets/releases/641.xml) Also using `<rule ref="rset1.xml,rset2.xml">` is deprecated. As before, only the first ruleset is considered, but now the rest is not ignored silently. Refs pmd#2612
Another change is that the error in case of unresolved ruleset doesn't just dump the entire classpath, which is unreadable. Instead the classpath is logged with a debug level at the start of execution.
oowekyala
added
is:deprecation
Generated by 🚫 Danger |
adangel
approved these changes
May 20, 2022
| @@ -539,7 +546,7 @@ private void parseRuleSetReferenceNode(RuleSetBuilder ruleSetBuilder, Element ru | |||
| // load the ruleset with minimum priority low, so that we get all rules, to be able to exclude any rule | |||
| // minimum priority will be applied again, before constructing the final ruleset | |||
| RuleSetFactory ruleSetFactory = toLoader().filterAbovePriority(RulePriority.LOW).warnDeprecated(false).toFactory(); | |||
| RuleSet otherRuleSet = ruleSetFactory.createRuleSet(RuleSetReferenceId.parse(ref).get(0)); | |||
There was a problem hiding this comment.
Ah... here we silently used only the first reference :)
| @@ -85,12 +76,21 @@ | |||
| @Deprecated | |||
| @InternalApi | |||
| public class RuleSetReferenceId { | |||
|
|
|||
| // todo this class has issues... What is even an "external" ruleset? | |||
| // terminology and API should be clarified. | |||
There was a problem hiding this comment.
Keep in mind at some point we marked it as InternalApi.
Clarification should happen anyway of course...
Comment on lines
+200
to
+201
| "Ruleset reference '" + tempRuleSetFileName + "' uses a deprecated form, use '" | ||
| + builtinRuleSet + "' instead" |
Co-authored-by: Andreas Dangel <andreas.dangel@adangel.org>
oowekyala
added a commit
to adangel/pmd
that referenced
this pull request
Jul 16, 2022
They were merged on separate branches
This was referenced Jan 11, 2023
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Some undocumented ruleset reference forms are deprecated:
Also using
<rule ref="rset1.xml,rset2.xml">is deprecated. As before, only the first ruleset is considered (which was undocumented), but now the rest is not ignored silently.I'll prepare the merge into PMD 7 and #3922 (which conflicts a lot).
Related issues
Ready?
./mvnw clean verifypasses (checked automatically by github actions)