AddMissingMethodImplementation should check for superclass implementations of the relevant method

[nmck257]

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]

@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; }"

[rgustafson-ie]

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

[cjobinabo]

@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.

[Jenson3210]

• Fixed in Skip adding methods already provided by a superclass #983 which adds a superclass hierarchy check to AddMissingMethodImplementation. I verified that with the classes provided, this is no longer happening.

@timtebeek can this be closed?