search

openrewrite / rewrite-migrate-java

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

`@PostConstruct` import being removed from beans #457

Open melloware opened 2 months ago

melloware commented 2 months ago

What version of OpenRewrite are you using?

Latest

How are you running OpenRewrite?

CLI using mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-migrate-java:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.migrate.jakarta.JakartaEE10

CDI Spec: https://jakarta.ee/specifications/cdi/2.0/cdi-spec-2.0.pdf

Before conversion:

package org.primefaces.showcase.view.overlay;

import org.primefaces.showcase.domain.Movie;

import javax.annotation.PostConstruct;
import javax.enterprise.context.RequestScoped;
import javax.inject.Named;
import java.util.ArrayList;
import java.util.List;

import javax.faces.application.FacesMessage;
import javax.faces.context.FacesContext;

import org.primefaces.event.SelectEvent;

@Named
@RequestScoped
public class MovieView {

    private Movie movie;

    private List<Movie> movieList;

    public List<Movie> getMovieList() {
        return movieList;
    }

    @PostConstruct
    public void init() {
        movieList = new ArrayList<Movie>();

        movieList.add(new Movie("The Lord of the Rings: The Two Towers", "Peter Jackson", "Fantasy, Epic", 179));
        movieList.add(new Movie("The Amazing Spider-Man 2", "Marc Webb", "Action", 142));
        movieList.add(new Movie("Iron Man 3", "Shane Black", "Action", 109));
        movieList.add(new Movie("Thor: The Dark World", "Alan Taylor", "Action, Fantasy", 112));
        movieList.add(new Movie("Avatar", "James Cameron", "Science Fiction", 160));
        movieList.add(new Movie("The Lord of the Rings: The Fellowship of the Ring", "Peter Jackson", "Fantasy, Epic", 165));
        movieList.add(new Movie("Divergent", "Neil Burger", "Action", 140));
        movieList.add(new Movie("The Hobbit: The Desolation of Smaug", "Peter Jackson", "Fantasy", 161));
        movieList.add(new Movie("Rio 2", "Carlos Saldanha", "Comedy", 101));
        movieList.add(new Movie("Captain America: The Winter Soldier", "Joe Russo", "Action", 136));
        movieList.add(new Movie("Fast Five", "Justin Lin", "Action", 132));
        movieList.add(new Movie("The Lord of the Rings: The Return of the King", "Peter Jackson", "Fantasy, Epic", 200));

    }

    public Movie getMovie() {
        return movie;
    }

    public void setMovie(Movie movie) {
        this.movie = movie;
    }

    public void onRowSelect(SelectEvent<Movie> event) {
        FacesMessage msg = new FacesMessage("Movie Selected", event.getObject().getMovie());
        FacesContext.getCurrentInstance().addMessage(null, msg);
    }
}

After Conversion:

package org.primefaces.showcase.view.overlay;

import org.primefaces.showcase.domain.Movie;

import jakarta.enterprise.context.RequestScoped;
import jakarta.inject.Named;
import java.util.ArrayList;
import java.util.List;

import jakarta.faces.application.FacesMessage;
import jakarta.faces.context.FacesContext;

import org.primefaces.event.SelectEvent;

@Named
@RequestScoped
public class MovieView {

    private Movie movie;

    private List<Movie> movieList;

    public List<Movie> getMovieList() {
        return movieList;
    }

    @PostConstruct
    public void init() {
        movieList = new ArrayList<Movie>();

        movieList.add(new Movie("The Lord of the Rings: The Two Towers", "Peter Jackson", "Fantasy, Epic", 179));
        movieList.add(new Movie("The Amazing Spider-Man 2", "Marc Webb", "Action", 142));
        movieList.add(new Movie("Iron Man 3", "Shane Black", "Action", 109));
        movieList.add(new Movie("Thor: The Dark World", "Alan Taylor", "Action, Fantasy", 112));
        movieList.add(new Movie("Avatar", "James Cameron", "Science Fiction", 160));
        movieList.add(new Movie("The Lord of the Rings: The Fellowship of the Ring", "Peter Jackson", "Fantasy, Epic", 165));
        movieList.add(new Movie("Divergent", "Neil Burger", "Action", 140));
        movieList.add(new Movie("The Hobbit: The Desolation of Smaug", "Peter Jackson", "Fantasy", 161));
        movieList.add(new Movie("Rio 2", "Carlos Saldanha", "Comedy", 101));
        movieList.add(new Movie("Captain America: The Winter Soldier", "Joe Russo", "Action", 136));
        movieList.add(new Movie("Fast Five", "Justin Lin", "Action", 132));
        movieList.add(new Movie("The Lord of the Rings: The Return of the King", "Peter Jackson", "Fantasy, Epic", 200));

    }

