views:

166

answers:

2

Code Contracts keep giving me "Possibly calling a method on a null reference" warnings for all of my LINQ statements, and I can't find a way to silence them. For example, the following method generates two such warnings because I'm accessing the "Make" and "Model" properties of the "car" object without first checking for null.

    public IEnumerable<string> GetCarModelsByMake(string make)
    {
        return from car in Cars
               where car.Make == make
               select car.Model;
    }

In my particular case, I know that the Cars collection will never contain any null entries, so I figured I could just add an Assume to the method to silence the static checker, like so:

    public IEnumerable<string> GetCarModelsByMake(string make)
    {
        Contract.Assume(Cars.All(car => car != null));

        return from car in Cars
               where car.Make == make
               select car.Model;
    }

But that doesn't work, presumably because it's a bit too much to expect the static checker to understand. So, I decided to just suppress the warnings using the following SuppressMessage attribute:

    [SuppressMessage("Microsoft.Contracts", "NonNull")]

But for some reason, that fails to suppress the warnings. I even tried the following SuppressMessage attributes as well, none of which worked:

    [SuppressMessage("Microsoft.Contracts", "Requires")]
    [SuppressMessage("Microsoft.Contracts", "Ensures")]
    [SuppressMessage("Microsoft.Contracts", "Invariant")]

I even tried to completely disable contract verification for the method using the ContractVerification attribute:

    [ContractVerification(false)]

But that didn't work either. So, I decided I'd just add an explicit null check in the "where" clause of the LINQ statement:

    public IEnumerable<string> GetCarModelsByMake(string make)
    {
        return from car in Cars
               where car != null && car.Make == make
               select car.Model;
    }

That successfully gets rid of the warning on the "where" clause, but it doesn't eliminate the warning for the "select" clause. In fact, the only way I've found to actually get rid of both warnings is to add null checks to every clause in the LINQ statement, like so:

    public IEnumerable<string> GetCarModelsByMake(string make)
    {
        return from car in Cars
               where car != null && car.Make == make
               select car == null ? null : car.Model;
    }

Clearly, that is not very clean or efficient code, and I don't actually intend to add such redundant null checks to all of my LINQ statements -- especially when I know the enumeration doesn't contain any null entries. The best solution to this problem would be for the static checker to understand Contract.Assume statements that ensure non-null values for each item in the collection, but if that can't be done, then at least honor the SuppressMessage attribute on the method.

+4  A: 

It may be complaining about null checking Cars. Try this:

public IEnumerable GetCarModelsByMake(string make)
{
    if (null == Cars)
        return new string[0];  // or null if you like

    return from car in Cars
        where car.Make == make
        select car.Model;
}

Remember, this LINQ statement is effectively the same as:

return Cars.Where(car => car.Make == make).Select(car => car.Model);

You'll get an exception if Cars is null.

Travis Heseman
+1 but I think that adding `Cars != null` to the class invariants would stay closer to the spirit of the original code, which obviously doesn't expect `Cars` to be `null`. http://msdn.microsoft.com/en-us/magazine/ee236408.aspx#id0070066
Wim Coenen
Absolutely; an invariant for this class would be better suited for Code Contract nature of this question.
Travis Heseman
I should have clarified that the class already has an invariant that guarantees Cars != null. That's not what the contract warnings are about though. The warnings are about the "car" variable being null, not the "Cars" collection.
James Messinger
Could you add another invariant like "!Cars.Contains(null)"?
Travis Heseman
A: 

Have you tried the latest version of Code Contracts? There was one released in October, and I can't reproduce this with it.

Alternatively, Code Contracts has its own ForAll method defined on the static Contracts class, which might work better with its logic than the LINQ extension All method.

Porges