`jakarta.annotation-api` dependency added in Java 11 migration should be removed when migrating to Jakarta

[abccbaandy]

What version of OpenRewrite are you using?

I am using IntelliJ IDEA OpenRewrite feature, guess it's latest version? It didn't print the version :(

How are you running OpenRewrite?

with this recipes only

org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_2

What is the smallest, simplest way to reproduce the problem?

N/A

What did you expect to see?

Noting.

What did you see instead?

in build.gradle, it add this line

jakarta.annotation:jakarta.annotation-api:1.3.5

What is the full stack trace of any errors you encountered?

N/A
But here is the log relate to this issue

Changes have been made to build.gradle by:
    org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_2
        org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_1
            org.openrewrite.java.spring.boot3.UpgradeSpringBoot_3_0
                org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_7
                    org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_6
                        org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_5
                            org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_4
                                org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_3
                                    org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_2
                                        org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_1
                                            org.openrewrite.java.spring.boot2.UpgradeSpringBoot_2_0
                                                org.openrewrite.java.spring.boot2.MigrateHibernateConstraintsToJavax
                                                    org.openrewrite.java.dependencies.AddDependency: {groupId=javax.validation, artifactId=validation-api, version=2.x, onlyIfUsing=javax.validation.constraints.*}
                                org.openrewrite.java.dependencies.UpgradeDependencyVersion: {groupId=org.springframework.boot, artifactId=*, newVersion=2.4.x, overrideManagedVersion=false}
                            org.openrewrite.java.dependencies.UpgradeDependencyVersion: {groupId=org.springframework.boot, artifactId=*, newVersion=2.5.x, overrideManagedVersion=false}
                            org.openrewrite.gradle.plugins.UpgradePluginVersion: {pluginIdPattern=org.springframework.boot, newVersion=2.5.x}
                            org.openrewrite.java.dependencies.ChangeDependency: {oldGroupId=mysql, oldArtifactId=mysql-connector-java, newGroupId=com.mysql, newArtifactId=mysql-connector-j, newVersion=8.0.x}
                        org.openrewrite.java.dependencies.UpgradeDependencyVersion: {groupId=org.springframework.boot, artifactId=*, newVersion=2.6.x, overrideManagedVersion=false}
                        org.openrewrite.gradle.plugins.UpgradePluginVersion: {pluginIdPattern=org.springframework.boot, newVersion=2.6.x}
                    org.openrewrite.java.dependencies.UpgradeDependencyVersion: {groupId=org.springframework.boot, artifactId=*, newVersion=2.7.x, overrideManagedVersion=false}
                    org.openrewrite.gradle.plugins.UpgradePluginVersion: {pluginIdPattern=org.springframework.boot, newVersion=2.7.x}
                org.openrewrite.java.migrate.UpgradeToJava17
                    org.openrewrite.java.migrate.Java8toJava11
                        org.openrewrite.java.migrate.javax.AddCommonAnnotationsDependencies
                            org.openrewrite.java.dependencies.AddDependency: {groupId=jakarta.annotation, artifactId=jakarta.annotation-api, version=1.3.x, onlyIfUsing=javax.annotation..*, acceptTransitive=true}
                        org.openrewrite.java.migrate.lombok.UpdateLombokToJava11
                            org.openrewrite.java.dependencies.UpgradeDependencyVersion: {groupId=org.projectlombok, artifactId=lombok, newVersion=1.18.*}

Are you interested in contributing a fix to OpenRewrite?

Sure, let me know what can I do.

[timtebeek]

Hi @abccbaandy ; thanks for the report! Good to see the recipes that made changes included here; that helps reference the code. The dependency is added from this recipe step

rewrite-migrate-java/src/main/resources/META-INF/rewrite/add-common-annotations-dependencies.yml

Lines 35 to 40 in 64f8669

	- org.openrewrite.java.dependencies.AddDependency:
	groupId: jakarta.annotation
	artifactId: jakarta.annotation-api
	version: 1.3.x
	onlyIfUsing: javax.annotation..*
	acceptTransitive: true

Notice how it's constrained to only add the dependency if you are using anything from the javax.annotation..* package. Can you confirm that is indeed the case?
We also should not add the dependency if it's already present transitively; that part might be sensitive to recipe execution order.

Could you let me know why you feel we should not add that dependency to your project in this case?
Is it because you were already running successfully on a more recent Java & Spring version without?
Or is the dependency already available transitively but not picked up some how?

[abccbaandy]

Hi @timtebeek , the Spring boot version was 2.X, so yes, I am use javax.annotation.
And the recipes change all of them to jakarta.annotation <= This is correct too.

But for the jakarta.annotation:jakarta.annotation-api:1.3.5 part. This is wrong because Spring boot 3.2.5 use jakarta.annotation:jakarta.annotation-api:2.1.1
ref: https://mvnrepository.com/artifact/org.springframework.boot/spring-boot-starter/3.2.5
And 1.3.5 don't have jakarta.annotation package.
So after the recipes run finish, I got jakarta.annotation in my code with jakarta.annotation-api:1.3.5 dependency which cause build fail.

Also, since the spring boot starter contain the jakarta.annotation dependency and maintain the version already, I don't think it's right to add jakarta relate dependency,

[timtebeek]

Thanks for that additional context! I think I have an understanding now of what's going on.

We recently added an explicit recipe to change or add jakarta.annotation-api as needed for folks that migrate to Java 11.

• feat: Missing "Add explicit Common Annotations dependencies" in "Migrate to Java 11" #456

The Java 11 migration is also run as part of a Spring Boot 3 migration, as that includes Java 17 migration, which includes Java 11.
These recipes are run in sequence, each performing the step necessary at that time. That means we need to add the dependency when going from 8 to 11, as at that point the imports have not yet changed. When we go from 11 to 17 and Jakarta though, we should remove that dependency again. It's only necessary for folks that wish to stay on Java 11 for whatever reason.

I imagine we could add a recipe to org.openrewrite.java.migrate.jakarta.JavaxMigrationToJakarta to remove explicit jakarta.annotation-api dependencies again if they also come in transitively. For that conditional we could use a precondition using DependencyInsight. Effectively that should then not result in any added dependency when folks leap from 8 to 17 + Jakarta.

Is this something you'd already be comfortable adding to src/main/resources/META-INF/rewrite/jakarta-ee-9.yml ? Even a draft PR with a unit test would be welcome; if not I'll try to fit it in but that could be a little while, as it's hard to get to everything.

[timtebeek]

I see you've started working toward this @abccbaandy ; thanks a lot! Feel free to open a draft pull request such that I can help out if needed.

[abccbaandy]

lol how do you found that?
Do you get any notify if anyone fork the project?

Anyways, I open a PR with some question, please help.
Thanks.

[abccbaandy]

Hi @timtebeek I have some new thought about this issue after study the source.

1. The DependencyInsight seems not support non-ResolvedDependency.
   Means when we have direct dependency, the transitive dependency with the same name is gone.
   ref
   https://github.com/openrewrite/rewrite/blob/main/rewrite-maven/src/main/java/org/openrewrite/maven/search/DependencyInsight.java#L119
   So, I guess this issue cause DependencyInsight onlyTransitive feature not possible.

2. I found there is an issue with similar problem.
   He just checks if the spring exist then bypass the add dependency

applicability:
  singleSource:
    - org.openrewrite.maven.search.DoesNotIncludeDependency:
        groupId: org.springframework.boot
        artifactId: spring-boot-starter-web

https://github.com/nmck257/rewrite-migrate-java/blob/29319b419d1daa228abd8ebc6958b1f3f725a8b2/src/main/resources/META-INF/rewrite/jakarta-ee-9.yml#L696
ori PR: #196

But it got removed in the latest master, I don't know why.

Can we use the same way to modify the org.openrewrite.java.migrate.javax.AddCommonAnnotationsDependencies recipe?
Btw, I don't know why DoesNotIncludeDependency only available in maven, but I think we can use DependencyInsight to check if spring exist?

[timtebeek]

Ah yes single-source applicability tests is what we called preconditions in Rewrite v7; I suspect it was dropped as we moved to Rewrite v8 and not revisited since. I can see that DoesNotIncludeDependency recipe itself does use DependencyInsight, with a hint about use with multi module projects.
https://github.com/openrewrite/rewrite/blob/74faca04cd0ffbbb3e4347c86460feb4e1ea139e/rewrite-maven/src/main/java/org/openrewrite/maven/search/DoesNotIncludeDependency.java#L66-L68

If we can make that work well as a precondition to check for the absence/presence of a transitive(!) dependency, for instance by looking at the MavenResolutionResult marker, then we could use this before we remove the dependency. I'd need to look a little deeper than I have time for this week, but hope the context helps you explore there.