ChangeType does not work on J.ClassDeclaration

[rlsanders4]

What's changed?

A test case was added to demonstrate an issue with using the ChangeType recipe on a J.ClassDeclaration object.

What's your motivation?

ChangeType fails to change the class name for a J.ClassDeclaration object. This may be related to

• Update ChangeType and ChangePackage to work with SourceFileWithReference #4648
  since we noticed this new regression in a local recipe today.

Anyone you would like to review specifically?

@timtebeek

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[rlsanders4]

Test results (updated)

Failed:

ChangeTypeTest > classDeclarationTest() FAILED
    org.opentest4j.AssertionFailedError: expected: <GoodbyeClass> but was: <HelloClass>
        at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
        at app//org.openrewrite.java.ChangeTypeTest.classDeclarationTest(ChangeTypeTest.java:2087)

Passed:

ChangeTypeTest > compilationUnitTest()

[timtebeek]

As also detailed in Slack, we're exploring if this is perhaps due to the way this is tested, as a test like this passes:

@Issue("https://github.com/openrewrite/rewrite/issues/775")
@Test
void changeTypeInTypeDeclaration() {
    rewriteRun(
      spec -> spec.recipe(new ChangeType("de.Class2", "de.Class1", false)),
      java(
        """
          package de;
          public class Class2 {}
          """,
        """
          package de;
          public class Class1 {}
          """,
        spec -> spec.afterRecipe(cu -> {
            String simpleName = cu.getClasses().get(0).getSimpleName();
            assertThat(simpleName).isEqualTo("Class1");
        })
      )
    );
}

[Laurens-W]

Thanks for the runnable reproducer!
I looked into it and this happened due to the intermediate visitor that was introduced to determine whether we should apply the java visitor or the reference visitor. Restoring the previous behavior only took a small change to that.

We do however think this is an unintentional use of the ChangeType recipe and are curious if you could provide some more context (on slack) of the sequence of changes you're making so we can look at better supporting your use case!

[rlsanders4]

@Laurens-W Thanks for the fix here! We have two general use cases in which we call ChangeType on only the J.ClassDeclaration and not the entire J.CompilationUnit.

1. We have a series of recipes that generate a new file, replace the J.CompilationUnit of an existing file with the J.CompilationUnit from the generated file, and then make a few additional modifications. At several points during these changes, there are Java syntax errors in the LST (e.g. the file name does not match the public class name) that can cause issues with calling ChangeType or other recipes on the entire J.CompilationUnit. However, we know that the J.ClassDeclaration object itself is free of syntax errors, so we use ChangeType to update the J.ClassDeclaration and then we fix the rest of the J.CompilationUnit manually. This use case is fairly specific to our situation, but the ability to call ChangeType on just a J.ClassDeclaration does provide a lot more flexibility in these cases.

2. Sometimes we want to change a type only in one specific class in the J.CompilationUnit, but keep this type in any other classes that may exist in the file. This is a more general use case that others may run into as well.