[karmakaze]

> I did consider two other approaches:

  Changing Ruleset from being []Rule to []*Rule, which would mean we no longer need to explicitly take a reference to the rule.
  Returning a Rule rather than a *Rule. This would still copy the Rule, but it should stay on the stack instead of moving to the heap.

> However, both of these would have resulted in a breaking change as this method is part of the public API.

The problem with heap allocated objects could be due to the incorrect public API.

The change that improves performance also gives out pointers to the actual elements of Ruleset itself permitting the caller to change the contents of Ruleset which wasn't possible before the speed-up. Perhaps you're already aware since change to []*Rule was being considered.

[mook]

The best part is, there's a subtle API breakage here: the returned *Rule is now a reference to an element of the []Rule, so if the caller was previously modifying the returned value it'll change the slice.

It's debatable what API guarantees existed around this though; most of the time this would be unspecified.

[ok_dad]

Sometimes it doesn't matter if a public API is incorrect, because it's set in stone for whatever reason, and you just need to fix the problem internally.

[spockz]

This is why I like https://github.com/openrewrite so much. One gets to tell users how to rewrite code automatically. It makes refactoring almost as easy as in a mono repo.

[bspammer]

This looks super interesting, but as far as I can tell they don’t go into how to inform downstream users to add your recipes to their maven/gradle configuration when you make a breaking change.

In my head, the ideal flow would be an annotation on your library function/class which triggers an IntelliJ suggestion for downstream users affected by your breaking change to run the automated refactor for them. Kinda like a more helpful @Deprecated.

[spockz]

It would definitely be nice to have this generated from a deprecated like annotation!

The maven plug-in seems to use the recipes directly as dependencies: https://github.com/openrewrite/rewrite-maven-plugin

In our internal situation the parent Pom could already control this plug-in definition including versions of the recipes. At the very least the recipes could follow the same version as the artefact.

I thought I saw somewhere where the recipe was bundled with the artefact itself. That is very neat for simple usecase.

However, it suffers from the same flaw as the native-image configuration for GraalVM in artifacts. Sometimes the configuration/recipe needs to change. Eg because of new insights or because it is incompatible with new versions of GraalVM/open rewrite. So I suppose having an extra version dimension Could solve this. Then one can always depend on the latest version of the recipe for that artifact.

[karmakaze]

The way to fix it in that manner here is to undo the 42% speedup and return the heap allocated object for the caller to mangle.

[account42]

Yes, if you are serious about backwards compat then you pay that cost and perhaps add a separate function that is faster for callers to opt into.

[karmakaze]

Effectively deprecating the old one and recommending the new one being a way to smoothly 'fix' an API.