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.

Sign up for GitHub

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

Jump to bottom

[java] Fix IllegalStateException in UseDiamondOperator rule #4357

Merged
merged 3 commits into from
Jan 25, 2023
Merged

[java] Fix IllegalStateException in UseDiamondOperator rule #4357

merged 3 commits into from
Jan 25, 2023

Conversation

jsotuyod
Copy link
Member

@jsotuyod jsotuyod commented Jan 25, 2023 •
edited by adangel
Loading

Rule: UseDiamondOperator

Describe the PR

  • An IllegalStateException is produced when the inference context corresponds to an invocation of an unresolved method.
  • The rule effectively can't determine if the diamond operator can be used, but the current message of "overload resolution is not complete" is noisy, not actionable to the user (the issue is a missing class in the auxclasspath, not the overload resolution itself), and prevents the rule from continuing to analyze the current file.
  • The current fix is not elegant, and probably subpar (maybe the Infer logic could remove the check and deal with this seemlessly?) but I fear that code is too complex for me to make that call / change at this time. @oowekyala I'd love your help here.

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)

@jsotuyod
Add failing test for UseDiamongOperatorRule
378ba33 
 - An IllegalStateException is produced when the inference context corresponds
to an invocation of an unresolved method.
 - The rule effectively can't determine if the diamond oeprator can be used, but the current message of
"overload resolution is not complete" is noisy, not actionable to the user
(the issue is a missing class in the auxclasspath, not the overload resolution),
and prevents the rule from continuing to analyze the current file.
@jsotuyod
Suppress the overload resolution failure and continue
8ef6042 
 - This is probably not the best solution (maybe the Infer logic needs to remove
the check and handle it seamlessly?) but that code is too complex for me to make that call
@jsotuyod jsotuyod added the a:bug PMD crashes or fails to analyse a file. label Jan 25, 2023
@jsotuyod jsotuyod added this to the 7.0.0 milestone Jan 25, 2023
@pmd-test
Copy link

pmd-test commented Jan 25, 2023 •
edited
Loading

2 Messages
📖 Compared to pmd/7.0.x:
This changeset changes 4 violations,
introduces 0 new violations, 34 new errors and 0 new configuration errors,
removes 0 violations, 0 errors and 0 configuration errors.
Full report
📖 Compared to master:
This changeset changes 49641 violations,
introduces 33909 new violations, 1480 new errors and 0 new configuration errors,
removes 195793 violations, 4 errors and 7 configuration errors.
Full report
✅ Compared to pmd/7.0.x:
This changeset changes 0 violations,
introduces 51 new violations, 1 new errors and 0 new configuration errors,
removes 0 violations, 1447 errors and 0 configuration errors.
Full report
✅ Compared to master:
This changeset changes 502 violations,
introduces 1102 new violations, 2 new errors and 0 new configuration errors,
removes 3 violations, 6 errors and 7 configuration errors.
Full report

Generated by 🚫 Danger

oowekyala
oowekyala reviewed Jan 25, 2023
View reviewed changes
Copy link
Member

@oowekyala oowekyala left a comment

Choose a reason for hiding this comment

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

Thanks for spotting this! I think it's not a good idea to catch the exception when these methods are not supposed to throw in the first place. I'll investigate this...

@jsotuyod
Copy link
Member Author

@oowekyala thanks for your help!

@jsotuyod jsotuyod merged commit 596cc26 into pmd/7.0.x Jan 25, 2023
@jsotuyod jsotuyod deleted the fix-usediamongoperator-rule branch January 25, 2023 13:19
@adangel adangel changed the title [java] Fix IllegalStateException in UseDiamongOperator rule [java] Fix IllegalStateException in UseDiamondOperator rule Jan 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@oowekyala oowekyala oowekyala left review comments

Assignees
No one assigned
Labels
a:bug PMD crashes or fails to analyse a file.
Projects
None yet
Milestone
7.0.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jsotuyod @pmd-test @oowekyala