More preparations for running Detekt with type resolution #5905
Conversation
sschuberth
requested review from
a team,
nnobelis,
MarcelBochtler and
oheger-bosch
as code owners
Codecov ReportBase: 58.30% // Head: 58.32% // Increases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #5905 +/- ##
============================================
+ Coverage 58.30% 58.32% +0.01%
+ Complexity 2154 2152 -2
============================================
Files 329 329
Lines 19157 19154 -3
Branches 3761 3764 +3
============================================
+ Hits 11170 11171 +1
+ Misses 6878 6875 -3
+ Partials 1109 1108 -1
Flags with carried forward coverage won't be shown. Click here to find out more.
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
...sid-webapp/src/main/kotlin/model/identification/markedAsIdentified/MarkedAsIdentifiedFile.kt
Show resolved
Hide resolved
This silences Detekt violations when used with type resolution. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
`shouldBeTypeOf` applies a contract. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Avoiding sequences makes the issue disappear. Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
Signed-off-by: Sebastian Schuberth <sschuberth@gmail.com>
sschuberth
force-pushed
the
detekt-type-resolution-2
branch
from
7dcfa66
to
bf0f647
Compare
|
Disabled auto-merge as I believe it'd be good to have another approval for the decision to ditch |
| @@ -277,7 +277,7 @@ fun <K, V : Collection<T>, T> Map<K, V>.zipWithCollections(other: Map<K, V>): Ma | |||
| zip(other) { a, b -> | |||
| when { | |||
| // When iterating over the combined key set, not both values can be null. | |||
| a == null -> b!! | |||
| a == null -> checkNotNull(b) | |||
There was a problem hiding this comment.
Java 14 improved the NullPointerExceptions to display the actual variable name which was null causing this exception.
checkNotNull() only shows Required value was null..
I'm not sure if this Java 14 feature would also work with NPEs from Kotlin, and if it would be more helpful. But it might be one thing to consider.
There was a problem hiding this comment.
Aside from that, when looking at this commit, if these are the only places where it had to be changed, I would also vote for keeping this the rule to disallow !!.
There was a problem hiding this comment.
Additionally, this rule does not to disallow every usage of !!.
There was a problem hiding this comment.
I'm not sure if this Java 14 feature would also work with NPEs from Kotlin, and if it would be more helpful.
That's a nice Java feature indeed, and I believe it would work with any JVM language; however, for the time being we're probably stuck with Java 11 anyway, also see #4948 😞
Aside from that, when looking at this commit, if these are the only places where it had to be changed
To be fair, there might be might more places (in non-main source sets) that would need to be changed when keeping this rule enabled and using Detekt with type resolution. I'm not yet done with looking at all the code, but I'll probably stop here for now with this preparation.
Please have a look at the individual commit messages for the details.