Improve correctness of UnusedPrivateProperty #5935
Conversation
| super.visitReferenceExpression(expression) | ||
| } | ||
|
|
||
| private fun skipNode(expression: KtReferenceExpression): Boolean { | ||
| return when { |
There was a problem hiding this comment.
note: adding a bit of structure as we can followup with more exceptions and rules for other edge cases
There was a problem hiding this comment.
example of a followup: ignore import statements as well
luanpotter
force-pushed
the
luan.unused-private-property-improvements
branch
from
c5983cb to
4385b42
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
detekt found more than 10 potential problems in the proposed changes. Check the Files changed tab for more details.
Codecov Report
@@ Coverage Diff @@
## main #5935 +/- ##
=========================================
Coverage 84.46% 84.46%
Complexity 3784 3784
=========================================
Files 546 546
Lines 12923 12928 +5
Branches 2268 2271 +3
=========================================
+ Hits 10915 10920 +5
Misses 877 877
Partials 1131 1131
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
luanpotter
force-pushed
the
luan.unused-private-property-improvements
branch
from
4385b42 to
6d11524
Compare
...s-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivatePropertySpec.kt
Fixed
Show fixed
Hide fixed
...s-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivatePropertySpec.kt
Fixed
Show fixed
Hide fixed
...s-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivatePropertySpec.kt
Fixed
Show fixed
Hide fixed
...s-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivatePropertySpec.kt
Fixed
Show fixed
Hide fixed
...s-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivatePropertySpec.kt
Fixed
Show fixed
Hide fixed
...s-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivatePropertySpec.kt
Fixed
Show fixed
Hide fixed
...s-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivatePropertySpec.kt
Fixed
Show fixed
Hide fixed
luanpotter
force-pushed
the
luan.unused-private-property-improvements
branch
from
6d11524 to
3dba288
Compare
luanpotter
force-pushed
the
luan.unused-private-property-improvements
branch
from
3dba288 to
b09a18d
Compare
| } | ||
|
|
||
| @Test | ||
| fun `package declarations are ignored`() { |
There was a problem hiding this comment.
I do think we only need this test to be added. The other cases are covered by the existing tests.
There was a problem hiding this comment.
good point! I had skimmed the file for these tests but initially failed to see them nested in the inner classes.
I removed the other two.
on that note though, lmk of including my 1 test under a new "irrelevant references are ignored" nested class (it seems all tests are nested somewhere)
I was planning on following this up with some other edge cases (eg import statements should be ignored as well)
but I am happy to put the test nested somewhere else if that makes sense, just lmk where it better fits
There was a problem hiding this comment.
Regarding test under new nested class 👍
Regarding follow up --> please submit a new PR, it's easier to review 👍
There was a problem hiding this comment.
yes, def meant followup PRs! thanks for reviewing so swiftly :)
Fixes #5934
Basically ignore package declarations when counting references for the purposes of identifying Unused Private Properties.
Followups
I believe there are many other edge cases that could be considered; for example, the right-hand side of a dot-expression is also being considered a use reference:
Should cause an error, but it doesn't. I'm not sure if there is a better solution here other than exploring each edge case individually, but I am happy to chip down on at least the more obvious ones on followup PRs.