Skip to content

Commit

Permalink
Don't double-parse files that appear in multiple, overlapping source …
Browse files Browse the repository at this point in the history 
…sets
  • Loading branch information
@sambsnyd
sambsnyd committed Sep 8, 2023
1 parent 1114a98 commit 5927ee4
Show file tree
Hide file tree
Showing 4 changed files with 134 additions and 13 deletions.
5 changes: 3 additions & 2 deletions plugin/build.gradle.kts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,6 @@ configurations.all {
}
}


java {
toolchain {
languageVersion.set(JavaLanguageVersion.of(17))
Expand Down Expand Up @@ -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")

Expand All @@ -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")

Expand Down Expand Up @@ -155,7 +156,7 @@ tasks.withType<Test>() {
}

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"

Expand Down
34 changes: 30 additions & 4 deletions plugin/src/main/java/org/openrewrite/gradle/RewritePlugin.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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;
}
Expand Down Expand Up @@ -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);
Expand All @@ -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;
}

Expand All @@ -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<String> 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);
}
}
}));
});
}
}
31 changes: 24 additions & 7 deletions plugin/src/main/java/org/openrewrite/gradle/isolated/DefaultProjectParser.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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());
}

/**
Expand Down Expand Up @@ -185,7 +184,7 @@ public List<String> getActiveStyles() {
}

public List<String> 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) {
Expand Down Expand Up @@ -623,18 +622,27 @@ public Stream<SourceFile> parse(Project subproject, Set<Path> alreadyParsed, Exe
List<NamedStyles> styles = getStyles();
@SuppressWarnings("deprecation")
JavaPluginConvention javaConvention = subproject.getConvention().findPlugin(JavaPluginConvention.class);
Set<SourceSet> sourceSets;
List<SourceSet> sourceSets;
List<Marker> 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") ||
Expand All @@ -643,7 +651,16 @@ public Stream<SourceFile> parse(Project subproject, Set<Path> alreadyParsed, Exe
sourceFileStream = sourceFileStream.concat(parseMultiplatformKotlinProject(subproject, exclusions, alreadyParsed, ctx));
}

Set<String> 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<SourceFile> sourceSetSourceFiles = Stream.of();
int sourceSetSize = 0;

Expand Down Expand Up @@ -950,7 +967,7 @@ private static Collection<String> mergeExclusions(Project project, Path baseDir,
private Collection<PathMatcher> pathMatchers(Path basePath, Collection<String> pathExpressions) {
return pathExpressions.stream()
.map(o -> basePath.getFileSystem().getPathMatcher("glob:" + o))
.collect(Collectors.toList());
.collect(toList());
}

private SourceFileStream parseMultiplatformKotlinProject(Project subproject, Collection<PathMatcher> exclusions, Set<Path> alreadyParsed, ExecutionContext ctx) {
Expand Down
77 changes: 77 additions & 0 deletions plugin/src/test/kotlin/org/openrewrite/gradle/RewriteRunTest.kt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
)
}
}

0 comments on commit 5927ee4

Please sign in to comment.