1

Assume the following code:

public class CC3
{
    private string _field;
    private bool _someFlag;

    public string Property
    {
        get { return _field; }
    }

    public bool SomeFlag
    {
        get { return _someFlag; }
    }

    public void SetField()
    {
        _field = " foo ";
        _someFlag = true;
    }

    public string Method()
    {
        Contract.Requires(SomeFlag);
        return Property.Trim();
    }
}

The static checker of Code Contracts complains about the return statement of Method:

Possibly calling a method on a null reference 'this.Property'

What do I have to do to enable the static checker to prove that Property can never be null if SomeFlag is true?

Daniel Hilgarth
  • 171,043
  • 40
  • 335
  • 443
  • Not sure whether relevant, but maybe [the assignments could be reordered by the compiler](http://msdn.microsoft.com/en-us/magazine/jj863136.aspx) leading to a race condition? – Rawling Feb 19 '13 at 16:55
  • @Rawling: Thanks for your comment. I don't think it's relevant. No matter the order in which the body of `SetField` is executed. When calling `Method` both assignments have happened. – Daniel Hilgarth Feb 19 '13 at 16:56
  • What if one thread is calling `Method` while another is calling `SetField`? (I don't know whether this is precluded by something outside the class.) – Rawling Feb 19 '13 at 17:00
  • @Rawling: I see, that's a good point! I added a `lock` statement inside `SetField` to remove this possible race condition, but it doesn't change the outcome. – Daniel Hilgarth Feb 19 '13 at 17:04
  • Locked against Method too? Otherwise... I can't think of anything else :p – Rawling Feb 19 '13 at 17:49
  • @Rawling: No, that lock wouldn't make sense. And it doesn't help :) – Daniel Hilgarth Feb 19 '13 at 17:50
  • I've been having a play with this and, unless I've missed anything, it just doesn't look like the static analyser is too bright. Even with a class with just a private, initialized string and a method that calls `Trim` on it, it complains unless the string is marked `readonly`... – Rawling Feb 21 '13 at 09:20
  • @Rawling: I guess you missed to define the invariants of the class. The static checker needs quite a lot of hints, that'S true... – Daniel Hilgarth Feb 21 '13 at 09:24

2 Answers2

0

You can give the static analysis a helping hand using Contract.Assume:

public string Method()
{
    Contract.Requires(SomeFlag);
    Contract.Assume(Property != null);
    return Property.Trim();
}

Or actually add the check as a Contract.Requires in its own right. After all, just because you can manually prove it to be true for now, you can't guarantee that will always be the case when the code gets modified. In fact, consider whether SomeFlag being true is actually a requirement at all. Perhaps this is a cleaner solution:

public string Method()
{
    Contract.Requires(Property != null);
    return Property.Trim();
}
Matthew Strawbridge
  • 19,940
  • 10
  • 72
  • 93
  • Thanks for your suggestion. I am aware of these methods to make the warning go away. Unfortunately, they don't apply to my code. What I posted here obviously is the bare minimum to reproduce the behavior. In my real code, I have a hierarchy of methods with the first one containing the check for the flag and one of the called methods accessing `Property`. The check for the flag is indeed the correct one, think about it as some kind of `IsValid` which checks a lot more than just the one property being not null. – Daniel Hilgarth Feb 20 '13 at 06:35
  • About changes to the class: That argument is not valid - it would invalidate everything the static checker can assume. If the class is modified, the static checker needs to be run again. – Daniel Hilgarth Feb 20 '13 at 06:37
  • That is exactly my point: changes to the class would invalidate everything the static checker could assume. If you have to help the checker by adding a `Contract.Assume` then you'll have to manually check that the assumption is still a valid one each time you change the class. – Matthew Strawbridge Feb 20 '13 at 07:07
0

The only way to prove that it is not null is to prove that it is not null. Ideally you could use an invariant if you would convert to auto properties. For this example you could rewrite the property to ensure that null is not a possible result:

public string Property
{
    get {
        Contract.Ensures(Contract.Result<string>() != null);
        return _field ?? String.Empty;
    }
}
Dandy
  • 272
  • 3
  • 11