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.

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

[core] Replace RuleViolationFactory implementations with ViolationDecorator #3203

Closed
oowekyala opened this issue Apr 8, 2021 · 1 comment
Labels
an:enhancement An improvement on existing features / rules
Projects
Milestone

Comments

@oowekyala
Copy link
Member

Is your feature request related to a problem? Please describe.
Currently RuleViolation has a few methods which are somewhat language-specific:

  • getPackageName
  • getClassName
  • getMethodName
  • getVariableName

This is fine, but the way these properties are filled in is very indirect and weird: you need an implementation of RuleViolation which sets fields of its superclass in its constructor. You also need an implementation of RuleViolationFactory which calls the constructor of your subclass. So we multiply implementations of these interfaces even though ParametricRuleViolation, the default RuleViolation implementation already has all the necessary fields. Those implementations are useless except to fill those properties.

Describe the solution you'd like

  • Expose a Map<String, String> as an attribute of RuleViolation, which contains key value pairs for these additional properties. Languages may add new properties at will.
  • Add a parameter with this type to the constructor of ParametricRuleViolation.
  • Add a new interface, ViolationDecorator, to pmd core. Violation decorators take a violation and add metadata to its attribute map. When creating a rule violation, we will just need to apply the registered decorator to add language-specific metadata, but the creating of the violation will not need any language-specific implementations. This way, RuleViolationFactory becomes redundant, and language-specific subclasses of RuleViolation too. So we reduce complexity when implementing a language. Here's a sketch of the interface:
interface ViolationDecorator {
   RuleViolation decorate(RuleViolation baseViolation, Node node);
}

and an implementation:

ViolationDecorator javaViolationDecorator = (baseViolation, node) -> {
    JavaNode node = (JavaNode) node;
    Map<String, String> metadata = new HashMap<>(baseViolation.getExtraAttributes()); // copy original map
    metadata.put(RuleViolation.VARIABLE_NAME, getVarName(javaNode));
    metadata.put(RuleViolation.METHOD_NAME, getMethodName(javaNode));
    // etc
    return new ParametricRuleViolation(baseViolation, metadata); // replace metadata, copy all other attributes of the base violation
};

Describe alternatives you've considered None

Additional context In #2807 I changed RuleViolationFactory a lot, with this idea in mind.

@oowekyala oowekyala added the an:enhancement An improvement on existing features / rules label Apr 8, 2021
@oowekyala oowekyala added this to the 7.0.0 milestone Apr 8, 2021
@oowekyala oowekyala added this to To do in PMD 7 Apr 8, 2021
@adangel adangel changed the title [core] Replace RuleviolationFactory implementations with ViolationDecorator [core] Replace RuleViolationFactory implementations with ViolationDecorator Jan 16, 2023
@adangel
Copy link
Member

adangel commented Jan 16, 2023

This is done for PMD 7 via #4050

@adangel adangel closed this as completed Jan 16, 2023
PMD 7 automation moved this from To do to Done Jan 16, 2023
@adangel adangel mentioned this issue Jan 23, 2023
55 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
an:enhancement An improvement on existing features / rules
Projects
No open projects
PMD 7
  
Done
Development

No branches or pull requests

2 participants