openrewrite/rewrite-migrate-java

AddMissingMethodImplementation should check for superclass implementations of the relevant method

Opened this issue · 3 comments

nmck257 commented

What problem are you trying to solve?

Currently, the AddMissingMethodImplementation checks primarily for existing method declarations on the visited class declaration. It does not check whether the visited class has a superclass which already provides an implementation of the relevant method.

What precondition(s) should be checked before applying this recipe?

Existing recipe, no suggested precondition changes

Describe the situation before applying the recipe

// surrounding code
interface MyInterface {
    void foo();
    void bar();
}
class MyBaseClass implements MyInterface {
    public void bar() {}
}
// code for the recipe to actually visit
class MyClass extends MyBaseClass {

}

Describe the situation after applying the recipe

If we run a recipe like this:

  - org.openrewrite.java.migrate.AddMissingMethodImplementation:
      fullyQualifiedClassName: MyInterface
      methodPattern: "*..* foo()"
      methodTemplateString: "public void foo() { /* added */ }"

...then we would get this:

class MyClass extends MyBaseClass {
    public void foo() { /* added */ }
}

But if we run this

  - org.openrewrite.java.migrate.AddMissingMethodImplementation:
      fullyQualifiedClassName: MyInterface
      methodPattern: "*..* bar()"
      methodTemplateString: "public void bar() { /* added */ }"

...then we would get no change, because MyClass already has an implementation of bar() from MyBaseClass.

Have you considered any alternatives or workarounds?

This behavior could also be disabled with a new option (default enabled)

Any additional context

Are you interested in contributing this recipe to OpenRewrite?

Maybe eventually; could also be a good first issue

timtebeek commented

@cjobinabo Since AddMissingMethodImplementation is heavily used in some of the recipes your team contributed I figured
let you know about this issue, as it might affect your users too. No expectation of course that you'll fix this; we're already glad with all the work that you've done, but thought it good to coordinate any changes here.

rewrite-migrate-java/src/main/resources/META-INF/rewrite/java-version-7.yml

Lines 31 to 41 in ee91666

displayName: Adds missing JDBC interface methods.
description: Add method implementations stubs to classes that implement JDBC interfaces.
recipeList:
- org.openrewrite.java.migrate.AddMissingMethodImplementation:
fullyQualifiedClassName: java.sql.Connection
methodPattern: "*..* abort(java.util.concurrent.Executor)"
methodTemplateString: "public void abort(java.util.concurrent.Executor executor) { \n\t// TODO Auto-generated method stub\n }"
- org.openrewrite.java.migrate.AddMissingMethodImplementation:
fullyQualifiedClassName: java.sql.Connection
methodPattern: "*..* getNetworkTimeout()"
methodTemplateString: "public int getNetworkTimeout() { \n\t// TODO Auto-generated method stub\n return 0; }"

  • 👍1
rgustafson-ie commented

I saw this in practice today, on subclasses of Spring's AbstractDataSource.

cjobinabo commented

@cjobinabo Since AddMissingMethodImplementation is heavily used in some of the recipes your team contributed I figured let you know about this issue, as it might affect your users too. No expectation of course that you'll fix this; we're already glad with all the work that you've done, but thought it good to coordinate any changes here.

rewrite-migrate-java/src/main/resources/META-INF/rewrite/java-version-7.yml

Lines 31 to 41 in ee91666

displayName: Adds missing JDBC interface methods.
description: Add method implementations stubs to classes that implement JDBC interfaces.
recipeList:
- org.openrewrite.java.migrate.AddMissingMethodImplementation:
fullyQualifiedClassName: java.sql.Connection
methodPattern: "*..* abort(java.util.concurrent.Executor)"
methodTemplateString: "public void abort(java.util.concurrent.Executor executor) { \n\t// TODO Auto-generated method stub\n }"
- org.openrewrite.java.migrate.AddMissingMethodImplementation:
fullyQualifiedClassName: java.sql.Connection
methodPattern: "*..* getNetworkTimeout()"
methodTemplateString: "public int getNetworkTimeout() { \n\t// TODO Auto-generated method stub\n return 0; }"

Thanks for the heads up, Tim (I know I'm super late to your message 😓). I'll try to keep an eye out for updates addressing this GitHub issue.

  • 👍1