`AddMissingMethodImplementation` generating stubs for methods with implementations

[williamkmorton]

trafficstars

What version of OpenRewrite are you using?

I am using

• Gradle plugin v6.29.0
• rewrite-migrate-java v2.31.0

How are you running OpenRewrite?

I am using the Gradle plugin, and my project is a single module project and applying rewrite-migrate-java through the composite rewrite-spring

plugins {
	java
	id("org.springframework.boot") version "3.4.1"
	id("io.spring.dependency-management") version "1.1.7"
        id("org.openrewrite.rewrite") version("6.29.0")
}

group = "com.example"
version = "0.0.1-SNAPSHOT"

java {
	toolchain {
		languageVersion = JavaLanguageVersion.of(21)
	}
}

repositories {
	mavenCentral()
}

dependencies {
	implementation("org.springframework.boot:spring-boot-starter-data-jdbc")
	implementation("org.springframework.boot:spring-boot-starter-web")
	testImplementation("org.springframework.boot:spring-boot-starter-test")
	testRuntimeOnly("org.junit.platform:junit-platform-launcher")
        rewrite("org.openrewrite.recipe:rewrite-spring:5.25.0")
}

rewrite {
        activeRecipe("org.openrewrite.java.spring.boot3.SpringBoot3BestPractices")
        setExportDatatables(true)
}

tasks.withType<Test> {
	useJUnitPlatform()
}

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

Use the configuation above and create a class like below that extends AbstractRoutingDataSource and run the recipe.

https://github.com/spring-projects/spring-framework/blob/main/spring-jdbc/src/main/java/org/springframework/jdbc/datasource/lookup/AbstractRoutingDataSource.java

Only determineCurrentLookupKey is abstract and needs to be overridden.

What did you expect to see?

package com.example.multitenancy;

import lombok.extern.slf4j.Slf4j;
import org.springframework.jdbc.datasource.lookup.AbstractRoutingDataSource;

@Slf4j
public class TenantRoutingDataSource extends AbstractRoutingDataSource {

    @Override
    protected Object determineCurrentLookupKey() {
        return TenantContext.getCurrentTenant();
    }
}

What did you see instead?

package com.example.multitenancy;

import java.sql.SQLException;
import java.util.logging.Logger;
import lombok.extern.slf4j.Slf4j;
import org.jetbrains.annotations.NotNull;
import org.springframework.jdbc.datasource.lookup.AbstractRoutingDataSource;

@Slf4j
public class TenantRoutingDataSource extends AbstractRoutingDataSource {

    @Override
    protected Object determineCurrentLookupKey() {
        return TenantContext.getCurrentTenant();
    }

    public boolean isWrapperFor(Class<?> iface) throws SQLException {
        // TODO Auto-generated method stub
        return iface != null && iface.isAssignableFrom(this.getClass());
    }

    public <T> T unwrap(Class<T> iface) throws SQLException {
        // TODO Auto-generated method stub
        try {
            if (iface != null && iface.isAssignableFrom(this.getClass())) {
                return (T) this;
            }
            throw new SQLException("Auto-generated unwrap failed; Revisit implementation");
        } catch (Exception e) {
            throw new SQLException(e);
        }
    }

    public Logger getParentLogger() {
        // TODO Auto-generated method stub
        return null;
    }
}

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

No errors encountered.

Changes have been made to src/main/java/com/example/multitenancy/TenantRoutingDataSource.java by:
    org.openrewrite.java.spring.boot3.SpringBoot3BestPractices
        org.openrewrite.java.migrate.UpgradeToJava21
            org.openrewrite.java.migrate.UpgradeToJava17
                org.openrewrite.java.migrate.Java8toJava11
                    org.openrewrite.java.migrate.UpgradeToJava8
                        org.openrewrite.java.migrate.UpgradeToJava7
                            org.openrewrite.java.migrate.UpgradeToJava6
                                org.openrewrite.java.migrate.JREWrapperInterface
                                    org.openrewrite.java.migrate.AddMissingMethodImplementation: {fullyQualifiedClassName=java.sql.Wrapper, methodPattern=*..* isWrapperFor(..), methodTemplateString=public boolean isWrapperFor(Class<?> iface) throws java.sql.SQLException { 
        // TODO Auto-generated method stub
 return iface != null && iface.isAssignableFrom(this.getClass()); }}
                                    org.openrewrite.java.migrate.AddMissingMethodImplementation: {fullyQualifiedClassName=java.sql.Wrapper, methodPattern=*..* unwrap(..), methodTemplateString=public <T> T unwrap(Class<T> iface) throws java.sql.SQLException { 
        // TODO Auto-generated method stub
 try { if (iface != null && iface.isAssignableFrom(this.getClass())) { return (T) this; } throw new java.sql.SQLException("Auto-generated unwrap failed; Revisit implementation"); } catch (Exception e) { throw new java.sql.SQLException(e); } }}
                            org.openrewrite.java.migrate.JREJdbcInterfaceNewMethods
                                org.openrewrite.java.migrate.AddMissingMethodImplementation: {fullyQualifiedClassName=javax.sql.CommonDataSource, methodPattern=*..* getParentLogger(), methodTemplateString=public java.util.logging.Logger getParentLogger() { 
        // TODO Auto-generated method stub
 return null; }}
        org.openrewrite.staticanalysis.CodeCleanup
            org.openrewrite.java.ShortenFullyQualifiedTypeReferences
Please review and commit the results.

Are you interested in contributing a fix to OpenRewrite?

[timtebeek]

hi @williamkmorton ; thanks for pointing this out! Indeed seems like an oversight on the recipe implementation. I'll link the relevant parts here.

The declarative recipe that makes the change is configured here: https://github.com/openrewrite/rewrite-migrate-java/blob/2f005afaad1f70f2f37fdfc1a0cc9ec3fb6365f1/src/main/resources/META-INF/rewrite/java-version-6.yml#L30-L42

The parts that should detect the already present methods is here: https://github.com/openrewrite/rewrite-migrate-java/blob/2f005afaad1f70f2f37fdfc1a0cc9ec3fb6365f1/src/main/java/org/openrewrite/java/migrate/AddMissingMethodImplementation.java#L82-L88

Likely we should not look at methods declared in the class, but look at methods available in the type, such that we also pick up methods defined on an extended class, or implemented interface default method:

Iterator<JavaType.Method> visibleMethods = classDecl.getType().getVisibleMethods();

Did you already explore such a fix perhaps?

[ghost]

I've only just started exploring OpenRewrite recipes and don't really know how things work under the hood but based on the information you've shared, it seems like a relatively easy fix. I'd be happy to give it a shot. Could you give me write access to the repo?

[timtebeek]

Thanks for the offer to help! We tend to work with forks for outside contributors, and you can open a pull request from there: https://github.com/openrewrite/rewrite-migrate-java/fork