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.

Sign up for GitHub

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

Jump to bottom

UnusedPrivateMember - added handling of overloaded array get operator… #3666

Merged
merged 2 commits into from
Jul 31, 2021
Merged

UnusedPrivateMember - added handling of overloaded array get operator… #3666

merged 2 commits into from
Jul 31, 2021

Conversation

DcortezMeleth
Copy link
Contributor

… method (#3640)

PR with changes removing UnusedPrivateMember rule false positives regarding overloaded array get operator method.

BraisGabin
BraisGabin reviewed Apr 9, 2021
View reviewed changes
...les-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivateMemberSpec.kt
this.firstOrNull { it.s == s }
}
"""
assertThat(subject.compileAndLintWithContext(env, code)).hasSize(0)
Copy link
Member

Choose a reason for hiding this comment

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

Could you add one test that shows that reports if it's not used?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure. Good call. Totally forgot about it.

...t-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivateMember.kt Outdated Show resolved Hide resolved
@DcortezMeleth DcortezMeleth force-pushed the feature/UnusedPrivateMember_array_get_overload branch from 4a531f7 to e4ade96 Compare April 14, 2021 13:09
@codecov
Copy link

codecov bot commented Apr 14, 2021 •
edited
Loading

Codecov Report

Merging #3666 (1785a15) into main (ce7bb32) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@             Coverage Diff              @@
##               main    #3666      +/-   ##
============================================
+ Coverage     83.48%   83.49%   +0.01%     
- Complexity     3159     3160       +1     
============================================
  Files           458      458              
  Lines          9047     9048       +1     
  Branches       1759     1760       +1     
============================================
+ Hits           7553     7555       +2     
- Misses          567      568       +1     
+ Partials        927      925       -2
Impacted Files Coverage Δ
...turbosch/detekt/rules/style/UnusedPrivateMember.kt 91.36% <100.00%> (+0.06%) ⬆️
...osch/detekt/rules/empty/EmptyDefaultConstructor.kt 80.00% <0.00%> (ø)
...b/arturbosch/detekt/rules/naming/FunctionNaming.kt 100.00% <0.00%> (+3.84%) ⬆️

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 ce7bb32...1785a15. Read the comment docs.

@DcortezMeleth DcortezMeleth force-pushed the feature/UnusedPrivateMember_array_get_overload branch from e4ade96 to 0088c54 Compare April 15, 2021 06:55
@DcortezMeleth DcortezMeleth force-pushed the feature/UnusedPrivateMember_array_get_overload branch 4 times, most recently from 6116def to e6e9904 Compare April 15, 2021 19:36
cortinico
cortinico reviewed Apr 29, 2021
View reviewed changes
...les-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivateMemberSpec.kt Show resolved Hide resolved
...t-rules-style/src/main/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivateMember.kt
@@ -44,6 +45,8 @@ import org.jetbrains.kotlin.resolve.calls.callUtil.getResolvedCall
import org.jetbrains.kotlin.types.expressions.OperatorConventions
import org.jetbrains.kotlin.util.OperatorNameConventions

private const val ARRAY_GET_METHOD_NAME = "get"
Copy link
Member

Choose a reason for hiding this comment

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

Should the set operator be tracked as well?

Copy link
Member

Choose a reason for hiding this comment

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

This can be done in a follow up PR (if someone wants to do it)

...les-style/src/test/kotlin/io/gitlab/arturbosch/detekt/rules/style/UnusedPrivateMemberSpec.kt Show resolved Hide resolved
@cortinico
Copy link
Member

Friendly ping to get the status of this PR @DcortezMeleth

If you need further support, let us know 👍
My comments were also sort of nits/nice-to-have. We should strive to merge this.

I also stumbled upon this same false positive in another project: https://github.com/cortinico/ktfmt-gradle/pull/32/files#diff-c3cf3f82913f1a5c2d33bd9b78e2b8cfcb4adbf4c82103d773017f79ed3334ae

@BraisGabin BraisGabin force-pushed the feature/UnusedPrivateMember_array_get_overload branch from e6e9904 to 8160568 Compare July 24, 2021 12:34
@BraisGabin
Copy link
Member

BraisGabin commented Jul 24, 2021 •
edited
Loading

I rebased the commit to fix the conflicts and added the missing test that @cortinico talk about.

@BraisGabin BraisGabin added this to the 1.18.0 milestone Jul 24, 2021
@BraisGabin BraisGabin force-pushed the feature/UnusedPrivateMember_array_get_overload branch from 8160568 to 1785a15 Compare July 30, 2021 07:23
@cortinico cortinico merged commit 2faba52 into detekt:main Jul 31, 2021
@cortinico cortinico mentioned this pull request Jul 31, 2021
Closed
@3flex 3flex mentioned this pull request Oct 1, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cortinico cortinico cortinico left review comments

@BraisGabin BraisGabin BraisGabin approved these changes

@chao2zhang chao2zhang chao2zhang approved these changes

Assignees
No one assigned
Labels
rules
Projects
None yet
Milestone
1.18.0
Development

Successfully merging this pull request may close these issues.
4 participants
@DcortezMeleth @cortinico @BraisGabin @chao2zhang