codenarc-developer

[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

Re: [Codenarc-developer] UnnecessaryNullCheckBeforeInstanceOf - false negative?

[Hamlet D. <ham...@ca...>]

Hi Chris, 

I think your fix is the correct solution. 

This should be a violation: 

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





----- Original Message -----
> 
> 
> 
> 
> 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
> ------------------------------------------------------------------------------
> Try before you buy = See our experts in action!
> The most comprehensive online learning library for Microsoft
> developers
> is just $99.99! Visual Studio, SharePoint, SQL - plus HTML5, CSS3,
> MVC3,
> Metro Style Apps, more. Free future releases when you subscribe now!
> http://p.sf.net/sfu/learndevnow-dev2
> _______________________________________________
> Codenarc-developer mailing list
> Cod...@li...
> https://lists.sourceforge.net/lists/listinfo/codenarc-developer
>