Skip to content

Add parser test demonstrating recovery from missing type arguments in generic names. #81062

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.

Sign up for GitHub

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

Merged
merged 8 commits into from
Nov 6, 2025
Merged

Add parser test demonstrating recovery from missing type arguments in generic names. #81062

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
8c96da6
Initial plan
Copilot Nov 5, 2025
516c5b7
Fix parsing of partially closed generic type names with omitted first…
Copilot Nov 5, 2025
4335265
Update tests for improved parsing of omitted type arguments (partial …
Copilot Nov 5, 2025
6ae8215
Complete test updates for improved parsing of omitted type arguments
Copilot Nov 5, 2025
cea68f0
Address PR feedback: use conditional expressions and add test variants
Copilot Nov 5, 2025
033c5d5
Add WorkItem attributes and binding diagnostics validation to tests
Copilot Nov 5, 2025
d4db8a1
Convert binding diagnostic test source to use raw string literals
Copilot Nov 6, 2025
e0dbd9c
Revert to using ParseTypeArgument unconditionally for better error me…
Copilot Nov 6, 2025
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
24 changes: 22 additions & 2 deletions src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6379,7 +6379,17 @@ private void ParseTypeArgumentList(out SyntaxToken open, SeparatedSyntaxListBuil
}

// first type
types.Add(this.ParseTypeArgument());
// Allow for a comma immediately after '<', treating it as an omitted first type argument.
// This handles cases like `M<,T>` where the first type argument is missing.
if (this.CurrentToken.Kind == SyntaxKind.CommaToken)
{
var omittedTypeArgumentInstance = _syntaxFactory.OmittedTypeArgument(SyntaxFactory.Token(SyntaxKind.OmittedTypeArgumentToken));
types.Add(omittedTypeArgumentInstance);
}
else
{
types.Add(this.ParseTypeArgument());
}
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Nov 5, 2025

Choose a reason for hiding this comment

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

@copilot switch this to a conditinoal expression like so:

types.Add(this.CurrentToken.Kind == SyntaxKind.CommaToken
    ? _syntaxFactory.OmittedTypeArgument(SyntaxFactory.Token(SyntaxKind.OmittedTypeArgumentToken))
    : this.ParseTypeArgument());

Do the same in the other code change you had below.

Copy link
Contributor Author

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

Done in commit cea68f0. Refactored both locations to use conditional expressions.


// remaining types & commas
while (true)
Expand Down Expand Up @@ -6418,7 +6428,17 @@ private void ParseTypeArgumentList(out SyntaxToken open, SeparatedSyntaxListBuil
if (this.CurrentToken.Kind == SyntaxKind.CommaToken || this.IsPossibleType())
{
types.AddSeparator(this.EatToken(SyntaxKind.CommaToken));
types.Add(this.ParseTypeArgument());
// Allow for a comma or '>' immediately after a comma, treating it as an omitted type argument.
// This handles cases like `M<T,>` or `M<,T,>` where type arguments are missing.
if (this.CurrentToken.Kind == SyntaxKind.CommaToken || this.CurrentToken.Kind == SyntaxKind.GreaterThanToken)
{
var omittedTypeArgumentInstance = _syntaxFactory.OmittedTypeArgument(SyntaxFactory.Token(SyntaxKind.OmittedTypeArgumentToken));
types.Add(omittedTypeArgumentInstance);
}
else
{
types.Add(this.ParseTypeArgument());
}
}
else if (this.SkipBadTypeArgumentListTokens(types, SyntaxKind.CommaToken) == PostSkipAction.Abort)
{
Expand Down
162 changes: 162 additions & 0 deletions src/Compilers/CSharp/Test/Syntax/Parsing/NameParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1490,5 +1490,167 @@ public void ParseGlobalAliasQualifiedNameAfterConditionalExpression()
}
EOF();
}

[Fact]
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Nov 5, 2025

Choose a reason for hiding this comment

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

@copilot the new tests (and any tests you modified) should have [Fact, WorkItem("...")] to associate them with the issue being fixed.

For tests that now no longer report a syntax error, we need to make a compilation and verify diagnostics on it to make sure that we are reporting a binding error.

Copy link
Contributor Author

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

Done in commit 033c5d5.

Added [WorkItem("https://github.com/dotnet/roslyn/issues/71379")] to:

  • The new test: ParseGenericNameInvocationWithPartiallyOmittedTypeArguments
  • Modified tests in SeparatedSyntaxListParsingTests: TypeArguments
  • Modified tests in TypeArgumentListParsingTests: TestGenericWithExtraCommasAndMissingTypes1 through TestGenericWithExtraCommasAndMissingTypes8

Also added binding diagnostics validation (via CreateCompilation().VerifyDiagnostics()) to all these tests to ensure proper semantic errors are reported even though syntax errors are eliminated.

public void ParseGenericNameInvocationWithPartiallyOmittedTypeArguments()
{
// Tests for partially closed generic type names where the first type argument is missing
var source = """
class C
{
void M<T1, T2>()
{
M<,int>(); // First argument omitted
M<string,>(); // Second argument omitted
M<,int,>(); // First and third arguments omitted (if we had 3 params)
Copy link
Member

@CyrusNajmabadi CyrusNajmabadi Nov 5, 2025

Choose a reason for hiding this comment

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

@copilot add variants with ,, as well.

Also, we should validate that we get proper binding diagnostics in these cases. We don't want this to accidentally be accepted.

Copy link
Contributor Author

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

Done in commit cea68f0. Added M<,,>() test variant with consecutive commas and validated proper binding diagnostics (ERR_BadArity) are generated for all cases to ensure they're not accidentally accepted.

}
}
""";

UsingTree(source);

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.ClassDeclaration);
{
N(SyntaxKind.ClassKeyword);
N(SyntaxKind.IdentifierToken, "C");
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.MethodDeclaration);
{
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.VoidKeyword);
}
N(SyntaxKind.IdentifierToken, "M");
N(SyntaxKind.TypeParameterList);
{
N(SyntaxKind.LessThanToken);
N(SyntaxKind.TypeParameter);
{
N(SyntaxKind.IdentifierToken, "T1");
}
N(SyntaxKind.CommaToken);
N(SyntaxKind.TypeParameter);
{
N(SyntaxKind.IdentifierToken, "T2");
}
N(SyntaxKind.GreaterThanToken);
}
N(SyntaxKind.ParameterList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
N(SyntaxKind.Block);
{
N(SyntaxKind.OpenBraceToken);
// M<,int>()
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.InvocationExpression);
{
N(SyntaxKind.GenericName);
{
N(SyntaxKind.IdentifierToken, "M");
N(SyntaxKind.TypeArgumentList);
{
N(SyntaxKind.LessThanToken);
N(SyntaxKind.OmittedTypeArgument);
{
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.CommaToken);
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.IntKeyword);
}
N(SyntaxKind.GreaterThanToken);
}
}
N(SyntaxKind.ArgumentList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
}
N(SyntaxKind.SemicolonToken);
}
// M<string,>()
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.InvocationExpression);
{
N(SyntaxKind.GenericName);
{
N(SyntaxKind.IdentifierToken, "M");
N(SyntaxKind.TypeArgumentList);
{
N(SyntaxKind.LessThanToken);
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.StringKeyword);
}
N(SyntaxKind.CommaToken);
N(SyntaxKind.OmittedTypeArgument);
{
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.GreaterThanToken);
}
}
N(SyntaxKind.ArgumentList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
}
N(SyntaxKind.SemicolonToken);
}
// M<,int,>()
N(SyntaxKind.ExpressionStatement);
{
N(SyntaxKind.InvocationExpression);
{
N(SyntaxKind.GenericName);
{
N(SyntaxKind.IdentifierToken, "M");
N(SyntaxKind.TypeArgumentList);
{
N(SyntaxKind.LessThanToken);
N(SyntaxKind.OmittedTypeArgument);
{
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.CommaToken);
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.IntKeyword);
}
N(SyntaxKind.CommaToken);
N(SyntaxKind.OmittedTypeArgument);
{
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.GreaterThanToken);
}
}
N(SyntaxKind.ArgumentList);
{
N(SyntaxKind.OpenParenToken);
N(SyntaxKind.CloseParenToken);
}
}
N(SyntaxKind.SemicolonToken);
}
N(SyntaxKind.CloseBraceToken);
}
}
N(SyntaxKind.CloseBraceToken);
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}
}
}
30 changes: 9 additions & 21 deletions src/Compilers/CSharp/Test/Syntax/Parsing/SeparatedSyntaxListParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,21 +43,9 @@ class C
// (7,9): error CS1003: Syntax error, ',' expected
// A<T U> a4;
Diagnostic(ErrorCode.ERR_SyntaxError, "U").WithArguments(",").WithLocation(7, 9),
// (9,9): error CS1031: Type expected
// A<T,> a6;
Diagnostic(ErrorCode.ERR_TypeExpected, ">").WithLocation(9, 9),
// (10,7): error CS1031: Type expected
// A<,T> a7;
Diagnostic(ErrorCode.ERR_TypeExpected, ",").WithLocation(10, 7),
// (11,9): error CS1003: Syntax error, ',' expected
// A<T U,,> a8;
Diagnostic(ErrorCode.ERR_SyntaxError, "U").WithArguments(",").WithLocation(11, 9),
// (11,11): error CS1031: Type expected
// A<T U,,> a8;
Diagnostic(ErrorCode.ERR_TypeExpected, ",").WithLocation(11, 11),
// (11,12): error CS1031: Type expected
// A<T U,,> a8;
Diagnostic(ErrorCode.ERR_TypeExpected, ">").WithLocation(11, 12));
Diagnostic(ErrorCode.ERR_SyntaxError, "U").WithArguments(",").WithLocation(11, 9));
N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.ClassDeclaration);
Expand Down Expand Up @@ -220,9 +208,9 @@ class C
N(SyntaxKind.IdentifierToken, "T");
}
N(SyntaxKind.CommaToken);
M(SyntaxKind.IdentifierName);
N(SyntaxKind.OmittedTypeArgument);
{
M(SyntaxKind.IdentifierToken);
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.GreaterThanToken);
}
Expand All @@ -244,9 +232,9 @@ class C
N(SyntaxKind.TypeArgumentList);
{
N(SyntaxKind.LessThanToken);
M(SyntaxKind.IdentifierName);
N(SyntaxKind.OmittedTypeArgument);
{
M(SyntaxKind.IdentifierToken);
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.CommaToken);
N(SyntaxKind.IdentifierName);
Expand Down Expand Up @@ -283,14 +271,14 @@ class C
N(SyntaxKind.IdentifierToken, "U");
}
N(SyntaxKind.CommaToken);
M(SyntaxKind.IdentifierName);
N(SyntaxKind.OmittedTypeArgument);
{
M(SyntaxKind.IdentifierToken);
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.CommaToken);
M(SyntaxKind.IdentifierName);
N(SyntaxKind.OmittedTypeArgument);
{
M(SyntaxKind.IdentifierToken);
N(SyntaxKind.OmittedTypeArgumentToken);
}
N(SyntaxKind.GreaterThanToken);
}
Expand Down
Loading
Loading