openrewrite / rewrite

Automated mass refactoring of source code.
https://docs.openrewrite.org
Apache License 2.0
2.22k stars 330 forks source link

StackOverflowError for switch expression in JavaVisitor #2357

Closed thomaszub closed 1 year ago

thomaszub commented 2 years ago

Hi,

parsing a switch expression leads to a StackOverflowError. Visiting the switch expression in JavaVisitor in line 1043 calls getType of J.SwitchExpression. This function creates a new JavaVisitor which calls visitSwitchExpression during traversing.

Kind regards Thomas

wjglerum commented 1 year ago

I encountered the same problem on some of my projects. And I can confirm it's due to the switch expressions. Any ideas how to fix this or to work around it?

fprochazka commented 1 year ago
same problem :-/ ``` [ERROR] Failed to execute goal org.openrewrite.maven:rewrite-maven-plugin:4.37.0:run (rewrite) on project core: Execution rewrite of goal org.openrewrite.maven:rewrite-maven-plugin:4.37.0:run failed: java.lang.StackOverflowError -> [Help 1] org.apache.maven.lifecycle.LifecycleExecutionException: Failed to execute goal org.openrewrite.maven:rewrite-maven-plugin:4.37.0:run (rewrite) on project core: Execution rewrite of goal org.openrewrite.maven:rewrite-maven-plugin:4.37.0:run failed: java.lang.StackOverflowError at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:375) at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:351) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:215) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:171) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:163) at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117) at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81) at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56) at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128) at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:294) at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192) at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105) at org.apache.maven.cli.MavenCli.execute (MavenCli.java:960) at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293) at org.apache.maven.cli.MavenCli.main (MavenCli.java:196) at jdk.internal.reflect.DirectMethodHandleAccessor.invoke (DirectMethodHandleAccessor.java:104) at java.lang.reflect.Method.invoke (Method.java:578) at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282) at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225) at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406) at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347) at org.codehaus.classworlds.Launcher.main (Launcher.java:47) Caused by: org.apache.maven.plugin.PluginExecutionException: Execution rewrite of goal org.openrewrite.maven:rewrite-maven-plugin:4.37.0:run failed: java.lang.StackOverflowError at org.apache.maven.plugin.DefaultBuildPluginManager.executeMojo (DefaultBuildPluginManager.java:148) at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute2 (MojoExecutor.java:370) at org.apache.maven.lifecycle.internal.MojoExecutor.doExecute (MojoExecutor.java:351) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:215) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:171) at org.apache.maven.lifecycle.internal.MojoExecutor.execute (MojoExecutor.java:163) at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:117) at org.apache.maven.lifecycle.internal.LifecycleModuleBuilder.buildProject (LifecycleModuleBuilder.java:81) at org.apache.maven.lifecycle.internal.builder.singlethreaded.SingleThreadedBuilder.build (SingleThreadedBuilder.java:56) at org.apache.maven.lifecycle.internal.LifecycleStarter.execute (LifecycleStarter.java:128) at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:294) at org.apache.maven.DefaultMaven.doExecute (DefaultMaven.java:192) at org.apache.maven.DefaultMaven.execute (DefaultMaven.java:105) at org.apache.maven.cli.MavenCli.execute (MavenCli.java:960) at org.apache.maven.cli.MavenCli.doMain (MavenCli.java:293) at org.apache.maven.cli.MavenCli.main (MavenCli.java:196) at jdk.internal.reflect.DirectMethodHandleAccessor.invoke (DirectMethodHandleAccessor.java:104) at java.lang.reflect.Method.invoke (Method.java:578) at org.codehaus.plexus.classworlds.launcher.Launcher.launchEnhanced (Launcher.java:282) at org.codehaus.plexus.classworlds.launcher.Launcher.launch (Launcher.java:225) at org.codehaus.plexus.classworlds.launcher.Launcher.mainWithExitCode (Launcher.java:406) at org.codehaus.plexus.classworlds.launcher.Launcher.main (Launcher.java:347) at org.codehaus.classworlds.Launcher.main (Launcher.java:47) Caused by: org.openrewrite.internal.RecipeRunException: java.lang.StackOverflowError at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:326) at org.openrewrite.TreeVisitor.reduce (TreeVisitor.java:238) at org.openrewrite.java.tree.J$SwitchExpression.getType (J.java:4653) at org.openrewrite.java.JavaVisitor.visitSwitchExpression (JavaVisitor.java:1042) at org.openrewrite.java.tree.J$SwitchExpression.acceptJava (J.java:4665) at org.openrewrite.java.tree.J.accept (J.java:64) at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:278) at org.openrewrite.TreeVisitor.reduce (TreeVisitor.java:238) at org.openrewrite.java.tree.J$SwitchExpression.getType (J.java:4653) at org.openrewrite.java.JavaVisitor.visitSwitchExpression (JavaVisitor.java:1042) ... at org.openrewrite.java.tree.J$SwitchExpression.acceptJava (J.java:4665) at org.openrewrite.java.tree.J.accept (J.java:64) at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:278) at org.openrewrite.TreeVisitor.reduce (TreeVisitor.java:238) at org.openrewrite.java.tree.J$SwitchExpression.getType (J.java:4653) at org.openrewrite.java.JavaVisitor.visitSwitchExpression (JavaVisitor.java:1042) Caused by: java.lang.StackOverflowError at java.lang.Exception. (Exception.java:103) at java.lang.RuntimeException. (RuntimeException.java:97) at org.openrewrite.internal.RecipeRunException. (RecipeRunException.java:31) at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:326) at org.openrewrite.TreeVisitor.visitAndCast (TreeVisitor.java:358) at org.openrewrite.java.JavaVisitor.visitSwitchExpression (JavaVisitor.java:1040) at org.openrewrite.java.tree.J$SwitchExpression.acceptJava (J.java:4665) at org.openrewrite.java.tree.J.accept (J.java:64) at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:278) at org.openrewrite.TreeVisitor.reduce (TreeVisitor.java:238) at org.openrewrite.java.tree.J$SwitchExpression.getType (J.java:4653) at org.openrewrite.java.JavaVisitor.visitSwitchExpression (JavaVisitor.java:1042) ... at org.openrewrite.java.tree.J$SwitchExpression.acceptJava (J.java:4665) at org.openrewrite.java.tree.J.accept (J.java:64) at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:278) at org.openrewrite.TreeVisitor.reduce (TreeVisitor.java:238) at org.openrewrite.java.tree.J$SwitchExpression.getType (J.java:4653) at org.openrewrite.java.JavaVisitor.visitSwitchExpression (JavaVisitor.java:1042) at org.openrewrite.java.tree.J$SwitchExpression.acceptJava (J.java:4665) at org.openrewrite.java.tree.J.accept (J.java:64) at org.openrewrite.TreeVisitor.visit (TreeVisitor.java:278) at org.openrewrite.TreeVisitor.reduce (TreeVisitor.java:238) ```
traceyyoshima commented 1 year ago

Hi @thomaszub, @wjglerum, @fprochazka thank you for reporting the issue!

Our switch parse tests, seem to work fine on java 8, 11, and 17. Do you have an example in the code where this occurs?

thomaszub commented 1 year ago

Hi @traceyyoshima,

this error occurs in JavaVisitor. Modifying the test in JavaVisitorTest triggers the error:

/*
 * Copyright 2020 the original author or authors.
 * <p>
 * Licensed under the Apache License, Version 2.0 (the "License");
 * you may not use this file except in compliance with the License.
 * You may obtain a copy of the License at
 * <p>
 * https://www.apache.org/licenses/LICENSE-2.0
 * <p>
 * Unless required by applicable law or agreed to in writing, software
 * distributed under the License is distributed on an "AS IS" BASIS,
 * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
 * See the License for the specific language governing permissions and
 * limitations under the License.
 */
package org.openrewrite.java

import org.junit.jupiter.api.Test
import org.openrewrite.ExecutionContext
import org.openrewrite.java.tree.J

interface JavaVisitorTest : JavaRecipeTest {

    @Suppress("RedundantThrows")
    @Test
    fun javaVisitorHandlesPaddedWithNullElem() = assertChanged(
        recipe = toRecipe {
            object : JavaIsoVisitor<ExecutionContext>() {
                override fun visitMethodInvocation(
                    method: J.MethodInvocation,
                    p: ExecutionContext
                ): J.MethodInvocation? {
                    val mi = super.visitMethodInvocation(method, p)
                    if ("removeMethod" == mi.simpleName) {
                        return null
                    }
                    return mi
                }
            }
        }.doNext(
            toRecipe {
                object : JavaIsoVisitor<ExecutionContext>() {
                    override fun visitMethodDeclaration(
                        method: J.MethodDeclaration,
                        p: ExecutionContext
                    ): J.MethodDeclaration {
                        var md = super.visitMethodDeclaration(method, p)
                        if (md.simpleName == "allTheThings") {
                            md = md.withTemplate(JavaTemplate.builder({ cursor }, "Exception").build(),
                                md.coordinates.replaceThrows())
                        }
                        return md
                    }
                }
            }
        ),
        before = """
            class A {
                void allTheThings() {
                    var val = switch("abc") {
                        case "a" -> 1;
                        case "b" -> 2;
                        default -> 0;
                    };
                    doSomething();
                    removeMethod();
                }
                void doSomething() {}
                void removeMethod() {}
            }
        """,
        after = """
            class A {
                void allTheThings() throws Exception {
                    doSomething();
                }
                void doSomething() {}
                void removeMethod() {}
            }
        """,
        cycles = 2,
        expectedCyclesThatMakeChanges = 2
    )
}
fprochazka commented 1 year ago

@traceyyoshima

Do you have an example in the code where this occurs?

That's something of a challenge, since the exceptions don't contain any messages and no debug info even with mvn -X is printed, with information on what source file is the plugin processing, so I don't know how to get you the sample easily. I would have to start removing code randomly from the affected maven module and it's quite large.

How can one get the sample in situations like this?

wjglerum commented 1 year ago

Hi @thomaszub, @wjglerum, @fprochazka thank you for reporting the issue!

Our switch parse tests, seem to work fine on java 8, 11, and 17. Do you have an example in the code where this occurs?

I could try to extract a little sample from our codebase later today. I have a small project where we only use 1 or 2 switch expressions, so that should be fairly easy to do.

traceyyoshima commented 1 year ago

Hi @thomaszub @fprochazka @wjglerum, thank you all for responding so quickly -- The test Thomas shared reproduces the issue. I'll use what he provided to track down the minimal amount of code to cause the SOE and look at this today.

wjglerum commented 1 year ago

Hi @thomaszub @fprochazka @wjglerum, thank you all for responding so quickly -- The test Thomas shared reproduces the issue. I'll use what he provided to track down the minimal amount of code to cause the SOE and look at this today.

Wow that was quick! I was about to look into a reproducer, but you already managed to provide a fix. Any chance that I could try this out locally already with a new build or version of openrewrite?

tkvangorder commented 1 year ago

You can consume snapshots of OpenRewrite via the sonatype snapshot repository:

https://oss.sonatype.org/content/repositories/snapshots/

We do not publish the recipe bom in snapshots, so you will need to explicitly set the various snapshot versions of the rewrite artifacts.