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

Fix for Issue 2040 #2072

Open
wants to merge 32 commits into
base: master
Choose a base branch
from

Conversation

baloghadamsoftware
Copy link
Contributor

@baloghadamsoftware baloghadamsoftware commented May 27, 2022

This PR fixes issue #2040 by limiting the errors reported by the detector ThrowingExceptions to cases where
java.lang.Exception or lava.lang.Throwable appears in the exception specification of th method but it is neither
inherited from an overridden method nor is is coming from another method that the method invokes. Syntethic
methods are also omitted as well as methods which throw generic exceptions.


Make sure these boxes are checked before submitting your PR -- thank you!

  • Added an entry into CHANGELOG.md if you have changed SpotBugs code

This PR fixes issue spotbugs#2040 by limiting the errors reported by the detector `ThrowingExceptions` to cases where
`java.lang.Exception` or `lava.lang.Throwable` appears in the exception specification of th method but it is neither
inherited from an overridden method nor is is coming from another method that the method invokes. Syntathic
methods are also omitted as well as methods which throw generic exceptions.
Ádám Balogh added 4 commits May 30, 2022 10:16
This PR fixes issue spotbugs#2040 by limiting the errors reported by the detector `ThrowingExceptions` to cases where
`java.lang.Exception` or `lava.lang.Throwable` appears in the exception specification of th method but it is neither
inherited from an overridden method nor is is coming from another method that the method invokes. Syntathic
methods are also omitted as well as methods which throw generic exceptions.
Copy link
Member

@KengoTODA KengoTODA left a comment

Choose a reason for hiding this comment

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

please keep adding test cases :)

@baloghadamsoftware
Copy link
Contributor Author

Thank you for your comments, @KengoTODA. You were right in all of them. I fixed every one and also added new test cases. Although I tested this PR on several medium-size projects, something was probably wrong with my CI loop because the tests seemed to have passed which is wrong because they should have crashed. Now with the new test cases I could fix these issues.

ThrawnCA
ThrawnCA previously approved these changes Sep 1, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
@baloghadamsoftware
Copy link
Contributor Author

Thanks, @ThrawnCA! You have exceptionally good eyes for typos.

@baloghadamsoftware baloghadamsoftware requested review from ThrawnCA and removed request for KengoTODA September 6, 2022 11:13
@baloghadamsoftware baloghadamsoftware requested review from KengoTODA and removed request for ThrawnCA and KengoTODA September 6, 2022 11:15
@gtoison
Copy link
Contributor

gtoison commented Aug 11, 2023

@JuditKnoll I think that this PR got stuck in limbo when Kengo retired from the project, would you have time to review it?
It should reduce the false positives

Copy link
Collaborator

@JuditKnoll JuditKnoll left a comment

Choose a reason for hiding this comment

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

It would be great to see more testcases, for example:

@github-actions github-actions bot added the Stale label Mar 4, 2024
@github-actions github-actions bot closed this Apr 8, 2024
@hazendaz hazendaz reopened this Apr 12, 2024
@github-actions github-actions bot removed the Stale label Apr 13, 2024
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.

None yet

6 participants