spring-projects / sts4

The next generation of tooling for Spring Boot, including support for Cloud Foundry manifest files, Concourse CI pipeline definitions, BOSH deployment manifests, and more... - Available for Eclipse, Visual Studio Code, and Theia
https://spring.io/tools
Eclipse Public License 1.0
869 stars 202 forks source link

refactoring recipe doesn't work with compile errors #1064

Open martinlippert opened 1 year ago

martinlippert commented 1 year ago

Steps to reproduce:

---
type: specs.openrewrite.org/v1beta/recipe
name: com.yourorg.AdaptToNewVersionOfLibrary
displayName: Adapt to changed method of Movie Service
recipeList:
  - org.openrewrite.java.ChangeMethodName:
      methodPattern: com.example.testlib.library.MovieService getMovies(..)
      newMethodName: getAllMovies

But when I execute this recipe, it doesn't do anything.

When I switch back to use the old version of the library, the compile errors go away again. Then, I can execute the recipe and it switches my method invocations to the new method name. Then, I can change the dependency in the pom.xml file to point to the new version and everything is fine, but the flow is still problematic.

Why is the recipe not able to update the method invocation? I guess this is due to the OpenRewrite parsing not being able to resolve the method invocation (this the old method doesn't exist anymore) and therefore doesn't match that against the method pattern in the recipe.

BoykoAlex commented 1 year ago

Rewrite recipes only work with all the code compiling and hence all types properly resolved. This also includes rewrite's recipe ChangeMethodName. However, this limitation does not mean that we cannot write a recipe that would work for the case above. The method's type would be "unknown" but the method target's type i think should be resolved as well as parameters. Therefore, the method name + types of the target and parameters should help to build the method signature for the method matcher. It wouldn't be easy if type of the target or types of parameters were not resolved though. This can be implemented as a separate recipe or perhaps included into the ChangeMethodName recipe...

martinlippert commented 1 year ago

Understand, but it seems to be like the fundamental design of OpenRewrite isn't really made for cases like this. But I would love to see a way to make things like this work in more general. Probably something to discuss with the OpenRewrite team.