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

False positive on UnnecessaryInnerClass calls to this #4866

Closed
dzirbel opened this issue May 26, 2022 · 0 comments · Fixed by #4884
Closed

False positive on UnnecessaryInnerClass calls to this #4866

dzirbel opened this issue May 26, 2022 · 0 comments · Fixed by #4884

Comments

@dzirbel
Copy link
Contributor

dzirbel commented May 26, 2022

Expected Behavior

If an inner class has a plan reference to this, UnnecessaryInnerClass should still raise an issue.

Observed Behavior

UnnecessaryInnerClass does not raise an issue, even though the class could be made non-inner.

Steps to Reproduce

@Test
fun `reports even with calls to this`() {
    val code = """
        class A {
            inner class B {
                fun foo() {
                    this
                }
            }
        }
    """.trimIndent()

    assertThat(subject.lintWithContext(env, code)).hasSize(1)
}

Context

I came across this issue while working on labeled expressions in #4865 but was unable to find a resolution. It seems that visitReferenceExpression() will be invoked for any plain call to this and resolve it in the example above as class A when it should be B. Even though it's a reference to this, the expression is not a KtThisExpression but a plain KtNameReferenceExpression with a referenced name of this; aside from checking against that referenced name I couldn't find a way to distinguish it from other expressions.

I don't have a particular use case, but it does seem to interfere with labeled expressions in some cases, e.g.

@Test
fun `reports irrelevant labeled expressions`() {
    val code = """
        class A {
            inner class B {
                fun inner() {
                    return Unit.apply { this@B }
                }
            }
        }
    """.trimIndent()

    assertThat(subject.lintWithContext(env, code)).hasSize(1)
}

this test will fail, not because of labeled expression as per #4865 but because of the use of this@B, even though that doesn't require B to be inner.

Your Environment

  • current main branch (fcd9c48159685fc049da6e6d2b3a77db3aea00bd)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants