openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.19k stars 329 forks source link

wrong maven coordinates for Groovy 4 #3406

Open blackdrag opened 1 year ago

blackdrag commented 1 year ago

As of Apache Groovy 4, Groovy aligned its maven coordinates more with the requirements of Apache. Which means that org.apache.groovy is the new namespace and org.codehaus.groovy will not be used anymore for any Groovy version beyond Apache Groovy 3

See for example: https://mvnrepository.com/artifact/org.apache.groovy/groovy-all/4.0.13

And see https://github.com/openrewrite/rewrite/blob/c27fcb269be0680e04929b5c015994bbf865bed7/rewrite-groovy/build.gradle.kts#L22 for how the latest version is requested but the namespace still points to Apache Groovy 3.

timtebeek commented 1 year ago

Thanks for chiming in here @blackdrag ! Looks like we might want to switch to a 3.x version range using the new namespace for now @joanvr , as I think there might be some conflicts with the Gradle API that we also include if we were to jump to Groovy 4.x.

In a broader sense it might be fun to explore how we can get Groovy users from 3.x to 4.x; the bare minimum would be this dependency change, but there's code changes we can do with OpenRewrite as well. Would that be something you'd be interested is contributing to @blackdrag ?

shanman190 commented 1 year ago

So I want to point out a few things real quick...

So far the rewrite-groovy module has almost solely been focused on supporting rewrite-gradle. Many Groovy cases still don't parse completely, if at all.

With that said, the indicated dependency is primarily being used to being in the Groovy compiler AST models for the sake of being able to parse Groovy source code into the rewrite-groovy LST.

From a Gradle perspective specifically, we're targeting Gradle 4.0 through latest which encompasses Groovy 2.5 and Groovy 3.x. I'd be very cautious about making a jump to the 4.x compiler AST model as in all likelihood we'd have to end up doing something like with the Java modules and having language+version specific libraries.

Based on the things that are happening in the Gradle ecosystem, it almost seems like Gradle is trying to begin phasing out the Groovy DSL entirely rather than upgrading to the 4.x Groovy version.

Also note, there was a full cutover with the 4.x release and it was not dual published at both new and old coordinates.

joanvr commented 1 year ago

After a bit of context research, I encountered the same things @shanman190 is saying in his comment. We do not actually have many recipes or care that much about pure groovy code refactoring, but rather to parse and refactor Gradle scripts. As the latest version of Gradle (8.2.1) it is still using Groovy 3.0.17, so there isn't much urgency to adopt Groovy 4.0. I've also explored what changes would be necessary to upgrade to Groovy 4.0, and (aside from the dependency coordinates change and the version bump) there are some extra changes needed. At least:

Also, we cannot move to the new groupId, because there are only 4.x releases there... So I think for now we are a bit blocked to support Groovy 4.x... Maybe it's worth it to increase the Groovy 3.x support and at some point move to 4.x if it's syntax is compatible enough with 3.x or maybe even create a separate rewrite-groovy-4 module for compatibility if needed (like we have for different versions of java).

WDYT @blackdrag? Would you be interested in contributing to Groovy parsing and recipes so we have fully support of it?

blackdrag commented 1 year ago

a couple of things

@shanman190 Gradle does not care about if Kotlin or Groovy is used as DSL, they would use XML if that is what the people want. I am aware you said "seems", still if you have a pointer that clarifies a plan to remove Groovy in the future I really would like to see that. Anyway, I know for a fact that Gradle checks the possibility to move to Groovy4, but they have some scenarios where Groovy4 behaves less than ideal. I think that is the major blocker for them preventing a move to Groovy4 right now.

@joanvr Then - I only discovered this project yesterday, so I did not try the rewrite-groovy module at all. I wonder though.. if the rewrite-groovy module is for Gradle only and not for Groovy, then why is it not together with the rewrite-gradle module?

As for the group-id.... It was actually a requirement from Apache to move the maven coordinates. The move has been delayed since Groovy 3 was already in the making when Groovy become part of Apache and a coordinate change mid-version was deemed unacceptable, but for Groovy 4 this is different. Afaik maven central and similar do not allow for the artifact to be published to multiple candidates. I may be wrong here or the information outdated though. I think I will bring this up on the Groovy mailing list.

As for changes in Groovy 4... looking at https://groovy-lang.org/releasenotes/groovy-4.0.html#Groovy4.0-naming-changes I do see a few things that could be interesting for a rewrite (some syntax migration maybe, some package naming updates surely). But I am not aware of breaking Syntax. Almost all programs from even Groovy 1 times should still compile.

Would I be interested in contributing. Interested yes, finding the time is more an issue. I can try if you tell me what you find exactly interesting.

timtebeek commented 1 year ago

Thanks for sharing those release notes @blackdrag ! Quickly scanning through those there's a couple items we can quite easily cover already with likely no more than a declarative yaml file:

Maven coordinate change In Groovy 4.0, the groupId of the maven coordinates for Groovy have changed from org.codehaus.groovy to org.apache.groovy. Please update your Gradle/Maven/other build settings appropriately.

Should be trivial with https://docs.openrewrite.org/recipes/java/dependencies/changedependency We can start a https://github.com/openrewrite/rewrite/tree/main/rewrite-groovy/src/main/resources/META-INF/rewrite/groovy-4.yml file to hold these recipes. This can change the groupId at the same time as changing the version number.

Legacy package removal The Java Platform Module System (JPMS) requires that classes in distinct modules have distinct package names (known as the "split packaging requirement"). Groovy has its own "modules" that weren’t historically structured according to this requirement.

Groovy 3 provided duplicate versions of numerous classes (in old and new packages) to allow Groovy users to migrate towards the new JPMS compliant package names. See the Groovy 3 release notes for more details. Groovy 4 no longer provides the duplicate legacy classes.

In short, time to stop using groovy.util.XmlSlurper and start using groovy.xml.XmlSlurper. Similarly, you should now be using groovy.xml.XmlParser, groovy.ant.AntBuilder, groovy.test.GroovyTestCase and the other classes mentioned in the prior mentioned Groovy 3 release notes.

Again another easy one through https://docs.openrewrite.org/recipes/java/changetype Don't be fooled by the java in the package name; it should work just as well for Groovy.

Module changes for groovy-all Based on user feedback and download statistics, we rejigged which modules are included in the groovy-all pom (GROOVY-9647). The groovy-yaml module is fairly widely used and is now included in groovy-all. The groovy-testng module is less widely used and is no longer included in groovy-all. Please adjust your build script dependencies if needed. If you are using the Groovy distribution, no changes are required since it includes the optional modules.

Also quite easy to do; we have https://docs.openrewrite.org/recipes/java/dependencies/adddependency to add a groovy-testng dependency onlyIfUsing some class from testng, such that applications should still work after running the recipe to upgrade, while only adding testng if necessary. Similarly we can remove groovy-yaml when using groovy-all.

In short we could say we have component recipes that you can configure and combine together to help users adopt the latest version, even if it's relatively small steps. By automating the path, even in part, you entice people to get on their way to upgrade rather than put if off because of perceived breaking changes.

shanman190 commented 1 year ago

@blackdrag, rewrite-groovy IS meant to parse both Groovy functional source as well as Groovy scripts. It's just that since it was created, it's been primarily focused on improving the Groovy scripts side of it's parsing functions.

Most recipe goals are probably going to take from using the Java LST elements via the JavaVisitor or GroovyVisitor. As Tim's pointed out, all of the Java recipes work for the Groovy LST due to its extension nature.

As far as Gradle Inc and Gradle Build Tool, they already of course have support for compiling Groovy 4.x source code without any issues, but they haven't adopted Groovy 4.x for the internal components of Gradle itself (localGroovy for Gradle plugins still provides Groovy 3.x for example on the latest release). As you know Groovy 4.x has been out for a while, so the absence of any migrate groovy 4.x for the internal Gradle build functionality, their ongoing investment in Kotlin DSL, and their recent switch to Kotlin DSL as the default recommended buildscript syntax are where my opinion of it seems like Gradle Inc is trying to move away from Groovy for DSL all together. https://blog.gradle.org/kotlin-dsl-is-now-the-default-for-new-gradle-builds

At least with my quick searching, it also doesn't seem to show anything on the public issue tracker about how they are planning to adopt Groovy 4.x for the internals.