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

Fixed bug reporting false positives with EmptyFunctionBlock #1690

Merged
merged 5 commits into from Jun 17, 2019

Conversation

cortinico
Copy link
Member

This rule (EmptyFunctionBlock) had a bug in reporting functions with override and empty
blocks (with or without comments).

I'm fixing it and adding a couple of tests to make sure it works properly.

Fixes #1684

Copy link
Contributor

@rock3r rock3r 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 the fix Nico! I think we can take the opportunity to improve the docs a bit further, while we're at it :)

@rock3r
Copy link
Contributor

rock3r commented Jun 13, 2019

Also the CI says you have conflicts :P

This rule had a bug in reporting functions with override and empty
blocks (with or without comments). I'm fixing it and adding a couple of
tests to make sure it works properly.

Fixes detekt#1684
@cortinico
Copy link
Member Author

Also the CI says you have conflicts :P

Yeah I should update my master from time to time 😆

@cortinico cortinico force-pushed the fix-1684-bug-empty-function-block branch from 6ebe673 to 0560f0a Compare June 13, 2019 20:59
Copy link
Member

@arturbosch arturbosch 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 the fix!

@arturbosch arturbosch added this to the 1.0.0 milestone Jun 15, 2019
@cortinico
Copy link
Member Author

This should be good to go now imho

Copy link
Member

@schalkms schalkms left a comment

Choose a reason for hiding this comment

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

Nice! Thank you very much!

Copy link
Contributor

@rock3r rock3r left a comment

Choose a reason for hiding this comment

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

LGTM! Thanks Nicola :)

@codecov-io
Copy link

Codecov Report

Merging #1690 into master will decrease coverage by 3.83%.
The diff coverage is 80%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1690      +/-   ##
============================================
- Coverage     79.81%   75.98%   -3.84%     
- Complexity     1594     1853     +259     
============================================
  Files           229      328      +99     
  Lines          4335     5521    +1186     
  Branches        829     1007     +178     
============================================
+ Hits           3460     4195     +735     
- Misses          428      790     +362     
- Partials        447      536      +89
Impacted Files Coverage Δ Complexity Δ
...rturbosch/detekt/rules/empty/EmptyFunctionBlock.kt 91.66% <ø> (+16.66%) 8 <0> (+2) ⬆️
.../gitlab/arturbosch/detekt/rules/empty/EmptyRule.kt 86.66% <80%> (-4.25%) 10 <0> (+2)
...sch/detekt/rules/providers/PotentialBugProvider.kt 100% <0%> (ø) 3% <0%> (ø) ⬇️
...tekt/formatting/wrappers/SpacingAroundOperators.kt 100% <0%> (ø) 3% <0%> (?)
...arturbosch/detekt/formatting/FormattingProvider.kt 100% <0%> (ø) 3% <0%> (?)
...bosch/detekt/formatting/wrappers/ImportOrdering.kt 100% <0%> (ø) 3% <0%> (?)
...in/io/gitlab/arturbosch/detekt/api/SplitPattern.kt 75% <0%> (ø) 10% <0%> (?)
...turbosch/detekt/formatting/wrappers/PackageName.kt 100% <0%> (ø) 3% <0%> (?)
...io/gitlab/arturbosch/detekt/api/CompositeConfig.kt 60% <0%> (ø) 4% <0%> (?)
...n/io/gitlab/arturbosch/detekt/api/ConsoleReport.kt 0% <0%> (ø) 0% <0%> (?)
... and 93 more

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 11abb88...0ac2a8d. Read the comment docs.

@arturbosch arturbosch merged commit 8f96fc8 into detekt:master Jun 17, 2019
@cortinico cortinico deleted the fix-1684-bug-empty-function-block branch June 19, 2019 15:20
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.

EmptyFunctionBlock false positive: overridden function with comment in body
6 participants