openrewrite / rewrite

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

Property in depdencyManagement of parent pom not changed, version tag added in submodul pom #4183

Open Evodam opened 8 months ago

Evodam commented 8 months ago

I just ran the following command:

mvn org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-hibernate:RELEASE -Drewrite.activeRecipes=org.openrewrite.hibernate.MigrateToHibernateDependencies61

The project on which I applied it has a parent-pom and multiple submodule-poms. The parent-pom has a dependencyManagement section where versions are defined using properties. The submodules only list the dependencies without version declaration.

This was before open rewrite: parent pom:

<dependency>
  <groupId>org.hibernate</groupId>
  <artifactId>hibernate-core</artifactId>
  <version>${hibernate.version}</version>
</dependency>

submodul pom:

<dependency>
  <groupId>org.hibernate</groupId>
  <artifactId>hibernate-core</artifactId>
</dependency>

This after: parent pom:

<dependency>
  <groupId>org.hibernate.orm</groupId>
  <artifactId>hibernate-core</artifactId>
  <version>6.1.0.Final</version>
</dependency>

submodul pom:

<dependency>
  <groupId>org.hibernate.orm</groupId>
  <artifactId>hibernate-core</artifactId>           
  <version>6.1.0.Final</version>
</dependency>

OpenRewrite replaced the property in the parent pom (instead of changing it) and added a version tag to the submodul pom although it isnt necessary there

timtebeek commented 8 months ago

Hi @Evodam that's unfortunate; I'm linking the relevant parts of the recipe https://github.com/openrewrite/rewrite-hibernate/blob/7612ecafb1dbbe1563b51a12ee9ba3dc69f0ab44/src/main/resources/META-INF/rewrite/hibernate-6.yml#L119-L143

Not quite sure yet why; might be something to fix in openrewrite/rewrite, but I've not yet found the time to explore. If you want to help out you can convert the above into a unit test such with the desired behavior. https://github.com/openrewrite/rewrite-hibernate/blob/main/src/test/java/org/openrewrite/hibernate/MigrateToHibernate61Test.java#L30

Evodam commented 8 months ago

@timtebeek

