Kapt: Strict mode. Issue an error if the converter fails to process a class (#KT-25518, #KT-24272)

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/internal/kapt/Kapt3KotlinGradleSubplugin.kt

@@ -341,6 +341,7 @@ class Kapt3KotlinGradleSubplugin : KotlinGradleSubplugin<KotlinCompile> {
         pluginOptions += SubpluginOption("useLightAnalysis", "${kaptExtension.useLightAnalysis}")
         pluginOptions += SubpluginOption("correctErrorTypes", "${kaptExtension.correctErrorTypes}")
         pluginOptions += SubpluginOption("mapDiagnosticLocations", "${kaptExtension.mapDiagnosticLocations}")
+        pluginOptions += SubpluginOption("strictMode", "${kaptExtension.strictMode}")
         pluginOptions += SubpluginOption("infoAsWarnings", "${project.isInfoAsWarnings()}")
         pluginOptions += FilesSubpluginOption("stubs", listOf(getKaptStubsDir()))
 

libraries/tools/kotlin-gradle-plugin/src/main/kotlin/org/jetbrains/kotlin/gradle/plugin/KaptExtension.kt

@@ -32,6 +32,8 @@ open class KaptExtension {
 
     open var mapDiagnosticLocations: Boolean = false
 
+    open var strictMode: Boolean = false
+
     @Deprecated("Use `annotationProcessor()` and `annotationProcessors()` instead")
     open var processors: String = ""
 

plugins/kapt3/kapt3-base/src/org/jetbrains/kotlin/kapt3/base/javac/KaptJavaLog.kt

@@ -224,10 +224,17 @@ class KaptJavaLog(
     }
 }
 
-fun KaptContext.kaptError(text: String): JCDiagnostic {
+private val LINE_SEPARATOR: String = System.getProperty("line.separator")
+
+fun KaptContext.kaptError(vararg line: String): JCDiagnostic {
+    val text = line.joinToString(LINE_SEPARATOR)
     return JCDiagnostic.Factory.instance(context).errorJava9Aware(null, null, "proc.messager", text)
 }
 
+fun KaptContext.reportKaptError(vararg line: String) {
+    compiler.log.report(kaptError(*line))
+}
+
 private fun JCDiagnostic.Factory.errorJava9Aware(
         source: DiagnosticSource?,
         pos: JCDiagnostic.DiagnosticPosition?,

plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/Kapt3Extension.kt

@@ -72,11 +72,15 @@ class ClasspathBasedKapt3Extension(
         val useLightAnalysis: Boolean,
         correctErrorTypes: Boolean,
         mapDiagnosticLocations: Boolean,
+        strictMode: Boolean,
         pluginInitializedTime: Long,
         logger: MessageCollectorBackedKaptLogger,
         compilerConfiguration: CompilerConfiguration
-) : AbstractKapt3Extension(paths, options, javacOptions, annotationProcessorFqNames,
+) : AbstractKapt3Extension(
-                           aptMode, pluginInitializedTime, logger, correctErrorTypes, mapDiagnosticLocations, compilerConfiguration) {
+    paths, options, javacOptions, annotationProcessorFqNames,
+    aptMode, pluginInitializedTime, logger, correctErrorTypes, mapDiagnosticLocations, strictMode,
+    compilerConfiguration
+) {
     override val analyzePartially: Boolean
         get() = useLightAnalysis
 
@@ -110,6 +114,7 @@ abstract class AbstractKapt3Extension(
         val logger: MessageCollectorBackedKaptLogger,
         val correctErrorTypes: Boolean,
         val mapDiagnosticLocations: Boolean,
+        val strictMode: Boolean,
         val compilerConfiguration: CompilerConfiguration
 ) : PartialAnalysisHandlerExtension() {
     private var annotationProcessingComplete = false
@@ -254,7 +259,12 @@ abstract class AbstractKapt3Extension(
     }
 
     private fun generateKotlinSourceStubs(kaptContext: KaptContextForStubGeneration) {
-        val converter = ClassFileToSourceStubConverter(kaptContext, generateNonExistentClass = true, correctErrorTypes = correctErrorTypes)
+        val converter = ClassFileToSourceStubConverter(
+            kaptContext,
+            generateNonExistentClass = true,
+            correctErrorTypes = correctErrorTypes,
+            strictMode = strictMode
+        )
 
         val (stubGenerationTime, kaptStubs) = measureTimeMillis {
             converter.convert()

plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/Kapt3Plugin.kt

@@ -56,6 +56,7 @@ import org.jetbrains.kotlin.kapt3.Kapt3CommandLineProcessor.Companion.VERBOSE_MO
 import org.jetbrains.kotlin.kapt3.Kapt3ConfigurationKeys.ANNOTATION_PROCESSOR_CLASSPATH
 import org.jetbrains.kotlin.kapt3.Kapt3ConfigurationKeys.APT_OPTIONS
 import org.jetbrains.kotlin.kapt3.Kapt3ConfigurationKeys.JAVAC_CLI_OPTIONS
+import org.jetbrains.kotlin.kapt3.Kapt3CommandLineProcessor.Companion.STRICT_MODE_OPTION
 import org.jetbrains.kotlin.kapt3.base.Kapt
 import org.jetbrains.kotlin.kapt3.base.KaptPaths
 import org.jetbrains.kotlin.kapt3.base.log
@@ -117,6 +118,9 @@ object Kapt3ConfigurationKeys {
 
     val MAP_DIAGNOSTIC_LOCATIONS: CompilerConfigurationKey<String> =
         CompilerConfigurationKey.create<String>(MAP_DIAGNOSTIC_LOCATIONS_OPTION.description)
+
+    val STRICT_MODE: CompilerConfigurationKey<String> =
+        CompilerConfigurationKey.create<String>(STRICT_MODE_OPTION.description)
 }
 
 class Kapt3CommandLineProcessor : CommandLineProcessor {
@@ -176,6 +180,9 @@ class Kapt3CommandLineProcessor : CommandLineProcessor {
 
         val MAP_DIAGNOSTIC_LOCATIONS_OPTION: CliOption =
             CliOption("mapDiagnosticLocations", "true | false", "Map diagnostic reported on kapt stubs to original locations in Kotlin sources", required = false)
+
+        val STRICT_MODE_OPTION: CliOption =
+            CliOption("strict", "true | false", "Show errors on incompatibilities during stub generation", required = false)
     }
 
     override val pluginId: String = ANNOTATION_PROCESSING_COMPILER_PLUGIN_ID
@@ -184,7 +191,7 @@ class Kapt3CommandLineProcessor : CommandLineProcessor {
             listOf(SOURCE_OUTPUT_DIR_OPTION, ANNOTATION_PROCESSOR_CLASSPATH_OPTION, APT_OPTIONS_OPTION, JAVAC_CLI_OPTIONS_OPTION,
                    CLASS_OUTPUT_DIR_OPTION, VERBOSE_MODE_OPTION, STUBS_OUTPUT_DIR_OPTION, APT_ONLY_OPTION, APT_MODE_OPTION,
                    USE_LIGHT_ANALYSIS_OPTION, CORRECT_ERROR_TYPES_OPTION, ANNOTATION_PROCESSORS_OPTION, INCREMENTAL_DATA_OUTPUT_DIR_OPTION,
-                   CONFIGURATION, MAP_DIAGNOSTIC_LOCATIONS_OPTION, INFO_AS_WARNINGS_OPTION)
+                   CONFIGURATION, MAP_DIAGNOSTIC_LOCATIONS_OPTION, INFO_AS_WARNINGS_OPTION, STRICT_MODE_OPTION)
 
     override fun processOption(option: CliOption, value: String, configuration: CompilerConfiguration) {
         when (option) {
@@ -204,6 +211,7 @@ class Kapt3CommandLineProcessor : CommandLineProcessor {
             CORRECT_ERROR_TYPES_OPTION -> configuration.put(Kapt3ConfigurationKeys.CORRECT_ERROR_TYPES, value)
             MAP_DIAGNOSTIC_LOCATIONS_OPTION -> configuration.put(Kapt3ConfigurationKeys.MAP_DIAGNOSTIC_LOCATIONS, value)
             INFO_AS_WARNINGS_OPTION -> configuration.put(Kapt3ConfigurationKeys.INFO_AS_WARNINGS, value)
+            STRICT_MODE_OPTION -> configuration.put(Kapt3ConfigurationKeys.STRICT_MODE, value)
             CONFIGURATION -> configuration.applyOptionsFrom(decodePluginOptions(value), pluginOptions)
             else -> throw CliOptionProcessingException("Unknown option: ${option.name}")
         }
@@ -235,6 +243,7 @@ class Kapt3ComponentRegistrar : ComponentRegistrar {
 
         val isVerbose = configuration.get(Kapt3ConfigurationKeys.VERBOSE_MODE) == "true"
         val infoAsWarnings = configuration.get(Kapt3ConfigurationKeys.INFO_AS_WARNINGS) == "true"
+        val strictMode = configuration.get(Kapt3ConfigurationKeys.STRICT_MODE) == "true"
         val messageCollector = configuration.get(CLIConfigurationKeys.MESSAGE_COLLECTOR_KEY)
                                ?: PrintingMessageCollector(System.err, MessageRenderer.PLAIN_FULL_PATHS, isVerbose)
         val logger = MessageCollectorBackedKaptLogger(isVerbose, infoAsWarnings, messageCollector)
@@ -307,6 +316,7 @@ class Kapt3ComponentRegistrar : ComponentRegistrar {
             logger.info("Correct error types: $correctErrorTypes")
             logger.info("Map diagnostic locations: $mapDiagnosticLocations")
             logger.info("Info as warnings: $infoAsWarnings")
+            logger.info("Strict mode: $strictMode")
             paths.log(logger)
             logger.info("Annotation processors: " + annotationProcessors.joinToString())
             logger.info("Javac options: $apOptions")
@@ -315,7 +325,8 @@ class Kapt3ComponentRegistrar : ComponentRegistrar {
 
         val kapt3AnalysisCompletedHandlerExtension = ClasspathBasedKapt3Extension(
             paths, apOptions, javacCliOptions, annotationProcessors,
-            aptMode, useLightAnalysis, correctErrorTypes, mapDiagnosticLocations, System.currentTimeMillis(), logger, configuration
+            aptMode, useLightAnalysis, correctErrorTypes, mapDiagnosticLocations, strictMode,
+            System.currentTimeMillis(), logger, configuration
         )
 
         AnalysisHandlerExtension.registerExtension(project, kapt3AnalysisCompletedHandlerExtension)
@@ -331,7 +342,6 @@ class Kapt3ComponentRegistrar : ComponentRegistrar {
             if (platform != JvmPlatform) return
             container.useInstance(KaptAnonymousTypeTransformer())
         }
-
     }
 
     /* This extension simply disables both code analysis and code generation.
@@ -358,7 +368,6 @@ class Kapt3ComponentRegistrar : ComponentRegistrar {
             return AnalysisResult.Companion.success(bindingTrace.bindingContext, module, shouldGenerateCode = false)
         }
     }
-
 }
 
 enum class AptMode {

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

@@ -32,6 +32,7 @@ import org.jetbrains.kotlin.kapt3.javac.KaptTreeMaker
 import org.jetbrains.kotlin.kapt3.javac.KaptJavaFileObject
 import org.jetbrains.kotlin.kapt3.base.plus
 import org.jetbrains.kotlin.kapt3.base.javac.kaptError
+import org.jetbrains.kotlin.kapt3.base.javac.reportKaptError
 import org.jetbrains.kotlin.kapt3.base.mapJList
 import org.jetbrains.kotlin.kapt3.base.mapJListIndexed
 import org.jetbrains.kotlin.kapt3.base.pairedListToMap
@@ -62,7 +63,8 @@ import com.sun.tools.javac.util.List as JavacList
 class ClassFileToSourceStubConverter(
         val kaptContext: KaptContextForStubGeneration,
         val generateNonExistentClass: Boolean,
-        val correctErrorTypes: Boolean
+        val correctErrorTypes: Boolean,
+        val strictMode: Boolean
 ) {
     private companion object {
         private val VISIBILITY_MODIFIERS = (Opcodes.ACC_PUBLIC or Opcodes.ACC_PRIVATE or Opcodes.ACC_PROTECTED).toLong()
@@ -285,7 +287,7 @@ class ClassFileToSourceStubConverter(
             isTopLevel: Boolean
     ): JCClassDecl? {
         if (isSynthetic(clazz.access)) return null
-        if (checkIfShouldBeIgnored(Type.getObjectType(clazz.name))) return null
+        if (!checkIfValidTypeName(clazz, Type.getObjectType(clazz.name))) return null
 
         val descriptor = kaptContext.origins[clazz]?.descriptor ?: return null
         val isNested = (descriptor as? ClassDescriptor)?.isNested ?: false
@@ -393,21 +395,41 @@ class ClassFileToSourceStubConverter(
                 enumValues + fields + methods + nestedClasses).keepKdocComments(clazz)
     }
 
-    private tailrec fun checkIfShouldBeIgnored(type: Type): Boolean {
+    private tailrec fun checkIfValidTypeName(containingClass: ClassNode, type: Type): Boolean {
         if (type.sort == Type.ARRAY) {
-            return checkIfShouldBeIgnored(type.elementType)
+            return checkIfValidTypeName(containingClass, type.elementType)
         }
 
-        if (type.sort != Type.OBJECT) return false
+        if (type.sort != Type.OBJECT) return true
 
         val internalName = type.internalName
         // Ignore type names with Java keywords in it
         if (internalName.split('/', '.').any { it in JAVA_KEYWORDS }) {
-            return true
+            if (strictMode) {
+                kaptContext.reportKaptError(
+                    "Can't generate a stub for '${containingClass.className}'.",
+                    "Type name '${type.className}' contains a Java keyword."
+                )
+            }
+
+            return false
         }
 
-        val clazz = kaptContext.compiledClasses.firstOrNull { it.name == internalName } ?: return false
+        val clazz = kaptContext.compiledClasses.firstOrNull { it.name == internalName } ?: return true
-        return checkIfInnerClassNameConflictsWithOuter(clazz)
+
+        if (doesInnerClassNameConflictWithOuter(clazz)) {
+            if (strictMode) {
+                kaptContext.reportKaptError(
+                    "Can't generate a stub for '${containingClass.className}'.",
+                    "Its name '${clazz.simpleName}' is the same as one of the outer class names.",
+                    "Java forbids it. Please change one of the class names."
+                )
+            }
+
+            return false
+        }
+
+        return true
     }
 
     private fun findContainingClassNode(clazz: ClassNode): ClassNode? {
@@ -416,7 +438,7 @@ class ClassFileToSourceStubConverter(
     }
 
     // Java forbids outer and inner class names to be the same. Check if the names are different
-    private tailrec fun checkIfInnerClassNameConflictsWithOuter(
+    private tailrec fun doesInnerClassNameConflictWithOuter(
             clazz: ClassNode,
             outerClass: ClassNode? = findContainingClassNode(clazz)
     ): Boolean {
@@ -424,7 +446,7 @@ class ClassFileToSourceStubConverter(
         if (treeMaker.getSimpleName(clazz) == treeMaker.getSimpleName(outerClass)) return true
         // Try to find the containing class for outerClassNode (to check the whole tree recursively)
         val containingClassForOuterClass = findContainingClassNode(outerClass) ?: return false
-        return checkIfInnerClassNameConflictsWithOuter(clazz, containingClassForOuterClass)
+        return doesInnerClassNameConflictWithOuter(clazz, containingClassForOuterClass)
     }
 
     private fun getClassAccessFlags(clazz: ClassNode, descriptor: DeclarationDescriptor, isInner: Boolean, isNested: Boolean) = when {
@@ -469,7 +491,7 @@ class ClassFileToSourceStubConverter(
         if (!isValidIdentifier(name)) return null
 
         val type = Type.getType(field.desc)
-        if (checkIfShouldBeIgnored(type)) {
+        if (!checkIfValidTypeName(containingClass, type)) {
             return null
         }
 
@@ -537,7 +559,9 @@ class ClassFileToSourceStubConverter(
 
         val parametersInfo = method.getParametersInfo(containingClass)
 
-        if (checkIfShouldBeIgnored(asmReturnType) || parametersInfo.any { checkIfShouldBeIgnored(it.type) }) {
+        if (!checkIfValidTypeName(containingClass, asmReturnType)
+            || parametersInfo.any { !checkIfValidTypeName(containingClass, it.type) }
+        ) {
             return null
         }
 

plugins/kapt3/kapt3-compiler/src/org/jetbrains/kotlin/kapt3/util/asmUtils.kt

@@ -50,4 +50,10 @@ private val ANNOTATION_TYPE_DESCRIPTOR_FOR_JVMOVERLOADS_GENERATED_METHODS: Strin
 
 private fun AnnotationNode.isJvmOverloadsGenerated(): Boolean {
     return this.desc == ANNOTATION_TYPE_DESCRIPTOR_FOR_JVMOVERLOADS_GENERATED_METHODS
-}
+}
+
+val ClassNode.className: String
+    get() = Type.getObjectType(name).className
+
+val ClassNode.simpleName: String
+    get() = name.substringAfterLast('/').substringAfterLast('$')

plugins/kapt3/kapt3-compiler/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3IntegrationTest.kt

@@ -171,7 +171,8 @@ abstract class AbstractKotlinKapt3IntegrationTest : CodegenTestCase() {
             PathUtil.getJdkClassesRootsFromCurrentJre() + PathUtil.kotlinPathsForIdeaPlugin.stdlibPath,
             emptyList(), javaSourceRoots, outputDir, outputDir, stubsOutputDir, incrementalDataOutputDir
         ), options, emptyMap(), emptyList(), STUBS_AND_APT, System.currentTimeMillis(),
-        MessageCollectorBackedKaptLogger(true), correctErrorTypes = true, mapDiagnosticLocations = true,
+        MessageCollectorBackedKaptLogger(true),
+        correctErrorTypes = true, mapDiagnosticLocations = true, strictMode = true,
         compilerConfiguration = CompilerConfiguration.EMPTY
     ) {
         internal var savedStubs: String? = null

plugins/kapt3/kapt3-compiler/test/org/jetbrains/kotlin/kapt3/test/AbstractKotlinKapt3Test.kt

@@ -165,9 +165,10 @@ abstract class AbstractKotlinKapt3Test : CodegenTestCase() {
         kaptContext: KaptContextForStubGeneration,
         javaFiles: List<File>,
         generateNonExistentClass: Boolean,
-        correctErrorTypes: Boolean
+        correctErrorTypes: Boolean,
+        strictMode: Boolean
     ): JavacList<JCCompilationUnit> {
-        val converter = ClassFileToSourceStubConverter(kaptContext, generateNonExistentClass, correctErrorTypes)
+        val converter = ClassFileToSourceStubConverter(kaptContext, generateNonExistentClass, correctErrorTypes, strictMode)
 
         val kaptStubs = converter.convert()
         val convertedFiles = kaptStubs.map { stub ->
@@ -251,9 +252,10 @@ open class AbstractClassFileToSourceStubConverterTest : AbstractKotlinKapt3Test(
         val generateNonExistentClass = wholeFile.isOptionSet("NON_EXISTENT_CLASS")
         val correctErrorTypes = wholeFile.isOptionSet("CORRECT_ERROR_TYPES")
         val validate = !wholeFile.isOptionSet("NO_VALIDATION")
+        val strictMode = wholeFile.isOptionSet("STRICT_MODE")
         val expectedErrors = wholeFile.getRawOptionValues(EXPECTED_ERROR).sorted()
 
-        val convertedFiles = convert(kaptContext, javaFiles, generateNonExistentClass, correctErrorTypes)
+        val convertedFiles = convert(kaptContext, javaFiles, generateNonExistentClass, correctErrorTypes, strictMode)
 
         kaptContext.javaLog.interceptorData.files = convertedFiles.map { it.sourceFile to it }.toMap()
         if (validate) kaptContext.compiler.enterTrees(convertedFiles)
@@ -307,7 +309,10 @@ open class AbstractClassFileToSourceStubConverterTest : AbstractKotlinKapt3Test(
 
 abstract class AbstractKotlinKaptContextTest : AbstractKotlinKapt3Test() {
     override fun check(kaptContext: KaptContextForStubGeneration, javaFiles: List<File>, txtFile: File, wholeFile: File) {
-        val compilationUnits = convert(kaptContext, javaFiles, generateNonExistentClass = false, correctErrorTypes = true)
+        val compilationUnits = convert(
+            kaptContext, javaFiles,
+            generateNonExistentClass = false, correctErrorTypes = true, strictMode = false
+        )
 
         kaptContext.doAnnotationProcessing(
             emptyList(),

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

@@ -214,6 +214,11 @@ public class ClassFileToSourceStubConverterTestGenerated extends AbstractClassFi
         runTest("plugins/kapt3/kapt3-compiler/testData/converter/kt19750.kt");
     }
 
+    @TestMetadata("kt24272.kt")
+    public void testKt24272() throws Exception {
+        runTest("plugins/kapt3/kapt3-compiler/testData/converter/kt24272.kt");
+    }
+
     @TestMetadata("kt25071.kt")
     public void testKt25071() throws Exception {
         runTest("plugins/kapt3/kapt3-compiler/testData/converter/kt25071.kt");

plugins/kapt3/kapt3-compiler/testData/converter/kt24272.kt

@@ -0,0 +1,15 @@
+// STRICT_MODE
+
+// EXPECTED_ERROR(java:23:50) cannot find symbol
+// EXPECTED_ERROR(java:28:49) cannot find symbol
+// EXPECTED_ERROR(other:-1:-1) Can't generate a stub for 'Foo$Bar$Bar'.
+
+class Foo(private val string: String) {
+    val bar = Bar("bar")
+
+    class Bar(val string: String) {
+        class Bar(val nested: String)
+
+        val bars: ArrayList<Bar> = ArrayList()
+    }
+}

plugins/kapt3/kapt3-compiler/testData/converter/kt24272.txt

@@ -0,0 +1,41 @@
+import java.lang.System;
+
+@kotlin.Metadata()
+public final class Foo {
+    @org.jetbrains.annotations.NotNull()
+    private final Foo.Bar bar = null;
+    private final java.lang.String string = null;
+
+    @org.jetbrains.annotations.NotNull()
+    public final Foo.Bar getBar() {
+        return null;
+    }
+
+    public Foo(@org.jetbrains.annotations.NotNull()
+    java.lang.String string) {
+        super();
+    }
+
+    @kotlin.Metadata()
+    public static final class Bar {
+        @org.jetbrains.annotations.NotNull()
+        private final java.util.ArrayList<Foo.Bar.Bar> bars = null;
+        @org.jetbrains.annotations.NotNull()
+        private final java.lang.String string = null;
+
+        @org.jetbrains.annotations.NotNull()
+        public final java.util.ArrayList<Foo.Bar.Bar> getBars() {
+            return null;
+        }
+
+        @org.jetbrains.annotations.NotNull()
+        public final java.lang.String getString() {
+            return null;
+        }
+
+        public Bar(@org.jetbrains.annotations.NotNull()
+        java.lang.String string) {
+            super();
+        }
+    }
+}