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

[MemberNameEqualsClassName] Support factory exemption for generic classes #3595

Merged
merged 1 commit into from
Mar 25, 2021

Conversation

josephlbarnett
Copy link
Contributor

Compare the referenced type name instead of the fully
qualified generic type name when possible.

Fixes #3591

@josephlbarnett
Copy link
Contributor Author

There may be a better way to do this, as I don't fully understand the kotlin compiler psi APIs, but this works for the test case included and falls back to the previous implementation for other cases.

@codecov
Copy link

codecov bot commented Mar 22, 2021

Codecov Report

Merging #3595 (71001e5) into main (8dbb01f) will decrease coverage by 0.00%.
The diff coverage is 66.66%.

❗ Current head 71001e5 differs from pull request most recent head 3017ef2. Consider uploading reports for the commit 3017ef2 to get more accurate results
Impacted file tree graph

@@             Coverage Diff              @@
##               main    #3595      +/-   ##
============================================
- Coverage     77.53%   77.53%   -0.01%     
- Complexity     2835     2837       +2     
============================================
  Files           464      464              
  Lines          8779     8783       +4     
  Branches       1720     1721       +1     
============================================
+ Hits           6807     6810       +3     
  Misses         1046     1046              
- Partials        926      927       +1     
Impacted Files Coverage Δ Complexity Δ
...h/detekt/rules/naming/MemberNameEqualsClassName.kt 77.14% <66.66%> (-1.65%) 13.00 <0.00> (ø)
...bosch/detekt/rules/complexity/MethodOverloading.kt 91.66% <0.00%> (+0.23%) 5.00% <0.00%> (ø%)
...b/arturbosch/detekt/rules/style/UseCheckNotNull.kt 90.90% <0.00%> (+0.90%) 5.00% <0.00%> (+2.00%)

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 8dbb01f...3017ef2. Read the comment docs.

@cortinico cortinico added this to the 1.17.0 milestone Mar 22, 2021
val code = """
data class GenericClass<T>(val wrapped: T) {
companion object {
fun <T> genericClass(wrapped: T): GenericClass<T> {
Copy link
Member

Choose a reason for hiding this comment

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

In this test is not clear if the problem was the generic in the class or the generic in the function. Could you simplify it? Or, if they are both, create two tests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added a non-generic factory function to the test, does that help?

Copy link
Member

Choose a reason for hiding this comment

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

Which is the problem exactly this code:

                    data class GenericClass(val wrapped: Any) {
                        companion object {
                            fun <T> genericClass(wrapped: T): GenericClass<T> {
                                return GenericClass(wrapped)
                            }
                        }
                    }

or this:

                    data class GenericClass<T>(val wrapped: T) {
                        companion object {
                            fun genericClass(wrapped: Any): GenericClass<Any> {
                                return GenericClass(wrapped)
                            }
                        }
                    }

Copy link
Contributor Author

Choose a reason for hiding this comment

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

the top one is closer to real world usage, but i suppose the bottom one is the minimal example? both fail without the change and pass with it, fwiw.

Compare the referenced type name instead of the fully
qualified generic type name when possible.

Fixes detekt#3591
@chao2zhang chao2zhang merged commit dfdec5c into detekt:main Mar 25, 2021
@cortinico cortinico changed the title Support factory exemption for generic classes [MemberNameEqualsClassName] Support factory exemption for generic classes May 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

MemberNameEqualsClassName factory function exemption doesn't work on generics
4 participants