15

I'm working on a large project where, even with 10s of 1000s of automated tests and 100% code coverage, we're getting a ridiculous number of errors. About 95% of errors we get are NullReferenceExceptions.

Is there any way to enforce null-checking at compile time?

Barring that, is there any way to automagically enforce null-checking in unit tests without having to write the tests for null cases myself?

Mehrdad Afshari
  • 414,610
  • 91
  • 852
  • 789
Juliet
  • 80,494
  • 45
  • 196
  • 228
  • 1
    Are the NullReferenceExceptions coming from the testing framework or from the actual code that's being tested? – Matthew Iselin Feb 25 '10 at 21:18
  • Which Build Server ? if TFS maybe use code analysis policy rules help. – Yoann. B Feb 25 '10 at 21:19
  • 1
    Perhaps add a rule to your style checker looking for `{} = null` and `return null;`? If you never set anything to null, the only things you have to check for null are the results of library calls. – Anon. Feb 25 '10 at 21:20
  • 3
    @Anon: And unitialized class field members of course.. – Patrick Feb 25 '10 at 21:24
  • @Matthew Iselin: the exceptions are come from code, not the testing framework. We have some automated end-to-end system and integration tests, and it seems to work well enough, but many null exceptions are discovered by our QA testers or users out in the field. – Juliet Feb 25 '10 at 21:26
  • @Yoann: we use TestDriven.NET with MbUnit, NCover, CCNet, and FxCop. – Juliet Feb 25 '10 at 21:27
  • How are you "fixing" these problems when you see them? You need to find the underlying cause, which might be a social cause, not a technical one, or it could be architectural (bad exception handling pattern). – John Saunders Feb 25 '10 at 23:30

13 Answers13

18

You should look into Code Contracts. The static checker is only available for the higher-end VS editions, but that's basically what you're after.

There are plenty of resources online, and <plug> you can also read a prerelease version of the chapter on Code Contracts from the 2nd edition of C# in Depth - download chapter 15 for free. </plug> (The chapter is slightly out of date with respect to the latest and greatest build of Code Contracts, but nothing huge.)

Jon Skeet
  • 1,421,763
  • 867
  • 9,128
  • 9,194
  • +1 Code Contracts will definitely stop your null references cold at compile time. You will not be able to build until you eliminate all possibilities of a null value being passed into a particular method/class. Also check out Pex which goes hand-in-hand with Code Contracts. – sidney.andrews Feb 25 '10 at 21:29
  • @Jon Skeet: I'm wrong or code contracts only works if the developper use Requires.Something in the code ? So if there is a developper mistake of checking using contracts, it will pass at compile time ? I think that Juliet want to check this after developpment time, when testing or building. – Yoann. B Feb 25 '10 at 21:58
  • @Yoann: Well yes, you have to express the contract in the code. How else are you going to distinguish between APIs which *can* accept nulls and those which can't? But the static checker *does* perform checking at compile time of the callers of the API. – Jon Skeet Feb 25 '10 at 22:13
  • I still wish they'd done it the same way as Spec#. Code Contracts are probably the best solution here but they are so... verbose. – Aaronaught Feb 25 '10 at 23:37
  • @Jon: That's why i adviced custom code analysis rules, but not sure it's possible to build custom rules checking null references. – Yoann. B Feb 25 '10 at 23:39
  • THANK YOU for a clear, constructive answer. The linked chapter is great. Will it remain available? I would like to link to it... – Pascal Cuoq Feb 25 '10 at 23:42
  • @Aaronaught: Agreed. I'm going to look at mixing PostSharp with Code Contracts to at least allow it to be done on attributes. @Yoann: So what advantage would the custom code analysis rules have over using code contracts? @Pascal: I don't know whether chapter 15 will stay free, to be honest. You could always suggest people buy the book ;) – Jon Skeet Feb 26 '10 at 06:21
  • +1, +answer: interesting solution. I've run it by my team, but it doesn't seem like they want to add anymore frameworks to existing code -- I'll keep pushing for code contracts and hopefully we'll see an improvement in our code. – Juliet Feb 26 '10 at 18:26
  • @Juliet - Says the coders who can't be bothered to check for nulls... – ChaosPandion Feb 26 '10 at 20:46
6

C# 8 has introduced Non-nullable reference types.

A .Net project can be modified to have the Nullable option enabled:

<LangVersion>8.0</LangVersion>
<Nullable>enable</Nullable>

The compiler will be able distinguish

  • string and string?

  • NonNullableClass and NullableClass?

Artur A
  • 7,115
  • 57
  • 60
4

100% code coverage means nothing.

It is a false sense of security.

The only thing you're measuring is that you're executing all the lines of code.

Not:

  • That those lines of code are all the lines of code that should've been there
  • That those lines of code are operating correctly (are you testing all edge cases?)

For instance, if your procedure to deal with a fire contains 1 step "run out of the building", then even if that happens in 100% of the cases, perhaps a better procedure would be to "alert the fire department, try to stop the fire, then run out if all else fails".

There is nothing built into C# that will help you with this without you specifically going in and adding code, either code contracts (.NET 4.0) or specific IF-statements (<4.0).

Lasse V. Karlsen
  • 380,855
  • 102
  • 628
  • 825
1

This isn't a technical solution, but a social one. Simply make it unacceptable in your environment to access a reference type without checking for null when the reference type has been modified in any way by outside code (another method call, etc). Unit Testing doesn't replace good old-fashioned code review.

