5

I've just started experimenting with CodeContracts in .NET 4 on an existing medium-sized project, and I'm surprised that the static checker is giving me compile-time warnings about the following piece of code:

public class Foo
{
   private readonly List<string> strs = new List<string>();

   public void DoSomething()
   {
       // Compiler warning from the static checker:
       // "requires unproven: source != null"
       strs.Add("hello");
   }
}

Why is the CodeContracts static checker complaining about the strs.Add(...) line? There's no possible way for strs to be null, right? Am I doing something wrong?

Judah Gabriel Himango
  • 58,906
  • 38
  • 158
  • 212
  • I don't have a copy of VS Premium to hand, but *that exact code* reports a problem? If so, I suggest you report it on the CodeContracts forum. That really surprises me. – Jon Skeet Aug 14 '10 at 19:19
  • I agree with Jon Skeet. I see how it could fail if `Foo` were marked `[Serializable]`. But if the checker can't prove this, what *can* it prove? – Niki Aug 14 '10 at 19:29
  • Jon, yes, that exact code actually causes a problem. I pasted this class into a test project, turned on static checking, and immediately I see a warning: "Possibly calling a method on a null reference". – Judah Gabriel Himango Aug 15 '10 at 19:26
  • I've reported it to the CodeContracts forum: http://social.msdn.microsoft.com/Forums/en/codecontracts/thread/25247563-6a77-461e-b591-8a2e14ddda9c – Judah Gabriel Himango Aug 15 '10 at 21:33

4 Answers4

8

The field may be marked readonly, but unfortunately the static checker isn't smart enough for this. Therefore, as the static checker isn't able to infer on its own that strs is never null, you must explicitly tell it through an invariant:

[ContractInvariantMethod]
private void ObjectInvariant() {
    Contract.Invariant(strs != null);
}
Rich
  • 3,081
  • 1
  • 22
  • 24
  • The thing with Code Contracts is to always be explicit. It doesn't "know" anything you haven't told it. – porges Aug 18 '10 at 05:41
  • 1
    *"It doesn't know anything you haven't told it."* And that is the major problem with code contracts, IMO. Way too much forced hand-holding, which results in major amounts of false positives. – Judah Gabriel Himango Dec 12 '11 at 19:32
  • 2
    To be fair, even readonly fields can be set through reflection, so in fact, `strs` *CAN* be `null` – Daniel Hilgarth Nov 08 '12 at 13:16
  • Sure, but we can also make System.String mutable through reflection; that doesn't mean we change the world to accommodate this edge case. – Judah Gabriel Himango Apr 12 '16 at 19:20
3

The following is valid code, which I would expect to generate the warning.

public class Foo 
{ 
   private readonly List<string> strs = new List<string>(); 

   public Foo()
   {
       // strs is readonly, so we can assign it here but nowhere else
       strs = ResultOfSomeFunction();
   }

   public void DoSomething() 
   { 
       // Compiler warning from the static checker: 
       // "requires unproven: source != null" 
       strs.Add("hello"); 
   } 
}

It's quite possible that their analyzer doesn't go so far as to make sure that you have nothing in a constructor that changes the value of strs. Or maybe you are somehow changing strs in a constructor and you don't realize it.

Gabe
  • 84,912
  • 12
  • 139
  • 238
1

Little correction: Pex uses Z3, an SMT solver while Clousot (the static checker code name) uses abstract interpretation and abstract domains.

Peli
  • 2,465
  • 16
  • 17
0

I'm not knowledgeable enough with the intricacies of .NET's object initialization semantics to answer your direct question. However, here's two tips:

  1. Microsoft Research's Pex can automatically generate unit tests that demonstrate exactly under what conditions a contract violation might occur. It uses the same theorem prover and static analyzer as CC does, so it's a fair bet, the two will agree.
  2. Proving contracts is equivalent to solving the Halting Problem, so just because CC can't prove that it cannot ever be null, doesn't mean it isn't true. IOW: CC might just be wrong and you need to help it along with a Contract.Assume (or, if you're feeling confident, Contract.Assert).

Interestingly, if you explicitly add the Object Invariant Assertion that strs can never be null, CC is able to prove that and, consequently, can also prove that strs.Add() will never be a null reference:

[ContractInvariantMethod]
private void StrsIsNotNull()
{
    Contract.Invariant(strs != null);
}

So, I guess my hunch #2 is correct: CC is just simply wrong in this case. (Or more precisely: the encoding of the semantics of C# into the theorem prover is incomplete.)

Jörg W Mittag
  • 363,080
  • 75
  • 446
  • 653
  • 1
    "Proving contracts is equivalent to solving the Halting Problem" - only very generally. Meanwhile there are a ton of commonly encountered specific cases that can be proven. As Skeet says, if it can't figure out the above, that would be weird. – Daniel Earwicker Aug 14 '10 at 19:25