Skip to content

[core] Update remaining properties to use PropertyFactory#2450

Merged
oowekyala merged 11 commits into
pmd:pmd/7.0.xfrom
oowekyala:update-all-properties
Jun 14, 2020
Merged

[core] Update remaining properties to use PropertyFactory#2450
oowekyala merged 11 commits into
pmd:pmd/7.0.xfrom
oowekyala:update-all-properties

Conversation

@oowekyala

@oowekyala oowekyala commented Apr 27, 2020

Copy link
Copy Markdown
Member

Describe the PR

  • Remove most remaining usages of old properties (eg BooleanProperty, StringProperty) to use PropertyFactory. These are stuck on master because they're public.
  • In some cases, when the properties were deprecated, remove them.
  • Some of the old properties are still there, when they use a null default value. This is something the new property framework disallows explicitly. Such properties should be converted to use an Optional value, but the framework doesn't support that yet. We can update them when the framework is refactored.
  • This also extracts something from the rule AvoidDuplicateLiterals: delimiter escaping in multi-valued properties. I don't think we need to completely remove delimiter logic, if anything, this enables backwards compatibility with the current framework. Keeping it also allows specifying some list values more concisely.

Related issues

#1432

Ready?

  • Added unit tests for fixed bug/feature
  • Passing all unit tests
  • Complete build ./mvnw clean verify passes (checked automatically by travis)
  • Added (in-code) documentation (if needed)

@oowekyala oowekyala added this to the 7.0.0 milestone Apr 27, 2020
@oowekyala oowekyala marked this pull request as draft May 1, 2020 15:41
@ghost

ghost commented May 21, 2020

Copy link
Copy Markdown
1 Message
📖 No java rules are changed!

Generated by 🚫 Danger

@oowekyala oowekyala marked this pull request as ready for review June 12, 2020 15:30
@oowekyala

Copy link
Copy Markdown
Member Author

I'm also going to merge this as it blocks #1432

@oowekyala oowekyala merged commit 75c7971 into pmd:pmd/7.0.x Jun 14, 2020
@oowekyala oowekyala deleted the update-all-properties branch June 14, 2020 17:06
@adangel adangel mentioned this pull request Jun 21, 2020
6 tasks
// What? TODO 7.0.0 Use a boolean property
public static final StringProperty COLOR = new StringProperty("color",
"Enables colors with anything other than 'false' or '0'.", "yes", 0);
// TODO should the "textcolor" renderer really support "optional" colors?

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On first glance, I'd say, we should remove this property, also the system property. I've added a task to #2524

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants