Improve parser recovery around nullable types in patterns

src/Compilers/CSharp/Portable/Parser/LanguageParser.cs

@@ -7164,18 +7164,77 @@ bool canFollowNullableType()
                     case ParseTypeMode.AfterIs:
                     case ParseTypeMode.DefinitePattern:
                     case ParseTypeMode.AsExpression:
-                        // These contexts might be a type that is at the end of an expression. In these contexts we only
-                        // permit the nullable qualifier if it is followed by a token that could not start an
-                        // expression, because for backward compatibility we want to consider a `?` token as part of the
-                        // `?:` operator if possible.
+                        // We are currently after `?` token after a nullable type pattern and need to decide how to
+                        // parse what we see next.  In the case of an identifier (e.g. `x ? a` there are two ways we can
+                        // see things
                         //
-                        // Similarly, if we have `T?[]` we do want to treat that as an array of nullables (following
-                        // existing parsing), not a conditional that returns a list.
+                        // 1. As a start of conditional expression, e.g. `var a = obj is string ? a : b`
+                        // 2. As a designation of a nullable-typed pattern, e.g. `if (obj is string? str)`
+                        //
+                        // Since nullable types (no matter reference or value types) are not valid in patterns by
+                        // default we are biased towards the first option and consider case 2 only for error recovery
+                        // purposes (if we parse here as nullable type pattern an error will be reported during
+                        // binding). This condition checks for simple cases, where we better use option 2 and parse a
+                        // nullable-typed pattern
+                        if (IsTrueIdentifier(this.CurrentToken))
+                        {
+                            // In a non-async method, `await` is a simple identifier.  However, if we see `x ? await`
+                            // it's almost certainly the start of an `await expression` in a conditional expression
+                            // (e.g. `x is Y ? await ...`), not a nullable type pattern (since users would not use
+                            // 'await' as the name of a variable).  So just treat this as a conditional expression.
+                            if (this.CurrentToken.ContextualKind == SyntaxKind.AwaitKeyword)
+                                return false;
+
+                            var nextTokenKind = PeekToken(1).Kind;
+
+                            // These token either 100% end a pattern or start a new one:
+
+                            // A literal token starts a new pattern. Can occur in list pattern with missing separation
+                            // `,`.  For example, in `x is [int[]? arr 5]` we'd prefer to parse this as a missing `,`
+                            // after `arr`
+                            if (SyntaxFacts.IsLiteral(nextTokenKind))
+                                return true;
+
+                            // A predefined type is basically the same case: `x is [string[]? slice char ch]`. We'd
+                            // prefer to parse this as a missing `,` after `slice`.
+                            if (SyntaxFacts.IsPredefinedType(nextTokenKind))
+                                return true;
+
+                            // `)`, `]` and `}` obviously end a pattern.  For example:
+                            // `if (x is int? i)`, `indexable[x is string? s]`, `x is { Prop: Type? typeVar }`
+                            if (nextTokenKind is SyntaxKind.CloseParenToken or SyntaxKind.CloseBracketToken or SyntaxKind.CloseBraceToken)
+                                return true;
+
+                            // `{` starts a new pattern.  For example: `x is A? { ...`. Note, that `[` and `(` are not
+                            // in the list because they can start an invocation/indexer
+                            if (nextTokenKind == SyntaxKind.OpenBraceToken)
+                                return true;
+
+                            // `,` ends a pattern in list/property pattern.  For example `x is { Prop1: Type1? type, Prop2: Type2 }` or
+                            // `x is [Type1? type, ...]`
+                            if (nextTokenKind == SyntaxKind.CommaToken)
+                                return true;
+
+                            // `;` end a pattern if it finishes an expression statement: var y = x is bool? b;
+                            if (nextTokenKind == SyntaxKind.SemicolonToken)
+                                return true;
+
+                            // EndOfFileToken is obviously the end of parsing. We are better parsing a pattern rather
+                            // than an unfinished conditional expression
+                            if (nextTokenKind == SyntaxKind.EndOfFileToken)
+                                return true;
+
+                            return false;
+                        }
+
+                        // If nothing from above worked permit the nullable qualifier if it is followed by a token that
+                        // could not start an expression If we have `T?[]` we do want to treat that as an array of
+                        // nullables (following existing parsing), not a conditional that returns a list.
                         return !CanStartExpression() || this.CurrentToken.Kind is SyntaxKind.OpenBracketToken;
                     case ParseTypeMode.NewExpression:
