Mitigate The Billion Dollar Mistake with Aspects

code 0 comments suggest edit

Tony Hoare, the computer scientist who implemented null references in ALGOL calls it his “billion-dollar mistake.”

I call it my billion-dollar mistake. It was the invention of the null reference in 1965. At that time, I was designing the first comprehensive type system for references in an object oriented language (ALGOL W). My goal was to ensure that all use of references should be absolutely safe, with checking performed automatically by the compiler. But I couldn’t resist the temptation to put in a null reference, simply because it was so easy to implement. This has led to innumerable errors, vulnerabilities, and system crashes, which have probably caused a billion dollars of pain and damage in the last forty years.

It may well be that a billion is a vast underestimate. But if you’re going to make a mistake, might as well go big. Respect!

To this day, we pay the price with tons of boilerplate code. For example, it’s generally good practice to add guard clauses for each potentially null parameter to a public method.

public void SomeMethod(object x, object y) {
  // Guard clauses
  if (x == null)
    throw new ArgumentNullException("x");
  if (y == null)
    throw new ArgumentNullException("y");

  // Rest of the method...
}

While it may feel like unnecessary ceremony, Jon Skeet gives some good reasons why guard clauses like this are a good idea in this StackOverflow answer:

Yes, there are good reasons:

  • It identifies exactly what is null, which may not be obvious from a NullReferenceException
  • It makes the code fail on invalid input even if some other condition means that the value isn’t dereferenced
  • It makes the exception occur before the method could have any other side-effects you might reach before the first dereference
  • It means you can be confident that if you pass the parameter into something else, you’re not violating theircontract
  • It documents your method’s requirements (using Code Contracts is even better for that of course)

I agree. The guard clauses are needed, but it’s time for some Real Talk™. This is shit work. And I hate shit work.

In this post,

  • I’ll explain the idea of non-nullable parameters and why I didn’t use CodeContracts in the hopes that heads off the first 10 comments asking “why didn’t you use CodeContracts dude?”
  • I’ll cover an approach using PostSharp to automatically validate null arguments.
  • I’ll then explain how I hope to create an even better approach.

Stick with me.

Non Null Parameters

With .NET languages such as C#, there’s no way to prevent a caller of a method from passing in a null value to a reference type argument. Instead, we simply end up having to validate the passed in arguments and ensure they’re not null.

In practice (at least with my code), the number of times I want to allow a null value is far exceeded by the number of times a null value is not valid. What I’d really like to do is invert the model. By default, a parameter cannot be null unless I explicitly say it can. In other words, make allowing null opt-in rather than opt-out as it is today.

I recall that there was some experimentation around this by Microsoft with the Spec# language that introduced a syntax to specify that a value cannot be null. For example…

public void Foo(string! arg);

…defines the argument to the method as a non-nullable string. The idea is this code would not compile if you attempt to pass in a null value for arg. It’s certainly not a trivial change as Craig Gidney writes in this post. He covers many of the challenges in adding a non-nullable syntax and then goes further to provide a proposed solution.

C# doesn’t have such a syntax, but it does have Code Contracts. After reading up on it, I really like the idea, but for me it suffers from one fatal flaw. There’s no way to apply a contract globally and then opt-out of it in specific places. I still have to apply the Contract calls to every potentially null argument of every method. In other words, it doesn’t satisfy my requirement to invert the model and make allowing null opt in rather than opt out. It’s still shit work. It’s also error-prone and I’m too lazy a bastard to get it right in every case.

IL Rewriting to the Rescue

So I figured I’d go off the deep end and experiment with Intermediate Language (IL)weaving with PostSharp to insert guard clauses automatically. Usually, any time I think about rewriting IL, I take a hammer to my head until the idea goes away. A few good whacks is plenty. However in this case, I thought it’d be a fun experiment to try. Not to mention I have a very hard head.

I chose to use PostSharp because it’s easy to get started with and it provides a simple, but powerful, API. It does have a few major downsides for what I want to accomplish that I’ll cover later.

I wrote an aspect, EnsureNonNullAspect, that you apply to a method, a class, or an assembly that injects on null checks for all public arguments and return values in your code. You can then opt out of the null checking using the AllowNullAttribute.

Here’s some examples of usage:

using NullGuard;

[assembly: EnsureNonNullAspect]

public class Sample 
{
    public void SomeMethod(string arg) {
        // throws ArgumentNullException if arg is null.
    }

    public void AnotherMethod([AllowNull]string arg) {
        // arg may be null here
    }

    public string MethodWithReturn() {
        // Throws InvalidOperationException if return value is null.
    }
   
    // Null checking works for automatic properties too.
    public string SomeProperty { get; set; }

    [AllowNull] // can be applied to a whole property
    public string NullProperty { get; set; }

    public string NullProperty { 
        get; 
        [param: AllowNull] // Or just the setter.
        set; 
}

For more examples, check out the automated tests in the NullGuard GitHub repository.

By default, the attribute only works for public properties, methods, and constructors. It also validates return values, out parameters, and incoming arguments.

If you need more fine grained control of what gets validated, the EnsureNonNullAspect accepts a ValidationFlags enum. For example, if you only want to validate arguments and not return values, you can specify: [EnsureNonNullAspect(ValidationFlags.AllPublicArguments)].

Downsides

This approach requires that the NullGuard and PostSharp libraries are redistributed with the application. Also, the generated code is a bit verbose. Here’s an example of the generated code of a previously one line method.

Another downside is that you’ll need to install the PostSharp Visual Studio extension and register for a license before you can fully use my library. The license for the free community edition is free, but it does add a bit of friction just to try this out.

I’d love to see PostSharp add support for generating IL that’s completely free of dependencies on the PostSharp assemblies. Perhaps by injecting just enough types into the rewritten assembly so it’s standalone.

Try it!

To try this out, install the NullGuard.PostSharp package from NuGet.  (It’s a pre-release library so make sure you include preleases when you attempt to install it).

Install-Package NullGuard.PostSharp IncludePrelease

Make sure you also install the PostSharp Visual Studio extension.

When you install the NuGet package into a project, it should modify that project to use PostSharp. If not, you’ll need to add an MSBuild task to run PostSharp against your project. Just look at Tests.csproj file in the NullGuard repository for an example.

If you just want to see things working, clone the NullGuard repository and run the unit tests.

File an issue if you have ideas on how to improve it or anything that’s wonky.

Alternative Approaches and What’s Next?

NullGuard.PostSharp is really an experiment. It’s my first iteration in solving this problem. I think it’s useful in its current state, but there are much better approaches I want to try.

  • Use Fody to write the guard blocks. Fody is an IL Weaver tool written by Simon Cropp that provides an MSBuild task to rewrite IL. The benefit of this approach is there is no need to redistribute parts of Fody with the application. The downside is Fody is much more daunting to use as compared to PostSharp. It leverages Mono.Cecil and requires a decent understanding of IL. Maybe I can convince Simon to help me out here. In the meanwhile, I better start reading up on IL. I think this will be the next approach I try. UPDATE: Turns out that in response to this blog post, the Fody team wrote NullGuard.Fody that does exactly this!
  • Use T4 to rewrite the source code. Rather than rewrite the IL, another approach would be to rewrite the source code much like T4MVC does with T4 Templates. One benefit of this approach is I could inject code contracts and get all the benefits of having them declared in the source code. The tricky part is doing this in a robust manner that doesn’t mess up the developer’s workflow.
  • Use Roslyn. It seems to me that Roslyn should be great for this. I just need to figure out how exactly I’d incorporate it. Modify source code or update the IL?
  • Beg the Code Contracts team to address this scenario. Like the Temptations, I ain’t too proud to beg.

Yet another alternative is to embrace the shit work, but write an automated test that ensures every argument is properly checked. I started working on a method you could add to any unit test suite that’d verify every method in an assembly, but it’s not done yet. It’s a bit tricky.

If you have a better solution, do let me know. I’d love for this to be handled by the language or Code Contracts, but right now those just don’t cut it yet.

Found a typo or error? Suggest an edit! If accepted, your contribution is listed automatically here.

Comments

avatar

48 responses