Payton Byrd
  • 956
  • 1
  • 12
  • 27
  • this leads to thousands of lines of new code that don't really add much value. if you get a null and you can't deal with a null value, don't check for it, just crash and burn. a better convention is "never ever under any circumstances pass null references to others in production code (nulls in test code where applicable is great to reduce clutter)" – sara Apr 06 '16 at 12:24
  • @kai - that is just crazy. You don't just let production applications crash and burn, and you have no control of whether methods return null in 3rd party APIs. – Payton Byrd Apr 06 '16 at 14:14
  • 1
    it is way way way better to crash (or at least terminate the current action/request) than it is to swallow an error or to let the system continue in an unknown state. of course you don't "let" your app crash, this shouldn't happen. if you get a null somewhere you have a bug and you should fix it so you don't get nulls there. of course application boundaries, UI code and 3rd party integration points are places where you have to validate stuff, but when you get into the domain model and the business logic, nulls mostly just cause harm and hurt readability. – sara Apr 06 '16 at 14:35
  • I agree that you should never introduce nulls within code you control, but you cannot ignore nulls and just let the error bubble up. The further the error bubbles up, the less likely the error has meaning to wherever you're catching the error. So, the two choices are wrap everything in try...catch, or test for the nulls and handle gracefully. – Payton Byrd Apr 06 '16 at 15:05
  • on the contrary, you WANT to be able to insert nulls when running your code from your test suite. it helps to show what really matters and what doesn't. like I said, of course you gotta validate user input or stuff you got from a web request etc, but I maintain that if you got a null in the domain model and weren't expecting it, you have a bug and it's folly to pretend like the application works when it doesn't. wrapping everything in try/catch or doing defensive checks for every LoC is precisely what you DON'T want to do. this is turning into a chat though, so I'm out. – sara Apr 06 '16 at 15:46
1

Is there any way to enforce null-checking at compile time?

Nope. The compiler cannot determine if the run-time reference variable is pointed to null.

And ruling out null producing statements (sets and returns) isn't enough either. Consider:

public class Customer
{
  public List<Order> Orders {get;set;}
}
  //now to use it
Customer c = new Customer;
Order o = c.Orders.First(); //oops, null ref exception;
Amy B
  • 108,202
  • 21
  • 135
  • 185
1

1) I think, the Resharper can suggest you to check some critical places in your code. For example, it suggests to add the [null reference check code] and adds it if you allow.

Try it. It will increase your experience if you need, of course.

2) Use "Fail Fast" pattern (or assert, assertions) in your code at the early stage of development application

garik
  • 5,669
  • 5
  • 30
  • 42
1

Defensive programming can only get you so far... maybe its just better to catch the exception and deal with it like any other.

CurtainDog
  • 3,175
  • 21
  • 17
  • In "dealing" with the exception, be sure to deal with why it happened. Why is this reference never being set? Was an exception thrown before it could be set? That happened to me today, and it was necessary to track down the reason for it (a missing resource causing an `ArgumentNullException`, which was logged and ignored). – John Saunders Feb 25 '10 at 23:29
  • Certain things, in particular io operations, you can never be sure will work. If yanking out a cable somewhere causes a method to return null (probably bad practice but you can't always get what you want) then you may as well catch it as an exception. – CurtainDog Feb 26 '10 at 00:36
0

neither of those is possible with C# 3. you would have to use something like Spec#... i think C#4 may have some of that built into it, but i'm not sure about that.

spec#: http://research.microsoft.com/en-us/projects/specsharp

Derick Bailey
  • 72,004
  • 22
  • 206
  • 219
0

You cannot have null checking at compile time as at compile time the objects are only Types and only at runtime the Types are converted to instances that have a concrete value ... here null.

AxelEckenberger
  • 16,628
  • 3
  • 48
  • 70
0

Maybe you should take a look at custom code analysis checkin policies for TFS

http://weblogs.asp.net/uruit/archive/2009/03/17/writing-custom-rules-for-tfs-2008-code-analysis-check-in-policy.aspx

Yoann. B
  • 11,075
  • 19
  • 69
  • 111
0

The .NET framework was looking to enforce compile time null reference checks by using an ! modifier.

public void MyMethod(!string cannotBeNull)

But alas, we don't have compile time checking. Your best bet is to minimize the amount occurrences for external callers to pass null values and then enforce null checks on public facing methods:

public class ExternalFacing
{
  public void MyMethod(string arg)
  {
     if (String.IsNullOrEmpty(arg))
        throw new ArgumentNullException(arg);

     implementationDependency.DoSomething(arg);
   }
}

internal class InternalClass
{
    public void DoSomething(string arg)
    {
         // shouldn't have to enforce null here.
    }
}

Then apply the appropriate unit tests to the External class to expect ArgumentNullExceptions.

bryanbcook
  • 16,210
  • 2
  • 40
  • 69
  • 1
    Not sure why this was downvoted, Spec# is a real thing that originated from the Microsoft Research lab. http://research.microsoft.com/en-us/projects/specsharp/ Code Contracts is a better option, but I'm not wrong. – bryanbcook Apr 23 '13 at 19:49
0

I may be wrong, but I think FxCop has a rule that suggests you add null reference checks to your code. You could try running your assemblies through the tool and see what it has to say.

Grant Palin
  • 4,546
  • 3
  • 36
  • 55
0

Check out Gendarme, it can be run post-build alongside your tests (possibly before them, if you wish) and has a few rules relating to null checks. You can also fairly trivially write your own.

Noon Silk
  • 54,084
  • 6
  • 88
  • 105