Skip to content

[core] Replace RuleViolationFactory implementations with ViolationDecorator #3203

@oowekyala

Description

@oowekyala

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.

Metadata

Metadata

Assignees

No one assigned

    Labels

    an:enhancementAn improvement on existing features / rules

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions