Improve error message for misplaced variable designator in property patterns#81287
Improve error message for misplaced variable designator in property patterns#81287CyrusNajmabadi merged 15 commits intomainfrom
Conversation
…nator Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
| EOF(); | ||
| } | ||
|
|
||
| [Fact, WorkItem("https://github.com/dotnet/roslyn/issues/78899")] |
There was a problem hiding this comment.
Updated all WorkItem URLs to reference the correct issue number 50220. (e25a7ad)
| public void DesignatorBeforePositionalPattern() | ||
| { | ||
| UsingExpression("o is Point p (1, 2)", | ||
| // (1,12): error CS8525: The variable designator must come after any property or positional patterns. Change 'p { ... }' to '{ ... } p'. |
There was a problem hiding this comment.
@copilot update the error string to drop the Change 'p { ... }' to '{ ... } p'. part
There was a problem hiding this comment.
Updated the error message to remove the "Change ... to ..." part. The new message is: "The variable designator '{0}' must come after any property pattern." (2fe3e0f)
| { | ||
| var designation = TryParseSimpleDesignation(inSwitchArmPattern); | ||
| if (designation != null) | ||
| { |
There was a problem hiding this comment.
@copilot let's simplify things. First, let's only fix the x { ... } case. We can ignore teh x ( ... ) case.
Second, intead of updating teh code here, you can update the parsePropertyPatternClause to see if we have identifier followed by { and to do the appropriate recovery tehre.
There was a problem hiding this comment.
Simplified to only handle the x { ... } case. Moved the logic into parsePropertyPatternClause to detect identifier followed by { and perform appropriate recovery. Removed tests for positional and combined patterns. (2fe3e0f)
Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
| { | ||
| // Check for the error case: identifier followed by '{' | ||
| // This handles "string s { Length: 5 }" which should be "string { Length: 5 } s" | ||
| if (this.IsTrueIdentifier() && this.IsValidPatternDesignation(inSwitchArmPattern) && this.PeekToken(1).Kind == SyntaxKind.OpenBraceToken) |
There was a problem hiding this comment.
@copilot this can be simplified. just check if we have a true identifier and we have a { one token ahead. If so, consume the token add teh error to it, and add it as a leading skipped token to the propertyPattern that is parsed out.
There was a problem hiding this comment.
Simplified the implementation. Now directly checks for true identifier followed by {, consumes the token with EatToken(), adds the error to it, and attaches it as leading skipped syntax to the property pattern. (c2cf58b)
Co-authored-by: CyrusNajmabadi <4564579+CyrusNajmabadi@users.noreply.github.com>
|
@dotnet/roslyn-compiler this is ready to review. |
|
@jcouv @RikkiGibson ptal. |
|
@333fred ptal :) |
Summary: Improve error messages when variable designation is in incorrect order with property pattern
Problem Statement
When users write
o is string s { Length: 5 }(with designation before property pattern), the compiler produced confusing error messages:error CS1026: ) expectederror CS1002: ; expectederror CS1513: } expectedThe correct syntax should be
o is string { Length: 5 } s(designation after property pattern).Solution
This PR adds a new error code
ERR_DesignatorBeneathPattern(CS8525) that provides a clear error message:Changes Made
✅ Error Code & Resources:
ERR_DesignatorBeneathPatternerror code (CS8525) toErrorCode.csCSharpResources.resxErrorFacts.cs✅ Parser Implementation:
parsePropertyPatternClauselocal function inParsePatternContinuedto detect identifier followed by{✅ Testing:
PatternParsingTests.cs:DesignatorBeforePropertyPattern- testsstring s { Length: 5 }Scope
This fix only handles the property pattern case (
x { ... }). Positional patterns (x ( ... )) are not covered by this change.Validation
Fixes #50220
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.