rewrite-spring icon indicating copy to clipboard operation
rewrite-spring copied to clipboard

UnnecessarySpringExtension recipe removes too much - Only SpringExtension.class should be removed

Open maecval opened this issue 11 months ago • 2 comments

What version of OpenRewrite are you using?

I am using

  • OpenRewrite 8.45.4
  • Maven plugin 6.1.3 (rewrite-maven-plugin:6.1.3)
  • rewrite-spring 6.1.0

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project. Command used to run it:

mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-spring:RELEASE -Drewrite.activeRecipes=org.openrewrite.java.spring.boot2.UnnecessarySpringExtension -Drewrite.exportDatatables=true

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

If a SpringBootTest class has an additional ExtendWith annotation which contains additional test extension classes the recipe removes the whole line istsead of just removing the SpringExtension.class and keeping the rest. Status before execution of recipe:

@ExtendWith({ SpringExtension.class, PactConsumerTestExt.class})
@SpringBootTest(...)
public class AnyTest {
    ...
}

What did you expect to see?

expected status after execution:

@ExtendWith({ PactConsumerTestExt.class})
@SpringBootTest(...)
public class AnyTest {
    ...
}

What did you see instead?

status after execution of recipe:

@SpringBootTest(...)
public class AnyTest {
    ...
}

What is the full stack trace of any errors you encountered?

Are you interested in contributing a fix to OpenRewrite?

Maybe :-)

maecval avatar Feb 14 '25 11:02 maecval

hi @maecval ; Thanks for the report! Can indeed confirm the issue with this unit test added to https://github.com/openrewrite/rewrite-spring/blob/70153755a1077235c1064e01b0703898ce0a650d/src/testWithSpringBoot_2_4/java/org/openrewrite/java/spring/boot2/UnnecessarySpringExtensionTest.java#L29

@Issue("https://github.com/openrewrite/rewrite-spring/issues/678")
@Test
void retainSecondary() {
    //language=java
    rewriteRun(
      java(
        """
          import org.junit.jupiter.api.extension.BeforeAllCallback;
          import org.junit.jupiter.api.extension.ExtensionContext;
          class AnotherExtension implements BeforeAllCallback {
              void beforeAll(ExtensionContext context) {
              }
          }
          """,
        SourceSpec::skip
      ),
      java(
        """
          import org.junit.jupiter.api.extension.ExtendWith;
          import org.springframework.boot.test.context.SpringBootTest;
          import org.springframework.test.context.junit.jupiter.SpringExtension;

          @SpringBootTest
          @ExtendWith({SpringExtension.class, AnotherExtension.class})
          class Test {
          }
          """,
        """
          import org.springframework.boot.test.context.SpringBootTest;

          @SpringBootTest
          @ExtendWith({AnotherExtension.class})
          class Test {
          }
          """
      )
    );
}

timtebeek avatar Feb 15 '25 00:02 timtebeek

Would welcome a draft PR that adds that test, and then whatever you can do to explore a fix. I imagine we'll want to explore the ExtendsWith annotation here: https://github.com/openrewrite/rewrite-spring/blob/70153755a1077235c1064e01b0703898ce0a650d/src/main/java/org/openrewrite/java/spring/boot2/UnnecessarySpringExtension.java#L84

As a simplest case we could not make any change at all when there is more than one argument. If you want to go more advanced you can remove the target argument from the annotation.

timtebeek avatar Feb 15 '25 00:02 timtebeek