Re: EAP 2.5 feedback?

Hi again Ilya

Having now used the NRE warning some more, I'm coming back with some comments.

When you use the attributes to specify behaviour in a method's paramteter
signature, that makes perfect sense. However, for locally defined variables,
it makes less sense.

foreach(Thing thing in things)
{
Thing newThing = thing.Copy();
newThing.DoSomething();
}

The call to DoSomething() will have a NRE. Typically, I would want it to
fail if Copy() had failed and returned null, so I would not want to add if
(newThing != null)..... Also, I would prefer not to add if (newThing ==
null) throw...... as this pollutes the code from my perspective.

Mayve NRE could have options so that I could turn off warnings for locally
defined variables, but keep those for parameters?

Does anyone else have views on this???

Cheers

Sean

Hello sean,

Thank you for your feedback

sk>> I don't like the null reference analysis and this is the first
sk>> thing I have ever turned off in R# since the 1.0 EAP! The thing
sk>> with this is that I write routines that expect the client to supply
sk>> good params. If it fails then good - fail and fail early! I found
sk>> the null reference analysis was inviting me to change code to be
sk>> 'safer' where I want the code to throw as soon as it's not happy.
sk>>

Could you please give an example in which false NRE (Null Reference
Exception) warning is issued? If parameters are not annotated with
attributes specified in Code Inspection options, it should not show
any warning. However, if you check it for null and then use parameter
in other branch it will consider that param CAN be null and warn you.
E.g.

private void foo(object p)
{
p.ToString(); // no warning
}
private void foo1(object p)
{
if (p == null)
{
}
p.ToString(); // warning
}
private void foo2( object p)
{
if (p == null) // "Expression is always false" warning
{
}
p.ToString(); // no warning
}



1 comment
Comment actions Permalink

Hello sean,

sk> foreach(Thing thing in things)
sk> {
sk> Thing newThing = thing.Copy();
sk> newThing.DoSomething();
sk> }

Do you have any NRE attributes on Copy() signature? If you annotate Copy()
with Nullable attribute, it sure means that newThing can be null and thus
triggers warning on the next line. If you don't put any attribute on Copy()
method, the state of newThing will be unknown (analyser has no idea about
null-state), so no NRE warning is displayed.

So you have full control about what is NRE-analysed in your code, via annotation
attributes. However, there are several cases, when NRE analysis uses special
knowledge about code. For example, if you use

void foo(ITypeB b)
{
ITypeA a = b as ITypeA;
}
and no implicit conversion exist (otherwise you will get redundand cast warning),
NRE analyser will assume that variable 'a' can be null. That is, if we change
your code slightly:

foreach(Thing thing in things)
{
Thing newThing = thing.Clone() as Thing; // cast here, Clone returns object
newThing.DoSomething();
}

You will have NRE in this place, regardless of attributes on Clone.

Does this make sense?

Sincerely,
Ilya Ryzhenkov

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


0

Please sign in to leave a comment.