Skip to content

Fix parsing of nested verbatim interpolated strings started with @$#44924

Merged
jcouv merged 2 commits intodotnet:masterfrom
rstarkov:master
Jul 6, 2020
Merged

Fix parsing of nested verbatim interpolated strings started with @$#44924
jcouv merged 2 commits intodotnet:masterfrom
rstarkov:master

Conversation

@rstarkov
Copy link
Contributor

@rstarkov rstarkov commented Jun 6, 2020

When parsing an interpolated string, the function that scans to find the end of an interpolated hole, ScanInterpolatedStringLiteralHoleBalancedText, does not correctly account for the possibility of a nested string starting with @$.

This PR fixes this by copying the code already present for the $@ case just a few cases earlier.

I've included a unit test which fails without this patch.

@rstarkov rstarkov requested a review from a team as a code owner June 6, 2020 23:28
@dnfadmin
Copy link

dnfadmin commented Jun 6, 2020

CLA assistant check
All CLA requirements met.

@jcouv jcouv added Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee. labels Jun 7, 2020
Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

Done with review pass (iteration 1)

@jcouv jcouv self-assigned this Jun 23, 2020
@rstarkov
Copy link
Contributor Author

Added the language version diagnostic and unit test for it. This test style seems superior so I've deleted the old test.

Note that the error location is the $ of the @$ sequence. Feels like ideally it should be the @ but I couldn't find an easy way to do that. Let me know if you want me to address this?

Copy link
Member

@jcouv jcouv left a comment

Choose a reason for hiding this comment

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

LGTM Thanks (iteration 2)

@jcouv
Copy link
Member

jcouv commented Jul 6, 2020

@dotnet/roslyn-compiler for a second review of community PR. Thanks

Copy link
Member

@gafter gafter left a comment

Choose a reason for hiding this comment

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

😷

@jcouv jcouv merged commit a75d0d6 into dotnet:master Jul 6, 2020
@ghost ghost added this to the Next milestone Jul 6, 2020
@jcouv
Copy link
Member

jcouv commented Jul 6, 2020

Merged/squashed. Thanks @rstarkov for the contribution! :-)

@JoeRobich JoeRobich modified the milestones: Next, 16.8.P1 Jul 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Area-Compilers Community The pull request was submitted by a contributor who is not a Microsoft employee.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants