Use caller reference in UseMethod

[fmodesto]

What's changed?

Modified UsesMethod to track the actual invocation target type, rather than just the method's declaring type.

Example Case:

class A { public void foo() {} }
interface I { void foo(); }
class B extends A implements I {}

class Test {
  void test() {
    new B().foo();
  }
}

The declaring type of the method foo is A. But that misses matches on the interface I

What's your motivation?

• UseMethod precondition check, fails to detect the usage #5376

Anything in particular you'd like reviewers to focus on?

This has a downstream regression in rewrite-static-analysis in CompareEnumsWithEqualityOperator. UseMethod matches for the exact java.lang.Enum equals(java.lang.Object) without overrides. This change would make the actual enum to use equals: com.pkg.MyEnum equals(java.lang.Object).

Checklist

• I've added unit tests to cover both positive and negative cases
• I've read and applied the recipe conventions and best practices
• I've used the IntelliJ IDEA auto-formatter on affected files

[timtebeek]

Thanks for the help! Copying the failure from the logs here to make it easier to follow up:

JavadocTest > methodFoundInSuperclassBecauseSuperclassFieldsTakesPresenceOverInterfaceFields() FAILED
    org.opentest4j.AssertionFailedError: expected: <SomeParent2> but was: <Test>
        at app//org.junit.jupiter.api.AssertionFailureBuilder.build(AssertionFailureBuilder.java:151)
        at app//org.junit.jupiter.api.AssertionFailureBuilder.buildAndThrow(AssertionFailureBuilder.java:132)
        at app//org.junit.jupiter.api.AssertEquals.failNotEqual(AssertEquals.java:197)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:182)
        at app//org.junit.jupiter.api.AssertEquals.assertEquals(AssertEquals.java:177)
        at app//org.junit.jupiter.api.Assertions.assertEquals(Assertions.java:1145)
        at app//org.openrewrite.java.tree.JavadocTest.lambda$methodFoundInSuperclassBecauseSuperclassFieldsTakesPresenceOverInterfaceFields$4(JavadocTest.java:1262)
        at app//org.openrewrite.java.tree.JavadocTest$$Lambda$1064/0x00000001005bb440.accept0(Unknown Source)
        at app//org.openrewrite.internal.ThrowingConsumer.accept(ThrowingConsumer.java:26)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:576)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:130)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:125)
        at app//org.openrewrite.java.tree.JavadocTest.methodFoundInSuperclassBecauseSuperclassFieldsTakesPresenceOverInterfaceFields(JavadocTest.java:1234)

rewrite/rewrite-java-tck/src/main/java/org/openrewrite/java/tree/JavadocTest.java

Lines 1232 to 1266 in b110d3a

	@Test
	void methodFoundInSuperclassBecauseSuperclassFieldsTakesPresenceOverInterfaceFields() {
	rewriteRun(
	java(
	"""
	import javax.swing.text.html.HTML.Tag;
	interface SomeInterface {
	int test();
	}
	abstract class SomeParent2 implements SomeInterface {
	abstract boolean test();
	}
	abstract class SomeParent extends SomeParent2 implements SomeInterface {
	}
	"""
	),
	java(
	"""
	class Test extends SomeParent implements SomeInterface {
	/**
	* @see #test()
	*/
	void method() {}
	}
	""",
	spec -> spec.afterRecipe(cu -> {
	assertEquals("test", cu.getTypesInUse().getUsedMethods().iterator().next().getName());
	assertEquals("SomeParent2", cu.getTypesInUse().getUsedMethods().iterator().next().getDeclaringType().getFullyQualifiedName());
	})
	)
	);
	}

[timtebeek]

Great to see! Linking this discussion here as I imagine we'd need that visit(memberRef.getContaining(), compareTo.getContaining()); reintroduced here before a merge:

• Temporarily rollback TypeUtils enhancements #5394 (comment)

[fmodesto]

Causes failures in rewrite-static-analysis

[fmodesto]

Not sure about this one, the issue is in rewrite-static-analysis CompareEnumsWithEqualityOperator UseMethod matches for the exact java.lang.Enum equals(java.lang.Object) without overrides, now it comes as the.actual.Enum equals(java.lang.Object)

[knutwannheden]

I am not opposed to this change. Just hesitate because it changes the TypesInUse cache.

[knutwannheden]

As ImmutableSet doesn't actually declare contains(Object) I think we should correct this:

Suggested change

	"com.google.common.collect.ImmutableSet contains(..)",
	"com.google.common.collect.ImmutableCollection contains(..)",

[knutwannheden]

I see that this is what the issue is about. Appears to be quite the corner case, as in this case the implementation is inherited from ImmutableCollection but at the same time ImmutableSet also implements Set, which indeed overrides the contains(Object) method.

Curiously enough opening the call graph for Set#contains(Object) in IDEA doesn't show this call site either.