3.6 KiB
C# Language Design Meeting for September 4, 2019
Agenda
[AllowNull]
on properties
Discussion
https://github.com/dotnet/roslyn/issues/37313
AllowNull and flow analysis
Some questions have come up concerning AllowNull
and property analysis, like in the following example:
class Program
{
private string _f = string.Empty;
[AllowNull]
public string P
{
get => _f;
set => _f = value ?? string.Empty;
}
static void Main(string[] args)
{
var p = new Program();
p.P = null; // No warning, as expected
Console.Write(p.P.Length); // unexpected warning
}
}
The tricky part here is that we are tracking flow state for properties, meaning if null
is stored in a
property, we treat that property as having a null state. Although there is an attribute, the flow state
of the variable wins, meaning that we think there is a null
inside P
, even though the stated type
is string
. Similarly, if you invert the attribute and use [NotNull]
on a string?
, the flow state
will win again.
There a couple ideas to address the problem, including
- Don't flow-track non-nullable fields/properties.
- Have
[AllowNull]
suppress both the warning, and suppress setting the flow state tonull
- Have the
[NotNull]
attribute suppress the flow state (which it currently doesn't), and require the property be written with a nullable type (string?
) and have[NotNull]
on the getter. - Stop tracking when any nullability attributes are present on properties.
2 and 3 are somewhat related and we could do both, in theory. The main problem is that it greatly complicates the user's understanding of when flow tracking is enabled, and there are also potentially downstream affects for type inference if we allow the rule for (2) to apply to parameters.
For (1) it seems plausible, since we would still have a warning on assigning a null to a non-nullable member. This would remove the flow-based subsequent warnings, but the user would still be warned at the point where the problem happens. However, it doesn't seem to solve the problem for generics, e.g.
class C<T> where T : new()
{
private T _f = new T();
[AllowNull]
public T P
{
get => _f ?? throw new Exception();
set
{
if (value is null)
{
throw new Exception();
}
return _f;
}
}
}
Here the generic property P
cannot be marked nullable because it is unconstrained.
However, 2 & 3 also have a problem around MaybeNull
. If a property is annotated MaybeNull
, then
the attribute would override the state, meaning that a null check is useless. If you check for null
it would not matter, because when you read the property again, the attribute would override the state
and the result would still be maybe null.
An idea to address this is a combination of (4) and (3), where we have special attribute behavior
for properties, and in that case NotNull
has precedence over nullable state, but nullable state
has precedence over MaybeNull
. In addition, AllowNull
modifies the state transition to use
the declared state if the input is null, while it otherwise uses the non-null state.
Conclusion
The proposal starting point is:
NotNull wins over tracked state, which wins over MaybeNull. AllowNull transforms an incoming maybe null state to the declared state.
There's an action item to go investigate how this will play into the rest nullability, and an open question of whether to treat fields like properties, or like local variables.