It seems Im not allowed to create a branch and Im not familiar with gradle, I cannot make the project compile and run the tests, but I hope this is sufficient:


    @Test
        void updateManagedDependency() {
            rewriteRun(
              mavenProject(
                "Sample",
                //language=xml
                pomXml("""
                        <?xml version="1.0" encoding="UTF-8"?>
                        <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                          <modelVersion>4.0.0</modelVersion>
                          <groupId>com.example</groupId>
                          <artifactId>demo</artifactId>
                          <version>0.0.1-SNAPSHOT</version>

                          <properties>
                            <hibernate.version>5.6.15.Final</hibernate.version>
                          </properties>

                          <dependencyManagement>
                            <dependencies>
                              <dependency>
                                <groupId>org.hibernate</groupId>
                                <artifactId>hibernate-core</artifactId>
                                <version>${hibernate.version}</version>
                              </dependency>
                            </dependencies>
                          </dependencyManagement>

                          <dependencies>
                              <dependency>
                                <groupId>org.hibernate</groupId>
                                <artifactId>hibernate-core</artifactId>
                            </dependency>
                          </dependencies>
                        </project>
                        """, spec -> spec.after(actual -> {
                      Matcher matcher = Pattern.compile("<version>(6\\.1\\.\\d+\\.Final)</version>").matcher(actual);
                      assertTrue(matcher.find());
                      return """
                        <?xml version="1.0" encoding="UTF-8"?>
                        <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                          <modelVersion>4.0.0</modelVersion>
                          <groupId>com.example</groupId>
                          <artifactId>demo</artifactId>
                          <version>0.0.1-SNAPSHOT</version>

                           <properties>
                            <hibernate.version>%s</hibernate.version>
                            </properties>

                           <dependencyManagement>
                            <dependencies>
                              <dependency>
                                <groupId>org.hibernate</groupId>
                                <artifactId>hibernate-core</artifactId>
                                <version>${hibernate.version}</version>
                              </dependency>
                            </dependencies>
                          </dependencyManagement>

                          <dependencies>
                            <dependency>
                              <groupId>org.hibernate.orm</groupId>
                              <artifactId>hibernate-core</artifactId>
                            </dependency>
                          </dependencies>
                        </project>
                        """.formatted(matcher.group(1));
                  })
                ),
                //language=java
                srcMainJava(
                  java("""
                    public class TestApplication {
                    }
                    """
                  )
                )
              )
            );
        }

    @Test
        void updateProperties() {
            rewriteRun(
              mavenProject(
                "Sample",
                //language=xml
                pomXml("""
                    <?xml version="1.0" encoding="UTF-8"?>
                    <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                      xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                      <modelVersion>4.0.0</modelVersion>
                      <groupId>com.example</groupId>
                      <artifactId>demo</artifactId>
                      <version>0.0.1-SNAPSHOT</version>

                      <properties>
                        <hibernate.version>5.6.15.Final</hibernate.version>
                      </properties>

                      <dependencies>
                        <dependency>
                          <groupId>org.hibernate</groupId>
                          <artifactId>hibernate-core</artifactId>
                          <version>${hibernate.version}</version>
                        </dependency>
                      </dependencies>
                    </project>
                    """, spec -> spec.after(actual -> {
                      Matcher matcher = Pattern.compile("<version>(6\\.1\\.\\d+\\.Final)</version>").matcher(actual);
                      assertTrue(matcher.find());
                      return """
                        <?xml version="1.0" encoding="UTF-8"?>
                        <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                          <modelVersion>4.0.0</modelVersion>
                          <groupId>com.example</groupId>
                          <artifactId>demo</artifactId>
                          <version>0.0.1-SNAPSHOT</version>

                           <properties>
                            <hibernate.version>%s</hibernate.version>
                            </properties>

                          <dependencies>
                            <dependency>
                              <groupId>org.hibernate.orm</groupId>
                              <artifactId>hibernate-core</artifactId>
                              <version>${hibernate.version}</version>
                            </dependency>
                          </dependencies>
                        </project>
                        """.formatted(matcher.group(1));
                  })
                ),
                //language=java
                srcMainJava(
                  java("""
                    public class TestApplication {
                    }
                    """
                  )
                )
              )
            );
        }

    @Test
        void updateDependencyManagementOnly() {
            rewriteRun(
                    mavenProject(
                                    "SampleParent",
                                    //language=xml
                                    pomXml("""
                                        <?xml version="1.0" encoding="UTF-8"?>
                                        <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                                          <modelVersion>4.0.0</modelVersion>
                                          <groupId>com.example</groupId>
                                          <artifactId>parent</artifactId>
                                          <version>0.0.1-SNAPSHOT</version>

                                          <properties>
                                            <hibernate.version>5.6.15.Final</hibernate.version>
                                          </properties>

                                          <dependencyManagement>
                                            <dependencies>
                                                <dependency>
                                                  <groupId>org.hibernate</groupId>
                                                  <artifactId>hibernate-core</artifactId>
                                                  <version>${hibernate.version}</version>
                                                </dependency>
                                            </dependencies>
                                          </dependencyManagement>
                                        </project>
                                        """, spec -> spec.after(actual -> {
                                          Matcher matcher = Pattern.compile("<version>(6\\.1\\.\\d+\\.Final)</version>").matcher(actual);
                                          assertTrue(matcher.find());
                                          return """
                                            <?xml version="1.0" encoding="UTF-8"?>
                                            <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                              xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                                              <modelVersion>4.0.0</modelVersion>
                                              <groupId>com.example</groupId>
                                              <artifactId>parent</artifactId>
                                              <version>0.0.1-SNAPSHOT</version>

                                               <properties>
                                                <hibernate.version>%s</hibernate.version>
                                                </properties>

                                              <dependencyManagement>
                                                <dependencies>
                                                  <dependency>
                                                    <groupId>org.hibernate</groupId>
                                                    <artifactId>hibernate-core</artifactId>
                                                    <version>${hibernate.version}</version>
                                                  </dependency>
                                                </dependencies>
                                              </dependencyManagement>
                                            </project>
                                            """.formatted(matcher.group(1));
                                      })
                                    ),
                                    //language=java
                                    srcMainJava(
                                      java("""
                                        public class TestApplication {
                                        }
                                        """
                                      )
                                    )
                                  ),

              mavenProject(
                "SampleChild",
                //language=xml
                pomXml("""
                        <?xml version="1.0" encoding="UTF-8"?>
                        <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                          xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                          <modelVersion>4.0.0</modelVersion>

                          <artifactId>child</artifactId>
                          <version>0.0.1-SNAPSHOT</version>

                          <parent>
                            <groupId>com.example</groupId>
                            <artifactId>parent</artifactId>
                            <version>0.0.1-SNAPSHOT</version>
                            </parent>

                          <dependencies>
                            <dependency>
                              <groupId>org.hibernate</groupId>
                              <artifactId>hibernate-core</artifactId>
                            </dependency>
                          </dependencies>
                        </project>
                        """, spec -> spec.after(actual -> {
                      Matcher matcher = Pattern.compile("<version>(6\\.1\\.\\d+\\.Final)</version>").matcher(actual);
                      assertTrue(matcher.find());
                      return """
                              <?xml version="1.0" encoding="UTF-8"?>
                              <project xmlns="http://maven.apache.org/POM/4.0.0" xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
                                xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 https://maven.apache.org/xsd/maven-4.0.0.xsd">
                                <modelVersion>4.0.0</modelVersion>
                                <artifactId>child</artifactId>
                                <version>0.0.1-SNAPSHOT</version>

                                  <parent>
                                  <groupId>com.example</groupId>
                                  <artifactId>parent</artifactId>
                                  <version>0.0.1-SNAPSHOT</version>
                                  </parent>

                                <dependencies>
                                  <dependency>
                                    <groupId>org.hibernate.orm</groupId>
                                    <artifactId>hibernate-core</artifactId>         
                                  </dependency>
                                </dependencies>
                              </project>
                              """.formatted(matcher.group(1));
                  })
                ),
                //language=java
                srcMainJava(
                  java("""
                    public class TestApplication2 {
                    }
                    """
                  )
                )
              )
            );
        }
Evodam commented 8 months ago

@timtebeek

I tried to analyse the problem. What I got so far:

first: Properties are not respected, it is only checked if the dependency tag itself has a version second: when it is checked whether a depedency is managed, a dependency with the new groupId and artifactId are searched, not for the old groupId and artifactId (this might be irrelevant if the files are processed in cycles)

In my comment above I added a third test case to simulate two different poms with a parent-child-relation.

Unfortunately I cannot get the the project to compile but I hope this could be helpful to find the bug.

timtebeek commented 4 months ago

Thanks for the detailed tests @Evodam ! And sorry to hear you had trouble getting the project to work. We have some instructions documented here if that is still of help to you: https://docs.openrewrite.org/reference/building-openrewrite-from-source

I've moved the issue as I believe it's not specific to rewrite-hibernate, and moving it here should make it easier to find & correlate the fix. We did have some recent changes to that particular recipe that might have already improved

We'd have to double check with the tests you've provided, which I'll try to fit in when I find the time. Thanks again!