Conversion of `Predicates.equalTo` may cause issues - the generic type is not always correctly inferred

[lredor]

• I discovered that the conversion of Predicates.equalTo, added with PR Convert Guava Predicates.equalTo and .not #887, may cause issues - the generic type is not always correctly inferred.

What version of OpenRewrite are you using?

I am using:

• Rewrite Maven Plugin v6.23.0
• Maven v3.9.5
• org.openrewrite.recipe:rewrite-migrate-java v3.21.0

How are you running OpenRewrite?

I used the Maven command line to launch the recipe on the Sirius Desktop repository :

mvn -U org.openrewrite.maven:rewrite-maven-plugin:6.23.0-SNAPSHOT:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-migrate-java:3.21.0-SNAPSHOT -Drewrite.activeRecipes=org.openrewrite.java.migrate.guava.NoGuava -Drewrite.exportDatatables=true -Dmaven.repo.local=./.repositoryMaven -f packaging/org.eclipse.sirius.parent/pom.xml

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

import com.google.common.base.Predicates;
import com.google.common.base.Predicate;

import java.util.Collection;

class Test {
    public static void test(Collection<String> aCollection, Predicate<Collection<String>> anotherPredicate) {
        Predicate<Collection<String>> combined = Predicates.and(Predicates.equalTo(aCollection), anotherPredicate);
    }
}

What did you expect to see?

• Since PR Convert Guava Predicates.equalTo and .not #887, the result produces the following compilation error: 'and(java.util.function.Predicate<? super java.lang.Object>)' in 'java.util.function.Predicate' cannot be applied to '(java.util.function.Predicate<java.util.Collection<java.lang.String>>)'.
  It seems that the isEqual expression must be stored in a variable to compile correctly. I'm not sure that extracting the variable is the most elegant solution, but it does fix the issue.

import java.util.function.Predicate;

import java.util.Collection;

class Test {
    public static void testResult(Collection<String> aCollection, Predicate<Collection<String>> anotherPredicate) {
        Predicate<Collection<String>> isEqualPredicate = Predicate.isEqual(aCollection);
        Predicate<Collection<String>> combined = isEqualPredicate.and(anotherPredicate);
    }
}

What did you see instead?

import java.util.function.Predicate;

import java.util.Collection;

class Test {
    public static void testResult(Collection<String> aCollection, Predicate<Collection<String>> anotherPredicate) {
        Predicate<Collection<String>> combined = Predicate.isEqual(aCollection).and(anotherPredicate);
    }
}

[timtebeek]

Thanks for the update. These things are always tricky with unexpected edge cases. Perhaps detection and then holding off on changes might be easiest in the short term.

[timtebeek]

Having a closer look I think what we could strive for is to add the type argument to the left hand side of the .and:

    @Issue("https://github.com/openrewrite/rewrite-migrate-java/issues/903")
    @Test
    void doNotConvertPredicatesEqualToWhenUsedInMethodChain() {
        // Converting Predicates.equalTo() to Predicate.isEqual() breaks type inference when chained with .and()
        // The issue is that Predicate.isEqual() returns Predicate<? super T> which causes compilation errors
        // when chained with other methods
        rewriteRun(
          //language=java
          java(
            """
              import com.google.common.base.Predicate;
              import com.google.common.base.Predicates;
              import java.util.Collection;

              class A {
                  public static Predicate<Collection<String>> combinedPredicate(Collection<String> aCollection) {
                      Predicate<Collection<String>> anotherPredicate = c -> !c.isEmpty();
                      return Predicates.and(Predicates.equalTo(aCollection), anotherPredicate);
                  }
              }
              """,
            """
              import java.util.Collection;
              import java.util.function.Predicate;

              class A {
                  public static Predicate<Collection<String>> combinedPredicate(Collection<String> aCollection) {
                      Predicate<Collection<String>> anotherPredicate = c -> !c.isEmpty();
                      return Predicate.<Collection<String>>isEqual(aCollection, anotherPredicate);
                  }
              }
              """
          )
        );
    }

Would you agree that fixes the type issues?

If so we can likely add special handling like we already have for J.MemberReference but then for J.MethodInvocation.

rewrite-migrate-java/src/main/java/org/openrewrite/java/migrate/guava/NoGuavaPredicatesAndOr.java

Lines 82 to 85 in 808ddc1

	if (result instanceof J.MemberReference && result.getType() != null) {
	String typeString = result.getType().toString().replace("com.google.common.base.", "");
	result = JavaTemplate.apply("((" + typeString + ") #{any()})", getCursor(), method.getCoordinates().replace(), result);
	}

[timtebeek]

I've pushed up a minimal fix for now, but we can reopen if you'd indeed like to pursue the above migration.

[lredor]

Would you agree that fixes the type issues?

I didn’t come across this kind of cast before, but that’s exactly what I had in mind. 👍

[lredor]

I didn't completely understand the fix of e464614. It was added to handle the Predicates.equalTo case, but it avoids handling the case below, which is not desirable.

class NoGuavaPredicatesAndOrTest implements RewriteTest {
...
    @Test
    void replacePredicatesAndWithMethodAsParameters() {
        //language=java
        rewriteRun(
          java(
            """
              import com.google.common.base.Predicate;
              import com.google.common.base.Predicates;
              import java.util.Objects;

              class Test {
                  Predicate<String> getAndPredicate() {
                      return Objects::nonNull;
                  }
                  Predicate<String> getMaxLengthPredicate() {
                      return s -> s.length() > 5;
                  }
                  Predicate<String> combined = Predicates.and(getAndPredicate(), getMaxLengthPredicate());
              }
              """,
            """
              import com.google.common.base.Predicate;
              import java.util.Objects;

              class Test {
                  Predicate<String> getAndPredicate() {
                      return Objects::nonNull;
                  }
                  Predicate<String> getMaxLengthPredicate() {
                      return s -> s.length() > 5;
                  }
                  Predicate<String> combined = getAndPredicate().and(getMaxLengthPredicate());
              }
              """
          )
        );
    }
...