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

[java] Make LawOfDemeter results deterministic #4549

Merged
merged 4 commits into from May 26, 2023

Conversation

adangel
Copy link
Member

@adangel adangel commented May 5, 2023

Describe the PR

I believe, this should fix the spurious differing results in the regression tester. At least, I could reproduce on first try the problem when reversing the order of the reaching assignments.

That should then make the test-case-3 of pmd/pmd-regression-tester#119 happy.

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)

@adangel adangel added the a:bug PMD crashes or fails to analyse a file. label May 5, 2023
@adangel adangel added this to the 7.0.0 milestone May 5, 2023
@pmd-test
Copy link

pmd-test commented May 5, 2023

1 Message
📖 Compared to master:
This changeset changes 11 violations,
introduces 2 new violations, 0 new errors and 0 new configuration errors,
removes 2 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 11 violations,
introduces 2 new violations, 0 new errors and 0 new configuration errors,
removes 2 violations, 0 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 4 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 12 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 4 violations,
introduces 0 new violations, 0 new errors and 0 new configuration errors,
removes 0 violations, 12 errors and 0 configuration errors.
Full report
Compared to master:
This changeset changes 13 violations,
introduces 2 new violations, 0 new errors and 0 new configuration errors,
removes 2 violations, 12 errors and 0 configuration errors.
Full report

Generated by 🚫 Danger

@adangel
Copy link
Member Author

adangel commented May 5, 2023

Ok, the baseline seems to have been created with a different order of the reaching assignments. I'll give it a try and remove the reversed() to see, whether we match the baseline then. Ideally we wouldn't see any changed violations.

@oowekyala
Copy link
Member

oowekyala commented May 5, 2023

Maybe a better fix would be to replace all new HashSet and new HashMap in DataflowPass.java with their LinkedHashX version... Then we wouldn't need the sorting at all, and we ensure other rules won't have this problem.

…gn/LawOfDemeterRule.java

Co-authored-by: Clément Fournier <clement.fournier@tu-dresden.de>
@adangel
Copy link
Member Author

adangel commented May 5, 2023

Maybe a better fix would be to replace all new HashSet and new HashMap in DataflowPass.java with their LinkedHashX version... Then we wouldn't need the sorting at all, and we ensure other rules won't have this problem.

I wanted to make it minimally invasive. I can give it a shot, though...

@adangel
Copy link
Member Author

adangel commented May 5, 2023

Maybe a better fix would be to replace all new HashSet and new HashMap in DataflowPass.java with their LinkedHashX version... Then we wouldn't need the sorting at all, and we ensure other rules won't have this problem.

I wanted to make it minimally invasive. I can give it a shot, though...

I've updated the Dataflow code now. I needed to swap the test case again - that means, if this was really the reason for the random results, then we should get the numbers from the first run again:

This changeset changes 13 violations,
introduces 2 new violations, 0 new errors and 0 new configuration errors,
removes 2 violations, 12 errors and 0 configuration errors.

But it should be stable now...

@oowekyala oowekyala merged commit 81233fa into pmd:master May 26, 2023
3 checks passed
@adangel adangel deleted the law-of-demeter-deterministic branch May 26, 2023 10:43
adangel added a commit to adangel/pmd that referenced this pull request May 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
a:bug PMD crashes or fails to analyse a file.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants