Should I use both NotNull and ContractAnnotation("null => halt")?

Question

It seems that NotNull and ContractAnnotation("key: null => halt") are fairly similar in their effect on R#. Is there something that I am missing? Should I always apply both?

Bibbye

Answer 1

They're very similar, but are semantically very different.

NotNull states that the target is expected to not be null, but makes no statements about the consequences. There might be a null check that throws an ArgumentNullException, or it might just use it without checking, and you could get a runtime NullReferenceException. Equally, the application might check for null, log it and continue safely.

ContractAnnotation("key: null => halt") tells ReSharper that if the key parameter is null, then the program flow terminates. ReSharper can use this like so:

string foo = null;
Assert.NotNull(foo);  // ContractAnnotation("null => halt")
Assert.Equal(12, foo.length);

The second assert in this snippet will be marked as dead code, because ReSharper knows the first assert will throw when passed a null.

However, if Assert.NotNull was just marked with the NotNull attribute, then ReSharper would highlight the foo parameter with a warning, telling you that you shouldn't be passing a null value, but it doesn't know what will happen if you do.

Subtle, but different. I'd stick to the NotNull attribute if you require the value to never be null, and use ContractAnnotation("null => halt") for an assert style method that will definitely and explicitly throw if passed a null.

Lanate

Thank you for the explanation. Still feels a bit wrong having to write null-checking/-annotating code thrice: the contract, the NotNull annotation and then the runtime guard. – Bibbye

My question was inspired by Microsoft's EF7 code. It seems that they are using contracts only for their guards class, and NotNull + guards for everything else: github.com/aspnet/EntityFramework/blob/dev/src/… Do you have any idea why? – Bibbye

@Bibbye oh wow, that annotations code looks straight out of my answer :) That's so cool! – Flats

@IgalTabachnik: I have seen your answer before :). I couldn't make my implementation any better then yours/their version. Do you have any idea why MS guys did not use JetBrains annotations throughout all their code? – Bibbye

I wouldn't regard it as writing it 3 ways. You write it once, then use an annotation to provide edit-time analysis of what you've done. If you write a null check, use ContractAnnotation to say you'll throw if it fails. If you don't, use NotNull to say you're assuming it's not null, without an explicit check. – Lanate

Answer 2

In short:

• Normally you should only use [NotNull]/[CanBeNull] annotations. They are validated by R# to notify if you are breaking contract. They are propagating through interface implementation and virtual method overrides. They are easy to understand.

• You can use [ContractAnnotation] if you need to be more expressive on specifying how method behaves. It is not a replacement of [NotNull]/[CanBeNull]. It allows you to link method inputs and outputs, link notnull/null/true/false values, specify inputs that will make you method halt. The string you are passing to [ContractAnnotation] called "function definition table", this is just an abstraction over method body behavior.

Typically need to use [ContractAnnotation] when [NotNull]/[CanBeNull] is not enough expressive in cases like:

if (!IsNotNullAndAlsoValid(obj)) return; // null => false

Assert.That(x != null); // false => halt

if (!map.TryGetValue(key, out value)) { // => false, value: null
  ...                                   // + key parameter annotated as [NotNull]
}

And yes, [NotNull] is semantically different from [ContractAnnotation("null => halt")]. This is easy to illustrate in the code, sometimes R# knows for sure that execution will halt because of NRE and highlight remaining code as 'dead':

C c = null;
c.InstanceMethod(); // possible NRE
c.SomethingElse();  // unreachable code

But when you are using [NotNull]-annotated APIs, R# just warns you about 'Possible NRE'. This is done because in the case R# has not enough information about some value being not null, it shouldn't highlight the rest of the methods as potentially unreachable:

C c = AlwaysTrue ? new C() : null;
Util.NotNullAnnotatedMethod(c); // possible NRE
Util.OtherMethod(c);            // no warnings here
if (c != null) { ... }          // no warnings here

While [ContractAnnotation] allows you to specify behavior more strictly, so that R# will rely on annotation more aggressively:

if (string.IsNullOrEmpty(text)) return; // text: null => false
if (text != null) { ... }               // condition always true
else { ... }                            // unreachable code

Shaped