-                        // A nullable qualifier is permitted as part of the type in a `new` expression.
-                        // e.g. `new int?()` is allowed.  It creates a null value of type `Nullable<int>`.
-                        // Similarly `new int? {}` is allowed.
+                        // A nullable qualifier is permitted as part of the type in a `new` expression. e.g. `new
+                        // int?()` is allowed.  It creates a null value of type `Nullable<int>`. Similarly `new int? {}`
+                        // is allowed.
                         return
                             this.CurrentToken.Kind is SyntaxKind.OpenParenToken or   // ctor parameters
                                                       SyntaxKind.OpenBracketToken or   // array type

src/Compilers/CSharp/Portable/Parser/LanguageParser_Patterns.cs

@@ -50,21 +50,21 @@ private bool ConvertExpressionToType(ExpressionSyntax expression, [NotNullWhen(t
             };
         }
 
-        private PatternSyntax ParsePattern(Precedence precedence, bool afterIs = false, bool whenIsKeyword = false)
+        private PatternSyntax ParsePattern(Precedence precedence, bool afterIs = false, bool inSwitchArmPattern = false)
         {
-            return ParseDisjunctivePattern(precedence, afterIs, whenIsKeyword);
+            return ParseDisjunctivePattern(precedence, afterIs, inSwitchArmPattern);
         }
 
-        private PatternSyntax ParseDisjunctivePattern(Precedence precedence, bool afterIs, bool whenIsKeyword)
+        private PatternSyntax ParseDisjunctivePattern(Precedence precedence, bool afterIs, bool inSwitchArmPattern)
         {
-            PatternSyntax result = ParseConjunctivePattern(precedence, afterIs, whenIsKeyword);
+            PatternSyntax result = ParseConjunctivePattern(precedence, afterIs, inSwitchArmPattern);
             while (this.CurrentToken.ContextualKind == SyntaxKind.OrKeyword)
             {
                 result = _syntaxFactory.BinaryPattern(
                     SyntaxKind.OrPattern,
                     result,
                     ConvertToKeyword(this.EatToken()),
-                    ParseConjunctivePattern(precedence, afterIs, whenIsKeyword));
+                    ParseConjunctivePattern(precedence, afterIs, inSwitchArmPattern));
             }
 
             return result;
@@ -101,16 +101,16 @@ private bool LooksLikeTypeOfPattern()
             return false;
         }
 
-        private PatternSyntax ParseConjunctivePattern(Precedence precedence, bool afterIs, bool whenIsKeyword)
+        private PatternSyntax ParseConjunctivePattern(Precedence precedence, bool afterIs, bool inSwitchArmPattern)
         {
-            PatternSyntax result = ParseNegatedPattern(precedence, afterIs, whenIsKeyword);
+            PatternSyntax result = ParseNegatedPattern(precedence, afterIs, inSwitchArmPattern);
             while (this.CurrentToken.ContextualKind == SyntaxKind.AndKeyword)
             {
                 result = _syntaxFactory.BinaryPattern(
                     SyntaxKind.AndPattern,
                     result,
                     ConvertToKeyword(this.EatToken()),
-                    ParseNegatedPattern(precedence, afterIs, whenIsKeyword));
+                    ParseNegatedPattern(precedence, afterIs, inSwitchArmPattern));
             }
 
             return result;
@@ -155,21 +155,21 @@ private bool ScanDesignation(bool permitTuple)
             }
         }
 
-        private PatternSyntax ParseNegatedPattern(Precedence precedence, bool afterIs, bool whenIsKeyword)
+        private PatternSyntax ParseNegatedPattern(Precedence precedence, bool afterIs, bool inSwitchArmPattern)
         {
             if (this.CurrentToken.ContextualKind == SyntaxKind.NotKeyword)
             {
                 return _syntaxFactory.UnaryPattern(
                     ConvertToKeyword(this.EatToken()),
-                    ParseNegatedPattern(precedence, afterIs, whenIsKeyword));
+                    ParseNegatedPattern(precedence, afterIs, inSwitchArmPattern));
             }
             else
             {
-                return ParsePrimaryPattern(precedence, afterIs, whenIsKeyword);
+                return ParsePrimaryPattern(precedence, afterIs, inSwitchArmPattern);
             }
         }
 
