From b74ef72af7387a14ceaf4ef96a0e256c2fac6a46 Mon Sep 17 00:00:00 2001 From: Yan Zhulanow Date: Thu, 2 Nov 2017 10:58:30 +0900 Subject: [PATCH] Kapt: Forbid illegal Java identifiers in Enum value literals --- .../kotlin/kapt3/javac/KaptJavaLog.kt | 33 +++++++++++++++++-- .../stubs/ClassFileToSourceStubConverter.kt | 7 +++- .../kapt3/test/AbstractKotlinKapt3Test.kt | 30 +++++++++++++++-- ...ileToSourceStubConverterTestGenerated.java | 6 ++++ .../testData/converter/invalidFieldName.kt | 10 ++++++ .../testData/converter/invalidFieldName.txt | 18 ++++++++++ .../testData/converter/strangeIdentifiers.kt | 2 ++ 7 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 plugins/kapt3/testData/converter/invalidFieldName.kt create mode 100644 plugins/kapt3/testData/converter/invalidFieldName.txt diff --git a/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/javac/KaptJavaLog.kt b/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/javac/KaptJavaLog.kt index 68637afb4be..551d8cf3f8b 100644 --- a/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/javac/KaptJavaLog.kt +++ b/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/javac/KaptJavaLog.kt @@ -17,12 +17,11 @@ package org.jetbrains.kotlin.kapt3.javac import com.sun.tools.javac.tree.JCTree -import com.sun.tools.javac.util.Context -import com.sun.tools.javac.util.JCDiagnostic -import com.sun.tools.javac.util.Log +import com.sun.tools.javac.util.* import org.jetbrains.kotlin.cli.common.messages.CompilerMessageSeverity import org.jetbrains.kotlin.cli.common.messages.CompilerMessageSeverity.* import org.jetbrains.kotlin.cli.common.messages.MessageCollector +import org.jetbrains.kotlin.kapt3.KaptContext import org.jetbrains.kotlin.kapt3.util.MessageCollectorBackedWriter import java.io.PrintWriter import javax.tools.JavaFileObject @@ -38,6 +37,29 @@ class KaptJavaLog( context.put(Log.outKey, noticeWriter) } + val reportedDiagnostics: List + get() = _reportedDiagnostics + + private val _reportedDiagnostics = mutableListOf() + + override fun flush(kind: WriterKind?) { + super.flush(kind) + + val diagnosticKind = when (kind) { + WriterKind.ERROR -> JCDiagnostic.DiagnosticType.ERROR + WriterKind.WARNING -> JCDiagnostic.DiagnosticType.WARNING + WriterKind.NOTICE -> JCDiagnostic.DiagnosticType.NOTE + else -> return + } + + _reportedDiagnostics.removeAll { it.type == diagnosticKind } + } + + override fun flush() { + super.flush() + _reportedDiagnostics.clear() + } + override fun report(diagnostic: JCDiagnostic) { if (diagnostic.type == JCDiagnostic.DiagnosticType.ERROR && diagnostic.code in IGNORED_DIAGNOSTICS) { return @@ -61,6 +83,7 @@ class KaptJavaLog( } } + _reportedDiagnostics += diagnostic super.report(diagnostic) } @@ -114,4 +137,8 @@ class KaptJavaLog( class DiagnosticInterceptorData { var files: Map = emptyMap() } +} + +fun KaptContext<*>.kaptError(text: String): JCDiagnostic { + return JCDiagnostic.Factory.instance(context).error(null, null, "proc.messager", text) } \ No newline at end of file diff --git a/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt b/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt index e1f5e1b66c2..90c5f057446 100644 --- a/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt +++ b/plugins/kapt3/src/org/jetbrains/kotlin/kapt3/stubs/ClassFileToSourceStubConverter.kt @@ -29,6 +29,7 @@ import org.jetbrains.kotlin.descriptors.annotations.Annotations import org.jetbrains.kotlin.kapt3.* import org.jetbrains.kotlin.kapt3.javac.KaptTreeMaker import org.jetbrains.kotlin.kapt3.javac.KaptJavaFileObject +import org.jetbrains.kotlin.kapt3.javac.kaptError import org.jetbrains.kotlin.kapt3.util.* import org.jetbrains.kotlin.name.FqName import org.jetbrains.kotlin.psi.* @@ -787,7 +788,11 @@ 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).takeIf { isValidIdentifier(it) } ?: "InvalidFieldName" + val valueName = (value[1] as String).takeIf { isValidIdentifier(it) } ?: run { + kaptContext.compiler.log.report(kaptContext.kaptError("'${value[1]}' is an invalid Java enum value name")) + "InvalidFieldName" + } + treeMaker.Select(treeMaker.Type(enumType), treeMaker.name(valueName)) } is List<*> -> treeMaker.NewArray(null, JavacList.nil(), mapJList(value) { convertLiteralExpression(it) }) diff --git a/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt b/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt index 8981b1abcf0..a8bb915fbaa 100644 --- a/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt +++ b/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt @@ -19,6 +19,7 @@ package org.jetbrains.kotlin.kapt3.test import com.intellij.openapi.util.text.StringUtil import com.sun.tools.javac.comp.CompileStates import com.sun.tools.javac.tree.JCTree.JCCompilationUnit +import com.sun.tools.javac.util.JCDiagnostic import com.sun.tools.javac.util.Log import org.jetbrains.kotlin.cli.common.messages.MessageRenderer import org.jetbrains.kotlin.cli.common.messages.PrintingMessageCollector @@ -28,6 +29,7 @@ import org.jetbrains.kotlin.codegen.state.GenerationState import org.jetbrains.kotlin.kapt3.Kapt3BuilderFactory import org.jetbrains.kotlin.kapt3.KaptContext import org.jetbrains.kotlin.kapt3.doAnnotationProcessing +import org.jetbrains.kotlin.kapt3.javac.KaptJavaLog import org.jetbrains.kotlin.kapt3.stubs.ClassFileToSourceStubConverter import org.jetbrains.kotlin.kapt3.util.KaptLogger import org.jetbrains.kotlin.resolve.jvm.extensions.AnalysisHandlerExtension @@ -38,6 +40,7 @@ import org.jetbrains.kotlin.test.util.trimTrailingWhitespacesAndAddNewlineAtEOF import org.jetbrains.kotlin.utils.PathUtil import java.io.File import java.nio.file.Files +import java.util.* import com.sun.tools.javac.util.List as JavacList abstract class AbstractKotlinKapt3Test : CodegenTestCase() { @@ -96,9 +99,17 @@ abstract class AbstractClassFileToSourceStubConverterTest : AbstractKotlinKapt3T override fun check(kaptContext: KaptContext, txtFile: File, wholeFile: File) { fun isOptionSet(name: String) = wholeFile.useLines { lines -> lines.any { it.trim() == "// $name" } } + fun getOptionValues(name: String) = wholeFile.useLines { lines -> + lines.filter { it.startsWith("// $name") } + .map { it.drop(name.length + 3).trim() } + .filter { it.isNotEmpty() } + .toList() + } + val generateNonExistentClass = isOptionSet("NON_EXISTENT_CLASS") val correctErrorTypes = isOptionSet("CORRECT_ERROR_TYPES") val validate = !isOptionSet("NO_VALIDATION") + val expectedErrors = getOptionValues("EXPECTED_ERROR").sorted() val javaFiles = convert(kaptContext, generateNonExistentClass, correctErrorTypes) @@ -111,8 +122,23 @@ abstract class AbstractClassFileToSourceStubConverterTest : AbstractKotlinKapt3T .let { removeMetadataAnnotationContents(it) } if (kaptContext.compiler.shouldStop(CompileStates.CompileState.ENTER)) { - Log.instance(kaptContext.context).flush() - error("There were errors during analysis. See errors above. Stubs:\n\n$actual") + val log = Log.instance(kaptContext.context) as KaptJavaLog + + val actualErrors = log.reportedDiagnostics + .filter { it.type == JCDiagnostic.DiagnosticType.ERROR } + .map { it.getMessage(Locale.US).lines().first() } + .sorted() + + log.flush() + + if (expectedErrors.isEmpty()) { + error("There were errors during analysis. See errors above. Stubs:\n\n$actual") + } else if (actualErrors != expectedErrors) { + error("Expected error matching assertion. Expected: \n" + + expectedErrors.joinToString("\n") { "'$it'" } + + "\n, found: \n" + + actualErrors.joinToString("\n") { "'$it'" }) + } } KotlinTestUtils.assertEqualsToFile(txtFile, actual) } diff --git a/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/ClassFileToSourceStubConverterTestGenerated.java b/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/ClassFileToSourceStubConverterTestGenerated.java index d3fd5c4af92..8ea449095e0 100644 --- a/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/ClassFileToSourceStubConverterTestGenerated.java +++ b/plugins/kapt3/test/org/jetbrains/kotlin/kapt3/test/ClassFileToSourceStubConverterTestGenerated.java @@ -120,6 +120,12 @@ public class ClassFileToSourceStubConverterTestGenerated extends AbstractClassFi doTest(fileName); } + @TestMetadata("invalidFieldName.kt") + public void testInvalidFieldName() throws Exception { + String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/converter/invalidFieldName.kt"); + doTest(fileName); + } + @TestMetadata("javaKeywords.kt") public void testJavaKeywords() throws Exception { String fileName = KotlinTestUtils.navigationMetadata("plugins/kapt3/testData/converter/javaKeywords.kt"); diff --git a/plugins/kapt3/testData/converter/invalidFieldName.kt b/plugins/kapt3/testData/converter/invalidFieldName.kt new file mode 100644 index 00000000000..e88918a4d1f --- /dev/null +++ b/plugins/kapt3/testData/converter/invalidFieldName.kt @@ -0,0 +1,10 @@ +// EXPECTED_ERROR 'WHI-TE' is an invalid Java enum value name +// EXPECTED_ERROR an enum annotation value must be an enum constant +// EXPECTED_ERROR cannot find symbol + +enum class Color { + BLACK, `WHI-TE` +} + +@Anno(Color.`WHI-TE`) +annotation class Anno(val color: Color) \ No newline at end of file diff --git a/plugins/kapt3/testData/converter/invalidFieldName.txt b/plugins/kapt3/testData/converter/invalidFieldName.txt new file mode 100644 index 00000000000..35c47513c05 --- /dev/null +++ b/plugins/kapt3/testData/converter/invalidFieldName.txt @@ -0,0 +1,18 @@ +@kotlin.Metadata() +@java.lang.annotation.Retention(value = java.lang.annotation.RetentionPolicy.RUNTIME) +@Anno(color = Color.InvalidFieldName) +public abstract @interface Anno { + + public abstract Color color(); +} + +//////////////////// + + +@kotlin.Metadata() +public enum Color { + /*public static final*/ BLACK /* = new BLACK() */; + + Color() { + } +} diff --git a/plugins/kapt3/testData/converter/strangeIdentifiers.kt b/plugins/kapt3/testData/converter/strangeIdentifiers.kt index 294bc63e1df..76f0db2d485 100644 --- a/plugins/kapt3/testData/converter/strangeIdentifiers.kt +++ b/plugins/kapt3/testData/converter/strangeIdentifiers.kt @@ -1,3 +1,5 @@ +// EXPECTED_ERROR '60x60' is an invalid Java enum value name + class `:)` { lateinit val f: String }