rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

Trailing comma in an array end up in white-space

Open dpozinen opened this issue 1 year ago • 2 comments

rewrite bom 2.6.4, wasn't an issue on 2.5.3

Test failures when there is a trailing comma in an annotation. If I remove it, tests pass.

java.lang.AssertionError: Source file was parsed into an LST that contains non-whitespace characters in its whitespace. This is indicative of a bug in the parser.

package com.playtika.services.infra.rewrite.spring;

import org.springframework.boot.test.context.SpringBootTest;

@SpringBootTest(
        properties = {
                "feign.circuitbreaker.enabled=true",
                "feign.circuitbreaker.group.enabled=true",
                "eureka.client.enabled=false"~~(non-whitespace)~~>,
        <~~},
        webEnvironment = SpringBootTest.WebEnvironment.NONE
)
public abstract class A {
}

So the comma at the end of properties in a test like this

    @Test
    void shouldChangePropertyKeyInAnnotationProperties() {
        rewriteRun(
                spec -> spec.recipe(new ChangePropertyKeyAnnotationAttributeValue(
                        "org.springframework.boot.test.context.SpringBootTest",
                        "properties", "feign.circuitbreaker.enabled",
                        "spring.cloud.openfeign.circuitbreaker.enabled")),
                java(
                        """
                                        package com.playtika.services.infra.rewrite.spring;

                                        import org.springframework.boot.test.context.SpringBootTest;

                                        @SpringBootTest(
                                                properties = {
                                                        "feign.circuitbreaker.enabled=true",
                                                        "feign.circuitbreaker.group.enabled=true",
                                                        "eureka.client.enabled=false", <-- THIS COMMA HERE
                                                },
                                                webEnvironment = SpringBootTest.WebEnvironment.NONE
                                        )
                                        public abstract class A {
                                        }
                                """
...
        ));
    }

dpozinen avatar Feb 13 '24 16:02 dpozinen

Hi! We recently added validation indeed to flush out LST issues in our parsers. It's helpful to improve the parsers, but indeed might cause a test to fail until we circle back to the parser. I'd encountered this case as well and logged a test last week https://github.com/openrewrite/rewrite/commit/78a55a46056808b4b630e781f39bad112abd3d63

We can keep your issue open to see that parser issue resolved; in the meantime you can remove that trailing comma from the test. Hope that helps!

timtebeek avatar Feb 13 '24 17:02 timtebeek

And perhaps good to note: This is only an internal modeling issue; your source code will still be reproduced faithfully even with that modeling mishap, as we have a separate check for that. So rest assured that any code changes will be just fine; it's only a matter of using compatible examples in tests until the parser correctly models the code.

timtebeek avatar Feb 14 '24 13:02 timtebeek