SonarLint warning, this code will always return the same value

Question

SonarLint is telling me:

"Refactor this code to not always return the same value."

But I don't seem to understand why

private List<InvoiceRejectionMessage> invoiceList = new ArrayList<>();

public String getMessage() {
    if(!invoiceList.isEmpty()) {
        return invoiceList
                .stream()
                .max(comparing(InvoiceRejectionMessage::getCreatedAt))
                .get()
                .getMessage();
    }
    return null;
}

InvoiceList is defined and will always be initialized, so it can't be null only empty. If it's empty, return null. If it's not, we're certain there's an element in there that can be returned by .max() and .get()

I don't feel comfortable refactoring this method just because Sonarlint tells me to, I'd rather know why I'm getting this warning

Answer 1

The rule associated to this hint is

squid:S3516 - Methods returns should not be invariant

The SonarQube implementation of this rule is available on GitHub.

Without seeing your code as a whole, I'm not 100% sure why this rule gets triggered. However, I suspect that Sonar is able to figure out that

1. invoiceList is unconditionally empty and; therefore
2. the if branch of your if-then-else statement never gets exercised; therefore,
3. the getMessage method unconditionally returns null.

---

Anyway, there is no need to treat the empty list as a special case; you can simplify the code in the following way, which will likely pacify Sonar:

private List<InvoiceRejectionMessage> invoiceList = new ArrayList<>();

public String getMessage() {
    return invoiceList
            .stream()
            .max(comparing(InvoiceRejectionMessage::getCreatedAt))
            .map(InvoiceRejectionMessage::getMessage)
            .orElse(null);
}

Incidentally, if you can change the class's API, its clients would benefit from changing getMessage's return type to Optional<String> (simply drop the last orElse(null) call):

public Optional<String> getMessage() {
    return invoiceList
            .stream()
            .max(comparing(InvoiceRejectionMessage::getCreatedAt))
            .map(InvoiceRejectionMessage::getMessage)
}