What version of OpenRewrite are you using?

I am using

• rewrite-xml v8.57.0

How are you running OpenRewrite?

I am writing my own declarative Recipe using ChangeTagAttribute and test it.

What is the smallest, simplest way to reproduce the problem?

The easiest way is to create your own declarative recipe using ChangeTagAttribute with no oldValue set and then test it using the framework. However, we can cut out the middle man and just test ChangeTagAttribute directly:

class ChangeTagAttributeTest implements RewriteTest {

  @Override
  public void defaults(RecipeSpec spec) {
    spec.recipe(new ChangeTagAttribute("/tag", "attribute", "newValue", null, null));
  }

  @Test
  void correctSiteXmlWillBeFormatted() {
    rewriteRun(
        xml(
            """
                    <tag attribute="oldValue">
                    </tag>
                    """,
            """
                    <tag attribute="newValue">
                    </tag>
                    """));
  }
}

What did you expect to see?

I expect the recipe to successfully finish. The second cycle should not perform any changes because newValue matches the already present value.

What did you see instead?

The classic error message Expected recipe to complete in 1 cycle, but took 2 cycles..

Why do you think this is an error?

The documentation claims that all recipes should be idempotent and not perform any unnecessary changes. Indeed, a recipe that does not perform a change to the content of the file should not need touch anything in the second cycle. This basic recipe does just that which means that any recipe using it will do the same.

I am aware the method has a test that accepts the changes in the second cycle.
However, the commit that added that line indicated that this was more of a quick and dirty fix to make a Test succeed and not fail for something that is ultimately inconsequential for anyone USING the recipe. It seems to me it mostly affects people TESTING the recipe.

Does a workaround exist?

You can set newValue: someNewValue, oldValue: ^(?!someNewValue).*$, regex: true`. That way it will only do the replacement if oldValue is not an exact copy of newValue. But that's more of a dirty workaround than anything else.

Are you interested in [contributing a fix to OpenRewrite]

The fix should be simple (if the new value is the same as the old one, do nothing).
However, because I can't be sure how big of a ripple effect removing this behavior has I'm hesitant to just throw it in. I'm more than happy to do it if this bug is deemed a real one and I am not just misunderstanding how it should behave.