Proposal: change 'is var x' to check require non-null x

[sharwell]

In most cases where var is allowed for inferring a type, the resulting inferred type is syntactically interchangeable. One exception is the following:

public void Method(object input)
{
  if (input is object value)
  {
    // value has the static type 'object'
    // this block executes only if input is non-null
  }

  if (input is var value)
  {
    // value has the static type 'object'
    // this block executes even if input is null
  }
}

This behavior has several problems:

1. Aside from the obvious is null, is var is the only cases where an is operator can produce a true result when the left hand side is null. This is confusing.

2. The pattern-matching is operator has the potential to support null checks on exposed values in concurrent code (Discussion on non-null reference types #788 (comment)), but usage in this case would require the type name always be explicitly specified.

3. If this is not changed, then after the language supports nullable reference types we will have a case where var cannot be replaced with the inferred type without a syntax error:

   string? local = ...;
   if (local is string a) { }
   if (local is var b) { } // b has the type 'string?'
   if (local is string? b) { } // syntax error, presumably equivalent to CS8116

[DavidArno]

I'm puzzled, why would:

if (local is string? b) { }

be a syntax error? If local is null, the expression is false and the if block won't execute. If not null, then the value is assigned to the nullable variable b and the block is executed.

Whether it was the correct decision to always have the is var pattern match, regardless of whether the value is null or not is surely moot: the decision was made and changing it now would be a breaking change (which seems to have no benefit.

[sharwell]

@DavidArno Currently both of the following are not allowed:

if (local is DateTime? b) { } // fails to parse
if (local is Nullable<DateTime> b) { } // CS8116: it is not legal to use nullable type 'DateTime?' in a pattern

which seems to have no benefit

Null checks against dotted chains can lead to incorrect null analysis in concurrent code. Null checks using is patterns can be consistently/correctly analyzed in concurrent code.

[sharwell]

📝 The most likely situation where this will cause changes is code that uses the following form:

case var _ when someExpression:

[HaloFour]

I'd have to dig but I'm pretty sure the entire debate over is var matching null already happened. Considering that's the behavior per spec and per legal C# 7.0 code I can't imagine that the team would reverse it.

Without getting into recursive patterns under what circumstances would if (input is var x) be preferred to if (input != null)? Flow analysis with non-nullable references would determine that input is not null at that point and allow direct referencing. Having to declare and use a new variable in scope that literally has the same meaning as the existing variable seems extraneous.

[sharwell]

@HaloFour this pattern would benefit

public string? Property { get; }

if (this.Property != null)
  MethodWithNotNullArg(this.Property); // I expect a warning here

if (this.Property is var value)
  MethodWithNotNullArg(value); // I would not expect a warning here, but doesn't work in current rules

// But we're being left with this as the only actually-safe form:
if (this.Property is string value)
  MethodWithNotNullArg(value);

[DavidArno]

@sharwell,

Again I'm not arguing whether is var always matching was the right decision or not, but that's the way it is, and two (more verbose) ways already exist to avoid warnings in your example:

if (this.Property is PropertyType value)
  MethodWithNotNullArg(value);

if (this.Property is var value && value != null)
  MethodWithNotNullArg(value);

And with read-only locals, what you want could be achieved with an is let pattern:

if (this.Property is let value)
  MethodWithNotNullArg(value);

(though I'm not sure I like that syntax as it doesn't "read nicely".)

[sharwell]

And with read-only locals, what you want could be achieved with an is let pattern:

In the absence of this proposal (is var checking for null) getting accepted, is let would be a suitable alternative.

[HaloFour]

I'd pay money to have is let also limit scope to when definitely assigned, avoiding the whole leaky pattern variable thing.

[Thaina]

I think it look good but what happen when we write

if(!(obj is let x))
{
    // x is nothing?
}

// x is nowhere to be used ?

Wish we would have is! !is or isnot somehow

[DavidArno]

@Thaina,

!(obj is let x) would be semantically identical ¹ to !(obj is ObjType x). So the same rules would apply:

if (!(s is string ss))
{
    var x = ss; // Error: ss is unassigned here and is unusable
}
else
{
    var y = ss; // ss is assigned and not null
}
var z = ss; // Error: ss may not be assigned and is unusable

if (!(s is let ss))
{
    var x = ss; // Error: ss is unassigned here and is unusable
}
else
{
    var y = ss; // ss is assigned and not null
}
var z = ss; // Error: ss may not be assigned and is unusable

¹ Semantically identical, other than the fact that the variable introduced via is let would be read-only.

[Thaina]

@DavidArno I was guessing like that but I'm not sure because there are mention about scope leakage. The z in your case would know nothing about ss because it outside of if scope

Is the else count as the same scope as if ?

[DavidArno]

@Thaina,

ss not being visible to z is an option. I like it in principle, but it would be inconsistent with other is patterns. I guess it depends on how much cash @HaloFour is prepared to stump up 😀. Of course, this could be an opportunity to fix the "this variable has leaked but isn't usable here" issue that x is T y patterns currently suffer from. Then it would be consistent.

It would make sense for else to be considered part of the if scope if such a scope restriction were introduced.

[alrz]

as it was mentioned before, var pattern deliberately doesn't check for null for two reasons,

• It's considered as "catch all" for switch, i.e. case var x would subsume all the subsequent patterns.
• When used in a recursive pattern to bind a value, being null is kinda irrelevant.

Anyways, I do think different scope/nullability for let besides being readonly is attractive but it feels like a JavaScript feature™.

[HaloFour]

I wonder if a behavioral change would be palatable when considered specifically alongside nullable reference types. Since that syntax/behavior does not exist today it theoretically wouldn't be a breaking change. Although using an older compiler would produce a different result.

public class Foo {
    public string Bar { get; set; }
    public string? Baz { get; set; }
}

///
var foo = MethodReturnsFoo();

if (foo.Bar is var bar) {
    // always true, bar could be null but shouldn't be
}

if (foo.Baz is var baz) {
    // only true if Baz returns non-null value
}

[alrz]

@HaloFour

What if null was a valid value? It shouldn't silently cause the whole pattern to fail if it's not explicitly specified to do so (using a new syntax).