7

I'm using ReSharpers [NotNull] annotations like this:

public void MyMethod([NotNull] string a)
{
    if(a == null) // Warning: Expression is always false.
    {
        throw new ArgumentNullException();
    }

    // ...
}

However, due to the NotNull annotation, ReSharper warns me about the unused precondition check, because the

expression is always false

But, as far as I understand those annotations, they only indicate that the parameter should never be null; i.e. they do not prohibit callers from passing null, e.g. as in

this.MyMethod(null);

or even less obvious (more like in real code)

string foo = null;
this.MyMethod(foo);

Therefore I feel like it does make sense to include precondition checks for null, but maybe I'm missing out on a concept or don't understand it correctly.

Does it make sense to include explicit nullability checks for with [NotNull] annotated parameters?

Thomas Flinkow
  • 4,845
  • 5
  • 29
  • 65
  • 1
    Your understanding is correct. `NotNull` basically means "don't pass null or method will throw exception", and that's what you are doing. And actually for me Resharper does not produce any warnings (like "expression is always false") for your code. – Evk Mar 02 '18 at 08:38
  • 1
    @Evk I might have changed something in the settings, I will try to change it back because obviously, "the expression is **always** false" is not correct. Apart from that, I take it that you would also include explicit nullabilty checks, right? – Thomas Flinkow Mar 02 '18 at 08:40
  • 3
    Yes, implementation of your method should not be related to those `Null`, `NotNull` etc decorations, just implement it as usual. They are only to notify caller (and by caller here I mean developer which uses Resharper) that he should better not pass null values, that's all. – Evk Mar 02 '18 at 08:43
  • 2
    @Evk thank you, I really like the wording *decorations*, that makes it quite clear. – Thomas Flinkow Mar 02 '18 at 08:44
  • 1
    `[NotNull]` only allows static analysis, not runtime. However, I suggest you to take a look at Fody.NullGuard, which can inject null checks based on usages of that attribute. – Matthias Mar 03 '18 at 14:47

1 Answers1

3

I tested this and when marking a parameter with [NotNull] you are basically telling that this parameter should not take a null string. When passing a null value you are getting a warning from Resharper that tells you "Possible null assignment for entity marked with [NotNull] attribute.", but you are not getting an error, so the program compiles. It is wise to guard against nulls in the method that is being called.

Alin
  • 394
  • 5
  • 14
  • 2
    Yep, that's exactly the point, it tells the callers that the parameter **should** not be `null`, but it afaik doesn't **enforce** the parameter to never be `null`. I'll follow your advise and guard against `null` parameters. – Thomas Flinkow Mar 02 '18 at 08:45