Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve generic type argument list error recovery #69734

Merged
merged 29 commits into from
Apr 2, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
29 commits
Select commit Hold shift + click to select a range
d7e9b38
Improve error recovery on missing > in method
Rekkonnect Aug 26, 2023
b4d81da
Add many more tests
Rekkonnect Aug 27, 2023
508a9c8
Recovery on properties
Rekkonnect Aug 27, 2023
1b5ff5b
Recovery on tuples and more tests
Rekkonnect Aug 27, 2023
54c64dd
Adjust tests
Rekkonnect Aug 27, 2023
4c92d12
Fix type argument list termination and tests
Rekkonnect Aug 28, 2023
a830762
Move another test
Rekkonnect Aug 28, 2023
bd3f3bf
Better recovery logic and another test
Rekkonnect Aug 28, 2023
a6d0ffe
Adjust test after improvement
Rekkonnect Aug 28, 2023
5246351
Apply review comments
Rekkonnect Aug 29, 2023
6dca754
Add more tests covering type arg list break
Rekkonnect Aug 29, 2023
604db40
Revert test adjustment
Rekkonnect Aug 29, 2023
8218e34
More comments and improve tests
Rekkonnect Sep 8, 2023
5a25a48
Recover removed test
Rekkonnect Sep 8, 2023
e3ed56a
Merge branch 'main' into generic-method-error-recovery
Rekkonnect Sep 8, 2023
5935734
Formatting expected diagnostics
Rekkonnect Sep 8, 2023
cb19e46
Review comments
Rekkonnect Sep 8, 2023
0349424
Add a lot more tests
Rekkonnect Sep 9, 2023
ef2a978
Add `this` and `operator`
Rekkonnect Sep 9, 2023
13194f0
Add comments for terminating tokens
Rekkonnect Sep 10, 2023
491d86b
Remove test failures for bad recoveries
Rekkonnect Sep 13, 2023
c71ea51
Fix spelling
Rekkonnect Sep 13, 2023
fa72e62
Handle pattern matching keywords + tests
Rekkonnect Sep 13, 2023
9601aa6
Fix tests
Rekkonnect Sep 15, 2023
7b0f635
Add more tests in method declarations
Rekkonnect Sep 15, 2023
80dc680
Review comments
Rekkonnect Sep 29, 2023
c5bcc66
Merge branch 'main' into generic-method-error-recovery
Rekkonnect Sep 30, 2023
0f2180c
Merge branch 'main' into generic-method-error-recovery
Rekkonnect Apr 2, 2024
dbceb8e
Tests + nits
Rekkonnect Apr 2, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
103 changes: 101 additions & 2 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
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;
}
Rekkonnect marked this conversation as resolved.
Show resolved Hide resolved

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;
333fred marked this conversation as resolved.
Show resolved Hide resolved
}

// 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:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we have tests for tuple types where the tuple type name is and or or and the tuple is unterminated?

return true;
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Sep 29, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you give an example of where this arises (in a code comment). #Closed

}

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
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