Skip to content

[core] Simplify and improve rule configuration validation #3868

@oowekyala

Description

@oowekyala

Is your feature request related to a problem? Please describe.
Currently rules are validated

  • during ruleset parsing, for properties (eg if you input an incorrect property value, an exception is reported)
  • before they're run using getDysfunctionReason, which reports configuration errors and removes the rule from the ruleset. (net.sourceforge.pmd.PmdAnalysis#removeBrokenRules)
  • when calling Rule::start, eg when there's a deprecated property the rule usually logs a warning
  • The dysfunctionReason comes from PropertySource. Each rule is a property source and can be potentially misconfigured

The problems with this:

  • there is no reference to the specific place in the ruleset xml you should edit to fix the warning/error
  • an IllegalArgumentException with a stack trace for a wrong property is really bad UX
  • the UX is inconsistent, some reporting goes through loggers, refs [core] Error reporting vs logging #3816
  • rulesets are mutable because of getDysfunctionReason, which I feel can be avoided
  • there is no clearly documented way to validate configuration for rule implementors

Describe the solution you'd like
The solution I'd like is too large to describe here, so I'll leave it for another ticket, but here are elements for what I'd like the UX to look like:

  • Property errors, whether they come from our framework or from user-specified (=rule implementation-specified) should be reported in a consistent way
  • messages should include a context from the XML ruleset file to make them easier to fix
  • all validation should be done during the XML parsing phase (at least in appearance)
  • these errors should not be treated as "ConfigErrors" -> this is the only thing we need ConfigError for, we can remove it afterwards ([core] Remove config errors #3901)
  • there should be a single place where the user can validate things in a rule (and report nice error messages, consistent with the rest). This is clearly documented and intuitive.

Describe alternatives you've considered

Additional context

Metadata

Metadata

Assignees

No one assigned

    Labels

    an:enhancementAn improvement on existing features / rulesin:ruleset-xmlAbout the ruleset schema/parser

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions