SimplifyBooleanExpression causes NullPointerException because condition is negated

[timo-abele]

How are you running OpenRewrite?

I am using the Maven plugin, and my project is a single module project.

<plugin>
    <groupId>org.openrewrite.maven</groupId>
    <artifactId>rewrite-maven-plugin</artifactId>
    <version>5.23.1</version>
    <configuration>
        <activeRecipes>
          <recipe>org.openrewrite.staticanalysis.SimplifyBooleanExpression</recipe>
        </activeRecipes>
    </configuration>
    <dependencies>
        <dependency>
            <groupId>org.openrewrite.recipe</groupId>
            <artifactId>rewrite-static-analysis</artifactId>
            <version>1.3.1</version>
        </dependency>
    </dependencies>
</plugin>

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

class A {
    boolean foo(String name) {
        return !(name != null ? !name.equals(System.out.toString()) : false);
    }
}

This is transformed to return name == null ? !name.equals(authority.name) : authority.name != null; if name is null.

What did you expect to see?

no change, or negation of the cases instead of the condition:

class A {
    boolean foo(String name) {
        return name != null ? name.equals(System.out.toString()) : !false;
    }
}

What did you see instead?

class A {
    boolean foo(String name) {
        return name == null ? !name.equals(System.out.toString()) : false;
    }
}

What is the full stack trace of any errors you encountered?

Standard NPE message when name is null

[timtebeek]

Thanks for the report! That's indeed a confusing amount of negation best left alone perhaps. 🤔

[knutwannheden]

Also note that a ? b : false can be simplified to a && b. So we might want to add that instead.

[knutwannheden]

I also just notice that the current recipe is incorrect, as the method invocation isn't null safe and would end up throwing an NPE. That definitely needs fixing.

[timo-abele]

Also note that a ? b : false can be simplified to a && b. So we might want to add that instead.

The false is a simplification on my part, in the original it was an expression. I should have probably used a variable instead of the literal false in the example.