From 5a19640de4ded1d1bf280c14611ad16fcab5fd09 Mon Sep 17 00:00:00 2001 From: Jiaxiang Chen Date: Tue, 6 Sep 2022 23:56:53 -0700 Subject: [PATCH] fix behavior for KSP error case with compilation. respect returnOkOnError when withCompilation is set true. --- .../ksp/KotlinSymbolProcessingExtension.kt | 3 + .../devtools/ksp/gradle/KspSubplugin.kt | 2 +- integration-tests/build.gradle.kts | 1 + .../devtools/ksp/test/KSPCmdLineOptionsIT.kt | 85 +++++++++++++++++++ .../resources/cmd-options/build.gradle.kts | 8 ++ .../resources/cmd-options/gradle.properties | 0 .../cmd-options/processors/build.gradle.kts | 23 +++++ .../src/main/kotlin/TestProcessor.kt | 37 ++++++++ ...ols.ksp.processing.SymbolProcessorProvider | 1 + .../resources/cmd-options/settings.gradle.kts | 19 +++++ .../cmd-options/workload/build.gradle.kts | 19 +++++ .../workload/src/main/kotlin/com/example/A.kt | 4 + 12 files changed, 201 insertions(+), 1 deletion(-) create mode 100644 integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt create mode 100644 integration-tests/src/test/resources/cmd-options/build.gradle.kts create mode 100644 integration-tests/src/test/resources/cmd-options/gradle.properties create mode 100644 integration-tests/src/test/resources/cmd-options/processors/build.gradle.kts create mode 100644 integration-tests/src/test/resources/cmd-options/processors/src/main/kotlin/TestProcessor.kt create mode 100644 integration-tests/src/test/resources/cmd-options/processors/src/main/resources/META-INF/services/com.google.devtools.ksp.processing.SymbolProcessorProvider create mode 100644 integration-tests/src/test/resources/cmd-options/settings.gradle.kts create mode 100644 integration-tests/src/test/resources/cmd-options/workload/build.gradle.kts create mode 100644 integration-tests/src/test/resources/cmd-options/workload/src/main/kotlin/com/example/A.kt diff --git a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt index 6787cb230f..f912513c7d 100644 --- a/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt +++ b/compiler-plugin/src/main/kotlin/com/google/devtools/ksp/KotlinSymbolProcessingExtension.kt @@ -129,6 +129,9 @@ abstract class AbstractKotlinSymbolProcessingExtension( if (finished) { if (!options.withCompilation) throw IllegalStateException("KSP is re-entered unexpectedly.") + if (!options.returnOkOnError && logger.hasError()) { + return AnalysisResult.compilationError(BindingContext.EMPTY) + } return null } diff --git a/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt b/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt index f266a42b27..144baa435b 100644 --- a/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt +++ b/gradle-plugin/src/main/kotlin/com/google/devtools/ksp/gradle/KspSubplugin.kt @@ -158,7 +158,7 @@ class KspGradleSubplugin @Inject internal constructor(private val registry: Tool options += SubpluginOption("projectBaseDir", project.project.projectDir.canonicalPath) options += SubpluginOption("allWarningsAsErrors", allWarningsAsErrors.toString()) options += FilesSubpluginOption("apclasspath", classpath.toList()) - // Turn this on by default to work KT-30172 around. It is off by default in the ccompiler plugin. + // Turn this on by default to work KT-30172 around. It is off by default in the compiler plugin. options += SubpluginOption( "returnOkOnError", project.findProperty("ksp.return.ok.on.error")?.toString() ?: "true" diff --git a/integration-tests/build.gradle.kts b/integration-tests/build.gradle.kts index 1d666749c5..305592c4d5 100644 --- a/integration-tests/build.gradle.kts +++ b/integration-tests/build.gradle.kts @@ -9,6 +9,7 @@ plugins { dependencies { testImplementation("junit:junit:$junitVersion") testImplementation(gradleTestKit()) + testImplementation("org.jetbrains.kotlin:kotlin-compiler:$kotlinBaseVersion") } tasks.named("test") { diff --git a/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt new file mode 100644 index 0000000000..80f8cce60d --- /dev/null +++ b/integration-tests/src/test/kotlin/com/google/devtools/ksp/test/KSPCmdLineOptionsIT.kt @@ -0,0 +1,85 @@ +package com.google.devtools.ksp.test + +import org.gradle.testkit.runner.GradleRunner +import org.jetbrains.kotlin.cli.common.ExitCode +import org.jetbrains.kotlin.cli.jvm.K2JVMCompiler +import org.junit.Assert +import org.junit.Rule +import org.junit.Test +import java.io.ByteArrayOutputStream +import java.io.File +import java.io.PrintStream +import java.net.URLClassLoader + +data class CompileResult(val exitCode: ExitCode, val output: String) + +class KSPCmdLineOptionsIT { + @Rule + @JvmField + val project: TemporaryTestProject = TemporaryTestProject("cmd-options") + + private fun runCmdCompiler(pluginOptions: List): CompileResult { + val gradleRunner = GradleRunner.create().withProjectDir(project.root) + gradleRunner.withArguments("clean", ":processors:build").build() + val processorJar = File(project.root, "processors/build/libs/processors-1.0-SNAPSHOT.jar") + val classLoader = URLClassLoader(arrayOf(processorJar.toURI().toURL()), javaClass.classLoader) + val compiler = classLoader.loadClass(K2JVMCompiler::class.java.name).newInstance() as K2JVMCompiler + val repoPath = "../build/repos/test/com/google/devtools/ksp/" + val kspPluginId = "com.google.devtools.ksp.symbol-processing" + val kspPluginJar = File("$repoPath/symbol-processing-cmdline/2.0.255-SNAPSHOT").listFiles()!!.filter { + it.name.matches(Regex(".*-\\d.jar")) + }.maxByOrNull { it.lastModified() }!! + val kspApiJar = File("$repoPath/symbol-processing-api/2.0.255-SNAPSHOT").listFiles()!!.filter { + it.name.matches(Regex(".*-\\d.jar")) + }.maxByOrNull { it.lastModified() }!! + val compilerArgs = mutableListOf( + "-no-stdlib", + "-Xplugin=${kspPluginJar.absolutePath}", + "-Xplugin=${kspApiJar.absolutePath}", + "-P", "plugin:$kspPluginId:apclasspath=${processorJar.absolutePath}", + "-P", "plugin:$kspPluginId:projectBaseDir=${project.root}/build", + "-P", "plugin:$kspPluginId:classOutputDir=${project.root}/build", + "-P", "plugin:$kspPluginId:javaOutputDir=${project.root}/build/out", + "-P", "plugin:$kspPluginId:kotlinOutputDir=${project.root}/build/out", + "-P", "plugin:$kspPluginId:resourceOutputDir=${project.root}/build/out", + "-P", "plugin:$kspPluginId:kspOutputDir=${project.root}/build/out", + "-P", "plugin:$kspPluginId:cachesDir=${project.root}/build/out", + "-P", "plugin:$kspPluginId:incremental=false", + "-d", "${project.root}/build/out" + ) + pluginOptions.forEach { + compilerArgs.add("-P") + compilerArgs.add("plugin:$kspPluginId:$it") + } + compilerArgs.add(File(project.root, "workload/src/main/kotlin/com/example/A.kt").absolutePath) + val outStream = ByteArrayOutputStream() + val exitCode = compiler.exec(PrintStream(outStream), *compilerArgs.toTypedArray()) + return CompileResult(exitCode, outStream.toString()) + } + + @Test + fun testWithCompilationOnError() { + val result = runCmdCompiler(listOf("apoption=error=true", "withCompilation=true")) + val errors = result.output.lines().filter { it.startsWith("error: [ksp]") } + val exitCode = result.exitCode + Assert.assertTrue(exitCode == ExitCode.COMPILATION_ERROR) + Assert.assertTrue( + errors.any { + it.startsWith("error: [ksp] java.lang.IllegalStateException: Error on request") + } + ) + } + + @Test + fun testWithCompilationOnErrorOk() { + val result = runCmdCompiler(listOf("apoption=error=true", "returnOkOnError=true", "withCompilation=true")) + val errors = result.output.lines().filter { it.startsWith("error: [ksp]") } + val exitCode = result.exitCode + Assert.assertTrue(exitCode == ExitCode.OK) + Assert.assertTrue( + errors.any { + it.startsWith("error: [ksp] java.lang.IllegalStateException: Error on request") + } + ) + } +} diff --git a/integration-tests/src/test/resources/cmd-options/build.gradle.kts b/integration-tests/src/test/resources/cmd-options/build.gradle.kts new file mode 100644 index 0000000000..c5737a2e0d --- /dev/null +++ b/integration-tests/src/test/resources/cmd-options/build.gradle.kts @@ -0,0 +1,8 @@ +plugins { + kotlin("jvm") +} + +repositories { + mavenCentral() + maven("https://maven.pkg.jetbrains.space/kotlin/p/kotlin/bootstrap/") +} diff --git a/integration-tests/src/test/resources/cmd-options/gradle.properties b/integration-tests/src/test/resources/cmd-options/gradle.properties new file mode 100644 index 0000000000..e69de29bb2 diff --git a/integration-tests/src/test/resources/cmd-options/processors/build.gradle.kts b/integration-tests/src/test/resources/cmd-options/processors/build.gradle.kts new file mode 100644 index 0000000000..999cb80ae5 --- /dev/null +++ b/integration-tests/src/test/resources/cmd-options/processors/build.gradle.kts @@ -0,0 +1,23 @@ +val kspVersion: String by project +val testRepo: String by project + +plugins { + kotlin("jvm") +} + +group = "com.example" +version = "1.0-SNAPSHOT" + +repositories { + maven(testRepo) + mavenCentral() + maven("https://maven.pkg.jetbrains.space/kotlin/p/kotlin/bootstrap/") +} + +dependencies { + implementation("com.google.devtools.ksp:symbol-processing-api:$kspVersion") +} + +sourceSets.main { + java.srcDirs("src/main/kotlin") +} diff --git a/integration-tests/src/test/resources/cmd-options/processors/src/main/kotlin/TestProcessor.kt b/integration-tests/src/test/resources/cmd-options/processors/src/main/kotlin/TestProcessor.kt new file mode 100644 index 0000000000..c38d2298ea --- /dev/null +++ b/integration-tests/src/test/resources/cmd-options/processors/src/main/kotlin/TestProcessor.kt @@ -0,0 +1,37 @@ +import com.google.devtools.ksp.processing.* +import com.google.devtools.ksp.symbol.* + +class TestProcessor : SymbolProcessor { + lateinit var codeGenerator: CodeGenerator + lateinit var logger: KSPLogger + lateinit var options: Map + var rounds = 0 + + fun init( + options: Map, + kotlinVersion: KotlinVersion, + codeGenerator: CodeGenerator, + logger: KSPLogger + ) { + this.logger = logger + this.options = options + this.codeGenerator = codeGenerator + } + + override fun process(resolver: Resolver): List { + if (options.containsKey("error")) { + throw IllegalStateException("Error on request") + } + return emptyList() + } +} + +class TestProcessorProvider : SymbolProcessorProvider { + override fun create( + env: SymbolProcessorEnvironment + ): SymbolProcessor { + return TestProcessor().apply { + init(env.options, env.kotlinVersion, env.codeGenerator, env.logger) + } + } +} diff --git a/integration-tests/src/test/resources/cmd-options/processors/src/main/resources/META-INF/services/com.google.devtools.ksp.processing.SymbolProcessorProvider b/integration-tests/src/test/resources/cmd-options/processors/src/main/resources/META-INF/services/com.google.devtools.ksp.processing.SymbolProcessorProvider new file mode 100644 index 0000000000..e6522f1952 --- /dev/null +++ b/integration-tests/src/test/resources/cmd-options/processors/src/main/resources/META-INF/services/com.google.devtools.ksp.processing.SymbolProcessorProvider @@ -0,0 +1 @@ +TestProcessorProvider \ No newline at end of file diff --git a/integration-tests/src/test/resources/cmd-options/settings.gradle.kts b/integration-tests/src/test/resources/cmd-options/settings.gradle.kts new file mode 100644 index 0000000000..79202ef612 --- /dev/null +++ b/integration-tests/src/test/resources/cmd-options/settings.gradle.kts @@ -0,0 +1,19 @@ +pluginManagement { + val kspVersion: String by settings + val kotlinVersion: String by settings + val testRepo: String by settings + plugins { + id("com.google.devtools.ksp") version kspVersion + kotlin("jvm") version kotlinVersion + } + repositories { + maven(testRepo) + gradlePluginPortal() + maven("https://maven.pkg.jetbrains.space/kotlin/p/kotlin/bootstrap/") + } +} + +rootProject.name = "cmd-options" + +include(":workload") +include(":processors") diff --git a/integration-tests/src/test/resources/cmd-options/workload/build.gradle.kts b/integration-tests/src/test/resources/cmd-options/workload/build.gradle.kts new file mode 100644 index 0000000000..a79cec868b --- /dev/null +++ b/integration-tests/src/test/resources/cmd-options/workload/build.gradle.kts @@ -0,0 +1,19 @@ +val testRepo: String by project + +plugins { + id("com.google.devtools.ksp") + kotlin("jvm") +} + +version = "1.0-SNAPSHOT" + +repositories { + maven(testRepo) + mavenCentral() + maven("https://maven.pkg.jetbrains.space/kotlin/p/kotlin/bootstrap/") +} + +dependencies { + implementation(kotlin("stdlib")) + ksp(project(":processors")) +} diff --git a/integration-tests/src/test/resources/cmd-options/workload/src/main/kotlin/com/example/A.kt b/integration-tests/src/test/resources/cmd-options/workload/src/main/kotlin/com/example/A.kt new file mode 100644 index 0000000000..ebf0688c55 --- /dev/null +++ b/integration-tests/src/test/resources/cmd-options/workload/src/main/kotlin/com/example/A.kt @@ -0,0 +1,4 @@ +package com.example + +fun main() { +}