Disallow is var x in c#8

[MkazemAkhgary]

There has been a lot of confusion and inconclusive discussion about behavior of this syntax. I think c# team should consider to eliminate this problem in appropriate time.

behavior of this syntax is inconsistent despite the behavior of classic x is Foo which returns false in case x is null even if variable is of provided type.

Also is var x is becoming a bad practice to force inline every variable declaration. We should avoid this.

We must not encourage use of it by introducing alternative syntax just to exclude null from its domain.

I tried to find a proverb from internet to describe current situation 😄 :
《The engine is falling to pieces while the joint owners of the car argue whether the footbrake or the handbrake should be applied》

Meanwhile we must not change behavior of this pattern because it would be very subtle and hard to debug for every developer who dumped some of this in his code base.

I guess the only solution is to disallow this syntax entirely. C#8 is good time line and as of c#7.3 there should be warning using it.

If you dont put value to my speaks, i think Eric lippert is a well trusted person here so read Q&A then tell me how his speaks renders this syntax inappropriate.

https://stackoverflow.com/a/7640437/4767498

Thanks for reading. Noticing and listenting to our requests.

Best regards.

---

Edited to include link

[HaloFour]

The var pattern will become more useful (and downright indispensable) when recursive patterns are added to the language, which should be in 7.3 or 8.0.

[alrz]

@HaloFour I think OP meant only the top level pattern in is operator, e.g. is var x.

[alrz]

behavior of this syntax is inconsistent despite the behavior of classic x is Foo which returns false in case x is null even if variable is of provided type.

x is T t is a type pattern and it's consistent with classic is operator x is T: both return false for null.

var x is the "value-binding" pattern, meaning that it just assigns the value regardless. Remember that null can be logically a valid value so you can't unconditionally exclude it from patterns. If you look at other languages you will see that the "value-binding" pattern does not do any null-check whatsoever, because it's not its job.

I guess the only solution is to disallow this syntax entirely. C#8 is good time line and as of c#7.3 there should be warning using it.

The is var x syntax is not actively harmful to take this breaking change. You can just don't use it. Something like #1064 could actually be helpful because an underscore could easily be confused considering the fact that it has been a valid identifier from C# 1.0.

Also is var x is becoming a bad practice to force inline every variable declaration. We should avoid this.

I read somewhere that people use class var {} to force explicit types. async void is a bad practice most of the time. Should we avoid those and hundred others too?

Now, when it has some known uses (for whatever reason) you can no longer suddenly make it an error. That's actually a factor to not take a breaking change.

We could just suggest to use declaration expressions instead via an IDE code action, whenever we have it.

[HaloFour]

If this were pre-C# 7.0 I'd probably agree with this proposal. I want to say that it's come up before. But at this point it's a part of the language, and one that people do seem to [ab]use for inline variable declaration. To remove it from the language is certainly a breaking change and quite impractical.

With C# 8.0 non-nullable reference types there will at least be the additional null checking in flow analysis which should detect when a potential null result from the var pattern is dereferenced. I think that will largely defang the issue here.

[alrz]

F# give warnings when a type test always hold (e.g. c is C)

let f (c : C) =
    match c with :? C -> () // WARNING

But not if you just use a variable (e.g. is var x)

let f (c : C) =
    match c with x -> () // OK

I think there is certainly some space to produce useful warnings here (as part of warning waves dotnet/roslyn#1580), but whether or not is var should be a part of that is up for debate.

Relates to dotnet/roslyn#16099

[DavidArno]

This change would break my code for no reason other than a few folk don't like the pattern. That would, in my view, be a ridiculous thing to do.

The correct thing to do would be to introduce declaration expressions, removing the need to use x is var y as a way of introducing a variable into an expression.

[Joe4evr]

Another thing to keep in mind is that is var is a potentially useful way to create a local variable from an external field/property/return value (should they be mutable).

if (_dependency.MutableValue is var localCopy && localCopy != null)
// instead of:
//var localCopy = _dependency.MutableValue;
//if (localCopy != null)

This might not seem like much now, but I'd consider it preparation for when property patterns land, at which point you can change it to is {} and get the null-check implicitly (potentially with more efficient code-gen).

[yaakov-h]

If you want to prohibit a language feature.......................................... write an analyzer. 😄

[Richiban]

var x is a perfectly valid pattern, although I'll agree that it doesn't make a great deal of sense when used on its own in an is context.

But this is currently valid, useful and already works:

public enum Color { Blue, Green, Red }

public Color ParseColorFromCommandLine()
{
	switch(Console.ReadLine())
	{
		case "Red": return Color.Red;
		case "Blue": return Color.Blue;
		case "Green": return Color.Green;
		case var other: throw new InvalidOperationException($"{other} is not a valid color");
	}
}

In the is situation, shouldn't the IDE warn on this anyway? I think that the best way to solve this confusion is just for users to get a siggly:

public void M(string s)
{	
	if(s is var x) // Warning: expression is always true
	{
		...
	}
	else
	{
		...
	}
}

[Joe4evr]

That could've been a useful suggestion when the first steps to pattern matching was still being worked on, but introducing new warnings for an existing feature is a breaking change for people who compile with /warnAsError (until the Warning Waves thing finally lands where people can opt-in to various new warnings).

Until then, you can make do with a custom analyzer.

[Richiban]

I could be wrong, but I thought that compiling with /warnAsError was an acceptance of possible breaking changes in the future? It seems pretty restrictive if the C# team can't ever introduce even a new warning...

[gafter]

There has been a lot of confusion and inconclusive discussion about behavior of this syntax. I think c# team should consider to eliminate this problem in appropriate time.

Discussion is not a problem. If you find it inconclusive, let me make it clear right now: we will not introduce a breaking change here. That should make this discussion quite conclusive. Problem solved.

If you dont put value to my speaks, i think Eric lippert is a well trusted person here so read Q&A then tell me how his speaks renders this syntax inappropriate.

Eric is correct here: expr is Type is false when expr is null. However, var isn't a type. Eric does not talk about var in that discussion. It is your mistake to think that var can be replaced by the type of the left-hand-side and retain the meaning of the construct. expr is var is not legal (unless a type named var is present).

Closing this issue, as the discussion is now conclusive and the language design team has no intention of making a breaking change. However, you are welcome to continue discussion here if you want.

[DarthVella]

In the is situation, shouldn't the IDE warn on this anyway? I think that the best way to solve this confusion is just for users to get a siggly

I thought the same, but on the else statement (CS0162, Unreachable code detected); it doesn't at the moement, and not on typed matches that can be proven true either.
It wouldn't help any potential confusion around absorbing null, though, and it does need the else statement.