diff --git a/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java b/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java index 8ad6ddfa7..122fbbb64 100644 --- a/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java +++ b/scip-gradle-plugin/src/main/java/org/scip_code/scip_java/gradle/ScipGradlePlugin.java @@ -9,7 +9,12 @@ import org.gradle.api.artifacts.Configuration; import org.gradle.api.initialization.Settings; import org.gradle.api.internal.GradleInternal; +import org.gradle.api.tasks.Input; +import org.gradle.api.tasks.SourceSet; +import org.gradle.api.tasks.SourceSetContainer; +import org.gradle.api.tasks.compile.CompileOptions; import org.gradle.api.tasks.compile.JavaCompile; +import org.gradle.process.CommandLineArgumentProvider; public class ScipGradlePlugin implements Plugin { @@ -61,35 +66,26 @@ private void configureProject(Project project) { task.getOptions().setIncremental(false); if (pluginAdded) { - // Groovy build scripts can append interpolated strings - // (GString) to compilerArgs, so despite the List - // signature the list may hold non-String elements. Iterate - // through a wildcard reference so elements are not implicitly - // cast to String, which throws ClassCastException (issue - // #801). - List args = task.getOptions().getCompilerArgs(); - - // It's important we don't add the plugin configuration more - // than once, as javac considers that an error. - boolean alreadyAdded = - args.stream() - .anyMatch(arg -> String.valueOf(arg).startsWith("-Xplugin:scip")); - if (!alreadyAdded) { - // The random timestamp ensures the sources are _always_ - // recompiled, so Gradle doesn't cache any state. - // TODO: before this plugin is published to Maven Central, we - // will need to revert this change - as it can have - // detrimental effect on people's builds. - task.getOptions() - .getCompilerArgs() - .add( - "-Xplugin:scip -targetroot:" - + targetRoot - + " -sourceroot:" - + sourceRoot - + " -randomtimestamp=" - + System.nanoTime()); - } + // An argument provider rather than compilerArgs, because builds + // may replace compilerArgs after we run (Spring's conventions + // call setCompilerArgs in afterEvaluate), which would silently + // drop the plugin. + // The random timestamp ensures the sources are _always_ + // recompiled, so Gradle doesn't cache any state. + // TODO: before this plugin is published to Maven Central, we + // will need to revert this change - as it can have + // detrimental effect on people's builds. + task.getOptions() + .getCompilerArgumentProviders() + .add( + new JavacPluginArgument( + task.getOptions(), + "-Xplugin:scip -targetroot:" + + targetRoot + + " -sourceroot:" + + sourceRoot + + " -randomtimestamp=" + + System.nanoTime())); } }); } @@ -127,16 +123,18 @@ private void configureProject(Project project) { /** * javac discovers {@code -Xplugin:} plugins from the annotation processor path when {@code * -processorpath} is set, and only falls back to the classpath when it isn't. Gradle populates - * {@code -processorpath} from the {@code annotationProcessor} (main) and {@code - * testAnnotationProcessor} (test) configurations, so whenever one of them declares a processor we - * must add the SCIP javac plugin to that same configuration or the corresponding compile task - * fails with "plug-in not found: scip". The two configurations are independent (test does not - * extend main), so each is checked separately. + * {@code -processorpath} from each source set's annotation processor configuration ({@code + * annotationProcessor} for main, {@code testAnnotationProcessor} for test), so whenever one of + * them declares a processor we must add the SCIP javac plugin to that same configuration or the + * corresponding compile task fails with "plug-in not found: scip". The configurations are + * independent (test does not extend main), so each is checked separately. */ private static boolean hasAnnotationProcessors(Project project, String configurationName) { try { Configuration config = project.getConfigurations().getByName(configurationName); - return config.isCanBeResolved() && !config.getDependencies().isEmpty(); + // All dependencies, including inherited ones: the Error Prone Gradle + // plugin makes annotationProcessor extend its own configuration. + return config.isCanBeResolved() && !config.getAllDependencies().isEmpty(); } catch (Exception exc) { return false; } @@ -144,13 +142,14 @@ private static boolean hasAnnotationProcessors(Project project, String configura private static boolean tryAddJavacPlugin(Project project, Object javacPluginDep) { try { - project.getDependencies().add("compileOnly", javacPluginDep); - project.getDependencies().add("testCompileOnly", javacPluginDep); - if (hasAnnotationProcessors(project, "annotationProcessor")) { - project.getDependencies().add("annotationProcessor", javacPluginDep); - } - if (hasAnnotationProcessors(project, "testAnnotationProcessor")) { - project.getDependencies().add("testAnnotationProcessor", javacPluginDep); + // Every source set, not just main and test: the plugin argument is added + // to every JavaCompile task. + for (SourceSet sourceSet : project.getExtensions().getByType(SourceSetContainer.class)) { + project.getDependencies().add(sourceSet.getCompileOnlyConfigurationName(), javacPluginDep); + String processors = sourceSet.getAnnotationProcessorConfigurationName(); + if (hasAnnotationProcessors(project, processors)) { + project.getDependencies().add(processors, javacPluginDep); + } } return true; } catch (Exception exc) { @@ -215,4 +214,35 @@ private static Object requiredExtra(Map extraProperties, String } return value; } + + /** Passes the SCIP javac plugin argument, unless the build already passes one itself. */ + private static class JavacPluginArgument implements CommandLineArgumentProvider { + private final CompileOptions options; + private final String argument; + + JavacPluginArgument(CompileOptions options, String argument) { + this.options = options; + this.argument = argument; + } + + @Input + public String getArgument() { + return argument; + } + + @Override + public Iterable asArguments() { + // Groovy build scripts can append interpolated strings (GString) to + // compilerArgs, so despite the List signature the list may hold + // non-String elements. Iterate through a wildcard reference so elements + // are not implicitly cast to String, which throws ClassCastException + // (issue #801). + List args = options.getCompilerArgs(); + + // javac rejects the plugin configuration if it's given more than once. + boolean alreadyAdded = + args.stream().anyMatch(arg -> String.valueOf(arg).startsWith("-Xplugin:scip")); + return alreadyAdded ? List.of() : List.of(argument); + } + } } diff --git a/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt b/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt index 900287442..2256cbc01 100644 --- a/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt +++ b/scip-java/src/main/kotlin/org/scip_code/scip_java/buildtools/GradleBuildTool.kt @@ -49,7 +49,7 @@ This means our SCIP compiler plugin was not attached to one or more JavaCompile 2. Another Gradle plugin is replacing the compiler arguments we add (rather than appending). Verify with: ./gradlew compileJava --info | grep -- '-Xplugin:scip' If '-Xplugin:scip' is missing from the printed javac command, another plugin - is overwriting JavaCompile.options.compilerArgs. + is overwriting JavaCompile.options.compilerArgumentProviders. """ ) return 1 diff --git a/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt b/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt index e2b466e83..0b7cb2a65 100644 --- a/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt +++ b/scip-java/src/test/kotlin/tests/GradleBuildToolTest.kt @@ -68,6 +68,15 @@ class GradleBuildToolTest : BuildToolHarness() { expectedScipFiles = 2, ) ) + // Regression test: processors inherited from another configuration (as + // with the Error Prone Gradle plugin) also make Gradle pass -processorpath. + add( + checkGradleBuild( + "inherited-annotation-path", + "gradle/inherited-annotation-path", + expectedScipFiles = 2, + ) + ) add( checkGradleBuild("build-with-Werror", "gradle/build-with-Werror", expectedScipFiles = 2) ) @@ -82,6 +91,20 @@ class GradleBuildToolTest : BuildToolHarness() { expectedScipFiles = 1, ) ) + // Regression test: a build that replaces compilerArgs after the plugin + // ran must still load the SCIP javac plugin. + add( + checkGradleBuild( + "replaced-compiler-args", + "gradle/replaced-compiler-args", + expectedScipFiles = 1, + ) + ) + // Regression test: source sets other than main and test need the SCIP + // javac plugin on their classpath too. + add( + checkGradleBuild("custom-source-set", "gradle/custom-source-set", expectedScipFiles = 2) + ) add( checkGradleBuild( "publishing", diff --git a/scip-java/src/test/resources/fixtures/gradle/custom-source-set/build.gradle b/scip-java/src/test/resources/fixtures/gradle/custom-source-set/build.gradle new file mode 100644 index 000000000..7d66d24dc --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/custom-source-set/build.gradle @@ -0,0 +1,14 @@ +plugins { + id 'java' +} +repositories { + mavenCentral() +} +// compileJava depends on compileExtraJava, which also needs the SCIP javac +// plugin on its classpath. +sourceSets { + extra +} +dependencies { + implementation sourceSets.extra.output +} diff --git a/scip-java/src/test/resources/fixtures/gradle/custom-source-set/src/extra/java/example/Greeting.java b/scip-java/src/test/resources/fixtures/gradle/custom-source-set/src/extra/java/example/Greeting.java new file mode 100644 index 000000000..4a82cb71e --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/custom-source-set/src/extra/java/example/Greeting.java @@ -0,0 +1,7 @@ +package example; + +public class Greeting { + public static String of(String name) { + return "Hello, " + name + "!"; + } +} diff --git a/scip-java/src/test/resources/fixtures/gradle/custom-source-set/src/main/java/example/Example.java b/scip-java/src/test/resources/fixtures/gradle/custom-source-set/src/main/java/example/Example.java new file mode 100644 index 000000000..01d3c6a0d --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/custom-source-set/src/main/java/example/Example.java @@ -0,0 +1,7 @@ +package example; + +public class Example { + public String greet(String name) { + return Greeting.of(name); + } +} diff --git a/scip-java/src/test/resources/fixtures/gradle/inherited-annotation-path/build.gradle b/scip-java/src/test/resources/fixtures/gradle/inherited-annotation-path/build.gradle new file mode 100644 index 000000000..5f71e584d --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/inherited-annotation-path/build.gradle @@ -0,0 +1,16 @@ +plugins { + id 'java' +} +repositories { + mavenCentral() +} +// The processor is only inherited by annotationProcessor, like the Error Prone +// Gradle plugin's 'errorprone' configuration. +configurations { + processors + annotationProcessor.extendsFrom(processors) +} +dependencies { + compileOnly 'org.immutables:value:2.9.2' + processors 'org.immutables:value:2.9.2' +} diff --git a/scip-java/src/test/resources/fixtures/gradle/inherited-annotation-path/src/main/java/WorkflowOptions.java b/scip-java/src/test/resources/fixtures/gradle/inherited-annotation-path/src/main/java/WorkflowOptions.java new file mode 100644 index 000000000..b57925691 --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/inherited-annotation-path/src/main/java/WorkflowOptions.java @@ -0,0 +1,7 @@ +package test; +import org.immutables.value.Value; +import java.util.Optional; +@Value.Immutable +public abstract class WorkflowOptions { + public abstract Optional getWorkflowIdReusePolicy(); +} diff --git a/scip-java/src/test/resources/fixtures/gradle/replaced-compiler-args/build.gradle b/scip-java/src/test/resources/fixtures/gradle/replaced-compiler-args/build.gradle new file mode 100644 index 000000000..1140b66ac --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/replaced-compiler-args/build.gradle @@ -0,0 +1,13 @@ +plugins { + id 'java' +} +repositories { + mavenCentral() +} +// Replaces compilerArgs after the SCIP plugin has configured the task, like +// Spring Framework's JavaConventions. +afterEvaluate { + tasks.withType(JavaCompile).configureEach { + options.compilerArgs = ["-Xlint:serial"] + } +} diff --git a/scip-java/src/test/resources/fixtures/gradle/replaced-compiler-args/src/main/java/example/Example.java b/scip-java/src/test/resources/fixtures/gradle/replaced-compiler-args/src/main/java/example/Example.java new file mode 100644 index 000000000..c7fa8bb6b --- /dev/null +++ b/scip-java/src/test/resources/fixtures/gradle/replaced-compiler-args/src/main/java/example/Example.java @@ -0,0 +1,7 @@ +package example; + +public class Example { + public String greet(String name) { + return "Hello, " + name + "!"; + } +}