Skip to content

Commit 97c47e4

Browse files
yigitting-yuan
authored andcommitted
Report INNER modifier for class desciptors
This PR fixes a bug where the inner modifier would be lost if the kotlin sources is in dependencies (when wrapped in a descriptor). Unfortunately, for descriptors, there is no easy way to distinguish between JAVA_STATIC and INNER. That is, if a java nested class that is annotated with STATIC is compiled, descriptor API will no longer report any modifiers for it. Similarly, if a Java inner class (that has no static annotation) is compiled, we'll start reporting INNER for it. Even though this is slightly confusing, it is at least consistent from a kotlin perspective. Also fixed a bug in multi-module testing setup where we were NOT compiling java sources in test modules. Fixes: #231 Test: javaModifiers more test cases report inner for descriptors
1 parent 7569b95 commit 97c47e4

File tree

4 files changed

+98
-3
lines changed

4 files changed

+98
-3
lines changed

compiler-plugin/src/main/kotlin/com/google/devtools/ksp/processor/JavaModifierProcessor.kt

+7-2
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,13 @@ class JavaModifierProcessor : AbstractTestProcessor() {
3030
}
3131

3232
override fun process(resolver: Resolver) {
33-
val symbol = resolver.getSymbolsWithAnnotation("Test").single() as KSClassDeclaration
34-
symbol.superTypes.single().resolve()!!.declaration.accept(ModifierVisitor(), Unit)
33+
resolver.getSymbolsWithAnnotation("Test")
34+
.map {
35+
it as KSClassDeclaration
36+
}
37+
.forEach {
38+
it.superTypes.single().resolve().declaration.accept(ModifierVisitor(), Unit)
39+
}
3540
}
3641

3742
inner class ModifierVisitor : KSTopDownVisitor<Unit, Unit>() {

compiler-plugin/src/main/kotlin/com/google/devtools/ksp/symbol/impl/binary/KSClassDeclarationDescriptorImpl.kt

+3
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,9 @@ class KSClassDeclarationDescriptorImpl private constructor(val descriptor: Class
117117
if (descriptor.kind == KtClassKind.ANNOTATION_CLASS) {
118118
modifiers.add(Modifier.ANNOTATION)
119119
}
120+
if (descriptor.isInner) {
121+
modifiers.add(Modifier.INNER)
122+
}
120123
modifiers
121124
}
122125

compiler-plugin/src/test/kotlin/com/google/devtools/ksp/test/AbstractKotlinKSPTest.kt

+31-1
Original file line numberDiff line numberDiff line change
@@ -101,7 +101,12 @@ abstract class AbstractKotlinKSPTest : KotlinBaseTest<AbstractKotlinKSPTest.KspT
101101
dependencies: List<File>,
102102
testProcessor: AbstractTestProcessor?) {
103103
val moduleRoot = module.rootDir
104-
module.writeJavaFiles(testFiles)
104+
val hasJavaSources = testFiles.any {
105+
it.isJavaFile()
106+
}
107+
if (hasJavaSources) {
108+
module.writeJavaFiles(testFiles)
109+
}
105110
val configuration = createConfiguration(
106111
ConfigurationKind.NO_KOTLIN_REFLECT,
107112
TestJdkKind.FULL_JDK_9,
@@ -140,6 +145,23 @@ abstract class AbstractKotlinKSPTest : KotlinBaseTest<AbstractKotlinKSPTest.KspT
140145
GenerationUtils.compileFiles(moduleFiles.psiFiles, environment, ClassBuilderFactories.TEST)
141146
} else {
142147
GenerationUtils.compileFilesTo(moduleFiles.psiFiles, environment, outDir)
148+
if (hasJavaSources) {
149+
// need to compile java sources as well
150+
val javaOutDir = module.rootDir.resolve("javaOut")
151+
val classpath = (dependencies + KotlinTestUtils.getAnnotationsJar() + module.outDir)
152+
.joinToString(":") { it.absolutePath }
153+
val options = listOf(
154+
"-classpath", classpath,
155+
"-d", javaOutDir.absolutePath
156+
)
157+
val javaFiles = module.javaSrcDir.listFilesRecursively()
158+
KotlinTestUtils.compileJavaFiles(
159+
javaFiles,
160+
options
161+
)
162+
// merge java outputs into module's out dir
163+
javaOutDir.copyRecursively(target = module.outDir, overwrite = false)
164+
}
143165
}
144166
}
145167

