Logo Questions Linux Laravel Mysql Ubuntu Git Menu
 

Need clarification on the meaning of Resharper NotNullAttribute

Consider the following code:

    public static void Foo()
    {
        Bar(null);
    }

    public static void Bar([NotNull] string s)
    {
        if (s == null)
            throw new ArgumentNullException("s");
    }

The [NotNull] attribute is used on Bar to tell callers that s shouldn't be null. This works fine, and I get a warning when I pass null to Bar (Possible 'null' assignment to entity marked with 'NotNull' attribute).

But it doesn't actually prevent me from passing null, so Bar must still check if s is null. So why am I also getting a warning on if (s == null) (Expression is always false) ?

As far as I can tell, this attribute has an ambiguous meaning; depending on context, it can mean two different things:

  • for the caller: don't pass a null argument
  • for the callee: this argument is not null

Am I using this attribute correctly, or am I missing something?

BTW, I'm using Resharper 7 EAP, so it might be a bug; however I want to make sure my usage is correct before I report it...


EDIT: just tried the same thing with R# 5.1 at work; it shows the warning on the call site, but not in the method. I'll report it on Jetbrain's Youtrack.


EDIT2: bug reported here

like image 942
Thomas Levesque Avatar asked May 22 '12 00:05

Thomas Levesque


1 Answers

As far as I can tell, you're using it right and ReSharper is wrong to tell you the comparison is always false. The [NotNull] attribute is no more than documentation and your code is right to double-check the input values. It wouldn't be the first time ReSharper made a wrong or stupid suggestion. The cool thing about JetBrains is that they have a public bug tracker where you can report this and get feedback straight from the devs.

That said, (and forgive me if you're aware of it), C# 4.0's Code Contracts makes this easy, predictable, and reliable:

Contract.RequiresAlways(s != null);
like image 114
Mahmoud Al-Qudsi Avatar answered Oct 03 '22 18:10

Mahmoud Al-Qudsi