Skip to content

[core] Add fluent API to report violations #5039

@oowekyala

Description

@oowekyala

Is your feature request related to a problem? Please describe.
The methods of RuleContext to declare violations are not the most readable, because they take many parameters. Especially since #4939, just adding a new method for every special case looks intenable.

Describe the solution you'd like
We should probably add a new fluent API to declare violations more nicely. We should take care of decoupling the information about where the violation is from the info about the message.

  • The location information:
    • can be a node
    • can be a node + a more specific FileLocation within the node
    • can be a token + a more specific FileLocation within the token (since [java] Fix #2996 - CommentSize/CommentContent suppression #4939).
      • In this case we need to find the nearest node, for purposes of suppression. This requires having the root of the tree at hand. This parameter seems superfluous and could maybe be saved in the RuleContext instead, but an obstacle to this is existing tests which need to be refactored.
  • The message:
    • can be the default message of the rule + some optional format arguments
    • can be a custom message + some optional format arguments
    • in the future for i18n ([core] Should violation messages be internationalised? #2951), we probably should consider replacing direct message strings with keys into a resource bundle.

Additional things a more flexible API would allow us to add would be custom severities/confidence level per violation: #4327

These different options multiply each other, so we would definitely reduce the complexity of the API if we made this a staged builder.

A possible inspiration for this API can be the SLF4J fluent API. My personal opinion is that we shouldn't add that much flexibility to the API. There should ideally be one obvious way to do each thing, not several equivalent ways. For instance adding format arguments one by one with addArgument like in SLF4J is not something I want here.

Additional notes based on the discussion in #4939 (comment):

  • The starting point of the fluent API should be an instance method in RuleContext
  • The finalizer method should be a verb (eg warn() or report()). It should return void to avoid any ambiguity (I've this ViolationMessage object, how do I report it? what do I do with it now?.
  • The first iterations of this will be @Experimental

Metadata

Metadata

Assignees

Labels

an:enhancementAn improvement on existing features / rules

Type

No type
No fields configured for issues without a type.

Projects

No projects

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions