[java] AvoidDeeplyNestedIfStmts violations can be unintentionally undetected

[HoushCE29]

Affects PMD Version: 5.3.3, 6.*, 7.0.0

Rule: design/AvoidDeeplyNestedIfStatements

Description: This rule has strange behavior where if you dispatch from an else block from somewhere else to a method containing violating deeply nested if code, it doesn't get caught as a violation. But if you do a simple if block but no else block, then it gets caught as a violation.

It's hard to explain without writing a book.... so see the sample code...

Code Sample demonstrating the issue:

This is a sample violating method:

private String doSomething(String somethingToDo) {
    if (condition) {
        if (anotherCondition) {
            if (problemDepthCondition) {
                return SOMETHING;
            }
        }
    }
    return SOMETHING_ELSE;
}

Here is code that the violation is caught for:

public String performSomething(String action) {
    if (action.equals(NOTHING)) {
        return NOTHING;
    }
    return doSomething(action);
}

However, this code (which is effectively the same) allows the violation:

public String performSomething(String action) {
    if (action.equals(NOTHING)) {
        return NOTHING;
    }
    // Only difference: using an else block
    else {
        return doSomething(action);
    }
}

Sure, the nested if's can and should be avoided and rewritten, but I would expect that if someone did code that way (which is what happened and why I am bringing up this issue) that it would be caught, even in the else block...

Please let me know if this actually isn't an issue (and is expected behavior), or if further clarification is needed.

Running PMD through: Gradle

[jsotuyod]

@HoushCE29 thanks for the report!

You seem to be using a really old PMD version (latest version is PMD 6.9.0, with 6.10.0 upcoming). Can you please double check the problem persists under 6.9.0?

[HoushCE29]

@jsotuyod Will do! I'll get back to you in the next week.

[HoushCE29]

@jsotuyod Confirmed, it does in fact do that same behavior with version 6.9.0.

Do you require a copy of my "test-case" mini Gradle project for further evaluation?

[jsotuyod]

@HoushCE29 shouldn't be necessary, thanks!

[adangel]

It seems to be related to https://sourceforge.net/p/pmd/bugs/44/ - I was wondering, whether we always excluded somehow the else-branch. And we do this pretty long now (since 2002 - 7e4cdbd).
I agree, that adding an else, should not silence the rule...