R# (4.0.738) incorrect "possible null reference" warning

I have the following code in a user control code-behind file:

panel.Visible = document != null && ShowHeader;
if( panel.Visible )
{
lblTitle.Text = document.Title;
}

R# erroneously warns that document.Title may produce an error.

Yours,
Morten

6 comments
Comment actions Permalink

Hello,

I have the following code in a user control code-behind file:

panel.Visible = document != null && ShowHeader;
if( panel.Visible )
{
lblTitle.Text = document.Title;
}
R# erroneously warns that document.Title may produce an error.


panel.Visible might, technically, change its value before the "if" check.


Serge Baltic
JetBrains, Inc — http://www.jetbrains.com
“Develop with pleasure!”


0
Comment actions Permalink

Moreover, it is not guaranteed that if you assign "true" to panel.Visible,
then you'll read the "true" from it

--
Eugene Pasynkov
Developer
JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"
"Serge Baltic" <baltic@intellij.net> wrote in message
news:dc0986bfb85d58ca46accb4fe770@news.intellij.net...

Hello,

>
>> I have the following code in a user control code-behind file:
>>
>> panel.Visible = document != null && ShowHeader;
>> if( panel.Visible )
>> {
>> lblTitle.Text = document.Title;
>> }
>> R# erroneously warns that document.Title may produce an error.
>

panel.Visible might, technically, change its value before the "if" check.

>

-
Serge Baltic
JetBrains, Inc - http://www.jetbrains.com
"Develop with pleasure!"

>



0
Comment actions Permalink

Eugene Pasynkov (JetBrains) wrote:

Moreover, it is not guaranteed that if you assign "true" to panel.Visible,
then you'll read the "true" from it


While all true, these "false positives" distract a lot from the ones you
really should be watching out for.

Let me rephrase the bug as a suggestion then: It'd be nice if R# knew
about classes shipping with .NET itself, as their behavior is pretty
static and it could reduce the number of warnings significantly.

Yours,
Morten

0
Comment actions Permalink

Hello Morten,

As far as I remember, the Visible property is not the one that should be
ignored. If you have panel inserted into invisible container, than setting
Visible to true will still result getting it as false. I may be wrong, but
I remember this property to be non-trivial in get/set policy.

Anyway, we feel that such non-straightforward logic is error prone. If you
don't like it, it may worth switching off standard library annotations. Currently
it can be done by erasing contents of ExternalAnnotations folder in ReSharper
installation dir and restarting VS. I think we may include global switch
to turn this off, but it is not yet decided.

Personally, I'd go with local variable:

var shouldShowHeader = document != null && ShowHeader;
panel.Visible = shouldShowHeader;
if( shouldShowHeader )
{
lblTitle.Text = document.Title;
}


Sincerely,
Ilya Ryzhenkov

JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"


MM> Eugene Pasynkov (JetBrains) wrote:
MM>
>> Moreover, it is not guaranteed that if you assign "true" to
>> panel.Visible, then you'll read the "true" from it
>>
MM> While all true, these "false positives" distract a lot from the ones
MM> you really should be watching out for.
MM>
MM> Let me rephrase the bug as a suggestion then: It'd be nice if R#
MM> knew about classes shipping with .NET itself, as their behavior is
MM> pretty static and it could reduce the number of warnings
MM> significantly.
MM>
MM> Yours,
MM> Morten


0
Comment actions Permalink

Hi,

that suggestion makes perfect sense to me, thanks!

Yours,
Morten

Ilya Ryzhenkov wrote:

Hello Morten,

As far as I remember, the Visible property is not the one that should be
ignored. If you have panel inserted into invisible container, than
setting Visible to true will still result getting it as false. I may be
wrong, but I remember this property to be non-trivial in get/set policy.
Anyway, we feel that such non-straightforward logic is error prone. If
you don't like it, it may worth switching off standard library
annotations. Currently it can be done by erasing contents of
ExternalAnnotations folder in ReSharper installation dir and restarting
VS. I think we may include global switch to turn this off, but it is not
yet decided.
Personally, I'd go with local variable:

var shouldShowHeader = document != null && ShowHeader;
panel.Visible = shouldShowHeader;
if( shouldShowHeader )
{
lblTitle.Text = document.Title;
}


Sincerely,
Ilya Ryzhenkov

JetBrains, Inc
http://www.jetbrains.com
"Develop with pleasure!"


MM> Eugene Pasynkov (JetBrains) wrote:
MM>

>>> Moreover, it is not guaranteed that if you assign "true" to
>>> panel.Visible, then you'll read the "true" from it
>>>

MM> While all true, these "false positives" distract a lot from the ones
MM> you really should be watching out for.
MM> MM> Let me rephrase the bug as a suggestion then: It'd be nice if R#
MM> knew about classes shipping with .NET itself, as their behavior is
MM> pretty static and it could reduce the number of warnings
MM> significantly.
MM> MM> Yours,
MM> Morten

0
Comment actions Permalink

This does seem to be a false positive -- it appears as though ReSharper needs to be taught that execution will stop if the NUnit assert fails:

Stream s = getStream();
Assert.IsTrue(s is FileStream); // execution stops if not a FileStream
var fs = s as FileStream; // will always be not null due to previous assertion
fs.Seek(0); // undeserved potential null

0

Please sign in to leave a comment.