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] Turn PropertySource into PropertyBundle #3869

Open
oowekyala opened this issue Mar 22, 2022 · 0 comments
Open

[core] Turn PropertySource into PropertyBundle #3869

oowekyala opened this issue Mar 22, 2022 · 0 comments
Labels
an:enhancement An improvement on existing features / rules
Milestone

Comments

@oowekyala
Copy link
Member

oowekyala commented Mar 22, 2022

Is your feature request related to a problem? Please describe.
The interface PropertySource is used to add the ability to have properties to rules and renderers and some other things. The interface is pervasive in our codebase and is a problem:

  • It's really not an interface, it's an abstract class, because it's so complicated to implement (and requires several fields) that if you implement PropertySource you have to extend transitively from AbstractPropertySource.
  • This strengthens the same dynamic between Rule and AbstractRule, if you extend Rule you have to extend AbstractRule. If you want a delegate rule you need to delegate 20 methods from PropertySource.
  • We can't replace PropertySource or make it evolve easily because every inheritor (every rule and renderer) is affected directly

Describe the solution you'd like

I think we would have much cleaner interfaces if we just renamed AbstractPropertySource to PropertyBundle (with the same functionality, but not abstract), and have the Rule interface have a method PropertyBundle getProperties(). We only need one implementation of the bundle.

Additionally, this allows us to implement custom PropertyBundle implementations, eg an IgnoredAnnotationsPropertyBundle which predeclares a property and adds nice getters. We can share property-related things between rules without having to put shared logic in a base rule class, which is more inflexible. This will also come in handy with language properties #2518.

Note that TreeRendererDescriptor already uses PropertySource in this way: instead of extending the interface it just uses an instance. I'd like to use the same model for #2518 and eventually for the large rule API restructuring needed for #3868

Describe alternatives you've considered

Additional context

@oowekyala oowekyala added the an:enhancement An improvement on existing features / rules label Mar 22, 2022
@oowekyala oowekyala mentioned this issue Apr 7, 2022
55 tasks
@oowekyala oowekyala added this to the 7.0.0 milestone May 7, 2022
@oowekyala oowekyala added this to To do in PMD 7 Jul 27, 2022
@adangel adangel modified the milestones: 7.0.0, 7.x Jan 23, 2023
@adangel adangel removed this from To do in PMD 7 Jan 23, 2023
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
None yet
Development

No branches or pull requests

2 participants