    public Movie getMovie() {
        return movie;
    }

    public void setMovie(Movie movie) {
        this.movie = movie;
    }

    public void onRowSelect(SelectEvent<Movie> event) {
        FacesMessage msg = new FacesMessage("Movie Selected", event.getObject().getMovie());
        FacesContext.getCurrentInstance().addMessage(null, msg);
    }
}

ERROR

Instead of converting import javax.annotation.PostConstruct; it just removed the import entirely.

timtebeek commented 2 months ago

Thanks for the report @melloware & welcome back! @cjobinabo Does this ring familiar to you or your team? Did we miss a test case to handle this properly?

melloware commented 2 months ago

Nice to be back! I am lurking always checking on your progress :)

melloware commented 2 months ago

I am looking through PR's and I can't see anything in particular that would have changed this? This was definitely not broken a couple of months ago when I did all my JakartaEE10 testing so it must have changed more recently?

melloware commented 2 months ago

Interesting in my log i see that it ran the correct recipes:

RNING] Changes have been made to primefaces-showcase\src\main\java\org\primefaces\showcase\view\overlay\MovieView.java by:
[WARNING]     org.openrewrite.java.migrate.jakarta.JakartaEE10
[WARNING]         org.openrewrite.java.migrate.jakarta.JavaxMigrationToJakarta
[WARNING]             org.openrewrite.java.migrate.jakarta.JavaxAnnotationMigrationToJakartaAnnotation
[WARNING]                 org.openrewrite.java.ChangeType: {oldFullyQualifiedTypeName=javax.annotation.PostConstruct, newFullyQualifiedTypeName=jakarta.annotation.PostConstruct}
[WARNING]             org.openrewrite.java.migrate.jakarta.JavaxEnterpriseToJakartaEnterprise
[WARNING]                 org.openrewrite.java.ChangePackage: {oldPackageName=javax.enterprise, newPackageName=jakarta.enterprise, recursive=true}
[WARNING]             org.openrewrite.java.migrate.jakarta.JavaxFacesToJakartaFaces
[WARNING]                 org.openrewrite.java.ChangePackage: {oldPackageName=javax.faces, newPackageName=jakarta.faces, recursive=true}
[WARNING]             org.openrewrite.java.migrate.jakarta.JavaxInjectMigrationToJakartaInject
[WARNING]                 org.openrewrite.java.ChangePackage: {oldPackageName=javax.inject, newPackageName=jakarta.inject, recursive=true}

But it definitely dopped the import.

The code I am running against is here: https://github.com/primefaces/primefaces/tree/master/primefaces-showcase

melloware commented 2 months ago

although interesting its a ChangeType instead of a ChangePackage?

bsmahi commented 2 months ago

@melloware I have tried replicating your issue, for me PostConstruct import is not deleted

diff --git a/devworkspace/oss/openrewrite-app/src/main/java/com/learnopenrewrite/app/Demo.java b/devworkspace/oss/openrewrite-app/src/main/java/com/learnopenrewrite/app/Demo.java
index 9c2ec3c..ef57d3e 100644
--- a/devworkspace/oss/openrewrite-app/src/main/java/com/learnopenrewrite/app/Demo.java
+++ b/devworkspace/oss/openrewrite-app/src/main/java/com/learnopenrewrite/app/Demo.java
@@ -1,6 +1,6 @@ org.openrewrite.config.CompositeRecipe
 package com.learnopenrewrite.app;

-import javax.annotation.PostConstruct;
+import jakarta.annotation.PostConstruct;

 public class Demo {

However, when I ran your code, am able to replicate that PostConstruct import statement got deleted.

Keep you posted.

Thanks, Mahi

melloware commented 2 months ago

Thanks @bsmahi i agree when I write a small test case it works but on that large project it is happening. Thanks for replicating!

timtebeek commented 2 months ago

We did a new release Wednesday which also includes these recent changes

Would you mind checking if this still affects you using these versions?

It's odd that this has started happening. We hadn't added an explicit unit test for this one as it's just a few lines of yaml https://github.com/openrewrite/rewrite-migrate-java/blob/6920ea6943c672aea6a7ffa0d7e1316c2208b05b/src/main/resources/META-INF/rewrite/jakarta-ee-9.yml#L116-L118

melloware commented 2 months ago

Confirmed this is still happening with latest release. It works in a small isolated test case but not in a large project like PrimeFaces Showcase its still stripping the import