Skip to content

Improve parser error recovery for property declarations with missing identifiers #81003

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

Open
wants to merge 13 commits into
base: main
Choose a base branch
from
+467 −112
Open

Improve parser error recovery for property declarations with missing identifiers #81003

Show file tree
Hide file tree
Changes from 9 commits
Commits
Show all changes
13 commits
Select commit Hold shift + click to select a range
de052c8
Initial plan
Copilot Nov 3, 2025
a498773
Implement error recovery for property declarations with missing ident…
Copilot Nov 3, 2025
39495f9
Update tests for improved error recovery behavior
Copilot Nov 3, 2025
b157606
Add comment to main test validating all members parse as properties
Copilot Nov 3, 2025
648c539
Refine
CyrusNajmabadi Nov 3, 2025
35a75ad
Merge branch 'copilot/improve-error-recovery' of https://github.com/d…
CyrusNajmabadi Nov 3, 2025
adff615
Update test
CyrusNajmabadi Nov 4, 2025
4eb8881
Refine
CyrusNajmabadi Nov 4, 2025
31418f6
Docs
CyrusNajmabadi Nov 4, 2025
d9c69ab
Add WorkItem attributes and fix formatting for new tests
Copilot Nov 4, 2025
c38401a
Restore
CyrusNajmabadi Nov 4, 2025
3c8097c
Update src/Compilers/CSharp/Portable/Parser/LanguageParser.cs
CyrusNajmabadi Nov 5, 2025
9d5f926
Merge remote-tracking branch 'upstream/main' into copilot/improve-err…
CyrusNajmabadi Nov 5, 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
114 changes: 94 additions & 20 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 @@ -15,6 +15,7 @@

namespace Microsoft.CodeAnalysis.CSharp.Syntax.InternalSyntax
{
using System.Security.Cryptography.X509Certificates;
using Microsoft.CodeAnalysis.Syntax.InternalSyntax;

internal sealed partial class LanguageParser : SyntaxParser
Expand Down Expand Up @@ -2811,8 +2812,8 @@ private MemberDeclarationSyntax ParseMemberDeclarationOrStatementCore(SyntaxKind
goto parse_member_name;
}

Debug.Assert(identifierOrThisOpt != null);

// identifierOrThisOpt may be null if we're doing error recovery for a property with missing identifier
Debug.Assert(identifierOrThisOpt != null || IsStartOfPropertyBody(index: 0));
if (TryParseIndexerOrPropertyDeclaration(attributes, modifiers, type, explicitInterfaceOpt, identifierOrThisOpt, typeParameterListOpt, out result))
{
return result;
Expand Down Expand Up @@ -2982,12 +2983,24 @@ this.CurrentToken.ContextualKind is not (SyntaxKind.PartialKeyword or SyntaxKind
return false;
}

private bool IsNoneOrIncompleteMember(SyntaxKind parentKind, SyntaxList<AttributeListSyntax> attributes, SyntaxListBuilder modifiers, TypeSyntax type,
ExplicitInterfaceSpecifierSyntax explicitInterfaceOpt, SyntaxToken identifierOrThisOpt, TypeParameterListSyntax typeParameterListOpt,
out MemberDeclarationSyntax result)
private bool IsNoneOrIncompleteMember(
SyntaxKind parentKind,
SyntaxList<AttributeListSyntax> attributes,
SyntaxListBuilder modifiers,
TypeSyntax type,
ExplicitInterfaceSpecifierSyntax explicitInterfaceOpt,
SyntaxToken identifierOrThisOpt,
TypeParameterListSyntax typeParameterListOpt,
out MemberDeclarationSyntax result)
{
if (explicitInterfaceOpt == null && identifierOrThisOpt == null && typeParameterListOpt == null)
{
if (looksLikeStartOfPropertyBody())
{
result = null;
return false;
}

if (attributes.Count == 0 && modifiers.Count == 0 && type.IsMissing && type.Kind != SyntaxKind.RefType)
{
// we haven't advanced, the caller needs to consume the tokens ahead
Expand Down Expand Up @@ -3025,6 +3038,46 @@ private bool IsNoneOrIncompleteMember(SyntaxKind parentKind, SyntaxList<Attribut

result = null;
return false;

bool looksLikeStartOfPropertyBody()
{
// Need to see at least some modifiers, or something that strongly looks like a property type in order
// for us to at least consider this a property. Otherwise, we end up cases where errant `{` and `=>`
// tokens would lead us to think we're a property in inappropriate places.
if (modifiers.Count == 0 && !looksLikePropertyType())
return false;

if (!IsStartOfPropertyBody(index: 0))
return false;

return true;
}

bool looksLikePropertyType()
{
var propertyType = type;

// We really want to see at least `Type {` or `Type =>` to think of this as a property. And we want the
// parser to be confident it got a good return type for the property, not something it was confused
// about and already reported errors on. Otherwise, we can too easily end up in cases where an error is
// next to a `{` and we don't want that to be a property.
if (propertyType.IsMissing || ContainsErrorDiagnostic(propertyType))
return false;

if (propertyType is RefTypeSyntax refType)
propertyType = refType.Type;

// Unlikely cases to actually be a property. Likely some runaway error recovery case. For example:
// with { } (likely a with expression)
// extension { } (likely an extension block being written)
// get { } (likely an accessor, not a property called 'get'). Same for set/add/remove/init.
if (propertyType is IdentifierNameSyntax { Identifier.ValueText: "extension" or "with" or "get" or "set" or "add" or "remove" or "init" })
return false;

// Can add more cases here if we find ourselves being too eager.

return true;
}
}

private bool ReconsideredTypeAsAsyncModifier(ref SyntaxListBuilder modifiers, ref TypeSyntax type, ref ResetPoint afterTypeResetPoint,
Expand All @@ -3050,21 +3103,28 @@ private bool ReconsideredTypeAsAsyncModifier(ref SyntaxListBuilder modifiers, re
return false;
}

private bool TryParseIndexerOrPropertyDeclaration(SyntaxList<AttributeListSyntax> attributes, SyntaxListBuilder modifiers, TypeSyntax type,
ExplicitInterfaceSpecifierSyntax explicitInterfaceOpt, SyntaxToken identifierOrThisOpt,
TypeParameterListSyntax typeParameterListOpt, out MemberDeclarationSyntax result)
private bool TryParseIndexerOrPropertyDeclaration(
SyntaxList<AttributeListSyntax> attributes,
SyntaxListBuilder modifiers,
TypeSyntax type,
ExplicitInterfaceSpecifierSyntax explicitInterfaceOpt,
SyntaxToken identifierOrThisOpt,
TypeParameterListSyntax typeParameterListOpt,
out MemberDeclarationSyntax result)
{
if (identifierOrThisOpt.Kind == SyntaxKind.ThisKeyword)
if (identifierOrThisOpt?.Kind == SyntaxKind.ThisKeyword)
{
result = this.ParseIndexerDeclaration(attributes, modifiers, type, explicitInterfaceOpt, identifierOrThisOpt, typeParameterListOpt);
return true;
}

// `{` or `=>` definitely start a property. Also allow
// `; {` and `; =>` as error recovery for a misplaced semicolon.
if (IsStartOfPropertyBody(this.CurrentToken.Kind) ||
(this.CurrentToken.Kind is SyntaxKind.SemicolonToken && IsStartOfPropertyBody(this.PeekToken(1).Kind)))
if (IsStartOfPropertyBody(index: 0))
{
// Error recovery: if we see a property body but no identifier, synthesize a missing identifier
identifierOrThisOpt ??= this.AddError(CreateMissingIdentifierToken(), ErrorCode.ERR_IdentifierExpected);

result = this.ParsePropertyDeclaration(attributes, modifiers, type, explicitInterfaceOpt, identifierOrThisOpt, typeParameterListOpt);
return true;
}
Expand All @@ -3073,8 +3133,24 @@ private bool TryParseIndexerOrPropertyDeclaration(SyntaxList<AttributeListSyntax
return false;
}

private static bool IsStartOfPropertyBody(SyntaxKind kind)
=> kind is SyntaxKind.OpenBraceToken or SyntaxKind.EqualsGreaterThanToken;
private bool IsStartOfPropertyBody(int index)
{
var kind = this.PeekToken(index).Kind;

// If we have `{` or `=>`, definitely a property body.
if (kind is SyntaxKind.OpenBraceToken or SyntaxKind.EqualsGreaterThanToken)
return true;

// For error recovery, also consider `; {` and `; =>` as starting a property body.
if (kind is SyntaxKind.SemicolonToken)
{
kind = this.PeekToken(index + 1).Kind;
if (kind is SyntaxKind.OpenBraceToken or SyntaxKind.EqualsGreaterThanToken)
return true;
}

return false;
}

// Returns null if we can't parse anything (even partially).
internal MemberDeclarationSyntax ParseMemberDeclaration(SyntaxKind parentKind)
Expand Down Expand Up @@ -3227,8 +3303,8 @@ private MemberDeclarationSyntax ParseMemberDeclarationCore(SyntaxKind parentKind
goto parse_member_name;
}

Debug.Assert(identifierOrThisOpt != null);

// identifierOrThisOpt may be null if we're doing error recovery for a property with missing identifier
Debug.Assert(identifierOrThisOpt != null || IsStartOfPropertyBody(index: 0));
if (TryParseIndexerOrPropertyDeclaration(attributes, modifiers, type, explicitInterfaceOpt, identifierOrThisOpt, typeParameterListOpt, out result))
{
return result;
Expand Down Expand Up @@ -3300,19 +3376,17 @@ private bool IsFieldDeclaration(bool isEvent, bool isGlobalScriptLevel)
// b) generic
// c) a property
// d) a method (unless we already know we're parsing an event)
var kind = this.PeekToken(1).Kind;

// Error recovery, don't allow a misplaced semicolon after the name in a property to throw off the entire parse.
//
// e.g. `public int MyProperty; { get; set; }` should still be parsed as a property with a skipped token.
if (!isGlobalScriptLevel &&
kind == SyntaxKind.SemicolonToken &&
IsStartOfPropertyBody(this.PeekToken(2).Kind))
IsStartOfPropertyBody(index: 1))
{
return false;
}

switch (kind)
switch (this.PeekToken(1).Kind)
{
case SyntaxKind.DotToken: // Goo. explicit
case SyntaxKind.ColonColonToken: // Goo:: explicit
Expand Down Expand Up @@ -4131,7 +4205,7 @@ private PropertyDeclarationSyntax ParsePropertyDeclaration(
}

// We know we are parsing a property because we have seen either an open brace or an arrow token
Debug.Assert(IsStartOfPropertyBody(this.CurrentToken.Kind));
Debug.Assert(IsStartOfPropertyBody(index: 0));

var accessorList = this.CurrentToken.Kind == SyntaxKind.OpenBraceToken
? this.ParseAccessorList(AccessorDeclaringKind.Property)
Expand Down
18 changes: 4 additions & 14 deletions src/Compilers/CSharp/Test/Semantic/Semantics/RefFieldTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21994,22 +21994,12 @@ public void RefScoped()
// (3,18): error CS1002: ; expected
// ref scoped R Property { get => throw null; }
Diagnostic(ErrorCode.ERR_SemicolonExpected, "Property").WithLocation(3, 18),
// (3,27): error CS1519: Invalid token '{' in class, record, struct, or interface member declaration
// (3,18): error CS0246: The type or namespace name 'Property' could not be found (are you missing a using directive or an assembly reference?)
// ref scoped R Property { get => throw null; }
Diagnostic(ErrorCode.ERR_InvalidMemberDecl, "{").WithArguments("{").WithLocation(3, 27),
// (3,27): error CS1519: Invalid token '{' in class, record, struct, or interface member declaration
Diagnostic(ErrorCode.ERR_SingleTypeNameNotFound, "Property").WithArguments("Property").WithLocation(3, 18),
// (3,27): error CS1001: Identifier expected
// ref scoped R Property { get => throw null; }
Diagnostic(ErrorCode.ERR_InvalidMemberDecl, "{").WithArguments("{").WithLocation(3, 27),
// (3,33): error CS1519: Invalid token '=>' in class, record, struct, or interface member declaration
// ref scoped R Property { get => throw null; }
Diagnostic(ErrorCode.ERR_InvalidMemberDecl, "=>").WithArguments("=>").WithLocation(3, 33),
// (3,33): error CS1519: Invalid token '=>' in class, record, struct, or interface member declaration
// ref scoped R Property { get => throw null; }
Diagnostic(ErrorCode.ERR_InvalidMemberDecl, "=>").WithArguments("=>").WithLocation(3, 33),
// (4,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(4, 1)
);
Diagnostic(ErrorCode.ERR_IdentifierExpected, "{").WithLocation(3, 27));

source =
@"ref struct R
Expand Down
30 changes: 6 additions & 24 deletions src/Compilers/CSharp/Test/Syntax/LexicalAndXml/LexicalErrorTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -172,15 +172,9 @@ public class \e
// (3,18): error CS1056: Unexpected character '\'
// public class \e
Diagnostic(ErrorCode.ERR_UnexpectedCharacter, "").WithArguments("\\").WithLocation(3, 18),
// (3,19): error CS0116: A namespace cannot directly contain members such as fields, methods or statements
// (3,20): error CS1001: Identifier expected
// public class \e
Diagnostic(ErrorCode.ERR_NamespaceUnexpected, "e").WithLocation(3, 19),
// (4,5): error CS1022: Type or namespace definition, or end-of-file expected
// {
Diagnostic(ErrorCode.ERR_EOFExpected, "{").WithLocation(4, 5),
// (6,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(6, 1));
Diagnostic(ErrorCode.ERR_IdentifierExpected, "").WithLocation(3, 20));
}

[Fact]
Expand All @@ -205,15 +199,9 @@ public class X\e
// (3,19): error CS1056: Unexpected character '\'
// public class X\e
Diagnostic(ErrorCode.ERR_UnexpectedCharacter, "").WithArguments("\\").WithLocation(3, 19),
// (3,20): error CS0116: A namespace cannot directly contain members such as fields, methods or statements
// (3,21): error CS1001: Identifier expected
// public class X\e
Diagnostic(ErrorCode.ERR_NamespaceUnexpected, "e").WithLocation(3, 20),
// (4,5): error CS1022: Type or namespace definition, or end-of-file expected
// {
Diagnostic(ErrorCode.ERR_EOFExpected, "{").WithLocation(4, 5),
// (6,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(6, 1));
Diagnostic(ErrorCode.ERR_IdentifierExpected, "").WithLocation(3, 21));
}

[Fact]
Expand Down Expand Up @@ -241,15 +229,9 @@ public class \eX
// (3,18): error CS1056: Unexpected character '\'
// public class \eX
Diagnostic(ErrorCode.ERR_UnexpectedCharacter, "").WithArguments("\\").WithLocation(3, 18),
// (3,19): error CS0116: A namespace cannot directly contain members such as fields, methods or statements
// (3,21): error CS1001: Identifier expected
// public class \eX
Diagnostic(ErrorCode.ERR_NamespaceUnexpected, "eX").WithLocation(3, 19),
// (4,5): error CS1022: Type or namespace definition, or end-of-file expected
// {
Diagnostic(ErrorCode.ERR_EOFExpected, "{").WithLocation(4, 5),
// (6,1): error CS1022: Type or namespace definition, or end-of-file expected
// }
Diagnostic(ErrorCode.ERR_EOFExpected, "}").WithLocation(6, 1));
Diagnostic(ErrorCode.ERR_IdentifierExpected, "").WithLocation(3, 21));
}

