Fixes for type attribution and TypeUtils#isAssignableTo()

[knutwannheden]

Try to fix #4423 by correcting the type attribution for some generics and the corresponding type assignability check.

• Fixes: JavaTemplateSemanticallyEqual fails to match AssertJ's assertThat("foo").isEqualTo("") #4423

[knutwannheden]

@timtebeek I believe this will cause some test failures, but I do believe that there is currently something missing in the type attribution, which causes the test from #4423 to fail.

• JavaTemplateSemanticallyEqual fails to match AssertJ's assertThat("foo").isEqualTo("") #4423

This is WIP and I am not sure when I will be able to pick this up. Maybe you find the time 😉

[timtebeek]

Indeed seeing three new falures; quick link to one of them here for reference.
:rewrite-java-test:test org.openrewrite.java.JavaTemplateSubstitutionsTest

diff --git a/Test.java b/Test.java	
index b025685..8109464 100644	
--- a/Test.java	
+++ b/Test.java	
@@ -1,6 +1,6 @@ 	
 import java.util.Map;	
 class Test {	
  void test(Map<String, ?> map) {	
-     System.out.println(map.get("test"));	
+     System.out.println(__P__.<error> << captured wildcard>>/*__p0__*/p();	
  }	
 }	

[knutwannheden]

@timtebeek I pushed another commit. Let's see if this helps. Got any idea if any recipes have bugs which can be explained by this?

[timtebeek]

@timtebeek I pushed another commit. Let's see if this helps. Got any idea if any recipes have bugs which can be explained by this?

Thanks a lot! I don't recall us using JavaTemplate.matches much outside of Refaster recipes, and within Refaster recipes I think we've mostly then had cases that failed to match. That's still a bug, but not as impactful as a false positive match would be. I do expect to see improvement to the cases matched through the ErrorProne Support set of recipes, and being able to leverage those more for our migrations as we've started to do in rewrite-testing-frameworks.

There had been a few earlier attempts at recipes that failed to match that could now be reevaluated:

• Add refaster recipes to simplify expected value rewrite-testing-frameworks#475
• Add TestNG -> AssertJ rules rewrite-testing-frameworks#520
• Quick attempt at idomatic AssertJ assertions through Refaster rewrite-testing-frameworks#550

Would you need me to apply the same fixes to the other versions of Java parsers?

[knutwannheden]

Yes, I believe we should apply the same fix to the other Java parsers.

[knutwannheden]

Also note that the bug could affect any recipes which direcly or indirectly use TypeUtils.isAssignableTo(). So this isn't restricted to Refaster or JavaTemplate.matches(), but it is still an uncommon corner case.

[timtebeek]

Thanks a lot for finding some time to squeeze this in! Should make those matchers a lot more effective, and unlocks more reuse through Refaster.

[timtebeek]

Seeing one downstream test failure (so far) in rewrite-static-analysis
https://github.com/openrewrite/rewrite-static-analysis/blob/195fbf806ce214305d59d92e81bf2bef7401cf9c/src/test/java/org/openrewrite/staticanalysis/ExplicitLambdaArgumentTypesTest.java#L618-L648

ExplicitLambdaArgumentTypesTest > extendsConstraint() FAILED
    org.opentest4j.AssertionFailedError: [Unexpected result in "com/test/A.java":
    diff --git a/com/test/A.java b/com/test/A.java
    index b70fd0e..46a199b 100644
    --- a/com/test/A.java
    +++ b/com/test/A.java
    @@ -4,6 +4,6 @@ 
 
     class A {
         void foo(List<? extends A> a) {
    -        a.forEach((A it) -> { });
    +        a.forEach((<captured wildcard> it) -> { });
         }
     }
    \ No newline at end of file
    ] 
    expected: 
      "package com.test;
  
      import java.util.List;
  
      class A {
          void foo(List<? extends A> a) {
              a.forEach((A it) -> { });
          }
      }"
     but was: 
      "package com.test;
  
      import java.util.List;
  
      class A {
          void foo(List<? extends A> a) {
              a.forEach((<captured wildcard> it) -> { });
          }
      }"
        at app//org.openrewrite.test.RewriteTest.assertContentEquals(RewriteTest.java:616)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:507)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:132)
        at app//org.openrewrite.test.RewriteTest.rewriteRun(RewriteTest.java:127)
        at app//org.openrewrite.staticanalysis.ExplicitLambdaArgumentTypesTest.extendsConstraint(ExplicitLambdaArgumentTypesTest.java:621)

[knutwannheden]

Looks like it requires slightly more work.