fix isVisibleFrom() for java package visibility

(cherry picked from commit 62b0b30)
google · Sep 1, 2021 · df04f42 · df04f42
1 parent 0e8d505
commit df04f42
Show file tree

Hide file tree

Showing 3 changed files with 32 additions and 5 deletions.
diff --git a/api/src/main/kotlin/com/google/devtools/ksp/utils.kt b/api/src/main/kotlin/com/google/devtools/ksp/utils.kt
@@ -150,7 +150,8 @@ fun KSDeclaration.getVisibility(): Visibility {
             this.modifiers.contains(Modifier.OVERRIDE) -> Visibility.PROTECTED
         this.modifiers.contains(Modifier.INTERNAL) -> Visibility.INTERNAL
         this.modifiers.contains(Modifier.PUBLIC) -> Visibility.PUBLIC
-        else -> if (this.origin != Origin.JAVA) Visibility.PUBLIC else Visibility.JAVA_PACKAGE
+        else -> if (this.origin != Origin.JAVA && this.origin != Origin.JAVA_LIB)
+            Visibility.PUBLIC else Visibility.JAVA_PACKAGE
     }
 }
 
@@ -234,7 +235,7 @@ fun KSDeclaration.closestClassDeclaration(): KSClassDeclaration? {
 // TODO: cross module visibility is not handled
 fun KSDeclaration.isVisibleFrom(other: KSDeclaration): Boolean {
     fun KSDeclaration.isSamePackage(other: KSDeclaration): Boolean =
-        this.containingFile?.packageName == other.containingFile?.packageName
+        this.packageName == other.packageName
 
     // lexical scope for local declaration.
     fun KSDeclaration.parentDeclarationsForLocal(): List<KSDeclaration> {
@@ -269,9 +270,9 @@ fun KSDeclaration.isVisibleFrom(other: KSDeclaration): Boolean {
         this.isPrivate() -> this.isVisibleInPrivate(other)
         this.isPublic() -> true
         this.isInternal() && other.containingFile != null && this.containingFile != null -> true
-        // Non-private symbols in Java are always visible in same package.
-        this.origin == Origin.JAVA -> this.isSamePackage(other)
-        this.isProtected() -> this.isVisibleInPrivate(other) || other.closestClassDeclaration()?.let {
+        this.isJavaPackagePrivate() -> this.isSamePackage(other)
+        this.isProtected() -> this.isVisibleInPrivate(other) || this.isSamePackage(other) ||
+            other.closestClassDeclaration()?.let {
             this.closestClassDeclaration()!!.asStarProjectedType().isAssignableFrom(it.asStarProjectedType())
         } ?: false
         else -> false

diff --git a/compiler-plugin/src/test/kotlin/com/google/devtools/ksp/processor/VisibilityProcessor.kt b/compiler-plugin/src/test/kotlin/com/google/devtools/ksp/processor/VisibilityProcessor.kt
@@ -17,12 +17,14 @@
 
 package com.google.devtools.ksp.processor
 
+import com.google.devtools.ksp.getClassDeclarationByName
 import com.google.devtools.ksp.getVisibility
 import com.google.devtools.ksp.isVisibleFrom
 import com.google.devtools.ksp.processing.Resolver
 import com.google.devtools.ksp.symbol.KSAnnotated
 import com.google.devtools.ksp.symbol.KSClassDeclaration
 import com.google.devtools.ksp.symbol.KSFunctionDeclaration
+import com.google.devtools.ksp.symbol.KSPropertyDeclaration
 
 class VisibilityProcessor : AbstractTestProcessor() {
     val results = mutableListOf<String>()
@@ -41,6 +43,16 @@ class VisibilityProcessor : AbstractTestProcessor() {
             "${it.simpleName.asString()}: ${it.getVisibility()},visible in A, B, D: " +
                 "${it.isVisibleFrom(symbolA)}, ${it.isVisibleFrom(symbolB)}, ${it.isVisibleFrom(symbolD)}"
         }.forEach { results.add(it) }
+        val javaClass = resolver.getClassDeclarationByName("JavaClass")!!
+        val kotlinClass = resolver.getClassDeclarationByName("KotlinClass")!!
+        javaClass.declarations.filterIsInstance<KSPropertyDeclaration>().map {
+            "${it.simpleName.asString()}: ${it.getVisibility()},visible in A, B, D: " +
+                "${it.isVisibleFrom(symbolA)}, ${it.isVisibleFrom(symbolB)}, ${it.isVisibleFrom(symbolD)}"
+        }.forEach { results.add(it) }
+        kotlinClass.declarations.filterIsInstance<KSPropertyDeclaration>().map {
+            "${it.simpleName.asString()}: ${it.getVisibility()},visible in A, B, D: " +
+                "${it.isVisibleFrom(symbolA)}, ${it.isVisibleFrom(symbolB)}, ${it.isVisibleFrom(symbolD)}"
+        }.forEach { results.add(it) }
         return emptyList()
     }
 }
diff --git a/compiler-plugin/testData/api/visibilities.kt b/compiler-plugin/testData/api/visibilities.kt
@@ -22,8 +22,22 @@
 // privateFun: PRIVATE,visible in A, B, D: false, false, false
 // protectedFun: PROTECTED,visible in A, B, D: true, false, true
 // <init>: PUBLIC,visible in A, B, D: true, true, true
+// javaPackageField: JAVA_PACKAGE,visible in A, B, D: true, false, true
+// x: INTERNAL,visible in A, B, D: false, false, false
 // END
 
+// MODULE: lib
+// FILE: JavaClass.java
+public class JavaClass {
+    int javaPackageField;
+}
+
+// FILE: lib.kt
+class KotlinClass {
+    internal val x: Int = 0
+}
+
+// MODULE: main(lib)
 // FILE: a.kt
 annotation class TestA
 annotation class TestB