Improve lexer error recovery with interpolated strings. #54875
Conversation
| TextWindow.AdvanceChar(2); | ||
|
|
||
| ErrorCode? error = null; | ||
| while (true) | ||
| { |
There was a problem hiding this comment.
no need for this 'if' check, the callers ensure it. so this became a Debug assert, with no else. #Resolved
Tangentially. THis is pure cleanup and improvement though that can go in outside of the raw string literal work. that's why i decoupled it and brought it into main. |
| } | ||
|
|
||
| [Fact, Trait(Traits.Feature, Traits.Features.CodeActionsAddParenthesesAroundConditionalExpressionInInterpolatedString)] | ||
| public async Task TestAddParenthesesClosingBracketInFalseCondition() | ||
| { | ||
| await TestInMethodAsync( | ||
| @"var s = $""{ true ? new int[0] [|:|] new int[] {} }"";", | ||
| @"var s = $""{ (true ? new int[0] : new int[] {} )}"";"); | ||
| @"var s = $""{ (true ? new int[0] : new int[] {}) }"";"); |
There was a problem hiding this comment.
these results are actually preferred as we do not have the bogus space after the WhenFalse part.
| @@ -88,71 +88,71 @@ public async Task TestAddParenthesesWithTrivia() | |||
| { | |||
| await TestInMethodAsync( | |||
| @"var s = $""{ /* Leading1 */ true /* Leading2 */ ? /* TruePart1 */ 1 /* TruePart2 */[|:|] /* FalsePart1 */ 2 /* FalsePart2 */ }"";", | |||
| @"var s = $""{ /* Leading1 */ (true /* Leading2 */ ? /* TruePart1 */ 1 /* TruePart2 */: /* FalsePart1 */ 2 /* FalsePart2 */ )}"";"); | |||
| @"var s = $""{ /* Leading1 */ (true /* Leading2 */ ? /* TruePart1 */ 1 /* TruePart2 */: /* FalsePart1 */ 2) /* FalsePart2 */ }"";"); | |||
There was a problem hiding this comment.
preferred as this is symettrical with what we do with Leading1. both the open and close parens tightly bind the expression code itself, not the surrounding trivia.
|
Looks like a couple of tests were affected (break in CI) |
Yeah, looks like the approach of picking the earliest error was necessary. I've moved to your model, but have preserved that logic. |
|
Yup. I'm set to squash things. Thanks! |
|
/azp run |
|
Azure Pipelines successfully started running 3 pipeline(s). |
|
@CyrusNajmabadi looks like there's a legitimate test failure. |
ghost
added this to the 
The current lexer strategy with interpolations is to bail early when it encounters something like a spurious newline in a non-verbatim interpolation. This means that simple user cases like:
Go entirely off the rails. since the lexer stops processing that at:
var v = $"Hello { @""leaving the rest to be restarted. This of course throws everything off as the contents of the string become code and then later quotes look to start new strings (like"; ...). Furthermore the close curlies found later then just destroy the rest of hte parsing stack, leading to everything being awful.This PR changes our strategy here to actually recognize when we are running into errant newlines (either in the interpolation code, or comments, or multiline verbatim literals), and while it treats that as an error, it still allows normal lexing of these constructs to proceed, producing a good tree with a good error message.
This cleanup helps the Raw-String-Literals work (esp. around how it handles interpolations). However, it can be done independently to keep things tidier and simpler.