Skip to content

Commit 4f49e0c

Browse files
DidierLoiseaugithub-actions[bot]timtebeek
authored
Correctly override parent property to set Java version (#549)
* Added test case for #523 Java version not detected in parent * Partially revert "Only bring down properties from the parent that are referenced explicitly" This partially reverts commit 12c8f37 * fix #523 restore behaviour of overriding properties from parent, but smartly - only override numeric values - if any property is detected, don't add maven.compiler.release * Formatting from github-actions Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> * Apply formatter to minimize diff * Fix failing UpgradeToJava17Test Need to update the model because we are working with the resolved properties now, not the requested ones. * Inline the optionals * Simplify visitTag * Inline pathToLocalParent variable and get propertyValue once * Remove unused imports --------- Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com> Co-authored-by: Tim te Beek <[email protected]>
1 parent 07542d2 commit 4f49e0c

File tree

2 files changed

+191
-96
lines changed

2 files changed

+191
-96
lines changed

‎src/main/java/org/openrewrite/java/migrate/maven/UpdateMavenProjectPropertyJavaVersion.java‎

Lines changed: 33 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -23,11 +23,12 @@
2323
import org.openrewrite.TreeVisitor;
2424
import org.openrewrite.maven.AddProperty;
2525
import org.openrewrite.maven.MavenIsoVisitor;
26-
import org.openrewrite.maven.tree.MavenResolutionResult;
2726
import org.openrewrite.xml.XPathMatcher;
2827
import org.openrewrite.xml.tree.Xml;
2928

30-
import java.util.*;
29+
import java.util.Arrays;
30+
import java.util.List;
31+
import java.util.Map;
3132
import java.util.stream.Collectors;
3233

3334
@Value
@@ -73,13 +74,12 @@ public String getDescription() {
7374
" * `maven.compiler.target`\n" +
7475
" * `maven.compiler.release`\n" +
7576
" * `release.version`\n\n" +
76-
"If none of these properties are in use and the maven compiler plugin is not otherwise configured adds the `maven.compiler.release` property.";
77+
"If none of these properties are in use and the maven compiler plugin is not otherwise configured, adds the `maven.compiler.release` property.";
7778
}
7879

7980
@Override
8081
public TreeVisitor<?, ExecutionContext> getVisitor() {
8182
return new MavenIsoVisitor<ExecutionContext>() {
82-
final Set<String> propertiesExplicitlyReferenced = new HashSet<>();
8383
boolean compilerPluginConfiguredExplicitly;
8484

8585
@Override
@@ -88,93 +88,57 @@ public Xml.Document visitDocument(Xml.Document document, ExecutionContext ctx) {
8888
Xml.Document d = super.visitDocument(document, ctx);
8989

9090
// Return early if the parent appears to be within the current repository, as properties defined there will be updated
91-
Optional<String> pathToLocalParent = d.getRoot().getChild("parent")
91+
if (d.getRoot().getChild("parent")
9292
.flatMap(parent -> parent.getChild("relativePath"))
93-
.flatMap(Xml.Tag::getValue);
94-
if (pathToLocalParent.isPresent()) {
93+
.flatMap(Xml.Tag::getValue)
94+
.isPresent()) {
9595
return d;
9696
}
9797

9898
// Otherwise override remote parent's properties locally
99-
MavenResolutionResult mrr = getResolutionResult();
100-
Map<String, String> currentProperties = mrr.getPom().getRequested().getProperties();
99+
Map<String, String> currentProperties = getResolutionResult().getPom().getProperties();
100+
boolean foundProperty = false;
101101
for (String property : JAVA_VERSION_PROPERTIES) {
102-
if (currentProperties.containsKey(property) || !propertiesExplicitlyReferenced.contains(property)) {
103-
continue;
102+
String propertyValue = currentProperties.get(property);
103+
if (propertyValue != null) {
104+
foundProperty = true;
105+
try {
106+
if (Float.parseFloat(propertyValue) < version) {
107+
d = (Xml.Document) new AddProperty(property, String.valueOf(version), null, false)
108+
.getVisitor()
109+
.visitNonNull(d, ctx);
110+
maybeUpdateModel();
111+
}
112+
} catch (NumberFormatException ex) {
113+
// either an expression or something else, don't touch
114+
}
104115
}
105-
d = (Xml.Document) new AddProperty(property, String.valueOf(version), null, false)
106-
.getVisitor()
107-
.visitNonNull(d, ctx);
108116
}
109117

110118
// When none of the relevant properties are explicitly configured Maven defaults to Java 8
111119
// The release option was added in 9
112120
// If no properties have yet been updated then set release explicitly
113-
if (version >= 9 &&
114-
!compilerPluginConfiguredExplicitly &&
115-
currentProperties.keySet()
116-
.stream()
117-
.noneMatch(JAVA_VERSION_PROPERTIES::contains)) {
121+
if (!foundProperty && version >= 9 && !compilerPluginConfiguredExplicitly) {
118122
d = (Xml.Document) new AddProperty("maven.compiler.release", String.valueOf(version), null, false)
119123
.getVisitor()
120124
.visitNonNull(d, ctx);
121-
HashMap<String, String> updatedProps = new HashMap<>(currentProperties);
122-
updatedProps.put("maven.compiler.release", version.toString());
123-
mrr = mrr.withPom(mrr.getPom().withRequested(mrr.getPom().getRequested().withProperties(updatedProps)));
124-
125-
d = d.withMarkers(d.getMarkers().setByType(mrr));
125+
maybeUpdateModel();
126126
}
127+
127128
return d;
128129
}
129130

130131
@Override
131132
public Xml.Tag visitTag(Xml.Tag tag, ExecutionContext ctx) {
132133
Xml.Tag t = super.visitTag(tag, ctx);
133-
Optional<String> s = t.getValue()
134-
.map(it -> it.replace("${", "").replace("}", "").trim())
135-
.filter(JAVA_VERSION_PROPERTIES::contains);
136-
if (s.isPresent()) {
137-
propertiesExplicitlyReferenced.add(s.get());
138-
} else if (JAVA_VERSION_XPATH_MATCHERS.stream().anyMatch(matcher -> matcher.matches(getCursor()))) {
139-
Optional<Float> maybeVersion = t.getValue().flatMap(
140-
value -> {
141-
try {
142-
return Optional.of(Float.parseFloat(value));
143-
} catch (NumberFormatException e) {
144-
return Optional.empty();
145-
}
146-
}
147-
);
148-
149-
if (!maybeVersion.isPresent()) {
150-
return t;
151-
}
152-
float currentVersion = maybeVersion.get();
153-
if (currentVersion >= version) {
154-
return t;
155-
}
156-
return t.withValue(String.valueOf(version));
157-
} else if (PLUGINS_MATCHER.matches(getCursor())) {
158-
Optional<Xml.Tag> maybeCompilerPlugin = t.getChildren().stream()
159-
.filter(plugin ->
160-
"plugin".equals(plugin.getName()) &&
161-
"org.apache.maven.plugins".equals(plugin.getChildValue("groupId").orElse("org.apache.maven.plugins")) &&
162-
"maven-compiler-plugin".equals(plugin.getChildValue("artifactId").orElse(null)))
163-
.findAny();
164-
Optional<Xml.Tag> maybeCompilerPluginConfig = maybeCompilerPlugin
165-
.flatMap(it -> it.getChild("configuration"));
166-
if (!maybeCompilerPluginConfig.isPresent()) {
167-
return t;
168-
}
169-
Xml.Tag compilerPluginConfig = maybeCompilerPluginConfig.get();
170-
Optional<String> source = compilerPluginConfig.getChildValue("source");
171-
Optional<String> target = compilerPluginConfig.getChildValue("target");
172-
Optional<String> release = compilerPluginConfig.getChildValue("release");
173-
if (source.isPresent()
174-
|| target.isPresent()
175-
|| release.isPresent()) {
176-
compilerPluginConfiguredExplicitly = true;
177-
}
134+
if (isPluginTag("org.apache.maven.plugins", "maven-compiler-plugin")) {
135+
t.getChild("configuration").ifPresent(compilerPluginConfig -> {
136+
if (compilerPluginConfig.getChildValue("source").isPresent() ||
137+
compilerPluginConfig.getChildValue("target").isPresent() ||
138+
compilerPluginConfig.getChildValue("release").isPresent()) {
139+
compilerPluginConfiguredExplicitly = true;
140+
}
141+
});
178142
}
179143
return t;
180144
}

‎src/test/java/org/openrewrite/java/migrate/maven/UpdateMavenProjectPropertyJavaVersionTest.java‎

Lines changed: 158 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -73,12 +73,57 @@ void basic() {
7373
<release.version>17</release.version>
7474
</properties>
7575
</project>
76+
"""
77+
)
78+
);
79+
}
80+
81+
@Test
82+
void basicWithVariables() {
83+
rewriteRun(
84+
//language=xml
85+
pomXml(
86+
"""
87+
<project>
88+
<groupId>com.example</groupId>
89+
<artifactId>foo</artifactId>
90+
<version>1.0.0</version>
91+
<modelVersion>4.0</modelVersion>
92+
<properties>
93+
<java.version>${release.version}</java.version>
94+
<jdk.version>11</jdk.version>
95+
<javaVersion>${release.version}</javaVersion>
96+
<jdkVersion>${jdk.version}</jdkVersion>
97+
<maven.compiler.source>${maven.compiler.release}</maven.compiler.source>
98+
<maven.compiler.target>${maven.compiler.release}</maven.compiler.target>
99+
<maven.compiler.release>11</maven.compiler.release>
100+
<release.version>11</release.version>
101+
</properties>
102+
</project>
103+
""",
104+
"""
105+
<project>
106+
<groupId>com.example</groupId>
107+
<artifactId>foo</artifactId>
108+
<version>1.0.0</version>
109+
<modelVersion>4.0</modelVersion>
110+
<properties>
111+
<java.version>${release.version}</java.version>
112+
<jdk.version>17</jdk.version>
113+
<javaVersion>${release.version}</javaVersion>
114+
<jdkVersion>${jdk.version}</jdkVersion>
115+
<maven.compiler.source>${maven.compiler.release}</maven.compiler.source>
116+
<maven.compiler.target>${maven.compiler.release}</maven.compiler.target>
117+
<maven.compiler.release>17</maven.compiler.release>
118+
<release.version>17</release.version>
119+
</properties>
120+
</project>
76121
""")
77122
);
78123
}
79124

80125
@Test
81-
void bringsDownExplicitlyUsedPropertyFromRemoteParent() {
126+
void overrideRemoteParent() {
82127
rewriteRun(
83128
//language=xml
84129
pomXml(
@@ -103,7 +148,8 @@ void bringsDownExplicitlyUsedPropertyFromRemoteParent() {
103148
SourceSpec::skip),
104149
mavenProject("example-child",
105150
//language=xml
106-
pomXml("""
151+
pomXml(
152+
"""
107153
<project>
108154
<parent>
109155
<groupId>com.example</groupId>
@@ -116,18 +162,6 @@ void bringsDownExplicitlyUsedPropertyFromRemoteParent() {
116162
<artifactId>example-child</artifactId>
117163
<version>1.0.0</version>
118164
<modelVersion>4.0</modelVersion>
119-
<build>
120-
<plugins>
121-
<plugin>
122-
<groupId>org.apache.maven.plugins</groupId>
123-
<artifactId>maven-compiler-plugin</artifactId>
124-
<version>3.8.0</version>
125-
<configuration>
126-
<release>${java.version}</release>
127-
</configuration>
128-
</plugin>
129-
</plugins>
130-
</build>
131165
</project>
132166
""",
133167
"""
@@ -145,21 +179,49 @@ void bringsDownExplicitlyUsedPropertyFromRemoteParent() {
145179
<modelVersion>4.0</modelVersion>
146180
<properties>
147181
<java.version>17</java.version>
182+
<javaVersion>17</javaVersion>
183+
<jdk.version>17</jdk.version>
184+
<jdkVersion>17</jdkVersion>
185+
<maven.compiler.release>17</maven.compiler.release>
186+
<maven.compiler.source>17</maven.compiler.source>
187+
<maven.compiler.target>17</maven.compiler.target>
188+
<release.version>17</release.version>
148189
</properties>
149-
<build>
150-
<plugins>
151-
<plugin>
152-
<groupId>org.apache.maven.plugins</groupId>
153-
<artifactId>maven-compiler-plugin</artifactId>
154-
<version>3.8.0</version>
155-
<configuration>
156-
<release>${java.version}</release>
157-
</configuration>
158-
</plugin>
159-
</plugins>
160-
</build>
161190
</project>
162-
""")
191+
"""
192+
)
193+
)
194+
);
195+
}
196+
197+
@Test
198+
void doNothingForExplicitPluginConfiguration() {
199+
// Use UseMavenCompilerPluginReleaseConfiguration for this case
200+
rewriteRun(
201+
//language=xml
202+
pomXml(
203+
"""
204+
<project>
205+
<groupId>com.example</groupId>
206+
<artifactId>example-child</artifactId>
207+
<version>1.0.0</version>
208+
<modelVersion>4.0</modelVersion>
209+
<build>
210+
<plugins>
211+
<plugin>
212+
<groupId>org.apache.maven.plugins</groupId>
213+
<artifactId>maven-compiler-plugin</artifactId>
214+
<version>3.8.0</version>
215+
<configuration>
216+
<release>11</release>
217+
<source>11</source>
218+
<target>11</target>
219+
</configuration>
220+
</plugin>
221+
</plugins>
222+
</build>
223+
</project>
224+
"""
163225
)
164226
);
165227
}
@@ -192,4 +254,73 @@ void addReleaseIfNoOtherChangeIsMade() {
192254
)
193255
);
194256
}
257+
258+
@Test
259+
void springBoot3ParentToJava17() {
260+
// Spring Boot Starter Parent already enforces Java 17
261+
rewriteRun(
262+
pomXml(
263+
//language=xml
264+
"""
265+
<project>
266+
<modelVersion>4.0.0</modelVersion>
267+
<parent>
268+
<groupId>org.springframework.boot</groupId>
269+
<artifactId>spring-boot-starter-parent</artifactId>
270+
<version>3.3.3</version>
271+
<relativePath/> <!-- lookup parent from repository -->
272+
</parent>
273+
274+
<groupId>com.mycompany.app</groupId>
275+
<artifactId>my-app</artifactId>
276+
<version>1</version>
277+
</project>
278+
"""
279+
)
280+
);
281+
}
282+
283+
@Test
284+
void springBoot3ParentToJava21() {
285+
rewriteRun(
286+
spec -> spec.recipe(new UpdateMavenProjectPropertyJavaVersion(21)),
287+
pomXml(
288+
//language=xml
289+
"""
290+
<project>
291+
<modelVersion>4.0.0</modelVersion>
292+
<parent>
293+
<groupId>org.springframework.boot</groupId>
294+
<artifactId>spring-boot-starter-parent</artifactId>
295+
<version>3.3.3</version>
296+
<relativePath/> <!-- lookup parent from repository -->
297+
</parent>
298+
299+
<groupId>com.mycompany.app</groupId>
300+
<artifactId>my-app</artifactId>
301+
<version>1</version>
302+
</project>
303+
""",
304+
//language=xml
305+
"""
306+
<project>
307+
<modelVersion>4.0.0</modelVersion>
308+
<parent>
309+
<groupId>org.springframework.boot</groupId>
310+
<artifactId>spring-boot-starter-parent</artifactId>
311+
<version>3.3.3</version>
312+
<relativePath/> <!-- lookup parent from repository -->
313+
</parent>
314+
315+
<groupId>com.mycompany.app</groupId>
316+
<artifactId>my-app</artifactId>
317+
<version>1</version>
318+
<properties>
319+
<java.version>21</java.version>
320+
</properties>
321+
</project>
322+
"""
323+
)
324+
);
325+
}
195326
}

0 commit comments

Comments
 (0)