Lint 8.2.0 alphas still do not support top-level function imports in kotlin [283693338]

Verified

Bug

Status Update

No update yet.

Description

za...@gmail.com

created issue #1

May 22, 2023 12:53AM

DESCRIBE THE ISSUE IN DETAIL:

Lint still misses top level function imports in Kotlin source files, despite being marked as fixed in AGP 7.1.0 according to here and here. I've tested this all the way up to AGP 8.2.0-alpha04

STEPS TO REPRODUCE:

Repro PR + test here: https://github.com/slackhq/slack-lints/pull/65
Observe the stub fails to compile due to not being able to resolve that top-level import

Studio Build: N/A
Version of Gradle Plugin: 8.2.0-alpha04
Version of Gradle: 8.1.1
Version of Java: 19
OS:

Comments

za...@gmail.com <za...@gmail.com> #2May 22, 2023 12:54AM

Sorry this should also say "in TestLintTask"!

js...@google.com <js...@google.com> #3May 22, 2023 06:35AM

Assigned to js...@google.com.

This import: https://github.com/slackhq/slack-lints/blob/main/slack-lint-checks/src/test/java/slack/lint/mocking/DataClassMockDetectorTest.kt#L32

import slack.test.mockito.mock

points to these: https://github.com/slackhq/slack-lints/blob/main/slack-lint-checks/src/test/java/slack/lint/mocking/MockFileStubs.kt#L51-L55

inline fun <reified T : Any> mock(): T = Mockito.mock(T::class.java)

inline fun <reified T : Any> mock(block: T.() -> Unit): T {
    return Mockito.mock(T::class.java).apply(block)
}

Even though the test said MockFileStubs, it's actually "source": https://github.com/slackhq/slack-lints/blob/main/slack-lint-checks/src/test/java/slack/lint/mocking/MockFileStubs.kt#L44

The point is, inline function with reified type parameter is not going to be modeled as light class / UAST.

Therefore, Lint's backup that looks up facade light class doesn't work: https://cs.android.com/android-studio/platform/tools/base/+/mirror-goog-studio-main:lint/libs/lint-tests/src/main/java/com/android/tools/lint/checks/infrastructure/ResolveChecker.kt;l=160-169;drc=f9c36e578a3ee73d04fa651d553287a90e5180c8

UAST has a backup too: so-called "fake" PSI. When compiler's light class (and thus UAST via delegation) won't create elements like that (because reified inline is not visible from JVM), but references are still available as source, UAST provides a UElement for that by creating a fake light element. In this case, though, that doesn't work either, because there are multiple candidate resolutions, and without specific call arguments, import resolution with just simple name reference can't conclude which one is the right one. This reminded me of a TODO I left 2 yrs ago... : https://github.com/JetBrains/intellij-community/blob/master/plugins/kotlin/uast/uast-kotlin-base/tests/test/org/jetbrains/uast/test/common/kotlin/UastResolveApiTestBase.kt#L90-L95

Anyhow, I found a Lint-level workaround w/ Analysis API. Will point that out once it's reviewed/landed/etc.

js...@google.com <js...@google.com> #4May 22, 2023 06:27PM

Marked as fixed.

Will point that out once it's reviewed/landed/etc.

Hm, cs.android.com stopped mirroring the internal changes since mid Apr. :\

Anyway, fix landed and will be included in 8.2.0-alpha06.

za...@gmail.com <za...@gmail.com> #5Jun 8, 2023 06:54PM

It gets farther now with lint 31.2.0-alpha07, but now there is a NSE thrown from deep in lint/PSI infrastructure

Repro: https://github.com/slackhq/slack-lints/pull/90

