AddMissingMethodImplementation generates wrong method for constructors

[ammachado]

What version of OpenRewrite are you using?

I am using

• OpenRewrite v8.9.4
• Maven/Gradle plugin RELEASE
• rewrite-migrate-java v2.3.0

How are you running OpenRewrite?

I'm applying rules using the maven plugin and a custom recipe artifact

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

package org.openrewrite.java.migrate;

import org.intellij.lang.annotations.Language;
import org.junit.jupiter.params.ParameterizedTest;
import org.junit.jupiter.params.provider.ValueSource;
import org.openrewrite.java.JavaParser;
import org.openrewrite.test.RewriteTest;

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

class AddMissingMethodImplementationTest implements RewriteTest {

    @ParameterizedTest
    @ValueSource(strings = {
      "*..* <constructor>(String)",
      "org.openrewrite.example.ImplementationClass <constructor>(String)",
    })
    void addMissingConstructor(String methodPattern) {
        @Language("java")
        final String abstractClassTemplate = """
          package org.openrewrite.example;

          public abstract class AbstractClass {
              private final String param;
              
              protected AbstractClass(String param) {
                  this.param = param;
              }
              
              public String getParam() {
                  return param;
              }
          }
          """;

        @Language("java")
        final String originalClass = """
          package org.openrewrite.example;

          public class ImplementationClass extends AbstractClass {
          }
          """;

        @Language("java")
        final String expected = """
          package org.openrewrite.example;
                    
          public class ImplementationClass extends AbstractClass {
                    
              public ImplementationClass(String param) {
                  super(param);
              }
          }
          """;

        rewriteRun(
          spec -> spec
            .parser(JavaParser.fromJavaVersion().dependsOn(abstractClassTemplate))
            .recipe(new AddMissingMethodImplementation(
              "org.openrewrite.example.AbstractClass",
              methodPattern,
              """
                public ImplementationClass(String param) {
                    super(param);
                }"""
            )),
          java(originalClass, expected)
        );
    }
}

What did you expect to see?

package org.openrewrite.example;
  
public class ImplementationClass extends AbstractClass {
    public ImplementationClass(String param) {
        super(param);
    }
}

What did you see instead?

 package org.openrewrite.example;
 
 public class ImplementationClass extends AbstractClass {
-    public ImplementationClass(String param) {
+    public Template ImplementationClass(String param) {
         super(param);
     }
 }

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

recipe completes successfully

Are you interested in contributing a fix to OpenRewrite?

[timtebeek]

Sorry to hear about your issues with this recent addition; adding constructors was not taken into account when this recipe was created. I'm not quite sure what would be needed to support that; perhaps we can start with a draft PR using the above test and work out the details from there. I suspect the JavaTemplate used might need to be made contextSensitive to start

rewrite-migrate-java/src/main/java/org/openrewrite/java/migrate/AddMissingMethodImplementation.java

Line 66 in 523cc18

private final JavaTemplate methodTemplate = JavaTemplate.builder(methodTemplateString).build();

Beyond that we're going to have to find out. Thanks at least for trying it out and reporting your findings already!

[ammachado]

Here's the template that is being parsed on https://github.com/openrewrite/rewrite/blob/main/rewrite-java/src/main/java/org/openrewrite/java/internal/template/JavaTemplateParser.java#L163:

package org.openrewrite.example;
import org.openrewrite.example.AbstractClass;
class __P__ {  static native <T> T p();  static native <T> T[] arrp();  static native boolean booleanp();  static native byte bytep();  static native char charp();  static native double doublep();  static native int intp();  static native long longp();  static native short shortp();  static native float floatp();}class __M__ {  static native Object any(Object o);  static native Object any(java.util.function.Predicate<Boolean> o);  static native <T> Object anyT();}public class ImplementationClass extends AbstractClass{
/*__TEMPLATE__*/
public class ImplementationClass extends AbstractClass {
    public ImplementationClass(String param) {
        super(param);
    }
}/*__TEMPLATE_STOP__*/
}

And here's the parsed tree (for brevity, I'm omiting the class __P__ and class __M__ parts:

----J.CompilationUnit
    |-------J.Package | "J.Package(id=c434b793-aa62-44fb-b854-7a0c886f7844, prefix=Space(comments=<0 comments>, whitespace=''), markers=Marker..."
    |       \---J.FieldAccess | "org.openrewrite.example"
    |           |---J.FieldAccess | "org.openrewrite"
    |           |   |---J.Identifier | "org"
    |           |   \-------J.Identifier | "openrewrite"
    |           \-------J.Identifier | "example"
    |-------J.Import | "import org.openrewrite.example.AbstractClass"
    |       \---J.FieldAccess | "org.openrewrite.example.AbstractClass"
    |           |---J.FieldAccess | "org.openrewrite.example"
    |           |   |---J.FieldAccess | "org.openrewrite"
    |           |   |   |---J.Identifier | "org"
    |           |   |   \-------J.Identifier | "openrewrite"
    |           |   \-------J.Identifier | "example"
    |           \-------J.Identifier | "AbstractClass"
    |  ...
    \---J.ClassDeclaration
        |---J.Modifier | "public"
        |---J.Identifier | "ImplementationClass"
        |-------J.Identifier | "AbstractClass"
        \---J.Block
            |-------J.VariableDeclarations | "<error>"
            |       \-------J.VariableDeclarations.NamedVariable | "<error>"
            |               \---J.Identifier | "<error>"
            |-------J.VariableDeclarations | " publicorg.openrewrite.example.ass ImplementationClass extends AbstractClass<error>"
            |       |---J.Modifier | "public"
            |       |---J.FieldAccess | "org.openrewrite.example.ass ImplementationClass extends AbstractClass"
            |       |   |---J.FieldAccess | "org.openrewrite.example"
            |       |   |   |---J.FieldAccess | "org.openrewrite"
            |       |   |   |   |---J.Identifier | "org"
            |       |   |   |   \-------J.Identifier | "openrewrite"
            |       |   |   \-------J.Identifier | "example"
            |       |   \-------J.Identifier | "AbstractClass"
            |       \-------J.VariableDeclarations.NamedVariable | "<error>"
            |               \---J.Identifier | "<error>"
            \-------J.ClassDeclaration
                    |---J.Identifier | "ImplementationClass"
                    |-------J.Identifier | "AbstractClass"
                    \---J.Block
                        \-------J.MethodDeclaration | "MethodDeclaration{org.openrewrite.example.ImplementationClass$ImplementationClass{name=<constructor>,return=org.openr..."
                                |---J.Identifier | "ImplementationClass"
                                |-----------J.VariableDeclarations | "Stringparam"
                                |           |---J.Identifier | "String"
                                |           \-------J.VariableDeclarations.NamedVariable | "param"
                                |                   \---J.Identifier | "param"
                                \---J.Block
                                    \-------J.MethodInvocation | "super(param)"
                                            |---J.Identifier | "super"
                                            \-----------J.Identifier | "param"