[Codenarc-developer] UnnecessaryNullCheckBeforeInstanceOf - false negative?

[Chris M. <chr...@ea...>]

Hamlet,

 

Out in the real world, I ran into what seemed like a false negative for
UnnecessaryNullCheckBeforeInstanceOf.

 

The rule produced no violation for this code, though I would have expected
one:

 

            boolean equals(Object object) {

                object != null && object instanceof RequestKey

            }

 

I added a failing test, and could "fix" the rule by overriding
visitBinaryExpression() instead of visitBooleanExpression().

 

But now that causes a violation for the highlighted line, failing the test:

 

    void testSuccessScenario() {

        final SOURCE = '''

            if (x instanceof MyClass) { }

            if (x instanceof MyClass) { } else if (x instanceof OtherClass)
{}

            (x instanceof MyClass) ? foo : bar

            if (x != null && x instanceof MyClass && x.isValid()) { }     //
violation

            if (x != null && y instanceof MyClass) { } // different
references

        '''

        assertNoViolations(SOURCE)

    }

 

Is there a reason that line should not cause a violation? 

 

Thanks.

Chris