Skip to content

Commit

Permalink
fix #523 restore behaviour of overriding properties from parent, but …
Browse files Browse the repository at this point in the history 
…smartly

- only override numeric values
- if any property is detected, don't add maven.compiler.release
  • Loading branch information
@DidierLoiseau
DidierLoiseau committed Sep 18, 2024
1 parent b446840 commit 2369654
Show file tree
Hide file tree
Showing 2 changed files with 160 additions and 57 deletions.
72 changes: 28 additions & 44 deletions src/main/java/org/openrewrite/java/migrate/maven/UpdateMavenProjectPropertyJavaVersion.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,15 +65,15 @@ public String getDisplayName() {
public String getDescription() {
//language=markdown
return "The Java version is determined by several project properties, including:\n\n" +
" * `java.version`\n" +
" * `jdk.version`\n" +
" * `javaVersion`\n" +
" * `jdkVersion`\n" +
" * `maven.compiler.source`\n" +
" * `maven.compiler.target`\n" +
" * `maven.compiler.release`\n" +
" * `release.version`\n\n" +
"If none of these properties are in use and the maven compiler plugin is not otherwise configured adds the `maven.compiler.release` property.";
" * `java.version`\n" +
" * `jdk.version`\n" +
" * `javaVersion`\n" +
" * `jdkVersion`\n" +
" * `maven.compiler.source`\n" +
" * `maven.compiler.target`\n" +
" * `maven.compiler.release`\n" +
" * `release.version`\n\n" +
"If none of these properties are in use and the maven compiler plugin is not otherwise configured, adds the `maven.compiler.release` property.";
}

@Override
Expand All @@ -96,28 +96,31 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {

// Otherwise override remote parent's properties locally
MavenResolutionResult mrr = getResolutionResult();
Map<String, String> currentProperties = mrr.getPom().getRequested().getProperties();
Map<String, String> currentProperties = mrr.getPom().getProperties();
boolean foundProperty = false;
for (String property : JAVA_VERSION_PROPERTIES) {
if (currentProperties.containsKey(property)) {
continue;
foundProperty = true;
try {
if (Float.parseFloat(currentProperties.get(property)) < version) {
d = (Xml.Document) new AddProperty(property, String.valueOf(version), null, false)
.getVisitor()
.visitNonNull(d, ctx);
}
} catch (NumberFormatException ex) {
// either an expression or something else, don't touch
}
}
d = (Xml.Document) new AddProperty(property, String.valueOf(version), null, false)
.getVisitor()
.visitNonNull(d, ctx);
}

// When none of the relevant properties are explicitly configured Maven defaults to Java 8
// The release option was added in 9
// If no properties have yet been updated then set release explicitly
if (version >= 9 &&
!compilerPluginConfiguredExplicitly &&
currentProperties.keySet()
.stream()
.noneMatch(JAVA_VERSION_PROPERTIES::contains)) {
if (!foundProperty && version >= 9 && !compilerPluginConfiguredExplicitly) {
d = (Xml.Document) new AddProperty("maven.compiler.release", String.valueOf(version), null, false)
.getVisitor()
.visitNonNull(d, ctx);
HashMap<String, String> updatedProps = new HashMap<>(currentProperties);
HashMap<String, String> updatedProps = new HashMap<>(mrr.getPom().getRequested().getProperties());
updatedProps.put("maven.compiler.release", version.toString());
mrr = mrr.withPom(mrr.getPom().withRequested(mrr.getPom().getRequested().withProperties(updatedProps)));

Expand All @@ -130,31 +133,12 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
Xml.Tag t = super.visitTag(tag, ctx);

if (JAVA_VERSION_XPATH_MATCHERS.stream().anyMatch(matcher -> matcher.matches(getCursor()))) {
Optional<Float> maybeVersion = t.getValue().flatMap(
value -> {
try {
return Optional.of(Float.parseFloat(value));
} catch (NumberFormatException e) {
return Optional.empty();
}
}
);

if (!maybeVersion.isPresent()) {
return t;
}
float currentVersion = maybeVersion.get();
if (currentVersion >= version) {
return t;
}
return t.withValue(String.valueOf(version));
} else if (PLUGINS_MATCHER.matches(getCursor())) {
if (PLUGINS_MATCHER.matches(getCursor())) {
Optional<Xml.Tag> maybeCompilerPlugin = t.getChildren().stream()
.filter(plugin ->
"plugin".equals(plugin.getName()) &&
"org.apache.maven.plugins".equals(plugin.getChildValue("groupId").orElse("org.apache.maven.plugins")) &&
"maven-compiler-plugin".equals(plugin.getChildValue("artifactId").orElse(null)))
"org.apache.maven.plugins".equals(plugin.getChildValue("groupId").orElse("org.apache.maven.plugins")) &&
"maven-compiler-plugin".equals(plugin.getChildValue("artifactId").orElse(null)))
.findAny();
Optional<Xml.Tag> maybeCompilerPluginConfig = maybeCompilerPlugin
.flatMap(it -> it.getChild("configuration"));
Expand All @@ -166,8 +150,8 @@ public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
Optional<String> target = compilerPluginConfig.getChildValue("target");
Optional<String> release = compilerPluginConfig.getChildValue("release");
if (source.isPresent()
|| target.isPresent()
|| release.isPresent()) {
|| target.isPresent()
|| release.isPresent()) {
compilerPluginConfiguredExplicitly = true;
}
}
Expand Down
145 changes: 132 additions & 13 deletions ...st/java/org/openrewrite/java/migrate/maven/UpdateMavenProjectPropertyJavaVersionTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,50 @@ void basic() {
);
}

@Test
void basicWithVariables() {
rewriteRun(
//language=xml
pomXml(
"""
<project>
<groupId>com.example</groupId>
<artifactId>foo</artifactId>
<version>1.0.0</version>
<modelVersion>4.0</modelVersion>
<properties>
<java.version>${release.version}</java.version>
<jdk.version>11</jdk.version>
<javaVersion>${release.version}</javaVersion>
<jdkVersion>${jdk.version}</jdkVersion>
<maven.compiler.source>${maven.compiler.release}</maven.compiler.source>
<maven.compiler.target>${maven.compiler.release}</maven.compiler.target>
<maven.compiler.release>11</maven.compiler.release>
<release.version>11</release.version>
</properties>
</project>
""",
"""
<project>
<groupId>com.example</groupId>
<artifactId>foo</artifactId>
<version>1.0.0</version>
<modelVersion>4.0</modelVersion>
<properties>
<java.version>${release.version}</java.version>
<jdk.version>17</jdk.version>
<javaVersion>${release.version}</javaVersion>
<jdkVersion>${jdk.version}</jdkVersion>
<maven.compiler.source>${maven.compiler.release}</maven.compiler.source>
<maven.compiler.target>${maven.compiler.release}</maven.compiler.target>
<maven.compiler.release>17</maven.compiler.release>
<release.version>17</release.version>
</properties>
</project>
""")
);
}

@Test
void overrideRemoteParent() {
rewriteRun(
Expand Down Expand Up @@ -147,6 +191,36 @@ void overrideRemoteParent() {
);
}

@Test
void doNothingForExplicitPluginConfiguration() {
// Use UseMavenCompilerPluginReleaseConfiguration for this case
rewriteRun(
//language=xml
pomXml("""
<project>
<groupId>com.example</groupId>
<artifactId>example-child</artifactId>
<version>1.0.0</version>
<modelVersion>4.0</modelVersion>
<build>
<plugins>
<plugin>
<groupId>org.apache.maven.plugins</groupId>
<artifactId>maven-compiler-plugin</artifactId>
<version>3.8.0</version>
<configuration>
<release>11</release>
<source>11</source>
<target>11</target>
</configuration>
</plugin>
</plugins>
</build>
</project>
""")
);
}

@Test
@Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/514")
void addReleaseIfNoOtherChangeIsMade() {
Expand Down Expand Up @@ -177,24 +251,69 @@ void addReleaseIfNoOtherChangeIsMade() {
}

@Test
void mavenUpgradeShouldUseDeclaredVersionInParent() {
void springBoot3ParentToJava17() {
// Spring Boot Starter Parent already enforces Java 17
rewriteRun(
pomXml(
//language=xml
"""
<project>
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.3.3</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
<version>1</version>
</project>
"""
)
);
}

@Test
void springBoot3ParentToJava21() {
rewriteRun(
spec -> spec.recipe(new UpdateMavenProjectPropertyJavaVersion(21)),
pomXml(
//language=xml
"""
<project>
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.3.3</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<project>
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.3.3</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
<version>1</version>
</project>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
<version>1</version>
</project>
""",
//language=xml
"""
<project>
<modelVersion>4.0.0</modelVersion>
<parent>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-starter-parent</artifactId>
<version>3.3.3</version>
<relativePath/> <!-- lookup parent from repository -->
</parent>
<groupId>com.mycompany.app</groupId>
<artifactId>my-app</artifactId>
<version>1</version>
<properties>
<java.version>21</java.version>
</properties>
</project>
"""
)
);
Expand Down

0 comments on commit 2369654

Please sign in to comment.