openrewrite / rewrite-kotlin

Work-in-progress implementation of Kotlin language support for OpenRewrite.
Apache License 2.0
43 stars 12 forks source link

TabsAndIndentVisitor misbehaves when visiting annotations individually #546

Closed nmck257 closed 8 months ago

nmck257 commented 9 months ago

What version of OpenRewrite are you using?

I am using

How are you running OpenRewrite?

RewriteTest

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

@Test
void foo() {
    rewriteRun(
            spec -> spec.recipe(RewriteTest.toRecipe(() -> new KotlinIsoVisitor<>() {
                @Override
                public J.Annotation visitAnnotation(J.Annotation annotation, ExecutionContext executionContext) {
                    J.Annotation a = super.visitAnnotation(annotation, executionContext);
                    return autoFormat(a, executionContext);
                }
            })).parser(KotlinParser.builder().logCompilationWarningsAndErrors(true)),
            kotlin("""
                    package org.sample

                    @SafeVarargs
                    @SuppressWarnings
                    class Foo {
                        val x =
                            3.plus(
                                3
                            )
                    }
                    """
            ));
}

What did you expect to see?

Success

What did you see instead?

org.opentest4j.AssertionFailedError: [The recipe must not make changes to "openRewriteFile.kt"] 
expected: "package org.sample

@SafeVarargs
@SuppressWarnings
class Foo {
    val x =
        3.plus(
            3
        )
}"
 but was: "package org.sample

@SafeVarargs
    @SuppressWarnings
class Foo {
    val x =
        3.plus(
            3
        )
}"

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

N/A

Are you interested in contributing a fix to OpenRewrite?

I probably could, if it feels like a low-priority which y'all aren't eager to fix.

The test case probably seems a bit contrived, but, an existing rewrite-spring recipe (ReplaceExtendWithAndContextConfiguration) does it: https://github.com/openrewrite/rewrite-spring/blob/847804059e1b568026a155cb17f3e732ad2fe6be/src/main/java/org/openrewrite/java/spring/boot2/ReplaceExtendWithAndContextConfiguration.java#L96

Arguably, that recipe is autoformatting a bit over-eagerly, but I think the scenario discovered here is still valid.

timtebeek commented 9 months ago

Really helpful to have the test provided, and a link to the Spring recipe that shows it's not an edge case. I'd say this is still a very valid one to pick up, as we should not create any unnecessary output changes. Help would be appreciated if possible!