openrewrite / rewrite-migrate-java

OpenRewrite recipes for migrating to newer versions of Java.
Apache License 2.0
92 stars 65 forks source link

Missing "Add explicit Common Annotations dependencies" in "Migrate to Java 11" #455

Closed ghusta closed 2 months ago

ghusta commented 2 months ago

When trying to apply the recipe Migrate to Java 11, there is no recipe for adding missing dependencies for Common Annotations (JSR-250). This dependency contains classes like javax.annotation.Generated or javax.annotation.security.RolesAllowed. It's also mentioned in JEP 320, for your information.

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

With Maven plugin, like :

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-migrate-java:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.migrate.Java8toJava11

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

This code should compile with a JDK 11 :

import javax.annotation.Generated;

@Generated("xxx")
public class A {
}

What did you expect to see?

In Maven pom.xml :

        <!-- Common Annotations -->
        <dependency>
            <groupId>javax.annotation</groupId>
            <artifactId>javax.annotation-api</artifactId>
            <version>1.3.2</version>
        </dependency>

Or rather the jakarta variant :

        <dependency>
            <groupId>jakarta.annotation</groupId>
            <artifactId>jakarta.annotation-api</artifactId>
            <version>1.3.5</version>
        </dependency>

What did you see instead?

Previous dependency declaration is absent.

Are you interested in [contributing a fix to OpenRewrite]

Why not

MBoegers commented 2 months ago

Similar behavior observed here https://github.com/openrewrite/rewrite-spring/issues/486 and we discussed another version at Javaland. It seems like there is a problem with the conditional adding of a dependency. I suspect that there is an issue when switching an Annotation (javax. --> jakarta.) in the code and a decoupled add dependency step.

ghusta commented 2 months ago

@MBoegers you mean there could be a problem if trying to use something like :

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

like in https://docs.openrewrite.org/recipes/java/migrate/javax/addjaxbdependencies ?

At that point, packages don't need to be updated to jakarta in the code IMO, first the dependency has to be added if needed.

MBoegers commented 2 months ago

@ghusta jakarta.annotation-api:1.3.5 does not change the namespace? sorry ATM I am not sure at which version @Generated is migrated into jakarta namespace.

But still, yes this Recipe org.openrewrite.java.dependencies.AddDependency in combination with onlyIfUsing is my prime suspect for this issue, https://github.com/openrewrite/rewrite-spring/issues/486 and another I cannot remember ATM. There appears to be a problem around.

Maybe we can reproduce the behavior with your AddDependency configuration, this would lower the search space enormous.

ghusta commented 2 months ago

No, the package is changed in further versions. In v2.0+ I think. See https://mvnrepository.com/artifact/jakarta.annotation/jakarta.annotation-api

ghusta commented 2 months ago

My first thought is a recipe that looks like this :

type: specs.openrewrite.org/v1beta/recipe
name: org.openrewrite.java.migrate.javax.AddCommonAnnotationsDependencies
displayName: Add explicit Common Annotations dependencies
description: >
  This recipe will add the necessary `annotation-api` dependency from Jakarta EE 8 to maintain compatibility with Java
  version 11 or greater.
tags:
  - javax
  - java11
  - jsr-250
  - jakarta
recipeList:
  # Add or update the jakarta.annotation-api to a maven project. This artifact still uses the javax.annotation name space.
  - org.openrewrite.java.dependencies.ChangeDependency:
      oldGroupId: javax.annotation
      oldArtifactId: javax.annotation-api
      newGroupId: jakarta.annotation
      newArtifactId: jakarta.annotation-api
      newVersion: 1.3.x
  - org.openrewrite.maven.ChangeManagedDependencyGroupIdAndArtifactId:
      oldGroupId: javax.annotation
      oldArtifactId: javax.annotation-api
      newGroupId: jakarta.annotation
      newArtifactId: jakarta.annotation-api
      newVersion: 1.3.x
  - org.openrewrite.java.dependencies.AddDependency:
      groupId: jakarta.annotation
      artifactId: jakarta.annotation-api
      version: 1.3.x
      onlyIfUsing: javax.annotation..*
      acceptTransitive: true

Maybe there are some other edge cases to work on...

timtebeek commented 2 months ago

Thanks a lot for both reporting and immediately picking this up! I've left some comments on the PR, but I think we should be able to get this in quickly.