Properties for Maven versions don't get properly updated.

[rdifrango]

In my project I use property placeholders for the version so something like;

<properties>
        <logback.version>1.2.3</logback.version>
</properties>

<dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-core</artifactId>
            <version>${logback.version}</version>
</dependency>

And when I run the org.openrewrite.maven.search.DependencyInsight it properly recognizes that it should be updated but it doesn't update the property and instead writes the following into the pom.xml

<!--~~>--><dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-core</artifactId>
            <version>${logback.version}</version>
        </dependency>

cc: @aegershman

[aegershman]

org.openrewrite.maven.search.DependencyInsight will find and mark direct and transitive dependencies based on a provided groupIdPattern and artifactIdPattern-- so, it looks like it's correct (unit tests as examples included here: https://github.com/openrewrite/rewrite/blob/6eba36c99a7dbe900ee9045cde64b6acd5e687de/rewrite-maven/src/test/kotlin/org/openrewrite/maven/search/DependencyInsightTest.kt)

I'm assuming the goal is to upgrade the dependency to a higher version, right? In which case, you'll probably want this in your rewrite.yml:

(more details at https://docs.openrewrite.org/reference/recipes/maven/upgradedependencyversion)

---
type: specs.openrewrite.org/v1beta/recipe
name: org.example.Example
displayName: example
recipeList:
  - org.openrewrite.maven.UpgradeDependencyVersion:
      groupId: ch.qos.logback
      artifactId: logback-core
      newVersion: 1.2.X # or "latest.release"

If you could try that out, that should update version within your <logback.version>..</> in your section (these unit tests should help show expected change results: https://github.com/openrewrite/rewrite/blob/6eba36c99a7dbe900ee9045cde64b6acd5e687de/rewrite-maven/src/test/kotlin/org/openrewrite/maven/UpgradeDependencyVersionTest.kt#L139)

Let me know if that's not what you had in mind 👍 (If it's not, it'd probably be helpful if you can share your <activeRecipes>...</> configuration and/or rewrite.yml to see what might be an issue)

[rdifrango]

that still didn't work, here's my plug-in config:

<plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>2.22.1</version>
            </plugin>
            <plugin>
                <groupId>org.openrewrite.maven</groupId>
                <artifactId>rewrite-maven-plugin</artifactId>
                <version>4.17.0-SNAPSHOT</version>
                <configuration>
                    <activeRecipes>
                        <recipe>com.yourorg.LogbackInsight</recipe>
                    </activeRecipes>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.openrewrite.recipe</groupId>
                        <artifactId>rewrite-logging-frameworks</artifactId>
                        <version>1.1.0-SNAPSHOT</version>
                    </dependency>
                </dependencies>
            </plugin>

---
type: specs.openrewrite.org/v1beta/recipe
name: com.yourorg.LogbackInsight
displayName: Find Logback Usage
recipeList:
  - org.openrewrite.maven.search.DependencyInsight:
      groupIdPattern: ch.qos.logback
      artifactIdPattern: "*"
      newVersion: "latest.release"
      scope: compile

[rdifrango]

and I'll say my assumption was that newVersion would simply default to the latest.release had I not specified it otherwise.

[rdifrango]

and here's my sample POM:

<?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 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>org.rdifrango</groupId>
    <artifactId>rewrite-example</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <maven.compiler.source>11</maven.compiler.source>
        <maven.compiler.target>11</maven.compiler.target>
        <slf4j.version>1.7.30</slf4j.version>
        <logback.version>1.2.3</logback.version>
    </properties>

    <build>
        <plugins>
            <plugin>
                <groupId>org.apache.maven.plugins</groupId>
                <artifactId>maven-surefire-plugin</artifactId>
                <version>2.22.1</version>
            </plugin>
            <plugin>
                <groupId>org.openrewrite.maven</groupId>
                <artifactId>rewrite-maven-plugin</artifactId>
                <version>4.17.0-SNAPSHOT</version>
                <configuration>
                    <activeRecipes>
                        <recipe>com.yourorg.LogbackInsight</recipe>
                        <!--                        <recipe>org.openrewrite.java.logging.slf4j.ParameterizedLogging</recipe>-->
                        <!--                        <recipe>org.openrewrite.java.logging.slf4j.Slf4jLogShouldBeConstant</recipe>-->
                    </activeRecipes>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.openrewrite.recipe</groupId>
                        <artifactId>rewrite-logging-frameworks</artifactId>
                        <version>1.1.0-SNAPSHOT</version>
                    </dependency>
                </dependencies>
            </plugin>
        </plugins>
    </build>

    <dependencies>
        <!--~~>--><dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-core</artifactId>
            <version>${logback.version}</version>
        </dependency>

        <dependency>
            <groupId>org.slf4j</groupId>
            <artifactId>slf4j-api</artifactId>
            <version>${slf4j.version}</version>
        </dependency>

        <!--~~>--><dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-classic</artifactId>
            <version>${logback.version}</version>
        </dependency>

        <dependency>
            <groupId>org.junit.jupiter</groupId>
            <artifactId>junit-jupiter-engine</artifactId>
            <version>5.6.0</version>
            <scope>test</scope>
        </dependency>
    </dependencies>
</project>

[rdifrango]

and I tried with newVersion: 1.2.X and newVersion: "1.2.X" and newVersion: 1.2.7 all resulted in:

<!--~~>--><dependency>
            <groupId>ch.qos.logback</groupId>
            <artifactId>logback-classic</artifactId>
            <version>${logback.version}</version>
        </dependency>

[aegershman]

@rdifrango the configuration option of newVersion doesn't apply to the recipe DependencyInsight (https://docs.openrewrite.org/reference/recipes/maven/search/dependencyinsight#options), it applies to a different recipe we're setting up in the recipeList: UpgradeDependencyVersion

Try replacing your entire rewrite.yml with this:

---
type: specs.openrewrite.org/v1beta/recipe
name: com.yourorg.LogbackInsight
displayName: Find Logback Usage
recipeList:
#  - org.openrewrite.maven.search.DependencyInsight:
#      groupIdPattern: ch.qos.logback
#      artifactIdPattern: "*"
#      scope: compile
  - org.openrewrite.maven.UpgradeDependencyVersion:
      groupId: ch.qos.logback
      artifactId: logback-core
      newVersion: latest.release

Note that I've replaced the entire recipeList entry of org.openrewrite.maven.search.DependencyInsight: with the org.openrewrite.maven.UpgradeDependencyVersion recipe. DependencyInsight is a search recipe whose only job is to show locations where a dependency shows up as either a top-level or transitive dependency. To actually upgrade it, you've got to use the org.openrewrite.maven.UpgradeDependencyVersion recipe.

Try copy-pasting that entire config I shared into your rewrite.yml and re-run mvn rewrite:run let me know if it looks any better?

[rdifrango]

@aegershman 🤦 - I was reading this and didn't catch that the recipe was only for insights not patching.

That said, I just ran it with that recipe and it didn't update the property:

<plugin>
                <groupId>org.openrewrite.maven</groupId>
                <artifactId>rewrite-maven-plugin</artifactId>
                <version>4.17.0-SNAPSHOT</version>
                <configuration>
                    <activeRecipes>
                        <recipe>com.yourorg.LogbackInsight</recipe>
                        <!--                        <recipe>org.openrewrite.java.logging.slf4j.ParameterizedLogging</recipe>-->
                        <!--                        <recipe>org.openrewrite.java.logging.slf4j.Slf4jLogShouldBeConstant</recipe>-->
                    </activeRecipes>
                </configuration>
                <dependencies>
                    <dependency>
                        <groupId>org.openrewrite.recipe</groupId>
                        <artifactId>rewrite-logging-frameworks</artifactId>
                        <version>1.1.0-SNAPSHOT</version>
                    </dependency>
                </dependencies>
            </plugin>

and

---
type: specs.openrewrite.org/v1beta/recipe
name: com.yourorg.LogbackInsight
displayName: Find Logback Usage
recipeList:
  - org.openrewrite.maven.UpgradeDependencyVersion:
      groupId: ch.qos.logback
      artifactId: logback-core
      newVersion: latest.release

[aegershman]

@rdifrango hmm so even with that config, and running rewrite:run or doing rewrite:dryRun it's saying "No changes were made"? Using your example as a template I validated this worked in the unit tests and ran it against a test maven project and it updated <logback.version> to the latest release of 1.2.8:

• unit test: https://github.com/openrewrite/rewrite/pull/1283
• a personal example project I had laying around: https://github.com/slugstack/slugstack-maven-scratchpad/pull/20

What does the output say when you run rewrite:run? Are there any changes?

[rdifrango]

sorry @aegershman let me re-run my example but the last time I ran with the exact POM above no changes were made.

[rdifrango]

ok, @aegershman - I re-ran it and seems to working now, so going to close the issue.