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 false positive in RethrowCaughtException for try with more than one catch (#4367) #4369

Merged
merged 1 commit into from Dec 21, 2021

Conversation

strkkk
Copy link
Contributor

@strkkk strkkk commented Dec 10, 2021

Closes #4367

  1. Fixes false positive
  2. One more test input is added (when same exception is thrown but with this qualifier)

Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

👏 really good first contribution :) Thanks!

@codecov
Copy link

codecov bot commented Dec 15, 2021

Codecov Report

Merging #4369 (a057707) into main (e2cc197) will increase coverage by 0.00%.
The diff coverage is 73.33%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #4369   +/-   ##
=========================================
  Coverage     84.33%   84.33%           
- Complexity     3269     3272    +3     
=========================================
  Files           473      473           
  Lines         10343    10351    +8     
  Branches       1825     1826    +1     
=========================================
+ Hits           8723     8730    +7     
  Misses          668      668           
- Partials        952      953    +1     
Impacted Files Coverage Δ
.../detekt/rules/exceptions/RethrowCaughtException.kt 81.81% <73.33%> (+3.24%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2cc197...a057707. Read the comment docs.

Copy link
Member

@cortinico cortinico left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member

@BraisGabin BraisGabin left a comment

Choose a reason for hiding this comment

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

Adding request for changes becuase right now it introduces a false-negative as commented in this thread: https://github.com/detekt/detekt/pull/4369/files#r767144893

@BraisGabin
Copy link
Member

Thank you so much! I'll merge it as soon as CI passes :). Greate job.

@BraisGabin
Copy link
Member

There is an issue in the code, could you check and fix it? https://github.com/detekt/detekt/runs/4594574549?check_suite_focus=true#step:4:356

@strkkk
Copy link
Contributor Author

strkkk commented Dec 21, 2021

@BraisGabin please trigger ci

@BraisGabin BraisGabin merged commit 7250c88 into detekt:main Dec 21, 2021
@strkkk strkkk deleted the 4367_false_positive_rethrow branch December 21, 2021 21:11
@cortinico cortinico added this to the 1.20.0 milestone Dec 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

RethrowCaughtException should not be triggered when there is more general exception below
3 participants