java.util.NoSuchElementException: Collection contains no element matching the predicate.
	at org.jetbrains.kotlin.analysis.project.structure.impl.KtModuleProviderImpl.getKtModuleForKtElement(KtModuleProviderImpl.kt:78)
	at org.jetbrains.kotlin.analysis.project.structure.ProjectStructureProviderKt.getKtModule(ProjectStructureProvider.kt:24)
	at org.jetbrains.kotlin.analysis.project.structure.ProjectStructureProviderKt.getKtModule$default(ProjectStructureProvider.kt:22)
	at org.jetbrains.kotlin.analysis.api.descriptors.KtFe10AnalysisSession.<init>(KtFe10AnalysisSession.kt:31)
	at org.jetbrains.kotlin.analysis.api.descriptors.KtFe10AnalysisSessionProvider.getAnalysisSession(KtFe10CliAnalysisSessionProvider.kt:24)
	at com.android.tools.lint.checks.infrastructure.ResolveCheckerKt$checkFile$1.visitImportStatement(ResolveChecker.kt:399)
	at org.jetbrains.uast.UImportStatement.accept(UImportStatement.kt:41)
	at org.jetbrains.uast.internal.ImplementationUtilsKt.acceptList(implementationUtils.kt:15)
	at org.jetbrains.uast.UFile.accept(UFile.kt:88)
	at com.android.tools.lint.detector.api.UastLintUtilsKt.acceptSourceFile(UastLintUtils.kt:826)
	at com.android.tools.lint.checks.infrastructure.ResolveCheckerKt.checkFile(ResolveChecker.kt:86)
	at com.android.tools.lint.checks.infrastructure.TestLintClient$2.parse(TestLintClient.java:895)
	at com.android.tools.lint.client.api.UElementVisitor.visitFile(UElementVisitor.kt:232)
	at com.android.tools.lint.client.api.LintDriver$visitUastDetectors$1.run(LintDriver.kt:2169)
	at com.android.tools.lint.client.api.LintClient.runReadAction(LintClient.kt:1700)
	at com.android.tools.lint.checks.infrastructure.TestLintClient.runReadAction(TestLintClient.java:908)
	at com.android.tools.lint.client.api.LintDriver$LintClientWrapper.runReadAction(LintDriver.kt:2871)
	at com.android.tools.lint.client.api.LintDriver.visitUastDetectors(LintDriver.kt:2169)
	at com.android.tools.lint.client.api.LintDriver.visitUast(LintDriver.kt:2154)
	at com.android.tools.lint.client.api.LintDriver.runFileDetectors(LintDriver.kt:1383)
	at com.android.tools.lint.client.api.LintDriver.checkProject(LintDriver.kt:1148)
	at com.android.tools.lint.client.api.LintDriver.checkProjectRoot(LintDriver.kt:619)
	at com.android.tools.lint.client.api.LintDriver.access$checkProjectRoot(LintDriver.kt:170)
	at com.android.tools.lint.client.api.LintDriver$analyze$1.invoke(LintDriver.kt:423)
	at com.android.tools.lint.client.api.LintDriver$analyze$1.invoke(LintDriver.kt:420)
	at com.android.tools.lint.client.api.LintDriver.doAnalyze(LintDriver.kt:500)
	at com.android.tools.lint.client.api.LintDriver.doAnalyze$default(LintDriver.kt:476)
	at com.android.tools.lint.client.api.LintDriver.analyze(LintDriver.kt:420)
	at com.android.tools.lint.checks.infrastructure.TestLintClient.analyze(TestLintClient.java:668)
	at com.android.tools.lint.checks.infrastructure.TestLintClient.analyze(TestLintClient.java:599)
	at com.android.tools.lint.checks.infrastructure.TestLintClient.checkLint(TestLintClient.java:338)
	at com.android.tools.lint.checks.infrastructure.TestLintRunner.checkLint(TestLintRunner.kt:326)
	at com.android.tools.lint.checks.infrastructure.TestLintRunner.runMode(TestLintRunner.kt:277)
	at com.android.tools.lint.checks.infrastructure.TestLintRunner.runMode(TestLintRunner.kt:226)
	at com.android.tools.lint.checks.infrastructure.TestLintRunner.runMode$default(TestLintRunner.kt:194)
	at com.android.tools.lint.checks.infrastructure.TestLintRunner.run(TestLintRunner.kt:131)
	at com.android.tools.lint.checks.infrastructure.TestLintTask.run(TestLintTask.java:1126)
	at slack.lint.mocking.DataClassMockDetectorTest.kotlinTests(DataClassMockDetectorTest.kt:74)

js...@google.com <js...@google.com> Jun 8, 2023 07:44PM

Status: Assigned (reopened)

js...@google.com <js...@google.com> #6Jun 8, 2023 09:42PM

Re: comment#4,

Now it's available: https://cs.android.com/android-studio/platform/tools/base/+/7cd31ca28e2abdeba4a8bbeef32dfe00b015fc47

and thus,

at org.jetbrains.kotlin.analysis.api.descriptors.KtFe10AnalysisSessionProvider.getAnalysisSession(KtFe10CliAnalysisSessionProvider.kt:24)
at com.android.tools.lint.checks.infrastructure.ResolveCheckerKt$checkFile$1.visitImportStatement(ResolveChecker.kt:399)

clearly pointed out the workaround I added.

js...@google.com <js...@google.com> #7Jun 8, 2023 09:49PM

Very last moment before NSE, VirtualFile comparison fails:

VirtualFile: /tmp/12746563826225783916/DataClassMockDetectorTest_null/default/app/src/slack/test/TestClass.kt

v.s.

VirtualFile: /tmp/12746563826225783916/DataClassMockDetectorTest_null/default/app/test/test/slack/test/TestClass.kt

then found that DataClassMockDetectorTest has slack.test.TestClass as testClass without specifying where to save this file, which ended up .../app/src/slack/test/TestClass.kt. Then, the main test input is test/test/slack/test/TestClass.kt even though it has MyTests class, which, with explicit to, is stored as .../app/test/test/slack/test/TEstClass.kt.

As a "mock" detector, all subclasses of MockDetector are expected to run only if JavaContext.isTestSource. Therefore, putting the main input under test/... makes sense. Recent changes on Lint's module structure building missed to include all kinds of test folders, though.

Thank you so much to test a new canary; I can reproduce this locally, and the fix is sent out.

js...@google.com <js...@google.com> #8Jun 9, 2023 07:01PM

Marked as fixed.

fix landed: https://cs.android.com/android-studio/platform/tools/base/+/2016d126ca372f1caa50a15bca5bd3ded4b232c3

and will be included in 8.2.0-alpha09.

You may need to configure Lint test driver to include test sources:

lint()
  .files(...)
  .issues(...)
  .configureDriver { it.checkTestSources = true }
  .run()
  ...

js...@google.com <js...@google.com> #9Sep 8, 2023 09:07PM

Verified by js...@google.com.

Seems to work: https://github.com/slackhq/slack-lints/commit/d7a89804de560aa7d45c0b2030b7178bd4ebb465