@@ -187,6 +209,14 @@ abstract class AbstractKotlinKSPTest : KotlinBaseTest<AbstractKotlinKSPTest.KspT
187209
private val TestModule.outDir:File
188210
get() = File(rootDir, "out")
189211

212+
private fun KspTestFile.isJavaFile() = name.endsWith(".java")
213+
214+
private fun File.listFilesRecursively(): List<File> = if (!this.exists()) {
215+
emptyList()
216+
} else {
217+
this.walkTopDown().filter { it.isFile }.toList()
218+
}
219+
190220
/**
191221
* TestFile class for KSP where we can also keep a reference to the [TestModule]
192222
*/

compiler-plugin/testData/api/javaModifiers.kt

+57
Original file line numberDiff line numberDiff line change
@@ -21,9 +21,35 @@
2121
// staticStr: PRIVATE
2222
// s1: FINAL JAVA_TRANSIENT
2323
// i1: PROTECTED JAVA_STATIC JAVA_VOLATILE
24+
// NestedC: PUBLIC JAVA_STATIC
25+
// InnerC: PUBLIC
2426
// intFun: JAVA_SYNCHRONIZED JAVA_DEFAULT
2527
// foo: ABSTRACT JAVA_STRICT
28+
// OuterJavaClass: PUBLIC
29+
// InnerJavaClass: PUBLIC
30+
// NestedJavaClass: PUBLIC JAVA_STATIC
31+
// OuterKotlinClass: OPEN
32+
// InnerKotlinClass: INNER
33+
// NestedKotlinClass: OPEN
34+
// DependencyOuterJavaClass: OPEN PUBLIC
35+
// DependencyNestedJavaClass: OPEN PUBLIC
36+
// DependencyInnerJavaClass: OPEN PUBLIC INNER
37+
// DependencyOuterKotlinClass: OPEN PUBLIC
38+
// DependencyInnerKotlinClass: FINAL PUBLIC INNER
39+
// DependencyNestedKotlinClass: OPEN PUBLIC
2640
// END
41+
// MODULE: module1
42+
// FILE: DependencyOuterJavaClass.java
43+
public class DependencyOuterJavaClass {
44+
public class DependencyInnerJavaClass {}
45+
public static class DependencyNestedJavaClass {}
46+
}
47+
// FILE: DependencyOuterKotlinClass.kt
48+
open class DependencyOuterKotlinClass {
49+
inner class DependencyInnerKotlinClass
50+
open class DependencyNestedKotlinClass
51+
}
52+
// MODULE: main(module1)
2753
// FILE: a.kt
2854
annotation class Test
2955

@@ -32,6 +58,18 @@ class Foo : C() {
3258

3359
}
3460

61+
@Test
62+
class Bar : OuterJavaClass()
63+
64+
@Test
65+
class Baz : OuterKotlinClass()
66+
67+
@Test
68+
class JavaDependency : DependencyOuterJavaClass()
69+
70+
@Test
71+
class KotlinDependency : DependencyOuterKotlinClass()
72+
3573
// FILE: C.java
3674

3775
public abstract class C {
@@ -47,4 +85,23 @@ public abstract class C {
4785
}
4886

4987
abstract strictfp void foo() {}
88+
89+
public static class NestedC {
90+
91+
}
92+
93+
public class InnerC {
94+
95+
}
96+
}
97+
98+
// FILE: OuterJavaClass.java
99+
public class OuterJavaClass {
100+
public class InnerJavaClass {}
101+
public static class NestedJavaClass {}
102+
}
103+
// FILE: OuterKotlinClass.kt
104+
open class OuterKotlinClass {
105+
inner class InnerKotlinClass
106+
open class NestedKotlinClass
50107
}

0 commit comments

Comments
 (0)