-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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.
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
base: main
Are you sure you want to change the base?
Conversation
…ifiers
Add error recovery in TryParseIndexerOrPropertyDeclaration to handle cases where a property has a missing identifier but the accessor list or expression body is present. When we detect a property body start token ('{' or '=>') but no identifier, we synthesize a missing identifier and continue parsing as a property rather than creating an incomplete member.
This improves parser recovery when users forget to add a property name, allowing the rest of the file to parse correctly instead of cascading errors.
Co-authored-by: CyrusNajmabadi <[email protected]>
Update RequiredModifierProperty tests to reflect the improved error recovery. With the new parser changes, properties with missing identifiers now produce clearer ERR_IdentifierExpected errors instead of generic ERR_InvalidMemberDecl errors, which is better for users. Co-authored-by: CyrusNajmabadi <[email protected]>
Add clarifying comment to PropertyWithMissingIdentifier_WithAccessors test to highlight that the key validation is that all members were parsed as properties (not incomplete members), which confirms the error recovery is working correctly. Co-authored-by: CyrusNajmabadi <[email protected]>
…otnet/roslyn into copilot/improve-error-recovery
| Diagnostic(ErrorCode.ERR_IdentifierExpected, "{").WithLocation(6, 27) | ||
| ); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
| Diagnostic(ErrorCode.ERR_IdentifierExpected, "{").WithLocation(6, 27) | |
| ); | |
| Diagnostic(ErrorCode.ERR_IdentifierExpected, "{").WithLocation(6, 27)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot fort all the tests you added, make sure this ); is on the prior line at the end. Also, add [Fact, WorkItem("...")] attributes for all new tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done in d9c69ab. Added WorkItem attributes and moved closing parenthesis to the end of the last Diagnostic line for all new tests.
- Add [Fact, WorkItem(...)] attributes to PropertyWithMissingIdentifier tests - Move closing parenthesis to end of last Diagnostic line for consistency Co-authored-by: CyrusNajmabadi <[email protected]>
| { | ||
| public ContainingSymbol { get; } | ||
| void M(object o, bool b0, bool b1) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this test was affected because the line above is public ContainingSymbol { get; }. Instead of going off the rails, we now properly parse and recover. So formatting works properly below.
|
@dotnet/roslyn-compiler ptal. |
|
Will try to review in depth tomorrow. |
Co-authored-by: Rikki Gibson <[email protected]>
Improve parser error recovery for property declarations with missing identifiers
Summary
Successfully implemented error recovery for property declarations with missing identifiers. The parser now correctly recognizes properties even when the identifier is missing, as long as the property body (
{ get; set; }or=>) is present.Plan:
Technical Implementation:
The fix adds error recovery in three key places:
Validation:
✅ Core Fix Validated: The test
PropertyWithMissingIdentifier_WithAccessorsdemonstrates that:PropertyDeclaration(notIncompleteMember)This matches the requirement from the issue: "The parser properly recovers by implying a missing identifier/type name on the property declaration."
Remaining Work:
18 existing tests need their expectations updated. These are NOT regressions - they represent IMPROVED error reporting:
ERR_InvalidMemberDeclerrorsERR_IdentifierExpectederrors (more helpful to users)The changes provide better error messages that guide users more precisely to the actual problem (missing identifier) rather than a vague "invalid member declaration" message.
Original prompt
💡 You can make Copilot smarter by setting up custom instructions, customizing its development environment and configuring Model Context Protocol (MCP) servers. Learn more Copilot coding agent tips in the docs.