Why does sonar think this expression is always false

user_1245992 0
Viewed 259

In my Java project SonarQube says that an expression is always false. However I cannot see why. This is the code in question:

    BaseException baseException = null;

    for (SpaceInfo i: spaceInfos) {
        try {
            processSingle(i.getSpaceKey(), i.getContentType());
        } catch (BaseException e) {
            baseException = BaseException.chain(baseException, e);
        }
    }

    // Here sonar say that this condition will always evaluate to false. 
    if (baseException != null) {
        throw baseException;
    }

However in my opinion if the processSingle method throws a BaseException then baseException should not be null and therefore the expression should not evaluate to false.

The processSingle method is declared as follows:

private void processSingle(String spaceKey, String contentType) throws BaseException

And there are definitely cases in which the processSingle method will throw a BaseException. So I think that Sonar is mistaken. Or is there something going on here that I am not seeing?

Screenshot of Sonar error

Update:

This is what BaseException.chain() does:

public static BaseException chain (BaseException a, BaseException b) {
    if (a == null) { return b; }
    a.setNextException(b);
    return a;
}

And this is the code of processSingle:

private void processSingle(String spaceKey, String contentType) throws BaseException {
    assert ContentTypes.Page.equals(contentType) || ContentTypes.BlogPost.equals(contentType);

    Content content;
    try {
        content = createEmptyContent(spaceKey, contentType);
    } catch (Exception e) {
        throw new MessageToContentProcessorProcessSingleException(contentType, spaceKey, e);
    }

    BaseException baseException = null;

    try {
        contentCreator.addMetadata(content);

    } catch (BaseException e) {
        baseException = BaseException.chain(baseException, e);
    }

    Pair<List<AttachmentInfo>, FailedToSaveAttachmentException> pair = contentCreator.saveAttachments(messageParser.getContent(), content);
    List<AttachmentInfo> attachments = pair.getLeft();
    baseException = BaseException.chain(baseException, pair.getRight());

    try {
        String html = htmlGenerator.generateHtml(attachments, messageParser.getContent());
        contentCreator.updateBodyOfContent(content, html);
    } catch (BaseException e) {
        baseException = BaseException.chain(baseException, e);
    }

    if (baseException != null) {
        throw new MessageToContentProcessorProcessSingleException(contentType, spaceKey, baseException);
    }
}
sonarqube java
2 Answers

Just for testing/curiosity, I would try:

} catch (BaseException e) {
    baseException = e;
}

this would show if Sonar thinks the exception can be thrown or not. Or if it is getting confused by the chain method or assignment statement (assigning to basseException but using it (still null) on the right side of assignment).

I know this is changing the logic, just for testing

even try (but I do not believe this would trick Sonar)

} catch (BaseException e) {
    var tmp = BaseException.chain(baseException, e);
    baseException = tmp;
}

Try changing chain() to help SonarQube:

public static BaseException chain (BaseException a, BaseException b) {
    if (a == null) { 
        return b; 
    } else {
        a.setNextException(b);
        return a;
    }
}

thinking about it, hardly possible to be the problem - almost trivial that a is not null here

Admin1

I would try and see if it works:

BaseException baseException;

for (SpaceInfo i: spaceInfos) {
    try {
        processSingle(i.getSpaceKey(), i.getContentType());
        baseException = null;
    } catch (BaseException e) {
        baseException = BaseException.chain(baseException, e);
    }
}
user_1847300
Related