From 5927ee44626e7dbfebbcb9596d089b5eeda0aba2 Mon Sep 17 00:00:00 2001 From: Sam Snyder Date: Thu, 7 Sep 2023 20:38:12 -0700 Subject: [PATCH] Don't double-parse files that appear in multiple, overlapping source sets --- plugin/build.gradle.kts | 5 +- .../org/openrewrite/gradle/RewritePlugin.java | 34 +++++++- .../gradle/isolated/DefaultProjectParser.java | 31 ++++++-- .../org/openrewrite/gradle/RewriteRunTest.kt | 77 +++++++++++++++++++ 4 files changed, 134 insertions(+), 13 deletions(-) diff --git a/plugin/build.gradle.kts b/plugin/build.gradle.kts index 194c0a184..003ddf497 100644 --- a/plugin/build.gradle.kts +++ b/plugin/build.gradle.kts @@ -67,7 +67,6 @@ configurations.all { } } - java { toolchain { languageVersion.set(JavaLanguageVersion.of(17)) @@ -109,6 +108,7 @@ dependencies { "rewriteDependencies"("org.openrewrite.gradle.tooling:model:$latest") "rewriteDependencies"("org.openrewrite:rewrite-maven") // Newer versions of checkstyle are compiled with a newer version of Java than is supported with gradle 4.x + @Suppress("VulnerableLibrariesLocal", "RedundantSuppression") "rewriteDependencies"("com.puppycrawl.tools:checkstyle:9.3") "rewriteDependencies"("com.fasterxml.jackson.module:jackson-module-kotlin:latest.release") @@ -126,6 +126,7 @@ dependencies { compileOnly("org.openrewrite:rewrite-xml") compileOnly("org.openrewrite:rewrite-yaml") compileOnly("org.openrewrite:rewrite-polyglot:$latest") + @Suppress("VulnerableLibrariesLocal", "RedundantSuppression") compileOnly("com.puppycrawl.tools:checkstyle:9.3") compileOnly("org.jetbrains.kotlin:kotlin-gradle-plugin:latest.release") @@ -155,7 +156,7 @@ tasks.withType() { } val gVP = tasks.register("generateVersionsProperties") { - val outputFile = file("$buildDir/rewrite/versions.properties") + val outputFile = file("${project.layout.buildDirectory.get().asFile}/rewrite/versions.properties") description = "Creates a versions.properties in $outputFile" group = "Build" diff --git a/plugin/src/main/java/org/openrewrite/gradle/RewritePlugin.java b/plugin/src/main/java/org/openrewrite/gradle/RewritePlugin.java index 8949fe13f..1549446dd 100644 --- a/plugin/src/main/java/org/openrewrite/gradle/RewritePlugin.java +++ b/plugin/src/main/java/org/openrewrite/gradle/RewritePlugin.java @@ -22,6 +22,11 @@ import org.gradle.api.plugins.quality.CheckstyleExtension; import org.gradle.api.plugins.quality.CheckstylePlugin; +import java.io.File; +import java.util.Comparator; +import java.util.HashSet; +import java.util.Set; + /** * When applied to the root project of a multi-project build, applies to all subprojects. * When applied to the root project the "rewrite" configuration and "rewrite" DSL created in the root project apply to @@ -37,7 +42,7 @@ public void apply(Project project) { if (!isRootProject && project.getRootProject().getPluginManager().hasPlugin("org.openrewrite.rewrite")) { return; } - if(project.getRootProject().getPluginManager().hasPlugin("io.moderne.rewrite")) { + if (project.getRootProject().getPluginManager().hasPlugin("io.moderne.rewrite")) { // Moderne plugin provides superset of rewrite plugin functionality, no need to apply both return; } @@ -67,7 +72,7 @@ public void apply(Project project) { .setResolveDependenciesTask(resolveRewriteDependenciesTask); rewriteDiscover.dependsOn(rewriteConf); - if(isRootProject) { + if (isRootProject) { project.allprojects(subproject -> configureProject(subproject, extension, rewriteDryRun, rewriteRun)); } else { configureProject(project, extension, rewriteDryRun, rewriteRun); @@ -78,14 +83,14 @@ private static void configureProject(Project project, RewriteExtension extension // DomainObjectCollection.all() accepts a function to be applied to both existing and subsequently added members of the collection // Do not replace all() with any form of collection iteration which does not share this important property project.getPlugins().all(plugin -> { - if(plugin instanceof CheckstylePlugin) { + if (plugin instanceof CheckstylePlugin) { // A multi-project build could hypothetically have different checkstyle configuration per-project // In practice all projects tend to have the same configuration CheckstyleExtension checkstyleExtension = project.getExtensions().getByType(CheckstyleExtension.class); extension.setCheckstyleConfigProvider(checkstyleExtension::getConfigFile); extension.setCheckstylePropertiesProvider(checkstyleExtension::getConfigProperties); } - if(!(plugin instanceof JavaBasePlugin)) { + if (!(plugin instanceof JavaBasePlugin)) { return; } @@ -99,6 +104,27 @@ private static void configureProject(Project project, RewriteExtension extension rewriteRun.dependsOn(compileTask); rewriteDryRun.dependsOn(compileTask); }); + + // Detect SourceSets which overlap other sourceSets and disable the compilation task of the overlapping + // source set. Some plugins will create source sets not intended to be compiled for their own purposes. + Set sourceDirs = new HashSet<>(); + project.afterEvaluate(unused -> javaConvention.getSourceSets().stream() + .sorted(Comparator.comparingInt(sourceSet -> { + if ("main".equals(sourceSet.getName())) { + return 0; + } else if ("test".equals(sourceSet.getName())) { + return 1; + } else { + return 2; + } + })).forEach(sourceSet -> { + for (File file : sourceSet.getAllSource().getSourceDirectories().getFiles()) { + if (!sourceDirs.add(file.getAbsolutePath())) { + Task compileTask = project.getTasks().getByPath(sourceSet.getCompileJavaTaskName()); + compileTask.setEnabled(false); + } + } + })); }); } } diff --git a/plugin/src/main/java/org/openrewrite/gradle/isolated/DefaultProjectParser.java b/plugin/src/main/java/org/openrewrite/gradle/isolated/DefaultProjectParser.java index 16d19d843..fd1ee6f1d 100644 --- a/plugin/src/main/java/org/openrewrite/gradle/isolated/DefaultProjectParser.java +++ b/plugin/src/main/java/org/openrewrite/gradle/isolated/DefaultProjectParser.java @@ -85,7 +85,6 @@ import java.util.function.Consumer; import java.util.function.Supplier; import java.util.function.UnaryOperator; -import java.util.stream.Collectors; import java.util.stream.Stream; import static java.util.Collections.*; @@ -119,7 +118,7 @@ public DefaultProjectParser(Project project, RewriteExtension extension) { OperatingSystemProvenance.current(), new BuildTool(randomId(), BuildTool.Type.Gradle, project.getGradle().getGradleVersion())) .filter(Objects::nonNull) - .collect(Collectors.toList()); + .collect(toList()); } /** @@ -185,7 +184,7 @@ public List getActiveStyles() { } public List getAvailableStyles() { - return environment().listStyles().stream().map(NamedStyles::getName).collect(Collectors.toList()); + return environment().listStyles().stream().map(NamedStyles::getName).collect(toList()); } public void discoverRecipes(boolean interactive, ServiceRegistry serviceRegistry) { @@ -623,18 +622,27 @@ public Stream parse(Project subproject, Set alreadyParsed, Exe List styles = getStyles(); @SuppressWarnings("deprecation") JavaPluginConvention javaConvention = subproject.getConvention().findPlugin(JavaPluginConvention.class); - Set sourceSets; + List sourceSets; List projectProvenance; if (javaConvention == null) { projectProvenance = sharedProvenance; - sourceSets = Collections.emptySet(); + sourceSets = emptyList(); } else { projectProvenance = new ArrayList<>(sharedProvenance); projectProvenance.add(new JavaProject(randomId(), subproject.getName(), new JavaProject.Publication(subproject.getGroup().toString(), subproject.getName(), subproject.getVersion().toString()))); - sourceSets = javaConvention.getSourceSets(); + sourceSets = javaConvention.getSourceSets().stream() + .sorted(Comparator.comparingInt(sourceSet -> { + if ("main".equals(sourceSet.getName())) { + return 0; + } else if ("test".equals(sourceSet.getName())) { + return 1; + } else { + return 2; + } + })).collect(toList()); } if (subproject.getPlugins().hasPlugin("org.jetbrains.kotlin.gradle.dsl.KotlinMultiplatformExtension") || @@ -643,7 +651,16 @@ public Stream parse(Project subproject, Set alreadyParsed, Exe sourceFileStream = sourceFileStream.concat(parseMultiplatformKotlinProject(subproject, exclusions, alreadyParsed, ctx)); } + Set sourceDirs = new HashSet<>(); + sourceSetLoop: for (SourceSet sourceSet : sourceSets) { + // Detect SourceSets which overlap other sourceSets and disable the compilation task of the overlapping + // source set. Skip parsing directories already covered by another source set. + for (File file : sourceSet.getAllSource().getSourceDirectories().getFiles()) { + if (!sourceDirs.add(file.getAbsolutePath())) { + continue sourceSetLoop; + } + } Stream sourceSetSourceFiles = Stream.of(); int sourceSetSize = 0; @@ -950,7 +967,7 @@ private static Collection mergeExclusions(Project project, Path baseDir, private Collection pathMatchers(Path basePath, Collection pathExpressions) { return pathExpressions.stream() .map(o -> basePath.getFileSystem().getPathMatcher("glob:" + o)) - .collect(Collectors.toList()); + .collect(toList()); } private SourceFileStream parseMultiplatformKotlinProject(Project subproject, Collection exclusions, Set alreadyParsed, ExecutionContext ctx) { diff --git a/plugin/src/test/kotlin/org/openrewrite/gradle/RewriteRunTest.kt b/plugin/src/test/kotlin/org/openrewrite/gradle/RewriteRunTest.kt index a86891f7d..41cd37646 100644 --- a/plugin/src/test/kotlin/org/openrewrite/gradle/RewriteRunTest.kt +++ b/plugin/src/test/kotlin/org/openrewrite/gradle/RewriteRunTest.kt @@ -972,4 +972,81 @@ class RewriteRunTest : RewritePluginTest { .`as`("Applicability test should have prevented this file from being altered") .isEqualTo("jonathan") } + + @Test + fun overlappingSourceSet(@TempDir buildRoot: File) { + gradleProject(buildRoot) { + rewriteYaml(""" + type: specs.openrewrite.org/v1beta/recipe + name: org.openrewrite.test.FindSneaky + displayName: Find lombok SneakyThrows annotation + description: Find lombok SneakyThrows annotation + recipeList: + - org.openrewrite.java.search.FindTypes: + fullyQualifiedTypeName: lombok.SneakyThrows + """) + buildGradle(""" + plugins { + id("java") + id("org.openrewrite.rewrite") + } + + repositories { + mavenLocal() + mavenCentral() + maven { + url = uri("https://oss.sonatype.org/content/repositories/snapshots") + } + } + + sourceSets { + create("overlapping") { + java.srcDir("src/test/java") + } + } + + rewrite { + activeRecipe("org.openrewrite.test.FindSneaky") + } + + dependencies { + testImplementation("org.projectlombok:lombok:latest.release") + } + """) + sourceSet("test") { + java(""" + package com.foo; + + import lombok.SneakyThrows; + + public class ATest { + + @SneakyThrows + public void fail() { + } + } + """) + } + } + + val result = runGradle(buildRoot, "rewriteRun") + val rewriteRunResult = result.task(":rewriteRun")!! + assertThat(rewriteRunResult.outcome).isEqualTo(TaskOutcome.SUCCESS) + val javaFile = buildRoot.resolve("src/test/java/com/foo/ATest.java") + assertThat(javaFile.readText()) + //language=java + .isEqualTo(""" + package com.foo; + + import lombok.SneakyThrows; + + public class ATest { + + @/*~~>*/SneakyThrows + public void fail() { + } + } + """.trimIndent() + ) + } }