Skip to content

Commit

Permalink
Improve parser recovery around nullable types in patterns (#72805)
Browse files Browse the repository at this point in the history
  • Loading branch information
@DoctorKrolic
DoctorKrolic committed Jul 8, 2024
1 parent f4fada1 commit 21fd45b
Show file tree
Hide file tree
Showing 15 changed files with 3,851 additions and 114 deletions.
79 changes: 70 additions & 9 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7223,18 +7223,79 @@ 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.
// Similarly, `async` can start a simple lambda in a conditional expression
// (e.g. `x is Y ? async a => ...`). The correct behavior is to treat `async` as a keyword
if (this.CurrentToken.ContextualKind is SyntaxKind.AsyncKeyword or 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;

// `;` ends 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
Expand Down
51 changes: 26 additions & 25 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser_Patterns.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -50,21 +50,21 @@ when ConvertExpressionToType(expr, out var leftType):
};
}

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;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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;
Expand All @@ -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:
Expand All @@ -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.
Expand All @@ -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;

Expand Down Expand Up @@ -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);
Expand All @@ -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 &&
Expand Down Expand Up @@ -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);

Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(
Expand All @@ -645,7 +646,7 @@ private ListPatternSyntax ParseListPattern(bool whenIsKeyword)
openBracket,
list,
this.EatToken(SyntaxKind.CloseBracketToken),
TryParseSimpleDesignation(whenIsKeyword));
TryParseSimpleDesignation(inSwitchArmPattern));
}
}
}
18 changes: 10 additions & 8 deletions src/Compilers/CSharp/Test/Emit2/Semantics/PatternMatchingTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]);
}
Expand Down
Loading

0 comments on commit 21fd45b

Please sign in to comment.