-        private PatternSyntax ParsePrimaryPattern(Precedence precedence, bool afterIs, bool whenIsKeyword)
+        private PatternSyntax ParsePrimaryPattern(Precedence precedence, bool afterIs, bool inSwitchArmPattern)
         {
             // handle common error recovery situations during typing
             var tk = this.CurrentToken.Kind;
@@ -192,10 +192,10 @@ private PatternSyntax ParsePrimaryPattern(Precedence precedence, bool afterIs, b
             switch (CurrentToken.Kind)
             {
                 case SyntaxKind.OpenBracketToken:
-                    return this.ParseListPattern(whenIsKeyword);
+                    return this.ParseListPattern(inSwitchArmPattern);
                 case SyntaxKind.DotDotToken:
                     return _syntaxFactory.SlicePattern(EatToken(),
-                        IsPossibleSubpatternElement() ? ParsePattern(precedence, afterIs: false, whenIsKeyword) : null);
+                        IsPossibleSubpatternElement() ? ParsePattern(precedence, afterIs: false, inSwitchArmPattern) : null);
                 case SyntaxKind.LessThanToken:
                 case SyntaxKind.LessThanEqualsToken:
                 case SyntaxKind.GreaterThanToken:
@@ -214,7 +214,8 @@ private PatternSyntax ParsePrimaryPattern(Precedence precedence, bool afterIs, b
             TypeSyntax? type = null;
             if (LooksLikeTypeOfPattern())
             {
-                type = this.ParseType(afterIs ? ParseTypeMode.AfterIs : ParseTypeMode.DefinitePattern);
+                type = this.ParseType(
+                    afterIs ? ParseTypeMode.AfterIs : ParseTypeMode.DefinitePattern);
                 if (type.IsMissing || !CanTokenFollowTypeInPattern(precedence))
                 {
                     // either it is not shaped like a type, or it is a constant expression.
@@ -223,7 +224,7 @@ private PatternSyntax ParsePrimaryPattern(Precedence precedence, bool afterIs, b
                 }
             }
 
-            var pattern = ParsePatternContinued(type, precedence, whenIsKeyword);
+            var pattern = ParsePatternContinued(type, precedence, inSwitchArmPattern);
             if (pattern != null)
                 return pattern;
 
@@ -262,14 +263,14 @@ bool CanTokenFollowTypeInPattern(Precedence precedence)
             }
         }
 
-        private PatternSyntax? ParsePatternContinued(TypeSyntax? type, Precedence precedence, bool whenIsKeyword)
+        private PatternSyntax? ParsePatternContinued(TypeSyntax? type, Precedence precedence, bool inSwitchArmPattern)
         {
             if (type?.Kind == SyntaxKind.IdentifierName)
             {
                 var typeIdentifier = (IdentifierNameSyntax)type;
                 var typeIdentifierToken = typeIdentifier.Identifier;
                 if (typeIdentifierToken.ContextualKind == SyntaxKind.VarKeyword &&
-                    (this.CurrentToken.Kind == SyntaxKind.OpenParenToken || this.IsValidPatternDesignation(whenIsKeyword)))
+                    (this.CurrentToken.Kind == SyntaxKind.OpenParenToken || this.IsValidPatternDesignation(inSwitchArmPattern)))
                 {
                     // we have a "var" pattern; "var" is not permitted to be a stand-in for a type (or a constant) in a pattern.
                     var varToken = ConvertToKeyword(typeIdentifierToken);
@@ -295,7 +296,7 @@ bool CanTokenFollowTypeInPattern(Precedence precedence)
                 var closeParenToken = this.EatToken(SyntaxKind.CloseParenToken);
 
                 parsePropertyPatternClause(out PropertyPatternClauseSyntax? propertyPatternClause0);
-                var designation0 = TryParseSimpleDesignation(whenIsKeyword);
+                var designation0 = TryParseSimpleDesignation(inSwitchArmPattern);
 
                 if (type == null &&
                     propertyPatternClause0 == null &&
@@ -333,12 +334,12 @@ bool CanTokenFollowTypeInPattern(Precedence precedence)
             {
                 return _syntaxFactory.RecursivePattern(
                     type, positionalPatternClause: null, propertyPatternClause,
-                    TryParseSimpleDesignation(whenIsKeyword));
+                    TryParseSimpleDesignation(inSwitchArmPattern));
             }
 
             if (type != null)
             {
-                var designation = TryParseSimpleDesignation(whenIsKeyword);
+                var designation = TryParseSimpleDesignation(inSwitchArmPattern);
                 if (designation != null)
                     return _syntaxFactory.DeclarationPattern(type, designation);
 
@@ -431,7 +432,7 @@ private CSharpSyntaxNode ParseExpressionOrPatternForSwitchStatement()
         {
             var savedState = _termState;
             _termState |= TerminatorState.IsExpressionOrPatternInCaseLabelOfSwitchStatement;
-            var pattern = ParsePattern(Precedence.Conditional, whenIsKeyword: true);
+            var pattern = ParsePattern(Precedence.Conditional, inSwitchArmPattern: true);
             _termState = savedState;
             return ConvertPatternToExpressionIfPossible(pattern);
         }
@@ -589,7 +590,7 @@ SeparatedSyntaxList<SwitchExpressionArmSyntax> parseSwitchExpressionArms()
 
                     var savedState = _termState;
                     _termState |= TerminatorState.IsPatternInSwitchExpressionArm;
-                    var pattern = ParsePattern(Precedence.Coalescing, whenIsKeyword: true);
+                    var pattern = ParsePattern(Precedence.Coalescing, inSwitchArmPattern: true);
                     _termState = savedState;
 
                     // We use a precedence that excludes lambdas, assignments, and a conditional which could have a
@@ -628,7 +629,7 @@ SeparatedSyntaxList<SwitchExpressionArmSyntax> parseSwitchExpressionArms()
             }
         }
 
-        private ListPatternSyntax ParseListPattern(bool whenIsKeyword)
+        private ListPatternSyntax ParseListPattern(bool inSwitchArmPattern)
         {
             var openBracket = this.EatToken(SyntaxKind.OpenBracketToken);
             var list = this.ParseCommaSeparatedSyntaxList(
@@ -645,7 +646,7 @@ private ListPatternSyntax ParseListPattern(bool whenIsKeyword)
                 openBracket,
                 list,
                 this.EatToken(SyntaxKind.CloseBracketToken),
-                TryParseSimpleDesignation(whenIsKeyword));
+                TryParseSimpleDesignation(inSwitchArmPattern));
         }
     }
 }

