False Positive on S2589 : Boolean expressions should not be gratuitous / StringUtils.trimWhitespace

SonarQube for IDE IntelliJ Platform
1

Please provide

  • Win 10
  • SonarLint plugin version: 10.4.2.78113
  • Programming language you’re coding in: Java

And a thorough description of the problem / question:

SonarLint shows a FP on this code :

import java.util.logging.Logger;

/**
 * Test rule S2589 (https://rules.sonarsource.com/java/RSPEC-2589/)
 */
class SonarRuleTest {

    private static final Logger log = Logger.getLogger(SonarRuleTest.class.getName());

    public static void main(String[] args) {
        new SonarRuleTest().testFP();
    }

    void testFP() {
        String input = null;
        String s1 = org.springframework.util.StringUtils.trimWhitespace(input);

        if (s1 != null) {
            log.info("not null");
            return;
        }

        log.info(s1);
    }

}

On line if (s1 != null) , it says : Remove this expression which always evaluates to “true”.
But s1 is always null in this case…

Here is the Maven pom :

<?xml version="1.0" encoding="UTF-8"?>
<project xmlns="http://maven.apache.org/POM/4.0.0"
         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
         xsi:schemaLocation="http://maven.apache.org/POM/4.0.0 http://maven.apache.org/xsd/maven-4.0.0.xsd">
    <modelVersion>4.0.0</modelVersion>

    <groupId>xxx</groupId>
    <artifactId>demo-fp-S2589</artifactId>
    <version>1.0-SNAPSHOT</version>

    <properties>
        <maven.compiler.source>17</maven.compiler.source>
        <maven.compiler.target>17</maven.compiler.target>
        <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
    </properties>

    <dependencies>
        <dependency>
            <groupId>org.springframework</groupId>
            <artifactId>spring-core</artifactId>
            <version>6.1.6</version>
        </dependency>
    </dependencies>

</project>
4

Hello @ghusta,

Thanks for reaching out. I think your problem is coming from the fact that our engine assumes that you can’t pass null to the method trimWhitespaces and thus guarantees that the result is not null; in your case, that’s, of course, not true. However, if you look at the documentation of StringUtils some of the parameters are annotated as Nullable, while others are not. Thus it’s not trivial to detect the right contract between the values.

As a side note:

Thanks for reaching out, but I’m not sure we can do much here due to the lack of a null/non-null contract.

Best,
Margarita

6

Thank you for your answer.

In fact I’ve simplified the example a bit.
Originally I had someting like

// request of type jakarta.servlet.http.HttpServletRequest
String myParam = request.getParameter("xxx");
String s1 = org.springframework.util.StringUtils.trimWhitespace(input);

But I don’t think it changes much things.

What I don’t understand, is if I create my own class MyOwnStringUtils with the same methods (trimWhitespace() and hasLength()), SonarLint doesn’t show this warning.

And with that version, I have error S2583 (Conditionally executed code should be reachable) :

    public static void foo(String input) {
        String s = StringUtils.trimWhitespace(input);
        if (s == null) {
            System.out.println("null");
        }
        System.out.println(s);
    }

Regards,

7

I think our SE engine (and IntelliJ Data flow analysis) make the same assumption that the function from StringUtils.trimWhitespace can’t return null. This is probably caused by the fact that it’s argument isn’t marked as nullable. While arguments of other functions in the class could be marked. This causes a mess in understanding what are the nullability constraints.

8

Thank you for your analysis.

So what should be done is to suggest Spring FWK team to add annotation @Nullable on that parameter. And maybe also on the method itself as it can return null ?

I found some unit tests on that method, confirming it can accept and return null.
See : spring-framework/spring-core/src/test/java/org/springframework/util/StringUtilsTests.java at main · spring-projects/spring-framework · GitHub

Regards,

9

Hey,

That would be nice to make these annotation usages consistent.

I also created a ticket on our side so we can try to fix the issue even without annotations:

https://sonarsource.atlassian.net/browse/SONARJAVA-4968

Best,
Margarita