Skip to content

[java] Extract reaching definitions analysis out of UnusedAssignmentRule#3311

Merged
adangel merged 44 commits into
pmd:pmd/7.0.xfrom
oowekyala:java-dfa-reaching-defs-unused-assignment-singular-field-that-kind-of-thing
Jun 25, 2021
Merged

[java] Extract reaching definitions analysis out of UnusedAssignmentRule#3311
adangel merged 44 commits into
pmd:pmd/7.0.xfrom
oowekyala:java-dfa-reaching-defs-unused-assignment-singular-field-that-kind-of-thing

Conversation

@oowekyala

@oowekyala oowekyala commented May 28, 2021

Copy link
Copy Markdown
Member

Describe the PR

This way the data-flow information it collects can be used by more rules. This reimplements several rules using this information.
SingularField and ImmutableField should be more precise and more general. MissingBreakInSwitch is updated according to #2894, ie, its implementation strategy uses control flow information to detect fall-through, instead of blindly counting breaks. The remaining piece to fix this issue, is to rename the rule to "FallThroughSwitchCase".

Also part of these changes aim to reduce the importance of AbstractLombokAwareRule, by identifying the actual annotations that each rule should care about instead of just ignoring all lombok annotations for all those rules.

Related issues

Updates rules:

  • AvoidThrowingNullPointerException
  • ImmutableField
  • SingularField
  • MissingBreakInSwitch

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)

This removes some noisyness that is better caught by unusedPrivateField
This uses the new dataflow and is in theory
more precise, but also will report more things.
…-assignment-singular-field-that-kind-of-thing
…-assignment-singular-field-that-kind-of-thing
…ed-assignment-singular-field-that-kind-of-thing
Update AvoidThrowingNullPointerException
…-assignment-singular-field-that-kind-of-thing
…-assignment-singular-field-that-kind-of-thing
…-assignment-singular-field-that-kind-of-thing
@oowekyala oowekyala added this to the 7.0.0 milestone May 28, 2021
@ghost

ghost commented May 28, 2021

Copy link
Copy Markdown
2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 0 violations,
introduces 2853 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 9904 violations,
introduces 273950 new violations, 2 new errors and 0 new configuration errors,
removes 16329 violations, 8 errors and 2 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 0 violations,
introduces 1054 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 9674 violations,
introduces 12040 new violations, 1 new errors and 0 new configuration errors,
removes 16174 violations, 8 errors and 2 configuration errors.
Full report
Compared to pmd/7.0.x:
This changeset changes 0 violations,
introduces 1054 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 9674 violations,
introduces 12040 new violations, 1 new errors and 0 new configuration errors,
removes 16174 violations, 8 errors and 2 configuration errors.
Full report

Generated by 🚫 Danger

@adangel adangel self-requested a review June 11, 2021 08:14

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

Thanks!

At some point, we should create unit tests for the dataflow implementation....

As far as I see, this PR

I'll add this to the release notes.

The other referenced missingbreakinswitch bugs are still open, I assume (at least, there are no test cases):

  • #659 still open
  • #2579 there is no test, but in github project, it is marked as done?

Then there is the renaming of "MissingBreakInSwitch" into "ImplicitSwitchFallThrough" (Problem 2 of #2894).
I'd suggest to make this a separate issue. Then we can rename it already in PMD 6 and remove the deprecated
rule reference in PMD 7.

@adangel

adangel commented Jun 25, 2021

Copy link
Copy Markdown
Member

The other referenced missingbreakinswitch bugs are still open, I assume (at least, there are no test cases):

Ok, I tested them. You've fixed them already.

@oowekyala

oowekyala commented Jun 25, 2021

Copy link
Copy Markdown
Member Author

Thanks for the review. I'll check whether the bugs of MissingBreakInSwitch are already solved and add tests if so you beat me to it :)

@adangel adangel self-assigned this Jun 25, 2021
adangel added a commit that referenced this pull request Jun 25, 2021
oowekyala:java-dfa-reaching-defs-unused-assignment-singular-field-that-kind-of-thing

[java] Extract reaching definitions analysis out of UnusedAssignmentRule
#3311
@adangel adangel merged commit 453a0c4 into pmd:pmd/7.0.x Jun 25, 2021
@oowekyala oowekyala deleted the java-dfa-reaching-defs-unused-assignment-singular-field-that-kind-of-thing branch June 25, 2021 15:09
@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

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants