Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

error: MigrateCollectionsSingletonList recipe error #186

Closed
kunli2 opened this issue Mar 2, 2023 · 1 comment · Fixed by #187
Closed

error: MigrateCollectionsSingletonList recipe error #186

kunli2 opened this issue Mar 2, 2023 · 1 comment · Fixed by #187
Assignees
Labels
bug Something isn't working

Comments

@kunli2
Copy link
Contributor

kunli2 commented Mar 2, 2023

Got this error when running recipe org.openrewrite.java.migrate.UpgradeToJava17 on repo https://github.com/spring-projects/spring-amqp

Error:

		connectionFactory.setConnectionListeners(Collections.singletonList(new ConnectionListener() {
			@Override
			public void onCreate(Connection connection) {
@ -79,7 +79,7 @@
				called.decrementAndGet();
			}
		})

Message:

Expected a template that would generate exactly one statement to replace one statement, but generated 2. Template:
List.of(__P__.<org.springframework.amqp.rabbit.connection.AbstractConnectionFactoryTests.1>/*__p0__*/p())

Detail:

java.lang.IllegalArgumentException: Expected a template that would generate exactly one statement to replace one statement, but generated 2. Template:
List.of(__P__.<org.springframework.amqp.rabbit.connection.AbstractConnectionFactoryTests.1>/*__p0__*/p())
org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.maybeReplaceStatement(JavaTemplateJavaExtension.java:462)
org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitMethodInvocation(JavaTemplateJavaExtension.java:428)
org.openrewrite.java.internal.template.JavaTemplateJavaExtension$1.visitMethodInvocation(JavaTemplateJavaExtension.java:59)
org.openrewrite.java.tree.J$MethodInvocation.acceptJava(J.java:3700)
org.openrewrite.java.tree.J.accept(J.java:64)
org.openrewrite.TreeVisitor.visit(TreeVisitor.java:276)
org.openrewrite.TreeVisitor.visit(TreeVisitor.java:172)
org.openrewrite.java.JavaTemplate.withTemplate(JavaTemplate.java:107)
...

Screen Shot 2023-03-01 at 2 44 29 PM

@kunli2 kunli2 self-assigned this Mar 2, 2023
@kunli2 kunli2 added the bug Something isn't working label Mar 2, 2023
@kunli2 kunli2 moved this to In Progress in OpenRewrite Mar 2, 2023
@kunli2 kunli2 changed the title Error: error: MigrateCollectionsSingletonList recipe error Mar 2, 2023
kunli2 added a commit that referenced this issue Mar 2, 2023
kunli2 added a commit that referenced this issue Mar 2, 2023
* fixes #186, fix a template error
@github-project-automation github-project-automation bot moved this from In Progress to Done in OpenRewrite Mar 2, 2023
@kunli2
Copy link
Contributor Author

kunli2 commented Mar 3, 2023

root cause

Source code

setConnectionListeners(Collections.singletonList(new ConnectionListener() {
         @Override
         public void onCreate() {
         }
     }));

this is the stub generated by BlockStatementTemplateGenerator.

class A {
    public void setConnectionListeners(List<? extends ConnectionListener> listeners) {
    }

    public void test() {
        __M__.any(
            /*__TEMPLATE__*/List.of(__P__.<A .1 >/*__p0__*/p())/*__TEMPLATE_STOP__*/
);
    }
}

since in the source, there is no assignment, the generic type of __P__ is not determined, and the code doesn't compile.

The stop comment is not able to be captured since it's somewhere before the target statement, so the order is the visitor caught /*__TEMPLATE_STOP__*/ first, and then caught /*__TEMPLATE__*/ (see the position in the tree below), the order is reversed than expected, and seems it's impossible to support this in current solution framework.

\---J.ClassDeclaration
    |---J.Identifier | "A"
    \---J.Block
        |-------J.MethodDeclaration | "MethodDeclaration{A{name=setConnectionListeners,return=void,parameters=[java.util.List<Generic{? extends {undefined}}..."
        |       |---J.Modifier | "public"
        |       |---J.Primitive | "void"
        |       |---J.Identifier | "setConnectionListeners"
        |       |-----------J.VariableDeclarations | "List<? extends ConnectionListener> listeners"
        |       |           |---J.ParameterizedType | "List<? extends ConnectionListener>"
        |       |           |   |---J.Identifier | "List"
        |       |           |   \-----------J.Wildcard | "? extends ConnectionListener"
        |       |           |               \---J.Identifier | "ConnectionListener"
        |       |           \-------J.VariableDeclarations.NamedVariable | "listeners"
        |       |                   \---J.Identifier | "listeners"
        |       \---J.Block
        \-------J.MethodDeclaration | "MethodDeclaration{A{name=test,return=void,parameters=[]}}"
                |---J.Modifier | "public"
                |---J.Primitive | "void"
                |---J.Identifier | "test"
                |-----------J.Empty
                \---J.Block
                    |-------J.MethodInvocation | "__M__.any( /*__TEMPLATE__*/List.of(__P__.<error><A.1>/*__p0__*/p())"        // JRightPadded.after = "/*__TEMPLATE_STOP__*/"
                    |       |-------J.Identifier | "__M__"
                    |       |---J.Identifier | "any"
                    |       \-----------J.MethodInvocation | "List.of(__P__.<error><A.1>/*__p0__*/p()"                        //  "/*__TEMPLATE__*/" is here, after the stop comment
                    |                   |-------J.Identifier | "List"
                    |                   |---J.Identifier | "of"
                    |                   \-----------J.FieldAccess | "__P__.<error>"
                    |                               |---J.Identifier | "__P__"
                    |                               \-------J.Identifier | "<error>"
                    \-------J.MethodInvocation | "p()"
                            |---J.Identifier | "p"
                            \-----------J.Empty

Compared to a working case:

List<LocalDate> list = Collections.singletonList(LocalDate.now());

The generated template stub is

class Test {
    List<String> list =
        /*__TEMPLATE__*/List.of(__P__.<String>/*__p0__*/p())/*__TEMPLATE_STOP__*/;
}

this works because there is no more statement to capture at the end, but not because the /*__TEMPLATE_STOP__*/ is successfully captured.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

1 participant