  1. Avatar for Jason Bock
    Jason Bock January 5th, 2013

    I wrote a framework using Cecil called Injectors that does this with IL rewriting- it's in Chapter 7 of my metaprogramming book. Roslyn could also handle this. 

  2. Avatar for Frank Quednau
    Frank Quednau January 5th, 2013

    We have opted for a Maybe struct and numerous extension methods to handle maybes. That wy, a null reference becomes something explicit. Projections and operations can be chained to be executed only if the underlying Maybe actually contains a value...

  3. Avatar for mattmc3
    mattmc3 January 5th, 2013

    One of the first thoughts I had about this was how this would handle optional parameters?  With optional parameters, the situation is reversed where the majority of the time you do want to allow null, and adding verbosity to the syntax for that would not be ideal.

    I really like the idea of using AOP for this.  It's sorely needed.   But, what I've often found is that null checking really isn't the only parameter checking you want to do.  For strings, it's more often checking !String.IsNullOrWhiteSpace().  And when you get into DBC (design-by-contract), you begin to see all the other assumptions that are part of your contract - not just non-nullability.  For my projects, I used a variation of the method described here:  http://rogeralsing.com/2008...

    This means that most of my methods start with something like:

    Verify.That(stringParam, "stringParam").IsNotNullOrWhiteSpace();
    Verify.That(intParam, "intParam")
      .IsGreaterThanOrEqualTo(0)
      .IsLessThan(100);

    It's worked really well for me.  I even share the fluent API for my unit testing, so instead of "Assert.AreEqual(expected, stringVariable)", I do "Test.That(stringVariable).IsEqualTo(expected)".

  4. Avatar for Ruben Bartelink
    Ruben Bartelink January 5th, 2013

    Re embracing it, I do just that, ably supported by AutoFixture.Idioms http://stackoverflow.com/a/... 
    See also http://autofixture.codeplex...

  5. Avatar for mattdgroves
    mattdgroves January 5th, 2013

    I like it--a good use of AOP.

  6. Avatar for Matt Calhoun
    Matt Calhoun January 5th, 2013

    Hi Phil, I think you'll find the code generated by PostSharp is actually much more concise with the paid version of the product. The community version is more verbose as demonstrated in your code example. ~Matt

  7. Avatar for haacked
    haacked January 5th, 2013

    As I mentioned, I allow conditional parameters to be null.

