Skip to content

[java] New rule PrimitiveWrapperInstantiation#3365

Merged
adangel merged 7 commits into
pmd:masterfrom
oowekyala:merge-wrapper-ctor-rules
Jul 29, 2021
Merged

[java] New rule PrimitiveWrapperInstantiation#3365
adangel merged 7 commits into
pmd:masterfrom
oowekyala:merge-wrapper-ctor-rules

Conversation

@oowekyala

@oowekyala oowekyala commented Jun 25, 2021

Copy link
Copy Markdown
Member

Describe the PR

This new rule merges the rules

  • ByteInstantiation
  • IntegerInstantiation
  • LongInstantiation
  • ShortInstantiation
  • BooleanInstantiation (this does a bit more - encourages to use boolean constants instead of new boolean or valueof)

The rule may have some overlap with #3364, if the constructor call is unnecessary because of autoboxing or some other case caught by #3364. This looks ok though

This also deprecates the rule UnnecessaryWrapperObjectCreation because it's useless, as described in #3364. The replacement hinted at in the updated rule description is #3364.

Related issues

Ready?

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

@oowekyala oowekyala added this to the 6.37.0 milestone Jun 25, 2021
@ghost

ghost commented Jun 25, 2021

Copy link
Copy Markdown
1 Message
📖 This changeset changes 0 violations,
introduces 535 new violations, 0 new errors and 0 new configuration errors,
removes 442 violations, 0 errors and 0 configuration errors.
Full report
This changeset changes 0 violations,
introduces 535 new violations, 0 new errors and 0 new configuration errors,
removes 442 violations, 0 errors and 0 configuration errors.
Full report
This changeset changes 0 violations,
introduces 484 new violations, 0 new errors and 0 new configuration errors,
removes 395 violations, 0 errors and 0 configuration errors.
Full report

Generated by 🚫 Danger

@oowekyala oowekyala mentioned this pull request Jul 9, 2021
4 tasks
@adangel adangel added the a:new-rule Proposal to add a new built-in rule label Jul 10, 2021
@adangel adangel self-requested a review July 24, 2021 09:16

@adangel adangel left a comment

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.

Looks good, thanks!

I know now, what the problem will be, when we integrate BooleanInstantiation into this rule: for Boolean, we actually need a very different violation message: We don't want to say "Do not use new Boolean(...), prefer Boolean.valueOf(...)", instead we want e.g. "Do not use new Boolean(...) or Boolean.valueOf(...), prefer Boolean.TRUE or Boolean.FALSE".
Keeping it as a XPath rule, this might not be possible. But if we reimplement this as a simple Java rule, we can issue different messages.

Maybe I'll have a look at this on Thursday - otherwise we can integrate BooleanInstantiation later.

Reminder: We need to mention the deprecated rules in the release notes - and of course also the new rule.

@adangel adangel self-assigned this Jul 27, 2021
@adangel adangel changed the title [java] New rule - PrimitiveWrapperConstructor [java] New rule PrimitiveWrapperInstantiation Jul 29, 2021
@adangel adangel added the is:deprecation The main focus is deprecating public APIs or rules, eg to make them internal, or removing them label Jul 29, 2021
@adangel adangel mentioned this pull request Jul 29, 2021
8 tasks
@adangel adangel marked this pull request as ready for review July 29, 2021 15:29
@adangel adangel merged commit c17deb5 into pmd:master Jul 29, 2021
@oowekyala oowekyala deleted the merge-wrapper-ctor-rules branch July 30, 2021 12:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a:new-rule Proposal to add a new built-in rule is:deprecation The main focus is deprecating public APIs or rules, eg to make them internal, or removing them

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants