-
-
Notifications
You must be signed in to change notification settings - Fork 778
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
ForbiddenVoid: New option 'ignoreUsageInGenerics' #1738
Conversation
26628f3
to
504743c
Compare
0baae20
to
d41e5ad
Compare
Please execute |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks! Looks good!
I think two additional test cases would be beneficial. Otherwise, this is an awesome 1st contribution.
@@ -69,8 +69,7 @@ class ForbiddenVoidSpec : Spek({ | |||
|
|||
class B : A() { | |||
override fun method(param: Foo<Void>) : Foo<Void> { | |||
@Suppress("ForbiddenVoid") | |||
return Foo<Void>() | |||
throw IllegalStateException() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why did you change this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It served no purpose for the particular test, but rather confused because of the additional @Suppress("ForbiddenVoid")
.
@@ -107,5 +106,42 @@ class ForbiddenVoidSpec : Spek({ | |||
assertThat(findings).hasSize(2) | |||
} | |||
} | |||
describe("ignoreUsageInGenerics is enabled") { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I would prefer to have the following test cases:
- nested generic void usage (e.g.
A<B<Void>>
) - Dictionary (e.g.
Map<Int, Void>
)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks!
Codecov Report
@@ Coverage Diff @@
## master #1738 +/- ##
============================================
+ Coverage 75.64% 79.81% +4.17%
- Complexity 1835 1936 +101
============================================
Files 332 324 -8
Lines 5531 5470 -61
Branches 1009 1007 -2
============================================
+ Hits 4184 4366 +182
+ Misses 807 581 -226
+ Partials 540 523 -17 Continue to review full report at Codecov.
|
Fixes #1724