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 of UnnecessaryInnerClass #4509

Merged
merged 3 commits into from Jan 23, 2022
Merged

Conversation

chao2zhang
Copy link
Member

@chao2zhang chao2zhang commented Jan 23, 2022

Main branch is failing because /home/runner/work/detekt/detekt/custom-checks/src/test/kotlin/io/github/detekt/custom/SpekTestDiscoverySpec.kt:16:5: Class 'VariableDeclarationsInSpekGroupsShouldOnlyBeSimple' does not require inner keyword. [UnnecessaryInnerClass] is a false positive.

  • This PR addresses the aforementioned issue by handling the class chain from the current class to the referred class.
  • This PR adds a passing unit test that failed before this PR.
  • Additionally, @Nested tests require the class to be inner. IDE prompts **Only non-static nested classes can serve as '@nested' test classes. **

@codecov
Copy link

codecov bot commented Jan 23, 2022

Codecov Report

Merging #4509 (2e9ee29) into main (8a69676) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff            @@
##               main    #4509   +/-   ##
=========================================
  Coverage     84.14%   84.15%           
+ Complexity     3315     3314    -1     
=========================================
  Files           476      476           
  Lines         10882    10875    -7     
  Branches       2015     2011    -4     
=========================================
- Hits           9157     9152    -5     
  Misses          692      692           
+ Partials       1033     1031    -2     
Impacted Files Coverage Δ
...rbosch/detekt/rules/style/UnnecessaryInnerClass.kt 94.02% <100.00%> (+2.13%) ⬆️

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 8a69676...2e9ee29. Read the comment docs.

@chao2zhang chao2zhang changed the title Fix false positive of UnncessaryInnerClass Fix false positive of UnnecessaryInnerClass Jan 23, 2022
@@ -233,6 +233,7 @@ style:
active: true
UnnecessaryInnerClass:
active: true
ignoreAnnotated: ['Nested']
Copy link
Contributor

Choose a reason for hiding this comment

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

I like this, because I may not necessarily reference anything in the outer class but still will be able to use inner classes to structure tests.

val fizz = foo
inner class C {
fun printFoo() {
println(foo)
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you mean to write println(fizz) here? At least that is what the test description suggests.

Copy link
Member Author

Choose a reason for hiding this comment

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

That's a great catch.

@chao2zhang chao2zhang merged commit ca9a3fa into detekt:main Jan 23, 2022
@chao2zhang chao2zhang added this to the 1.20.0 milestone Jan 23, 2022
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.

None yet

4 participants