Skip to content

Commit

Permalink
Improve generic type argument list error recovery (#69734)
Browse files Browse the repository at this point in the history 
* Improve error recovery on missing > in method

* Add many more tests

* Recovery on properties

* Recovery on tuples and more tests

* Adjust tests

* Fix type argument list termination and tests

* Move another test

* Better recovery logic and another test

* Adjust test after improvement

* Apply review comments

* Add more tests covering type arg list break

* Revert test adjustment

* More comments and improve tests

* Recover removed test

* Formatting expected diagnostics

Co-authored-by: Cyrus Najmabadi <cyrus.najmabadi@gmail.com>

* Review comments

* Add a lot more tests

* Add `this` and `operator`

* Add comments for terminating tokens

* Remove test failures for bad recoveries

* Fix spelling

* Handle pattern matching keywords + tests

* Fix tests

* Add more tests in method declarations

* Review comments

* Tests + nits

---------

Co-authored-by: Cyrus Najmabadi <cyrus.najmabadi@gmail.com>
  • Loading branch information
@Rekkonnect @CyrusNajmabadi
Rekkonnect and CyrusNajmabadi committed Apr 2, 2024
1 parent 2d8c140 commit 399051e
Show file tree
Hide file tree
Showing 7 changed files with 7,475 additions and 84 deletions.
103 changes: 101 additions & 2 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5801,6 +5801,7 @@ private ScanTypeFlags ScanPossibleTypeArgumentList(
}

ScanTypeFlags result = ScanTypeFlags.GenericTypeOrExpression;
ScanTypeFlags lastScannedType;

do
{
Expand All @@ -5819,7 +5820,8 @@ private ScanTypeFlags ScanPossibleTypeArgumentList(
return result;
}

switch (this.ScanType(out _))
lastScannedType = this.ScanType(out _);
switch (lastScannedType)
{
case ScanTypeFlags.NotType:
greaterThanToken = null;
Expand Down Expand Up @@ -5918,6 +5920,25 @@ private ScanTypeFlags ScanPossibleTypeArgumentList(

if (this.CurrentToken.Kind != SyntaxKind.GreaterThanToken)
{
// Error recovery after missing > token:

// In the case of an identifier, we assume that there could be a missing > token
// For example, we have reached C in X<A, B C
if (this.CurrentToken.Kind is SyntaxKind.IdentifierToken)
{
greaterThanToken = this.EatToken(SyntaxKind.GreaterThanToken);
return result;
}

// As for tuples, we do not expect direct invocation right after the parenthesis
// EXAMPLE: X<(string, string)(), where we imply a missing > token between )(
// as the user probably wants to invoke X by X<(string, string)>()
if (lastScannedType is ScanTypeFlags.TupleType && this.CurrentToken.Kind is SyntaxKind.OpenParenToken)
{
greaterThanToken = this.EatToken(SyntaxKind.GreaterThanToken);
return result;
}

greaterThanToken = null;
return ScanTypeFlags.NotType;
}
Expand Down Expand Up @@ -5968,7 +5989,34 @@ private void ParseTypeArgumentList(out SyntaxToken open, SeparatedSyntaxListBuil
{
break;
}
else if (this.CurrentToken.Kind == SyntaxKind.CommaToken || this.IsPossibleType())

// We prefer early terminating the argument list over parsing until exhaustion
// for better error recovery
if (tokenBreaksTypeArgumentList(this.CurrentToken))
{
break;
}

// We are currently past parsing a type and we encounter an unexpected identifier token
// followed by tokens that are not part of a type argument list
// Example: List<(string a, string b) Method() { }
// current token: ^^^^^^
if (this.CurrentToken.Kind is SyntaxKind.IdentifierToken && tokenBreaksTypeArgumentList(this.PeekToken(1)))
{
break;
}

// This is for the case where we are in a this[] accessor, and the last one of the parameters in the parameter list
// is missing a > on its type
// Example: X this[IEnumerable<string parameter] =>
// current token: ^^^^^^^^^
if (this.CurrentToken.Kind is SyntaxKind.IdentifierToken
&& this.PeekToken(1).Kind is SyntaxKind.CloseBracketToken)
{
break;
}

if (this.CurrentToken.Kind == SyntaxKind.CommaToken || this.IsPossibleType())
{
types.AddSeparator(this.EatToken(SyntaxKind.CommaToken));
types.Add(this.ParseTypeArgument());
Expand All @@ -5980,6 +6028,57 @@ private void ParseTypeArgumentList(out SyntaxToken open, SeparatedSyntaxListBuil
}

close = this.EatToken(SyntaxKind.GreaterThanToken);

static bool tokenBreaksTypeArgumentList(SyntaxToken token)
{
var contextualKind = SyntaxFacts.GetContextualKeywordKind(token.ValueText);
switch (contextualKind)
{
// Example: x is IEnumerable<string or IList<int>
case SyntaxKind.OrKeyword:
// Example: x is IEnumerable<string and IDisposable
case SyntaxKind.AndKeyword:
return true;
}

switch (token.Kind)
{
// Example: Method<string(argument)
// Note: We would do a bad job handling a tuple argument with a missing comma,
// like: Method<string (int x, int y)>
// but since we do not look as far as possible to determine whether it is
// a tuple type or an argument list, we resort to considering it as an
// argument list
case SyntaxKind.OpenParenToken:

// Example: IEnumerable<string Method<T>() --- (< in <T>)
case SyntaxKind.LessThanToken:
// Example: Method(IEnumerable<string parameter)
case SyntaxKind.CloseParenToken:
// Example: IEnumerable<string field;
case SyntaxKind.SemicolonToken:
// Example: IEnumerable<string Property { get; set; }
case SyntaxKind.OpenBraceToken:
// Example:
// {
// IEnumerable<string field
// }
case SyntaxKind.CloseBraceToken:
// Examples:
// - IEnumerable<string field = null;
// - Method(IEnumerable<string parameter = null)
case SyntaxKind.EqualsToken:
// Example: IEnumerable<string Property => null;
case SyntaxKind.EqualsGreaterThanToken:
// Example: IEnumerable<string this[string key] { get; set; }
case SyntaxKind.ThisKeyword:
// Example: static IEnumerable<string operator +(A left, A right);
case SyntaxKind.OperatorKeyword:
return true;
}

return false;
}
}

private PostSkipAction SkipBadTypeArgumentListTokens(SeparatedSyntaxListBuilder<TypeSyntax> list, SyntaxKind expected)
Expand Down
25 changes: 8 additions & 17 deletions src/Compilers/CSharp/Test/Semantic/Semantics/NullableReferenceTypesTests.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153546,27 +153546,18 @@ public override void M1<T>(C<System.Nullable<T?> x)
class C<T> {}
");
comp.VerifyDiagnostics(
// (11,32): error CS0305: Using the generic type 'C<T>' requires 1 type arguments
// (11,26): error CS0115: 'B.M1<T>(C<T??>)': no suitable method found to override
// public override void M1<T>(C<System.Nullable<T?> x)
Diagnostic(ErrorCode.ERR_BadArity, "C<System.Nullable<T?> x").WithArguments("C<T>", "type", "1").WithLocation(11, 32),
// (11,54): error CS1003: Syntax error, ',' expected
Diagnostic(ErrorCode.ERR_OverrideNotExpected, "M1").WithArguments("B.M1<T>(C<T??>)").WithLocation(11, 26),
// (11,54): error CS1003: Syntax error, '>' expected
// public override void M1<T>(C<System.Nullable<T?> x)
Diagnostic(ErrorCode.ERR_SyntaxError, "x").WithArguments(",").WithLocation(11, 54),
// (11,54): error CS0246: The type or namespace name 'x' could not be found (are you missing a using directive or an assembly reference?)
Diagnostic(ErrorCode.ERR_SyntaxError, "x").WithArguments(">").WithLocation(11, 54),
// (11,54): error CS0453: The type 'T?' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Nullable<T>'
// public override void M1<T>(C<System.Nullable<T?> x)
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "x").WithArguments("x").WithLocation(11, 54),
// (11,55): error CS1003: Syntax error, '>' expected
Diagnostic(ErrorCode.ERR_ValConstraintNotSatisfied, "x").WithArguments("System.Nullable<T>", "T", "T?").WithLocation(11, 54),
// (11,54): error CS0453: The type 'T' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Nullable<T>'
// public override void M1<T>(C<System.Nullable<T?> x)
Diagnostic(ErrorCode.ERR_SyntaxError, ")").WithArguments(">").WithLocation(11, 55),
// (11,55): error CS1001: Identifier expected
// public override void M1<T>(C<System.Nullable<T?> x)
Diagnostic(ErrorCode.ERR_IdentifierExpected, ")").WithLocation(11, 55),
// (11,55): error CS0453: The type 'T?' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Nullable<T>'
// public override void M1<T>(C<System.Nullable<T?> x)
Diagnostic(ErrorCode.ERR_ValConstraintNotSatisfied, "").WithArguments("System.Nullable<T>", "T", "T?").WithLocation(11, 55),
// (11,55): error CS0453: The type 'T' must be a non-nullable value type in order to use it as parameter 'T' in the generic type or method 'Nullable<T>'
// public override void M1<T>(C<System.Nullable<T?> x)
Diagnostic(ErrorCode.ERR_ValConstraintNotSatisfied, "").WithArguments("System.Nullable<T>", "T", "T").WithLocation(11, 55)
Diagnostic(ErrorCode.ERR_ValConstraintNotSatisfied, "x").WithArguments("System.Nullable<T>", "T", "T").WithLocation(11, 54)
);
}

Expand Down
Loading

0 comments on commit 399051e

Please sign in to comment.