rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

RemoveRedundantDependencyVersions - restore ability to remove newer version pins

Open nmck257 opened this issue 1 year ago • 1 comments

What problem are you trying to solve?

Add an option for the functionality removed in this commit: https://github.com/openrewrite/rewrite/commit/687fbb55709b7d6ee0a648c425d8b0b94fce65d5 (from @sambsnyd )

What precondition(s) should be checked before applying this recipe?

N/A

Describe the situation before applying the recipe

              <project>
                  <modelVersion>4.0.0</modelVersion>
                  <groupId>org.sample</groupId>
                  <artifactId>sample</artifactId>
                  <version>1.0.0</version>
                  <parent>
                      <groupId>org.springframework.boot</groupId>
                      <artifactId>spring-boot-starter-parent</artifactId>
                      <version>2.7.18</version>
                      <relativePath/>
                  </parent>
                  <build>
                      <plugins>
                          <plugin>
                              <groupId>pl.project13.maven</groupId>
                              <artifactId>git-commit-id-plugin</artifactId>
                              <!-- version in pom is 4.9.10, pin is newer-->
                              <version>4.9.11</version>
                          </plugin>
                      </plugins>
                  </build>
              </project>

Describe the situation after applying the recipe

              <project>
                  <modelVersion>4.0.0</modelVersion>
                  <groupId>org.sample</groupId>
                  <artifactId>sample</artifactId>
                  <version>1.0.0</version>
                  <parent>
                      <groupId>org.springframework.boot</groupId>
                      <artifactId>spring-boot-starter-parent</artifactId>
                      <version>2.7.18</version>
                      <relativePath/>
                  </parent>
                  <build>
                      <plugins>
                          <plugin>
                              <groupId>pl.project13.maven</groupId>
                              <artifactId>git-commit-id-plugin</artifactId>
                          </plugin>
                      </plugins>
                  </build>
              </project>

Have you considered any alternatives or workarounds?

Any additional context

This (removed) behavior can be useful if you want to strictly rely on parent-provided versions, and implicitly "distrust" the validity of existing pinned versions (or anticipate that they will become a problem eventually as the parent version increases)

Are you interested in contributing this recipe to OpenRewrite?

Yes -- mainly opening this issue first in case anyone has design input.

The least-intrusive implementation would be a new, additional flag, such as removeNewerVersions, which works in conjunction with onlyIfVersionsMatch. But the naming/usage starts to get fuzzy -- what if someone sets onlyIfVersionsMatch true and also removeNewerVersions true? We can validate those cases out, but, it's unintuitive.

Could also argue that onlyIfVersionsMatch is now an inaccurate name, since on false, it does not remove non-matching newer versions.

Replacing this field with some kind of removalStrategy enum could be nice, with values like matching, older, any, but replacing an arg like that can be disruptive for existing usage, and naming the new arg also takes some good thought (I don't love the value names I just suggested, eg).

Eager for any opinions

nmck257 avatar Apr 16 '24 13:04 nmck257

maybe replace onlyIfVersionsMatch with onlyIfManagedVersionIs, with the value being a comparator symbol? eg =, <, >=? and maybe * for "remove any relevant version regardless of value"?

default would be = to match default behavior of existing arg; could also interpret any number of = characters all the same way in case someone has muscle memory for == or ===

or maybe avoiding special characters is desirable for yaml, and instead could do eq, lt, gte, etc

nmck257 avatar Apr 16 '24 17:04 nmck257