Null check

Hi Ilya,

I downloaded your code, fixed it to compile and run with VS2005.


I get (false!) warnings "Parameter '...' of external visible member
should be checked for null" on the following code:


public bla(Fasel fasel) {
Expect.IsNotNull(fasel);
...
}

public class Expect {
public static void IsNotNull(object toTest) {
if (toTest == null) {
throw ....
}
}
}



Sincerely,
Stefan Lieser

2 comments
Comment actions Permalink

Hello Stefan,

I'm not going to check through the whole call graph. It would be too complex.
Also, having such method IsNotNull is error prone - you can accidentaly pass
valuetype into it. It will be boxed and check will pass, though it is not
even necessary to do the check. This method doesn't save much typing and
do not improve readability in my opinion - direct check for null is better.
I made live template to create check-and-throw construct and use it all the
time.

However, false positives are possible of course. It is planned to implement
exclusion system, when you will be able to ignore specific suggestions in
specific places. For now, you can just turn off parameter verification test.
Actually, you can write your own analysis for your specific case, it's quite
easy. Alternatively, you can adjust ParameterVerificationAnalyser - change
it to accept IInvocationExpression via AnalyserRegistration attribute, and
add few lines of code to check that invoked method is IsNotNull and argument
resolves to parameter, then set _parameterShouldBeVerified[parameter] = false;


Sincerely,
Ilya Ryzhenkov


SL> Hi Ilya,
SL>
SL> I downloaded your code, fixed it to compile and run with VS2005.
SL>
SL> I get (false!) warnings "Parameter '...' of external visible member
SL> should be checked for null" on the following code:
SL>
SL> public bla(Fasel fasel) {
SL> Expect.IsNotNull(fasel);
SL> ...
SL> }
SL> public class Expect {
SL> public static void IsNotNull(object toTest) {
SL> if (toTest == null) {
SL> throw ....
SL> }
SL> }
SL> }
SL> Sincerely,
SL> Stefan Lieser


0
Comment actions Permalink

Hello Ilya,

> Also, having such method IsNotNull is error prone - you can

accidentaly pass valuetype into it. It will be boxed and check will
pass, though it is not even necessary to do the check.


You're right! Thanks for that tip, I will think about it.

This method
doesn't save much typing and do not improve readability in my opinion -
direct check for null is better. I made live template to create
check-and-throw construct and use it all the time.


For me it is more explicit to express the expectation.

Compare both versions:

public void Foo(Bar bar) {
Expect.IsNotNull(bar);
...
}

versus

public void Foo(Bar bar) {
if (bar == null) {
throw new ArgumentNullException();
}
...
}

However, false positives are possible of course. It is planned to
implement exclusion system, when you will be able to ignore specific
suggestions in specific places.


I would prefer to make the tests as good as possible so that false
positives are not reported. But I don't know if this is possible in the
"null check" case.

For now, you can just turn off parameter
verification test. Actually, you can write your own analysis for your
specific case, it's quite easy. Alternatively, you can adjust
ParameterVerificationAnalyser - change it to accept
IInvocationExpression via AnalyserRegistration attribute, and add few
lines of code to check that invoked method is IsNotNull and argument
resolves to parameter, then set _parameterShouldBeVerified[parameter] =
false;


I will take a look at that!


Sincerely,
Stefan Lieser

0

Please sign in to leave a comment.