Skip to content

[java] New rule UnnecessaryBoxing#3364

Merged
adangel merged 9 commits into
pmd:pmd/7.0.xfrom
oowekyala:new-rule-UnnecessaryConversion
Jul 24, 2021
Merged

[java] New rule UnnecessaryBoxing#3364
adangel merged 9 commits into
pmd:pmd/7.0.xfrom
oowekyala:new-rule-UnnecessaryConversion

Conversation

@oowekyala

@oowekyala oowekyala commented Jun 25, 2021

Copy link
Copy Markdown
Member

Describe the PR

Add the new rule UnnecessaryBoxing, as per #2973. The old rules are not removed yet. We should have another rule that flags usages of primitive constructors actually (#3365).

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 7.0.0 milestone Jun 25, 2021
@ghost

ghost commented Jun 25, 2021

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 0 violations,
introduces 1493 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 10075 violations,
introduces 276363 new violations, 4 new errors and 0 new configuration errors,
removes 16336 violations, 8 errors and 2 configuration errors.
Full report

Generated by 🚫 Danger

@adangel adangel added the a:new-rule Proposal to add a new built-in rule label Jul 10, 2021

@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.

👍

The following might be a false-positive:

return Integer.valueOf(matchValue).hashCode();

where matchValue is a primitive int field (https://github.com/checkstyle/checkstyle/blob/checkstyle-8.10/src/main/java/com/puppycrawl/tools/checkstyle/filters/IntMatchFilter.java#L50).
In order to call hashCode() we must wrap this int or use Integer.hashCode(matchValue) instead.... I guess, that's actually what should be done in that case, calling directly Integer.hashCode..., but it didn't come into my mind at first... Looking at the implementation, for integers hashcode is the integer value itself.... so technically you could also write here simply return matchValue.
Also I think, this violation is reported accidentally. But there are not many other methods, if any at all, where you need a wrapper object....
I'm inclined to accept this violation 😄


So, having this rule in PMD 7 means, we can deprecate the other rules (*Instantiation) but only in PMD 7, but still need to update the rules, so that they run in PMD 7. We can the finally remove the rules with PMD 8.

Hang on... confusing: So #3365 for PMD 6 will create a replacement for *Instantiation rules, so we can deprecate them with PMD 6 and remove them with PMD 7. Good.

So, this new rule "UnnecessaryBoxing" replaces actually just "UnnecessaryWrapperObjectCreation" (which is probably of no use anyway), right?
So technically, we need to update UnnecessaryWrapperObjectCreation for PMD 7, deprecate it there and remove it with PMD 8.

@adangel adangel self-assigned this Jul 24, 2021
adangel added a commit that referenced this pull request Jul 24, 2021
@adangel adangel merged commit 150cc84 into pmd:pmd/7.0.x Jul 24, 2021
@oowekyala oowekyala deleted the new-rule-UnnecessaryConversion branch July 24, 2021 13:49
@adangel adangel mentioned this pull request Jul 29, 2021
8 tasks
@adangel adangel mentioned this pull request Jan 23, 2023
55 tasks
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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants