Status Update
No update yet.
Comments
js...@google.com
@Test
fun checkK2Uast() {
getExecutor().with(BooleanOption.LINT_USE_K2_UAST, true).run( ":lint")
val file = project.file("lint-results.txt")
assertThat(file).exists()
assertThat(file).contains("build.gradle:4: Warning: no Java language level directives")
// TODO(b/285413332)
// assertThat(file).contains("MyClass.java:5: Warning: Use Boolean.valueOf(true) instead")
// assertThat(file).contains("0 errors, 3 warnings")
}
So... if UseValueOf issue is expected to be raised on MyClass.java, technically, we should not see any behavioral differences when switching between K1 v.s. K2 UAST, since it's about Kotlin UAST backed by different kotlinc frontends. That is, it would be better to test a project w/ Kotlin source files.
However, handling both inputs / inter-op is also important, of course. I wonder, though, if this is already fixed by
sp...@google.com
However, handling both inputs / inter-op is also important, of course.
Yes, I filed the bug assuming we want the same behavior regardless of whether we're using K2.
I wonder, though, if this is already fixed by
http://ag/23488359 ?
Unfortunately not.
js...@google.com
I wonder, though, if this is already fixed by
http://ag/23488359 ?Unfortunately not.
Besides Module building is handling only direct source file roots (like .../blahblah.kt), not general root directory (like app/src).
Though, I'm still not able to test due to
js...@google.com
Besides
https://cs.android.com/android-studio/platform/tools/base/+/0912096446fb212131ce487660bf28b6bd40efeb , there is one more recent change:https://cs.android.com/android-studio/platform/tools/base/+/4d26c1afa9ae0be9bd67f68836190e8969245835 where I found that Module building is handling only direct source file roots (like.../blahblah.kt), not general root directory (likeapp/src).
I can confirm that's the case:
tools/base $ git log
commit f2f16428b963d102baf008eb0c7d90f09b0b9dc3 (HEAD -> agp-flag-for-k2-uast)
Author: Scott Pollom <spollom@google.com>
Date: Tue May 30 12:11:05 2023 -0700
Add BooleanOption to allow running lint with K2 UAST
Bug: 283870344
Test: LintUseK2UastTest, LintStandaloneTest, etc.
Change-Id: I381e36ac19192f9e2e1d3404107d6b2c86ff5642
commit 4d26c1afa9ae0be9bd67f68836190e8969245835 (m/studio-main, goog/studio-main)
Author: Jinseong Jeon <jsjeon@google.com>
Date: Thu Jun 1 11:11:27 2023 -0700
Build .kts module(s) for script files
Bug: 271032477
Test: a couple of Lint tests that input .kts
Change-Id: I59139471e47758c79d258063569f4ae694cb39a5
...
tools/base $ git diff
diff --git a/build-system/integration-test/lint/src/test/java/com/android/build/gradle/integration/lint/LintStandaloneTest.kt b/build-system/integration-test/lint/src/test/java/com/android/build/gradle/integration/lint/LintStandaloneTest.kt
index 21d07afcc6..6147b55358 100644
--- a/build-system/integration-test/lint/src/test/java/com/android/build/gradle/integration/lint/LintStandaloneTest.kt
+++ b/build-system/integration-test/lint/src/test/java/com/android/build/gradle/integration/lint/LintStandaloneTest.kt
@@ -155,9 +155,8 @@ class LintStandaloneTest(private val runLintInProcess: Boolean) {
val file = project.file("lint-results.txt")
assertThat(file).exists()
assertThat(file).contains("build.gradle:4: Warning: no Java language level directives")
- // TODO(b/285413332)
- // assertThat(file).contains("MyClass.java:5: Warning: Use Boolean.valueOf(true) instead")
- // assertThat(file).contains("0 errors, 3 warnings")
+ assertThat(file).contains("MyClass.java:5: Warning: Use Boolean.valueOf(true) instead")
+ assertThat(file).contains("0 errors, 3 warnings")
}
tools $ ./gradlew :base:build-system:integration-test:lint:test
...
> Task :base:build-system:integration-test:lint:test
com.android.build.gradle.integration.lint.LintBaselineTest > checkBaselineFilteringJarWarnings[lintBaselinesContinue = false, runLintInProcess = true] SKIPPED
com.android.build.gradle.integration.lint.LintBaselineTest > checkBaselineFilteringJarWarnings[lintBaselinesContinue = false, runLintInProcess = false] SKIPPED
BUILD SUCCESSFUL in 4m 21s
256 actionable tasks: 132 executed, 124 up-to-date
js...@google.com
Hm :( I saw
Not true that <C:\Users\ContainerAdministrator\android-tests\5\LintK2UastStandaloneTest\testUseK2Uast\lintStandalone\lint-results.txt> contains <MyClass.java:5: Warning: Use Boolean.valueOf(true) instead>. It is <build.gradle:4: Warning: no Java language level directives [JavaPluginLanguageLevel]
apply plugin: 'java-library'
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
settings.gradle:4: Warning: no Java language level directives [JavaPluginLanguageLevel]
repositories {
~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Explanation for issues of type "JavaPluginLanguageLevel":
In modules using plugins deriving from the Gradle java plugin (e.g.
java-library or application), the java source and target compatibility
default to the version of the JDK being used to run Gradle, which may cause
compatibility problems with Android (or other) modules.
You can specify an explicit sourceCompatibility and targetCompatibility in
this module to maintain compatibility no matter which JDK is used to run
Gradle.
0 errors, 2 warnings
>
...
js...@google.com
Oh, the target is studio-win, which we (or I) never tested K2 UAST, AA module structure, etc., where file path lookup can be different. :panic:
js...@google.com
Re:
tools $ gradlew.bat :base:build-system:integration-test:lint:test --tests=LintStandaloneTest.checkK2Uast*
Note: don't add quotes around --tests filter.
js...@google.com
I finally got to the bottom of this; filed an upstream issue:
So, that false negative started from here:
When JavaPerformanceDetector visits new Boolean(true) as a constructor call, from the expression type PsiType:Boolean, it wants to retrieve PsiClass, which is supposed to be a stub-based representation of java.lang.Boolean that came from the underlying JRT. While tracking down how Java definition is resolved in JavaPsiFacade, I found that facade didn't have anything about JRT, which led me to investigate how JvmDependenciesIndexImpl is populated in AA standalone.
Similar to legacy KotlinCoreEnvironment, AA standalone, in particular, StandaloneProjectFactory uses same CoreJarFileSystem from IJ platform to read .jar and same CoreJrtFileSystem (partially copied from IJ full) to read JRT. Those two file systems are not only shared with IJ and legacy frontend too, but also use / as file separator and !/ as JAR separator frequently. Thus, editing them doesn't seem like a right place; rather, any paths passed to those should be massaged upfront, and that does the trick.
Note that Paths.get in Windows even changes JAR separator(?!) e.g., jrt:///path/to/jdk/home!/java.base v.s. C:\src\studio-main\prebuilts\studio\jdk\jdk17\win!\java.base, so all the following steps in CoreJrtFileSystem that split the path into home path (/path/to/jdk/home before !/) v.s. path in image (java.base after !/) and walk from that home path to find child VirtualFile for module path simply doesn't work if !\ is used as JAR separator.
You can find more about JRT module paths from the code comments I left a while ago:
Here is a seemingly simple (yet non-trivial to figure out) fix:
Description
See test added to LintStandaloneTest in
Jinseong, I assigned this to you, but feel free to reassign if you're not the right assignee