src/Compilers/CSharp/Test/Emit2/Semantics/PatternMatchingTests.cs

@@ -4773,26 +4773,28 @@ public static void Main(object o)
     private static object M() => null;
 }";
             var compilation = CreateCompilation(program).VerifyDiagnostics(
-                // (9,32): error CS1525: Invalid expression term 'break'
-                //             case Color? Color2:
-                Diagnostic(ErrorCode.ERR_InvalidExprTerm, "").WithArguments("break").WithLocation(9, 32),
-                // (9,32): error CS1003: Syntax error, ':' expected
-                //             case Color? Color2:
-                Diagnostic(ErrorCode.ERR_SyntaxError, "").WithArguments(":").WithLocation(9, 32),
                 // (8,18): error CS0118: 'Color' is a variable but is used like a type
                 //             case Color Color:
                 Diagnostic(ErrorCode.ERR_BadSKknown, "Color").WithArguments("Color", "variable", "type").WithLocation(8, 18),
                 // (9,25): error CS0103: The name 'Color2' does not exist in the current context
                 //             case Color? Color2:
-                Diagnostic(ErrorCode.ERR_NameNotInContext, "Color2").WithArguments("Color2").WithLocation(9, 25)
-                );
+                Diagnostic(ErrorCode.ERR_NameNotInContext, "Color2").WithArguments("Color2").WithLocation(9, 25),
+                // (9,32): error CS1525: Invalid expression term 'break'
+                //             case Color? Color2:
+                Diagnostic(ErrorCode.ERR_InvalidExprTerm, "").WithArguments("break").WithLocation(9, 32),
+                // (9,32): error CS1003: Syntax error, ':' expected
+                //             case Color? Color2:
+                Diagnostic(ErrorCode.ERR_SyntaxError, "").WithArguments(":").WithLocation(9, 32));
+
             var tree = compilation.SyntaxTrees.Single();
             var model = compilation.GetSemanticModel(tree);
 
             var colorDecl = GetPatternDeclarations(tree, "Color").ToArray();
             var colorRef = GetReferences(tree, "Color").ToArray();
+
             Assert.Equal(1, colorDecl.Length);
             Assert.Equal(2, colorRef.Length);
+
             Assert.Null(model.GetSymbolInfo(colorRef[0]).Symbol);
             VerifyModelForDeclarationOrVarSimplePattern(model, colorDecl[0], colorRef[1]);
         }