Ability to rewrite breaking changes

alexey-anufriev commented 2 weeks ago

What problem are you trying to solve?

In my case I distributed a library along with rewrite plugin. For releases when a library contains breaking changes I bundle respective recipe that should fix breaking changes and streamline adoption of new major versions of the lib.

Unfortunately, since rewrite plugin requires maven to run all phases until process-test-classes it also does compile pahse. But since I have breaking changes it fails.

Describe the solution you'd like

a. Additional goal, like earlyRun that runs at generate-sources for example, and does not require compilation. b. Configuration for the phase.

Additional context

https://github.com/openrewrite/rewrite-maven-plugin/blob/main/src/main/java/org/openrewrite/maven/RewriteRunMojo.java#L29

Are you interested in contributing this feature to OpenRewrite?

Yes.

timtebeek commented 2 weeks ago

hi! We already have ways of handling breaking changes across different versions of libraries, as documented here. It'll come down to how you write your recipes, and what instructions you give folks on how to upgrade.

We've found it best to have a project that compiles before running a recipe; that way we can be sure to have complete type information for any matching that needs to happen on LST elements to then reliably make changes. An earlyRun would have no such guarantees, and would lead to a more fragile migration process, given the nature of how we operate.

If your library is public feel free to link that here such that we can see what accommodations would help you fit that model. Or chime into our Slack for a synchronous answer to any follow up questions you might have.

alexey-anufriev commented 2 weeks ago

hi @timtebeek, than you for the reply!

Unfortunately my lib is private so I cannot share the concrete case, but I can give an example: class has been renamed. Old one does not exists anymore, and we have just a new one. In this case project cannot compile, and recipe cannot run.

Keeping two classes for gradual migration is not an option.

alexey-anufriev commented 2 weeks ago

Hm, I have just tried to run runNoFork and was able to apply breaking changes. Does it work as expected?

timtebeek commented 2 weeks ago

I'm surprised to hear you're seeing any other results with runNoFork; the basic flow I'd expect is lets' say

You have a project A that uses library X with version 1.
You develop a version 2 of library X, with a breaking change, without backwards compatibility
You develop a recipe that automates that breaking change, and distribute that in a recipe module
You run the recipe agianst an unmodified project A that still uses library v1
The recipe applies the breaking change and bumps the library version to v2

That way there's no point where project A does not compile, and you're able to adopt breaking changes.

Does that help clarify the flow to give you confidence in automating breaking changes?

timtebeek commented 2 weeks ago

I've also logged this issue to improve our documentation; so if you have any suggestions for improvement please chime in there

https://github.com/openrewrite/rewrite-docs/issues/287

alexey-anufriev commented 2 weeks ago

I'm surprised to hear you're seeing any other results with runNoFork; the basic flow I'd expect is lets' say

You have a project A that uses library X with version 1.

You develop a version 2 of library X, with a breaking change, without backwards compatibility

You develop a recipe that automates that breaking change, and distribute that in a recipe module

You run the recipe agianst an unmodified project A that still uses library v1

The recipe applies the breaking change and bumps the library version to v2

That way there's no point where project A does not compile, and you're able to adopt breaking changes.

Does that help clarify the flow to give you confidence in automating breaking changes?

In my case version bump comes automatically, with Dependabot PR. And then, within the same PR, developer can run a migration that came with a new version of the lib and adjust the code to make it work, then commit extra changes and now PR should be ready to be merged.

timtebeek commented 2 weeks ago

Ah that's helpful context as to how you ended up with a broken project in the first place. Thanks! I can't speak to the specifics of Dependabot or how to recipes into a flow there; I do see there's a couple long standing open issue around pre commit hooks that don't give a lot of confidence those will be resolved soon.

From our end we require a project to compile before we're able to reliably match LST elements for replacements. If external tools bump the dependency early, then the only way to make that work would be with a backwards compatible intermediate version containing both the old and new class.

As an alternative you can of course run recipes proactively on your main branches to both bump the dependency version as well as make the required breaking changes. We develop tooling at Moderne to help you write such recipes and execute them across thousands of repositories, and push up your own pull requests from there. Might be worth evaluating if this is a good fit for you there.

I hope that any of the above context is helpful to you there; at this moment I don't see any feasible actions for us to handle non-compiling projects in a reliable fashion, as the missing type information will not allow you to match the now missing older classes you want to change to their moved replacements.

alexey-anufriev commented 2 weeks ago

As you already explained:

An earlyRun would have no such guarantees, and would lead to a more fragile migration process, given the nature of how we operate.

so if this is not acceptable I can understand this and I would not repeat my proposal again.

The only thing that makes me curious is noFrok mode. Have you tried this? For me, it is a good workaround that I still can use.

timtebeek commented 2 weeks ago

I'm not sure how the noFork mode would be any different; especially in CI environments where there would not be any previous cache with access to the older classes in the previous version of the dependency for type attribution. If that works for you then I'd consider it an undocumented unstable workaround, but not something I'd be confident in building upon. The flow we recommend and support is outlined above; I hope that you'll find a way to make that work for you as well.

Please don't take the above as harsh; online communication can seem colder than intended; it's meant as a friendly way to guide towards established patterns taking into account how we OpenRewrite operates, to build out something you can be sure works in all cases. With that I hope you agree with me closing the issue, as I don't think there's anything else we can do here.

alexey-anufriev commented 2 weeks ago

Thank you @timtebeek for support! Appreciate your help.

openrewrite / rewrite-maven-plugin