Kapt3: Ignore declarations with illegal Java identifiers (KT-16153)

Author: yanex

plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt

@@ -172,7 +172,7 @@ class ClassFileToSourceStubConverter(
             return null
         }
 
-        val simpleName = getClassName(clazz, descriptor, isDefaultImpls, packageFqName)
+        val simpleName = getValidIdentifierName(getClassName(clazz, descriptor, isDefaultImpls, packageFqName)) ?: return null
 
         val interfaces = mapJList(clazz.interfaces) {
             if (isAnnotation && it == "java/lang/annotation/Annotation") return@mapJList null
@@ -245,7 +245,7 @@ class ClassFileToSourceStubConverter(
 
         return treeMaker.ClassDef(
                 modifiers,
-                treeMaker.name(getNonKeywordName(simpleName)),
+                treeMaker.name(simpleName),
                 genericType.typeParameters,
                 if (hasSuperClass) genericType.superClass else null,
                 genericType.interfaces,
@@ -279,7 +279,7 @@ class ClassFileToSourceStubConverter(
         val descriptor = kaptContext.origins[field]?.descriptor
 
         val modifiers = convertModifiers(field.access, ElementKind.FIELD, packageFqName, field.visibleAnnotations, field.invisibleAnnotations)
-        val name = treeMaker.name(getNonKeywordName(field.name))
+        val name = getValidIdentifierName(field.name) ?: return null
         val type = Type.getType(field.desc)
 
         // Enum type must be an identifier (Javac requirement)
@@ -298,7 +298,7 @@ class ClassFileToSourceStubConverter(
                           ?: convertValueOfPrimitiveTypeOrString(value)
                           ?: if (isFinal(field.access)) convertLiteralExpression(getDefaultValue(type)) else null
 
-        return treeMaker.VarDef(modifiers, name, typeExpression, initializer)
+        return treeMaker.VarDef(modifiers, treeMaker.name(name), typeExpression, initializer)
     }
 
     private fun convertMethod(method: MethodNode, containingClass: ClassNode, packageFqName: String): JCMethodDecl? {
@@ -318,7 +318,7 @@ class ClassFileToSourceStubConverter(
         }
 
         val isConstructor = method.name == "<init>"
-        val name = treeMaker.name(getNonKeywordName(method.name))
+        val name = getValidIdentifierName(method.name, canBeConstructor = true) ?: return null
 
         val modifiers = convertModifiers(
                 if (containingClass.isEnum() && isConstructor)
@@ -344,7 +344,7 @@ class ClassFileToSourceStubConverter(
                     info.visibleAnnotations,
                     info.invisibleAnnotations)
 
-            val name = treeMaker.name(getNonKeywordName(info.name))
+            val name = treeMaker.name(getValidIdentifierName(info.name) ?: "p${index}_" + info.name.hashCode())
             val type = treeMaker.Type(info.type)
             treeMaker.VarDef(modifiers, name, type, null)
         }
@@ -388,7 +388,7 @@ class ClassFileToSourceStubConverter(
         }
 
         return treeMaker.MethodDef(
-                modifiers, name, returnType, genericSignature.typeParameters,
+                modifiers, treeMaker.name(name), returnType, genericSignature.typeParameters,
                 genericSignature.parameterTypes, genericSignature.exceptionTypes,
                 body, defaultValue)
     }
@@ -452,9 +452,21 @@ class ClassFileToSourceStubConverter(
         return ifNonError()
     }
 
-    private fun getNonKeywordName(name: String): String {
+    fun getValidIdentifierName(name: String, canBeConstructor: Boolean = false): String? {
+        if (canBeConstructor && name == "<init>") {
+            return name
+        }
+
         // In theory, this could lead to member/parameter name clashes, though it's supposed to be extremely rare.
         if (name in JAVA_KEYWORDS) return '_' + name + '_'
+
+        if (name.isEmpty()
+            || !Character.isJavaIdentifierStart(name[0])
+            || name.drop(1).any { !Character.isJavaIdentifierPart(it) }
+        ) {
+            return null
+        }
+
         return name
     }
 
@@ -532,7 +544,8 @@ class ClassFileToSourceStubConverter(
         val useSimpleName = '.' in fqName && fqName.substringBeforeLast('.', "") == packageFqName
         val name = if (useSimpleName) treeMaker.FqName(fqName.substring(packageFqName!!.length + 1)) else treeMaker.Type(annotationType)
         val values = mapPairedValuesJList<JCExpression>(annotation.values) { key, value ->
-            treeMaker.Assign(treeMaker.SimpleName(key), convertLiteralExpression(value))
+            val name = getValidIdentifierName(key) ?: return@mapPairedValuesJList null
+            treeMaker.Assign(treeMaker.SimpleName(name), convertLiteralExpression(value))
         }
         return treeMaker.Annotation(name, values)
     }
@@ -564,7 +577,7 @@ class ClassFileToSourceStubConverter(
             is Array<*> -> { // Two-element String array for enumerations ([desc, fieldName])
                 assert(value.size == 2)
                 val enumType = Type.getType(value[0] as String)
-                val valueName = value[1] as String
+                val valueName = getValidIdentifierName(value[1] as String) ?: "InvalidFieldName"
                 treeMaker.Select(treeMaker.Type(enumType), treeMaker.name(valueName))
             }
             is List<*> -> treeMaker.NewArray(null, JavacList.nil(), mapJList(value) { convertLiteralExpression(it) })

plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/ClassFileToSourceStubConverterTestGenerated.java

@@ -222,6 +222,12 @@ public void testSeveralPackageParts() throws Exception {
         doTest(fileName);
     }
 
+    @TestMetadata("strangeIdentifiers.kt")
+    public void testStrangeIdentifiers() throws Exception {
+        String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/converter/strangeIdentifiers.kt");
+        doTest(fileName);
+    }
+
     @TestMetadata("strangeNames.kt")
     public void testStrangeNames() throws Exception {
         String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/converter/strangeNames.kt");

plugins/kapt3/testData/converter/strangeIdentifiers.kt

@@ -0,0 +1,38 @@
+class `:)` {
+    lateinit val f: String
+}
+
+// Commented declarations won't compile with the current Kotlin
+class Test {
+    class `(^_^)`
+
+    lateinit val simpleName: String
+    lateinit val `strange name`: String
+//    lateinit val strangeType: List<`!A@`>
+
+    fun simpleFun() {}
+
+    @Anno(name = "Woofwoof", size = StrangeEnum.`60x60`, `A B` = "S")
+    fun simpleFun2(a: String, b: String) {}
+
+    fun `strange!Fun`() {}
+//    fun strangeFun2(a: String, b: `A()B()`) {}
+//    fun strangeFun3(a: String, b: `A B`) {}
+    fun strangeFun4(a: String, `A()B()`: String) {}
+//    fun strangeFun5(a: `A B`.C) {}
+}
+
+enum class StrangeEnum(val size: String) {
+    `60x60`("60x60"),
+    `70x70`("70x70"),
+    `80x80`("80x80"),
+    InvalidFieldName("0x0") // Workaround to pass javac analysis
+}
+
+annotation class Anno(val size: StrangeEnum, val name: String, val `A B`: String)
+
+class `!A@`
+class `A()B()`
+class `A B` {
+    class C
+}

plugins/kapt3/testData/converter/strangeIdentifiers.txt

@@ -0,0 +1,56 @@
+@java.lang.annotation.Retention(value = java.lang.annotation.RetentionPolicy.RUNTIME)
+public abstract @interface Anno {
+
+    public abstract StrangeEnum size();
+
+    public abstract java.lang.String name();
+}
+
+////////////////////
+
+
+public enum StrangeEnum {
+    /*public static final*/ InvalidFieldName /* = new InvalidFieldName(null) */;
+    @org.jetbrains.annotations.NotNull()
+    private final java.lang.String size = null;
+
+    @org.jetbrains.annotations.NotNull()
+    public final java.lang.String getSize() {
+        return null;
+    }
+
+    StrangeEnum(@org.jetbrains.annotations.NotNull()
+    java.lang.String size) {
+    }
+}
+
+////////////////////
+
+
+public final class Test {
+    @org.jetbrains.annotations.NotNull()
+    public java.lang.String simpleName;
+
+    @org.jetbrains.annotations.NotNull()
+    public final java.lang.String getSimpleName() {
+        return null;
+    }
+
+    public final void simpleFun() {
+    }
+
+    @Anno(name = "Woofwoof", size = StrangeEnum.InvalidFieldName)
+    public final void simpleFun2(@org.jetbrains.annotations.NotNull()
+    java.lang.String a, @org.jetbrains.annotations.NotNull()
+    java.lang.String b) {
+    }
+
+    public final void strangeFun4(@org.jetbrains.annotations.NotNull()
+    java.lang.String a, @org.jetbrains.annotations.NotNull()
+    java.lang.String p1_1899121793) {
+    }
+
+    public Test() {
+        super();
+    }
+}

plugins/kapt3/testData/converter/strangeNames.txt

@@ -1,26 +1 @@
-public final class My $ name {
-    @org.jetbrains.annotations.NotNull()
-    private final java.lang.String (@\u2022@) = "";
-    @org.jetbrains.annotations.NotNull()
-    private final java.lang.String \u041a\u043e\u0442\u043b\u0438\u043d-\u043a\u043e\u043c\u043f\u0438\u043b\u044f\u0442\u043e\u0440 = "";
 
-    public final void (^_^)(int )))))) {
-    }
-
-    @org.jetbrains.annotations.NotNull()
-    public final java.lang.String get(@\u2022@)() {
-        return null;
-    }
-
-    @org.jetbrains.annotations.NotNull()
-    public final java.lang.String get\u041a\u043e\u0442\u043b\u0438\u043d-\u043a\u043e\u043c\u043f\u0438\u043b\u044f\u0442\u043e\u0440() {
-        return null;
-    }
-
-    public final void \u3053\u3000\u3068\u3000\u308a\u3000\u3093\uff01() {
-    }
-
-    public My $ name() {
-        super();
-    }
-}