    Null checking isn't the only parameter checking, but its certainly the most common and rote. It's the source of a lot of bugs for me. In this case, I must invoke the 80/20 rule as the target of my solution. :)

  8. Avatar for Danny Tuppeny
    Danny Tuppeny January 6th, 2013

    I believe you've overlooked a feature of Code Contracts; it can infer where things should be not null based on your code accessing instance methods/properties :-)

  9. Avatar for Chris Airey
    Chris Airey January 6th, 2013

    My suggested solution would only cover dependencies in an IoC scenario, not global code. You can use the interceptors that are provided by Castle as an example. This will give you access to before invoke and after invoke of the method, allowing you to check for method parameters for null. Many IoC support this and even MEF does too. This would give you runtime checking rather than at compile time however.

  10. Avatar for Omer Raviv - BugAid Software
    Omer Raviv - BugAid Software January 6th, 2013

    I think ultimately, begging the Code Contracts team to allow this
    is the way to go, since you'll want to ensure we'll have good static code analysis tools that lets you know when your code is
    broken before you even run it. I personally use Resharper's [NotNull] and [CanBeNull] attributes, and I've found that the static code analysis saves me from my own dumb mistakes countless times. (See: http://blogs.jetbrains.com/... )

  11. Avatar for Shvedov Alexander
    Shvedov Alexander January 6th, 2013

    Hi, I'm developer at JetBrains ReSharper team and we are using R# itself to statically verify all the reference type usages and detect all the possible null dereferencings.

    To be efficient and precise, nullness inspection requires annotations a lot (just like in Spec#, but with standard .NET attributes [NotNull]/[CanBeNull]). Annotations are kind of extension for .NET type system, used by R# to "typecheck" all the reference types usages (and nullable types too). We are shipping packs of annotations for BCLs with ReSharper and users may annotate their projects too (just by declaring [NotNull]/[CanBeNull] attributes). We are using this annotations even in public surface (since our SDK developers are R# users :))).

    Benefits:
    - we have pretty HUGE codebase and pay ZERO cpu cycles for silly null checks. This is critical for tools like R# with a LOT of code executing every time users typing single character in VS editor
    - everything is statically proven to be correct, R# immediately forces user to check for null before calls where needed
    - nullness inspection are like very limited/specialized CodeContracts running in a real time immediately after every code change
    - nullness annotations can be easily erased from release version assemblies if you want (with [Conditional] attribute) so you can pay zero metadata bytes
    - explicit arguments checks looks noisier than [NotNull]/[CanBeNull] annotations on parameters/return values/fields

    Disadvantages:
    - code with [NotNull]/[CanBeNull] still looks noisier than code without checks at all
    - possible violations of nullability are just R# warnings, projects still compiles fine
    - R# can't prove nullness in some really complex situations and may force you to check some variable one more time
    - methods for custom null checks like MySuperCoolWay.ToCheckForNull() should be annotated to make R# known about null-checking behavior
    - ReSharper is required :)

    Why we do not use IL weavers:
    - build time is critical for projects of R# scale, weavers takes to much time (ccrewrite from CodeContracts is so much slow, for example)
    - why ever emit null checks if everything can be proven statically? :)))

  12. Avatar for gfraiteur
    gfraiteur January 6th, 2013

    I just wanted to say thank you for writing about PostSharp, and to mention two improvements. 

    First, moving some logic to build time will make the aspect much faster at run time. See the fork https://github.com/gfraiteu... for details (I sent a pull request). The code being generated by PostSharp Professional Edition will be much simpler also because it will analyze the aspect and only generate the code that supports features actually used by the aspect.

    Second, PostSharp 3 CTP (which can be installed from Visual Studio Extension Manager) contains a ready-made implementation of the aspect, but in an "opt-in" version (what you did is "opt out"). Just position the caret on a parameter and the aspect will be offered as a smart tag. This is a *very* fresh feature :). This option will generate code that is almost as good as hand-written code.

    -gael

  13. Avatar for Steve
    Steve January 6th, 2013

    Can IoC handle cases if the code is later mutated to null. I really think that null state needs to be handled when the state is being changed as opposed to compile, or init time.

  14. Avatar for Lee Smith
    Lee Smith January 6th, 2013

    You just wrap the nullable object in a struct. Then you only need to write the "s**t code" once:

    class Program    {        public class NullableObject        {        }        public struct ParameterObject        {            private readonly NullableObject value;            public ParameterObject(NullableObject obj)            {                if (obj == null) throw new ArgumentNullException("obj");                value = obj;            }            public NullableObject Value            {                get { return value; }            }        }        public static NullableObject Method1(ParameterObject obj)        {            return obj.Value;        }        static void Main(string[] args)        {            Method1(null);  //does not compile        }    }

  15. Avatar for Lee Smith
    Lee Smith January 6th, 2013

    You just wrap the nullable object in a struct. Then you only need to write the "s**t code" once:

    class Program    {        public class NullableObject        {        }        public struct ParameterObject        {            private readonly NullableObject value;            public ParameterObject(NullableObject obj)            {                if (obj == null) throw new ArgumentNullException("obj");                value = obj;            }            public NullableObject Value            {                get { return value; }            }        }        public static NullableObject Method1(ParameterObject obj)        {            return obj.Value;        }        static void Main(string[] args)        {            Method1(null);  //does not compile        }    }

  16. Avatar for Lee Smith
    Lee Smith January 6th, 2013

    Grrr wish your comments would leave in my line breaks!

  17. Avatar for Harry McIntyre
    Harry McIntyre January 6th, 2013

    There's a very interesting library/tool called Genuilder that intercepts your source during the msbuild process, and lets y ou modify it before it is compiled. It seems really clever but I think no-one has ever heard of it. What's really cool is that the VS intellisense engine picks up on code constructs you generate with it. I guess it's not too dissimilar to what Roslyn will offer.

    http://genuilder.codeplex.com/
    https://github.com/andrewda... - an example that makes properties on a class virtual

  18. Avatar for Mauricio Scheffer
    Mauricio Scheffer January 6th, 2013

    Just use an option type, like all sane statically typed languages (F#/OCaml/Scala/Haskell). Use Sasa ( http://sourceforge.net/proj... ) or FSharpx ( https://github.com/fsharp/f... ). No need to reinvent the wheel.

  19. Avatar for Chris
    Chris January 7th, 2013

    We do something very similar - we adopted the Option<t> type from F# into our C# setup. If something can be "null", we force it to be an Option which forces the consumer to handle both cases.

  20. Avatar for hcoder
    hcoder January 7th, 2013

    interesting ...
     

  21. Avatar for Mark Seemann
    Mark Seemann January 7th, 2013

    As for the automated test approach, I've been doing this for years with the AutoFixture.Idioms library. https://github.com/AutoFixt...

    To use it on an entire assembly, you could do something like this:

    var assembly = // some assembly
    var fixture = new Fixture();
    var assertion = new GuardClauseAssertion(fixture);
    assertion.Verify(assembly);

    You can fine-tune the behavior by filtering classes, or only looking at properties, constructors or whatever.

  22. Avatar for Alex Dresko
    Alex Dresko January 7th, 2013

    "...it’s time for some Real Talk™. This is shit work. And I hate shit work."

    Awesome. 

  23. Avatar for haacked
    haacked January 7th, 2013

    Hi Matt, would be nice to evaluate that myself before I invest in that solution.

  24. Avatar for haacked
    haacked January 7th, 2013

    Hi Shvedov. I didn't realize R# did this! I'll have to take a look as I'm a long time R# user. :)

  25. Avatar for distantcam
    distantcam January 7th, 2013

    Another disadvantage compared to IL weaving of the code you want is that every method and every property is going to run through this interceptor. And I noticed you've got a lot of reflection going on there.
    Compared with a Fody solution, which after the post-compile puts all of this logic into the assembly you're compiling, and has no runtime reflection to perform.

  26. Avatar for haacked
    haacked January 7th, 2013

    The creator of PostSharp sent a pull-request that does all the reflection at compile time. So the way I did it was naive and apparently we can do most of the work at compile time with PostSharp.

    Having said that, Simon Cropp sent me something to look at with Fody so I'll be checking out both approaches.

  27. Avatar for haacked
    haacked January 7th, 2013

    Thanks Mark! That looks like it's worth checking out.

  28. Avatar for Jeff in Colorado
    Jeff in Colorado January 7th, 2013

    Lame

  29. Avatar for Minnesota Steve
    Minnesota Steve January 8th, 2013

    I was not impressed with Code Contracts as it introduced a lot of noise and made the source practically unreadable.  It's quite likely we were using it wrong.  We started by basically eliminating the warnings it reported and found ourselves in a maze of twisty little passages, all alike.

    What I don't particularly like about Aspects is that they aren't always clear to the developer who comes along after you and tries to read the code and wonders why it's behaving differently from the norm and has to dig through everything to find how it's working.

    This is one of those areas I always start questioning how long can I shave a yak.   Sure I can check if something is null when it's called, but the bug is upstream.  How did I ever get a null object in the first place?

  30. Avatar for gorlok
    gorlok January 8th, 2013

    Don't fight your language: abrace it. Programming is for braves. NPE exists for fun :D

  31. Avatar for haacked
    haacked January 8th, 2013

    Well if you're writing a library, then the caller is mistakenly calling you and it's helpful to report it to them immediately. If it's your own code at fault, an ArgumentNullException gives you more information. When all you have is a stack trace from a user of your software, an ANE has way more info than an NRE. :)

  32. Avatar for knocte
    knocte January 8th, 2013

    Don't work on "embracing the shit work", as it's already done in the Gendarme static analysis tool (similar to FxCop) by one or more rules!

  33. Avatar for distantcam
    distantcam January 8th, 2013

    An analysis tool will tell you to add a guard clause, but you still need to add it manually. That's the shit work. If we can write a tool to detect when we need a guard, we should be able to write a tool that will just automatically add the guard anyway.

  34. Avatar for Eamon Nerbonne
    Eamon Nerbonne January 9th, 2013

    Optional parameters are best avoided - I'm pretty sure I'd *welcome* a change that makes them harder to use as a quick workaround.

  35. Avatar for Eamon Nerbonne
    Eamon Nerbonne January 9th, 2013

     That doesn't work reliably since structs' members are default initialized - and for references that means null.  So an uninitialized member or explicit default value will bypass your null check.

  36. Avatar for Eamon Nerbonne
    Eamon Nerbonne January 9th, 2013

    That's fine when you *want* a null-like non-value option, but the point is what do you do in the vast majority of the cases where you don't?

  37. Avatar for Alejjak Darkok
    Alejjak Darkok January 9th, 2013

    i remember these null checks beign called "Precondition assertions" which were typically done in C/C++ , i think the problem is not due to "nulls" but to the fact that programmers arent friends of asserting a functions' precondition. Take an integer parameter for example, we still have the same "null" problem if we assume all input will be positive integers, or whether the default values like 0 are correct. It should be a good programming standard to ALWAYS check for parameter preconditions and save a lot of headaches in the future (such as DateTime types with SQL ranges being different)

  38. Avatar for haacked
    haacked January 9th, 2013

    Yep. The point of this isn't to remove the responsibility of checking preconditions. It's to automatically check the _most common_ precondition. That of an argument not being null.

    The number of methods receiving reference arguments far exceeds the number of methods that take in an int, for example. At least in my code. 

    So I want to focus my time on checking preconditions that are specific to my domain such as making sure an int is in the correct range. I don't want to pepper my code with all these reference null checks to satisfy the language. Make sense?

  39. Avatar for Mass Mass
    Mass Mass January 9th, 2013

    I prefer the AOP way.

  40. Avatar for Dom Crossley
    Dom Crossley January 10th, 2013

    I coincidentally had a similar idea to this a few weeks ago.  I had some prototype code kicking around that used the Roslyn API to do method rewriting to automatically add guard clauses. 

    I've tidied the code up a bit and put it on github if you're interested.

    https://github.com/flashcur...

  41. Avatar for Mass Mass
    Mass Mass January 10th, 2013

    sdfsdf

  42. Avatar for Sam Critchley
    Sam Critchley January 20th, 2013

    Might be missing something but Fody already has a ready-to-go Null guard add-in - https://github.com/Fody/Nul...

  43. Avatar for haacked
    haacked January 21st, 2013

    Sam, the Fody NullGuard was written in response to this post! :) I'll edit the post to make that clear.

  44. Avatar for Blair Conrad
    Blair Conrad November 25th, 2014

    Sir, I'm not sure how interested you are in maintaining outgoing links, but Fody NullGuard's proper URL seems to have become

    https://github.com/Fody/Nul...

  45. Avatar for haacked
    haacked November 25th, 2014

    Thanks! I fixed it and it should publish in a few minutes.

  46. Avatar for Steve
    Steve July 21st, 2015

    I know this is old, but can you explain how to implement the AutoGuards library?

  47. Avatar for Matthew Krieger
    Matthew Krieger September 7th, 2015

    Phil - << Also, the generated code is a bit verbose. Here’s an example of the generated code of a previously one line method. >>

    Its the IL that's more verbose but IL is rarely viewed, and on top of that the extra compute cycles due to the addl code won't be material in the vast majority of use cases, is this really a downside?

  48. Avatar for Ulrich
    Ulrich April 16th, 2017

    For ReSharper users, there exists the "Implicit Nullability" extension which brings "not null by default" semantics into ReSharper's static nullability analysis. It also brings the static analysis in sync with Fody NullGuard.

    => https://github.com/ulrichb/...