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.

https://github.com/openrewrite/rewrite-migrate-java/blob/ee91666b5972ea931910fd6df87294907023f120/src/main/resources/META-INF/rewrite/java-version-7.yml#L31-L41

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

https://github.com/openrewrite/rewrite-migrate-java/blob/ee91666b5972ea931910fd6df87294907023f120/src/main/resources/META-INF/rewrite/java-version-7.yml#L31-L41

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.