0

I have a class that has a property declared as string? Email { get; set; } and a method that will return false if the email is empty or null for validation. There are other more complex checks in this method, so I am excluding them below for brevity. Although the Email was checked for null and will return False if it is null, the compiler still complains that it could be null.


public string? Email { get; set; }

private bool IsFormValid()
{
   if (string.IsNullOrEmpty(Email))
   {
      return false;
   }

   return true;
}

public void Submit()
{
    if (IsFormValid()) 
    {
        Submit(Email);     // Compiler warns that Email may be null here
    }
}

Does the compiler not recognize that Email would not be null in this scenario, or am I missing something? Is there a way to mark that Email is not null at this point other than rechecking Email is not null?

public void Submit()
{
    if (IsFormValid() && Email is not null)   // I don't want to do this since it was already checked
    {
        Submit(Email);     
    }
}
Scott Collins
  • 75
  • 6
  • There is sadly none, if you directly do not check if email is not null then the warining will still show. You could still turn this waring off. Read here to understand more https://learn.microsoft.com/en-us/cpp/error-messages/compiler-warnings/compiler-warning-level-3-c4996?view=msvc-160 – Alen.Toma Apr 15 '21 at 21:08
  • There are attributes you can use like `NotNullWhen(true)` to indicate that if you pass something to a method and it returns `true` that the value is not `null`. I'm not sure if there's an attribute for methods to indicate that a property is not null. – juharr Apr 15 '21 at 21:08
  • The compiler don't do flow analysis and knows nothing about what `IsFormValid` does. You efectively know that, at that point, the email cannot be null, but the compiler just sees a nullable variable being used in a non-nulable parameter, therefore it emits the warning. Try using a cast after the successful validation. – Alejandro Apr 15 '21 at 21:13
  • check out this page https://learn.microsoft.com/en-us/dotnet/csharp/language-reference/operators/null-forgiving – Rex Henderson Apr 15 '21 at 21:25
  • look at section that reads ..."If you can modify the IsValid method, you can use the NotNullWhen attribute to inform the compiler that an argument of the IsValid method cannot be null when the method returns true:" – Rex Henderson Apr 15 '21 at 21:25
  • It is literally not possible for the compiler in the general case to make the determination you want it to make. See "halting problem". You are getting exactly the error you should be getting. – Peter Duniho Apr 15 '21 at 21:29
  • @Alen.Toma: That is totally correct and expected behavior, luckily it give us that warning for this unsafe using of sequential access to the property. – Christoph Apr 15 '21 at 21:32
  • @juharr: He would still have to take a local reference to `Email` and check it again. – Christoph Apr 15 '21 at 21:33
  • @RexHenderson: Does not work in his context, he still would have to take a local reference to `Email` and pass it to a (static) method that has a parameter that is decorated with that attribute - for that reason he better checks with `is not null` as this check is already performed (not thread-safe). – Christoph Apr 15 '21 at 21:36
  • We are talking about static compile time checking features. Of course this cannot take threading into account. – Rex Henderson Apr 15 '21 at 21:39

1 Answers1

0

It does not work like that because it could be null. How can you be sure the Email is not changed to null since you called IsFormValid() by another thread? You either decide that you are sure to be in a single threaded context and overwrite the warning with !, or (preferred) you take a local reference to the Email, check it and then pass it to Submit.

public void Submit() {
    if (IsFormValid())
    {
        Submit(Email!);
    }
}

or

public void Submit() {
    string? email = Email;
    if (IsFormValid() && email is not null) {
        Submit(email);
    }
}
Christoph
  • 3,322
  • 2
  • 19
  • 28
  • The warning occurs even with local references, unless the null check is done in the `Submit` method itself. Definitely seems like a compiler shortcoming to me, though I understand it's probably for performance reasons that it doesn't check every method call for null checks. – Extragorey Dec 07 '21 at 00:57
  • It's worth noting you can use the `System.Diagnostics.CodeAnalysis.NotNullWhen` attribute (or similar) on a parameter to indicate that it's not null when the method returns a given value. – Extragorey Dec 07 '21 at 01:06