[Fact]
Expand Down
63 changes: 61 additions & 2 deletions src/Compilers/CSharp/Test/Syntax/Parsing/DeclarationParsingTests.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2974,8 +2974,67 @@ public void TestStructBadExpressionProperty()

Assert.Equal(3, file.Errors().Length);
Assert.Equal(ErrorCode.ERR_SemicolonExpected, (ErrorCode)file.Errors()[0].Code);
Assert.Equal(ErrorCode.ERR_InvalidMemberDecl, (ErrorCode)file.Errors()[1].Code);
Assert.Equal(ErrorCode.ERR_InvalidMemberDecl, (ErrorCode)file.Errors()[2].Code);
Assert.Equal(ErrorCode.ERR_TypeExpected, (ErrorCode)file.Errors()[1].Code);
Assert.Equal(ErrorCode.ERR_IdentifierExpected, (ErrorCode)file.Errors()[2].Code);

UsingTree(text,
// (3,18): error CS1002: ; expected
// public int P readonly => 0;
Diagnostic(ErrorCode.ERR_SemicolonExpected, "readonly").WithLocation(3, 18),
// (3,27): error CS1031: Type expected
// public int P readonly => 0;
Diagnostic(ErrorCode.ERR_TypeExpected, "=>").WithLocation(3, 27),
// (3,27): error CS1001: Identifier expected
// public int P readonly => 0;
Diagnostic(ErrorCode.ERR_IdentifierExpected, "=>").WithLocation(3, 27));

