AddMissingMethodImplementation generating stubs for methods with implementations

[williamkmorton]

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:

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

Lines 30 to 42 in 2f005af

	name: org.openrewrite.java.migrate.JREWrapperInterface
	displayName: Add missing `isWrapperFor` and `unwrap` methods.
	description: Add method implementations stubs to classes that implement `java.sql.Wrapper`.
	recipeList:
	- org.openrewrite.java.migrate.jacoco.UpgradeJaCoCo
	- org.openrewrite.java.migrate.AddMissingMethodImplementation:
	fullyQualifiedClassName: java.sql.Wrapper
	methodPattern: "*..* isWrapperFor(..)"
	methodTemplateString: "public boolean isWrapperFor(Class<?> iface) throws java.sql.SQLException { \n\t// TODO Auto-generated method stub\n 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 { \n\t// TODO Auto-generated method stub\n 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); } }"

The parts that should detect the already present methods is here:

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

Lines 82 to 88 in 2f005af

	// If the class already has method, don't make any changes to it.
	if (classDecl.getBody().getStatements().stream()
	.filter(statement -> statement instanceof J.MethodDeclaration)
	.map(J.MethodDeclaration.class::cast)
	.anyMatch(methodDeclaration -> methodMatcher.matches(methodDeclaration, classDecl))) {
	return classDecl;
	}

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?

[williamkmorton]

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