Update missing maven.compiler.* properties
From org.springframework.boot:spring-boot-starter-parent:2.7.3 to org.springframework.boot:spring-boot-starter-parent:3.1.x the properties maven.compiler.source and maven.compiler.target are missing.
Inspecting Spring boot 3.1 projects, the new properties are java.version and maven.compiler.release
What's changed?
The recipe spring-boot-31.yml will replace the affected properties by the new properties used in Spring boot 3.1
# Since JDK 17 `release` is used and defined in Spring Boot parent project
- org.openrewrite.maven.RenamePropertyKey:
oldKey: maven.compiler.target
newKey: maven.compiler.release
# Remove from the project <properties> section
- org.openrewrite.maven.RemoveProperty:
propertyName: maven.compiler.source
# Then, replace any usage with `release` coming from parent
- org.openrewrite.maven.RenamePropertyKey:
oldKey: maven.compiler.source
newKey: maven.compiler.release
What's your motivation?
I want to upgrade some Spring Boot applications whose parent is org.springframework.boot:spring-boot-starter-parent:2.7.3
Anyone you would like to review specifically?
@timtebeek
Checklist
- [ ] I've added unit tests to cover both positive and negative cases
- [ ] I've read and applied the recipe conventions and best practices
- [ ] I've used the IntelliJ IDEA auto-formatter on affected files
I tested the SNAPSHOT locally in my project and it works, however, the test in this draft doesn't run. Please, could you give some feedback before trying to fix the test. Thanks!
I'm not sure that the specific replacements you're doing are generically accurate.
This page has a nice summary of source, target, and release: https://www.baeldung.com/java-source-target-options
...and IIRC java.version is (by convention) used to set the value of those properties in the spring boot parent.
Maybe an alternative would be a recipe to simplify these properties? Eg, if source and target have the same value, replace with release (though this might belong in rewrite-migrate-java), or if both java.version and release are declared (And project is using spring parent), deduplicate to just set Java.version?
Hi @nmck257 , thanks for your suggestion. I've reviewed the spring boot code for both versions, and I think we should replace maven.compiler.source and maven.compiler.target usages with maven.compiler.release. Spring boot 3.1 defines maven.compiler.release=17.
For 2.7.x, the 1.8 version was used.
https://github.com/spring-projects/spring-boot/blob/5490e73922b37a7f0bdde43eb318cb1038b45d60/spring-boot-project/spring-boot-starters/spring-boot-starter-parent/build.gradle#L24C30-L25C43
And from 3.1.x, the 17 version is used.
https://github.com/spring-projects/spring-boot/blob/3.1.x/spring-boot-project/spring-boot-starters/spring-boot-starter-parent/build.gradle#L24
Removing any maven.compiler.source and maven.compiler.target references.
On the other hand, the project I want to upgrade has the following pom.xml configuration.
<plugin>
<groupId>org.springframework.boot</groupId>
<artifactId>spring-boot-maven-plugin</artifactId>
<executions>
<execution>
<!-- Spring Boot Actuator displays build-related information
if a META-INF/build-info.properties file is present -->
<goals>
<goal>build-info</goal>
</goals>
<configuration>
<additionalProperties>
<encoding.source>${project.build.sourceEncoding}</encoding.source>
<encoding.reporting>${project.reporting.outputEncoding}</encoding.reporting>
<java.source>${maven.compiler.source}</java.source>
<java.target>${maven.compiler.target}</java.target>
</additionalProperties>
</configuration>
</execution>
</executions>
</plugin>
If we upgrade to 3.1, replacing just the values could make sense but maybe we want to use only java.release property there... wdyt?
@ilozano2
Consolidating maven.compiler.source and maven.compiler.target down to maven.compiler.release at Spring Boot 3.1 feels reasonable; I don't that there's any likely scenario where using the old properties individually is desirable for a Spring Boot app.
A step further, it may be more idiomatic to just replace all those properties with java.version, which any project using spring-boot-starter-parent as a direct (or transitive) parent would have defined as the value for the relevant maven.compiler... props.
For the snippet you gave, I believe that the spring-boot-maven-plugin additionalProperties is only used to provide metadata to the actuator at runtime, and is not relevant to compilation: https://docs.spring.io/spring-boot/docs/current/maven-plugin/reference/htmlsingle/#build-info
...in which case, for that section, you might want to replace references to the old compiler properties in the values, but should probably not change any keys defined, as they're arbitrary, and changing them might break some contract between the app we're refactoring and anything else which calls that app's actuator.
@nmck257 I think this should cover all the cases using the built-in recipes
# Since JDK 17 `release` is used and defined in Spring Boot parent project
- org.openrewrite.maven.RenamePropertyKey:
oldKey: maven.compiler.target
newKey: maven.compiler.release
# Remove from the project <properties> section
- org.openrewrite.maven.RemoveProperty:
propertyName: maven.compiler.source
# Then, replace any usage with `release` coming from parent
- org.openrewrite.maven.RenamePropertyKey:
oldKey: maven.compiler.source
newKey: maven.compiler.release
Spring Boot 3.1 is using release, so we don't need source either target. Replacing target with release is ok. On the other hand, we could drop source, but there is an edge case that the developer uses in some place (like in the snippet of my previous comment); in that case we need to find a replacement and release should be ok by default.
That seems plausible -- I think a question then is if those changes belong in rewrite-migrate-java, instead, and should be applied for any project on jdk 8 and above.
The spring parent's main contribution in this space is defining a more-intuitively-named java.version property and wiring that up to the relevant maven.compiler.* properties.
About java.version, that's right, it is a more general way to define source&target in Spring. If we keep this change on spring-rewrite, we could use java.version instead of maven.compiler.release.
Otherwise, we should use the maven.compiler.* options only for Maven projects because the java.version property is Spring-specific.
About your first point, I guess moving this change to rewrite-migrate-java makes sense.
I tried to fix the spring boot parent issue, but the general problem is that we are upgrading Java and we are using the release property.
Do you think we should close this PR and start a new PR for a recipe UpgradeJavaSourceTargetRecipe in rewrite-migrate-java, which upgrades the maven.compiler.source and maven.compiler.target properties to maven.compiler.release in case the project is Maven?
I believe https://docs.openrewrite.org/recipes/gradle/updatejavacompatibility is similar to what I want, implemented for Gradle.
Yeah, I think moving over to rewrite-migrate-java makes sense.
Part of me would suggest leaving a small recipe like PreferJavaVersionProperty here in rewrite-spring which could replace maven.compiler.release with java.version.
But, if I remember correctly, the rewrite-migrate-java project already has some awareness of the java.version property (eg for incrementing it on upgrades), and it's probably simplest overall to keep all this complexity in one place (rewrite-migrate-java) even if some of it feels more in the spring domain.
Sorry @nmck257 , just one more thing before closing this PR. I think this rewrite gradle recipe is similar to what we discussed. I am tempted to port the recipe to Maven recipe. Does it make sense? I am still landing at OR code 👼
@ilozano2 - hmm - the goal of that recipe is certainly similar, but I expect the implementation details are fairly different. The code for org.openrewrite.gradle.UpdateJavaCompatibility might be a useful reference point, but I suspect that an implementation for a similar Maven recipe wouldn't really look like a port in the end, given how different Maven and Gradle are.
That said, since that recipe exists in rewrite-gradle, it could be reasonable to ask whether the recipe you're drafting might belong in rewrite-maven instead of rewrite-migrate-java.
I haven't watched too closely at how we've chosen to organize Maven/Gradle/cross-cutting recipes thus far -- do you have any opinion, @timtebeek ?
Hi! Typically we only have the parser, markers, and building blocks in openrewrite/rewrite, and place any other more complex or use case specific recipes in modules where appropriate. There's no hard line here, but there is a good sibling/reference recipe in rewrite-migrate-java: https://github.com/openrewrite/rewrite-migrate-java/blob/b442db77b5d0d9f976f511d51764bb3f2e67a757/src/main/java/org/openrewrite/java/migrate/maven/UseMavenCompilerPluginReleaseConfiguration.java#L36
I suggest to use that as reference implementation and contribute to rewrite-migrate-java. Thanks both!
Thanks both! Given that the proposed change is not specific to Spring, I propose we move this effort to rewrite-migrate-java with these related existing recipes:
Hope to see you contribute there, and let me know if you'll be at Spring I/O @ilozano2 ! :)