N(SyntaxKind.CompilationUnit);
{
N(SyntaxKind.StructDeclaration);
{
N(SyntaxKind.PublicKeyword);
N(SyntaxKind.StructKeyword);
N(SyntaxKind.IdentifierToken, "S");
N(SyntaxKind.OpenBraceToken);
N(SyntaxKind.FieldDeclaration);
{
N(SyntaxKind.PublicKeyword);
N(SyntaxKind.VariableDeclaration);
{
N(SyntaxKind.PredefinedType);
{
N(SyntaxKind.IntKeyword);
}
N(SyntaxKind.VariableDeclarator);
{
N(SyntaxKind.IdentifierToken, "P");
}
}
M(SyntaxKind.SemicolonToken);
}
N(SyntaxKind.PropertyDeclaration);
{
N(SyntaxKind.ReadOnlyKeyword);
M(SyntaxKind.IdentifierName);
{
M(SyntaxKind.IdentifierToken);
}
M(SyntaxKind.IdentifierToken);
N(SyntaxKind.ArrowExpressionClause);
{
N(SyntaxKind.EqualsGreaterThanToken);
N(SyntaxKind.NumericLiteralExpression);
{
N(SyntaxKind.NumericLiteralToken, "0");
}
}
N(SyntaxKind.SemicolonToken);
}
N(SyntaxKind.CloseBraceToken);
}
N(SyntaxKind.EndOfFileToken);
}
EOF();
}

[Fact]
Expand Down
Loading
Loading