rewrite icon indicating copy to clipboard operation
rewrite copied to clipboard

`StackOverflowError` on `@java.lang.annotation.Documented` due to self reference

Open jow290764 opened this issue 9 months ago • 4 comments

What version of OpenRewrite are you using?

I am using rewrite-java 8.23.1 through importing 2.9.0 rewrite-recipe-bom

How are you running OpenRewrite?

In production we plan to use the Maven plugin, and projects there are usually multi module projects, however I can reproduce the error using a junit test in a single module setup

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

The following produces a stackoverflow when used with MyRecipe. For details about MyRecipe see the attached maven project, which contains a downsized version of our original recipe.

	@Test
	void testReproduce() {
		rewriteRun(
			java(
				"""
					package com.yourorg;

					import java.io.Serializable;
					import javax.swing.JFrame;
					import de.parcit.base.resource.bl.shared.ResourceHelper;
					
					class A extends JFrame implements Serializable {
					}
				""",
				"""
					package com.yourorg;

					import java.io.Serializable;
					import javax.swing.JFrame;
					import de.parcit.base.resource.bl.shared.ResourceHelper;
					
					class A extends JFrame implements Serializable, Cloneable {
					}
				"""
			)
		);
		Assertions.assertTrue(true);
	}

What did you expect to see?

I did expect to not encounter a StackOverflowError caused by the self referencing @Documented annotation on JavaBean annotation on JFrame

@JavaBean(defaultProperty = "JMenuBar", description = "A toplevel window which can be minimized to an icon.")
@SwingContainer(delegate = "getContentPane")
@SuppressWarnings("serial") // Same-version serialization only
public class JFrame  extends Frame implements WindowConstants,
@Documented
@Target({TYPE})
@Retention(RUNTIME)
public @interface JavaBean {

The self reference from Documented to Documented causes the StackOverflowError

@Documented
@Retention(RetentionPolicy.RUNTIME)
@Target(ElementType.ANNOTATION_TYPE)
public @interface Documented {
}

What did you see instead?

I see a StackOverflowError when replacing interface implementations

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

too long, but includes repetitions of the following till StackOverflow is encountered

    at org.openrewrite.java.JavaTypeVisitor.visitClass (JavaTypeVisitor.java:121)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:188)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:185)
    at org.openrewrite.java.JavaTypeVisitor.visit (JavaTypeVisitor.java:75)
    at org.openrewrite.java.JavaTypeVisitor.lambda$visitClass$2 (JavaTypeVisitor.java:121)
    at org.openrewrite.internal.ListUtils.map (ListUtils.java:176)
    at org.openrewrite.java.JavaTypeVisitor.visitClass (JavaTypeVisitor.java:121)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:188)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:185)
    at org.openrewrite.java.JavaTypeVisitor.visit (JavaTypeVisitor.java:75)
    at org.openrewrite.java.JavaTypeVisitor.lambda$visitClass$2 (JavaTypeVisitor.java:121)
    at org.openrewrite.internal.ListUtils.map (ListUtils.java:176)
    at org.openrewrite.java.JavaTypeVisitor.visitClass (JavaTypeVisitor.java:121)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:188)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:185)
    at org.openrewrite.java.JavaTypeVisitor.visit (JavaTypeVisitor.java:75)
    at org.openrewrite.java.JavaTypeVisitor.lambda$visitClass$2 (JavaTypeVisitor.java:121)
    at org.openrewrite.internal.ListUtils.map (ListUtils.java:176)
    at org.openrewrite.java.JavaTypeVisitor.visitClass (JavaTypeVisitor.java:121)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:188)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:185)
    at org.openrewrite.java.JavaTypeVisitor.visit (JavaTypeVisitor.java:75)
    at org.openrewrite.java.JavaTypeVisitor.lambda$visitClass$2 (JavaTypeVisitor.java:121)
    at org.openrewrite.internal.ListUtils.map (ListUtils.java:176)
    at org.openrewrite.java.JavaTypeVisitor.visitClass (JavaTypeVisitor.java:121)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:188)
    at org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1$1.visitClass (JavaTemplateJavaExtension.java:185)
    at org.openrewrite.java.JavaTypeVisitor.visit (JavaTypeVisitor.java:75)
    at org.openrewrite.java.JavaTypeVisitor.lambda$visitClass$2 (JavaTypeVisitor.java:121)
    at org.openrewrite.internal.ListUtils.map (ListUtils.java:176)

Are you interested in contributing a fix to OpenRewrite?

Would have to ask my employer.

jow290764 avatar Apr 30 '24 14:04 jow290764

I can't see my uploaded zip containing a small test case for reproduction, but I hope you have received it.

jow290764 avatar Apr 30 '24 14:04 jow290764

Oh wow, thanks for not including the full stacktrace in the case of a stack overflow. 😅 Weird to see that self referencing documented annotation causing issues here; definitely something we should fix in our parser. /cc @knutwannheden

I suspect we might even be able to trim down the reproduction test even further, just looking at how that's defined. https://docs.oracle.com/javase/8/docs/api/java/lang/annotation/Documented.html

Thanks a lot for the level of detail in your report; much appreciated that we immediately have all the details visible here.

timtebeek avatar Apr 30 '24 14:04 timtebeek

Hmm; I've tried to replicate this with a smaller unit test added to JavaParserTest, but no luck so far with

    @Test
    @Issue("https://github.com/openrewrite/rewrite/issues/4161")
    void documentedSelfReference() {
        rewriteRun(
          java(
            """
              import javax.swing.JFrame;
              class A extends JFrame {
              }
              """
          )
        );
    }

The reason I'm trying to replicate with a smaller sample is because I don't have access to the de.parcit.base classes listed above, and I wonder if those factor in to the failures you're seeing. Are you able to produce a test that fails without using external dependencies?

timtebeek avatar May 02 '24 10:05 timtebeek

Sorry for not responding right away. I was on vacation for a few days.

Yes, I am able to reproduce without external dependencies. For your convenience I have added an updated test case: OpenRewriteProblem-2.zip OpenRewriteProblem-2.zip

The reason why your test is not reproducing the problem is probably because you are not trying to apply the template Templates.implementCloneableTemplate included in the test case. The problem happens during template application.

classDecl = Templates.implementCloneableTemplate.apply( updateCursor(classDecl), implementsClauseExists ? classDecl.getCoordinates().replaceImplementsClause() : classDecl.getCoordinates().addImplementsClause());

jow290764 avatar May 06 '24 07:05 jow290764

Looking over the zip I'm wondering if it'd be possible to replicate this with a self contained unit test; now it still refers to an external recipe, two visitors and a utility class. Ideally it'd be a a single unit test using spec-> spec.recipe(toRecipe(() -> new JavaVisitor<>(){ ... })), stripped from anything not necessary to replicate. As much as the zip is appreciated, I'm not finding the time to dive in there to resolve this among all the other items.

timtebeek avatar Aug 12 '24 16:08 timtebeek

I can replicate this as well with a nonsense example (excluding any preconditions and just mutating the class declaration directly):

package com.sample.test;

import org.junit.jupiter.api.Test;
import org.openrewrite.ExecutionContext;
import org.openrewrite.java.JavaIsoVisitor;
import org.openrewrite.java.JavaParser;
import org.openrewrite.java.JavaTemplate;
import org.openrewrite.java.tree.J;
import org.openrewrite.test.RecipeSpec;
import org.openrewrite.test.RewriteTest;

import static org.openrewrite.java.Assertions.java;

public class StackOverFlowErrorOnAnnotation implements RewriteTest {

    @Override
    public void defaults(RecipeSpec spec) {
        spec.recipe(RewriteTest.toRecipe(() -> {
            return new JavaIsoVisitor<>() {

                @Override
                public J.ClassDeclaration visitClassDeclaration(J.ClassDeclaration classDecl, ExecutionContext executionContext) {
                    J.ClassDeclaration classDeclaration = super.visitClassDeclaration(classDecl, executionContext);
                    classDeclaration = JavaTemplate
                      .builder("Object")
                      .imports("java.lang.Object")
                      .javaParser(JavaParser.fromJavaVersion())
                      .build()
                      .apply(getCursor(), classDeclaration.getCoordinates().addImplementsClause());

                    return classDeclaration;
                }
            };
        }))
          .parser(JavaParser.fromJavaVersion()
            .classpath("junit-jupiter-api"));
    }

    @Test
    void stackOverflowOnImplements() {
        rewriteRun(
          // language=java
          java(
            """
              package com.sample.test;

              import org.junit.jupiter.api.DisplayName;

              @DisplayName("GIVEN a test with a '@DisplayName' annotation")
              abstract class MyClassWithDisplayNameAnnotation {}
              """,
            """
              package com.sample.test;

              import org.junit.jupiter.api.DisplayName;

              @DisplayName("GIVEN a test with a '@DisplayName' annotation")
              abstract class MyClassWithDisplayNameAnnotation implements Object {}
              """
          )
        );
    }

}

The DisplayName annotation is also annotated with the self referencing Documented annotation.

darthblonderss avatar Aug 21 '24 19:08 darthblonderss

Thanks a lot @darthblonderss ; great to have a self-contained example. Did you already briefly explore a cause or fix?

timtebeek avatar Aug 21 '24 20:08 timtebeek