AssertJ: Refactor `assertThat` with `<` or `>` in the expression to `assertThat(actual).isGreaterThan(expected)` or `assertThat(actual).isLessThan(expected)`

[puskarpeter]

What problem are you trying to solve?

Using the AssertJ best practices Sonar complains about the following:

What precondition(s) should be checked before applying this recipe?

AssertJ available

Describe the situation before applying the recipe

class A {
    void foo(Long bar, Long foo) {
        assertThat(bar > 0).isTrue();
        assertThat(foo < 0).isTrue();
    }
}

Describe the situation after applying the recipe

class A {
    void foo(Long bar, Long foo) {
        assertThat(bar).isGreaterThan(0);
        assertThat(foo).isLessThan(0);
    }
}

Have you considered any alternatives or workarounds?

IDEA does not have quickfix for this, requires manual intervention.

Any additional context

This was a result after transforming this expression from Junit5

class A {
    void foo(Long bar, Long foo) {
        assertTrue(bar > 0);
        assertTrue(foo < 0);
    }
}

Are you interested in contributing this recipe to OpenRewrite?

Unfortunately I do not have the capacity for it right now.

[Stephan202]

This should be covered by these rules.

[timtebeek]

Thanks yes I had expected coverage there as well, but I think in our case we only match for double > double instead of the other numerical types as well. Something to improve in our matcher perhaps, if safe to do so. /cc @knutwannheden

[timtebeek]

Been trying for a bit here to replicate any mismatch, but this test still passes:

    @Test
    void matchBinaryLessThanForAnyNumericalType() {
        rewriteRun(
          spec -> spec.recipe(toRecipe(() -> new JavaVisitor<>() {
              @Override
              public J visitBinary(J.Binary binary, ExecutionContext ctx) {
                  return JavaTemplate.matches("#{left:any(double)} < #{right:any(double)}", getCursor()) ?
                    SearchResult.found(binary) : super.visitBinary(binary, ctx);
              }
          })),
          java(
            """
              class Test {
                  void foo(byte b, short s, int i, long l, float f, double d, Number n) {
                      boolean c1 = 0 < b;
                      boolean c2 = 0 < s;
                      boolean c3 = 0 < i;
                      boolean c4 = 0 < l;
                      boolean c5 = 0 < f;
                      boolean c6 = 0 < d;
                      boolean c7 = 0 < n.doubleValue();
                  }
              }
              """,
            """
              class Test {
                  void foo(byte b, short s, int i, long l, float f, double d, Number n) {
                      boolean c1 = /*~~>*/0 < b;
                      boolean c2 = /*~~>*/0 < s;
                      boolean c3 = /*~~>*/0 < i;
                      boolean c4 = /*~~>*/0 < l;
                      boolean c5 = /*~~>*/0 < f;
                      boolean c6 = /*~~>*/0 < d;
                      boolean c7 = /*~~>*/0 < n.doubleValue();
                  }
              }
              """
          )
        );
    }

[timtebeek]

For comparison; the generated recipe for AssertJPrimitiveRules.AssertThatIsLessThan uses:

        @Override
        public TreeVisitor<?, ExecutionContext> getVisitor() {
            JavaVisitor<ExecutionContext> javaVisitor = new AbstractRefasterJavaVisitor() {
                JavaTemplate before$0;
                JavaTemplate before$1;
                JavaTemplate after;

                @Override
                public J visitMethodInvocation(J.MethodInvocation elem, ExecutionContext ctx) {
                    JavaTemplate.Matcher matcher;
                    if (before$0 == null) {
                        before$0 = JavaTemplate.builder("org.assertj.core.api.Assertions.assertThat(#{actual:any(double)} < #{expected:any(double)}).isTrue()")
                                .bindType("org.assertj.core.api.AbstractBooleanAssert<?>")
                                .javaParser(JavaParser.fromJavaVersion().classpath(JavaParser.runtimeClasspath()))
                                .build();
                    }

[timtebeek]

How are you running your recipes @puskarpeter ? Because that JavaParser.runtimeClasspath() fails in some environments; there's an improvement I've been meaning to push to EPS, but that's waiting for this change to go first:

• Upgrade OpenRewrite 3.14.0 -> 3.16.0 PicnicSupermarket/error-prone-support#1853

[puskarpeter]

mvn -v 
Apache Maven 3.8.9 (e26b057cc3a17459358ef53e4d0e2e381bf08a1c)
Maven home: /home/peter-puskar/.sdkman/candidates/maven/current
Java version: 21.0.8, vendor: Ubuntu, runtime: /usr/lib/jvm/java-21-openjdk-amd64
Default locale: en_US, platform encoding: UTF-8
OS name: "linux", version: "6.16.8-061608-generic", arch: "amd64", family: "unix"

Also tried with maven 3.9.11 and java11 with no differences.
I tried running the recipe in 3 different ways and all produce the same result:

1. running with mvn -U org.openrewrite.maven:rewrite-maven-plugin:run -Drewrite.recipeArtifactCoordinates=org.openrewrite.recipe:rewrite-testing-frameworks:3.16.3 -Drewrite.activeRecipes=org.openrewrite.java.testing.assertj.Assertj -Drewrite.exportDatatables=true
2. adding the configuration in the root pom of the project with

<project>
  <build>
    <plugins>
      <plugin>
        <groupId>org.openrewrite.maven</groupId>
        <artifactId>rewrite-maven-plugin</artifactId>
        <version>6.18.0</version>
        <configuration>
          <exportDatatables>true</exportDatatables>
          <activeRecipes>
            <recipe>org.openrewrite.java.testing.assertj.Assertj</recipe>
          </activeRecipes>
        </configuration>
        <dependencies>
          <dependency>
            <groupId>org.openrewrite.recipe</groupId>
            <artifactId>rewrite-testing-frameworks</artifactId>
            <version>3.16.3</version>
          </dependency>
        </dependencies>
      </plugin>
    </plugins>
  </build>
</project>

and running mvn rewrite:run
3. running via IDEA rewrite plugin with this recipe:

type: specs.openrewrite.org/v1beta/recipe
name: assertj
recipeList:
  - org.openrewrite.java.testing.assertj.Assertj

Also for the matcher context, the actual field type that did not get converted is int since that is used in junit assertion like
assertTrue(0 > foo.compareTo(bar));