Warn when auto-property setter is more tolerant than getter

[RikkiGibson]

Related to #49903

SharpLab

#nullable enable
using System.Diagnostics.CodeAnalysis;

var c = new C();
c.Prop = null;
c.Prop.ToString(); // no warning, but crashes at runtime

public class C
{
    [AllowNull]
    public string Prop { get; set; }
}

In this scenario, the setter accepts a maybe-null argument but the getter claims it will return a not-null value. A manually implemented property can fulfill this by replacing a null argument with a non-null default value, but an auto-property can't do that.

We should give a warning on the declaration of this property--perhaps one indicating that an auto-property setter should not be nullable when the getter is non-nullable.

We could also give the warning in a "roundabout" way by just making sure we

1. type the synthesized backing field based on the getter's return type and
2. make sure the nullable walker analyzes the synthesized assignment of the 'value' param to the backing field in the setter

Note that this behavior is specific to properties--fields aren't much of a concern because they don't have the behavior of assuming that more tolerant setters "sanitize" their inputs.

[RikkiGibson]

While working on https://github.com/dotnet/csharplang/blob/main/proposals/field-keyword.md#nullability it occurred to me that this would be fairly straightforward to accomplish by simply nullable-analyzing the synthesized auto-property accessors.

This would lean on the existing analogy that the following two properties are equivalent (as much as is possible):

string Prop1 { get; set; }
string Prop2 { get => field; set => field = value; }

However, it would raise some questions about whether/when nullability attributes MaybeNull/NotNull/AllowNull/DisallowNull should automatically flow from the property to the field. For example, if someone already has a [MaybeNull] T Prop { get; } property, it is probably their intent that the field may be null. Some design thought would be needed, especially to reduce nuisance warnings for valid properties.

Here I will call out a few source locations which we could change in order to deliver that functionality. The first does the actual analysis while the second ensure the diagnostic location is on the appropriate accessor keyword instead of on the whole property.

https://github.com/dotnet/roslyn/blob/bdebb57e8817bdfa1b43f53e905db9b4dfa95736/src/Compilers/CSharp/Portable/Compiler/MethodCompiler.cs#L1888-L1891

                     if (sourceMethod is SourcePropertyAccessorSymbol { IsAutoPropertyAccessor: true })
                     {
+                        var autoPropertyBody = MethodBodySynthesizer.ConstructAutoPropertyAccessorBody(sourceMethod);
+                        Debug.Assert(diagnostics.AccumulatesDiagnostics);
+                        NullableWalker.AnalyzeIfNeeded(
+                           compilationState.Compilation,
+                           sourceMethod,
+                           autoPropertyBody,
+                           diagnostics.DiagnosticBag,
+                           useConstructorExitWarnings: false,
+                           initialNullableState: null,
+                           getFinalNullableState: false,
+                           baseOrThisInitializer: null,
+                           out _);
+                        return autoPropertyBody;
                     }

---

https://github.com/dotnet/roslyn/blob/bdebb57e8817bdfa1b43f53e905db9b4dfa95736/src/Compilers/CSharp/Portable/Compiler/MethodBodySynthesizer.cs#L179

-            CSharpSyntaxNode syntax = property.CSharpSyntaxNode;
+            CSharpSyntaxNode syntax = accessor.SyntaxNode;