SonarLint Use the primitive boolean expression here

Question

I have the following class Properties:

class Properties {
    private Boolean enabled;

    public Boolean getEnabled() {
        return enabled;
    }
}

If I write the following code, SonarLint gives me a warning on the if condition saying "Use the primitive boolean expression here.".

if (!properties.getEnabled()) {
    return true;
}
// more code

Changing the if condition to the following shuts up the warning. But that less readable, that can't be what SonarLint wants or?

if (properties.getEnabled().equals(Boolean.FALSE)) {
    return true;
}
// more code

What exactly does SonarLint want me to do here? What is the problem?

What does it mean for `enabled` to be `null`? https://imgur.com/gallery/80Indtp — Andy Turner, Nov 25 '19 at 15:36
Maybe declare your *enabled* attribute as a primitive boolean. I think Sonarlint is trying to prevent a Null Pointer Exception — D. Lawrence, Nov 25 '19 at 15:36
@D.Lawrence Indeed, putting a null check before it does silence the warning. A useful warning, but a very confusing message. Thank you — findusl, Nov 25 '19 at 15:40
@AndyTurner nice image. It should not be null, it is annotated with NotNull as well. However because of the framework that sets the value it is a non primitive. Yeah not good framework one may argue and one may be right. But not really my choice. With Lawrences tip SonarLint also accepts it. Thanks you for trying and responding so quick — findusl, Nov 25 '19 at 15:42

Damonio · Answer 1 · 2020-01-29T15:24:24.367

23

As other already mentioned, Sonar wants you to make sure that you don't have any null pointer exception, or at least that's what i have seen too when i do a check before trying to validate against the variable:

if i have the next, Sonar complains

if (properties.getEnabled()) {
       // Your code
}

But if i add a quick validation against nulls, Sonar stops complaining about it

if (properties.getEnabled() != null && properties.getEnabled()) {
       // Your code
}

Now, as you mentioned you can use the Boolean class to use the next

Boolean.TRUE.equals(properties.getEnabled());

As

if (Boolean.TRUE.equals(properties.getEnabled())){
       // Your code
}

It sounds like it's too verbose with Java But internally, they check if the object is of instance Boolean, so they discard the possibility to be null, as explained here: Is null check needed before calling instanceof?

You can check from the git repo what it's accepted and what is not:

https://github.com/SonarSource/sonar-java/blob/master/java-checks/src/test/files/checks/BoxedBooleanExpressionsCheck.java

edited Jan 29 '20 at 15:24

answered Jan 28 '20 at 22:22

Damonio

258
2
8

Interesting that sonar would accept the null check as you declared it, because a getter is just a function that can return a different value on every call. – findusl Jan 29 '20 at 08:25
I don't see why sonar won't allow me to, that's the reason that you want to verify that the getter doesn't return a null, since in every case there might be a possibility that your object contains true/false/null, only in runtime you will know the value – Damonio Jan 29 '20 at 15:27
but you do not clarify it. The first call to getEnabled may return something else than the second call. in order to be save here you'd have to save the result to a local variable and then work with that one. – findusl Jan 29 '20 at 16:13
I get the point that it might be different but even if you save the result to a local variable, you are only going to store the reference to the Boolean, not the actual value, in which that can change if another thread uses the same object. It might be that the reason is that the object was within the scope of a function and not a member variable, which for sonar, it was guaranteed that no other thread will modify that object. But if there might be a thread insecurity then, as you mentioned storing the actual value in a local variable would be the best. – Damonio Jan 29 '20 at 19:54

score 1 · Answer 2 · answered Apr 02 '20 at 17:48

1

Use org.apache.commons.lang3.BooleanUtils, it's a null safe way:

if (BooleanUtils.isNotTrue(properties.getEnabled())) {
    return true;
}

answered Apr 02 '20 at 17:48

saffer

111
4

1

You dont need any library. Just use this `Boolean.TRUE.equals(properties.getEnabled());` And it is null safe – Snigdhajyoti Sep 14 '21 at 11:29

score 0 · Answer 3 · answered Feb 27 '20 at 09:13

0

Try .booleanValue() Like this if(properties.getEnabled().booleanValue()) { } Hope so it will help u.

answered Feb 27 '20 at 09:13

Rohit Chaurasiya

422
4
8

It will throw null pointer if `properties.getEnabled()` returns a null – Snigdhajyoti Sep 14 '21 at 11:31

score 0 · Answer 4 · answered Mar 24 '22 at 09:48

0

This seems to have a simple answer. If the property is nullable, then the sanity at getter should help.

class Properties {
    private Boolean enabled;

    public Boolean isEnabled() {
        return enabled == true;
    }
}

answered Mar 24 '22 at 09:48

Mukund Desai

94
2

score -2 · Answer 5 · edited Sep 14 '21 at 11:25

-2

try this :

public boolean x (){ 
    boolean prop=properties.getEnabled() 
    if (prop) { 
        return true; 
    }else return false; 
}

edited Sep 14 '21 at 11:25

Suraj Rao

28,850
10
94
99

answered Sep 14 '21 at 11:24

fady manaa

9
1

1

It will throw null pointer if `properties.getEnabled()` returns a null – Snigdhajyoti Sep 14 '21 at 11:30

SonarLint Use the primitive boolean expression here

5 Answers5