Skip to content
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

Fix parsing interpolated strings with unmatched braces #14182

Merged
merged 4 commits into from
Nov 7, 2022

Conversation

abonie
Copy link
Member

@abonie abonie commented Oct 26, 2022

Should fix #14160

@abonie
Copy link
Member Author

abonie commented Oct 27, 2022

Looking at the failing tests, I am starting to have some doubts about this fix.

There are tests with inputs like:

let TripleInTripleInterpolated   = $"""123{456}789{"""012"""}345"""
let TripleInSingleInterpolated   = $"123{456}789{"""012"""}345"
let TripleInVerbatimInterpolated = $"123{456}789{"""012"""}345"
let TripleInterpolatedInTripleInterpolated   = $"""123{456}789{$"""012"""}345"""
let TripleInterpolatedInSingleInterpolated   = $"123{456}789{$"""012"""}345"
let TripleInterpolatedInVerbatimInterpolated = $"123{456}789{$"""012"""}345"

ensuring there's an "Invalid interpolated string" error for each line. And with my change, we will only correctly report the error on the first line, so it seems like legitimate regression.

However, I am not so sure if this was fully intentional to begin with. We happen to have nicer errors in this case, because the lexer is optimistic and continues to hope that the open { will be matched by closing }, even though there already was an error in between, that might have been the end of the string. And in the case of this particular test it happens to be true - all { are matched with }.

On the other hand, right now input like this:

let s = $"Unescaped curly brace or unclosed interpolation expr -> {"

let s2 = "Some legit string literal that did nothing wrong"

let s3 = "Another good citizen"

will result in false positive errors for s2 and s3 string literals (because the lexer keeps thinking it is in the context of an open interpolation expression). And of course FSI is broken by input like that (see #14160).

So there's definitely something to fix, but idk if it is ok to regress on cases like the first example (to be clear though, it would still be a compilation error).

@abonie abonie force-pushed the incomplete_interpolation_fsi_bug branch 2 times, most recently from 67ebe1f to e93cd97 Compare November 4, 2022 11:24
Two tests for unmatched curly brace in interpolated string
This splits two unit tests that were testing multiple cases in one go
into multiple unit tests - one per each case.

Note, that original tests were expecting an accurate error for each
of multiple lines of malformed string literals. They would no longer
pass with changes introduced by previous commits in this PR.
@abonie abonie force-pushed the incomplete_interpolation_fsi_bug branch from e93cd97 to 107a2b0 Compare November 4, 2022 11:27
@abonie abonie changed the title Incomplete interpolation fsi bug Fix parsing interpolated strings with unmatched braces Nov 4, 2022
@abonie abonie marked this pull request as ready for review November 4, 2022 13:23
@abonie abonie requested a review from a team as a code owner November 4, 2022 13:23
@0101 0101 merged commit 947f2bb into dotnet:main Nov 7, 2022
abonie added a commit that referenced this pull request Feb 16, 2023
github-actions bot pushed a commit that referenced this pull request Feb 16, 2023
abonie added a commit that referenced this pull request Feb 16, 2023
vzarytovskii pushed a commit that referenced this pull request Feb 16, 2023
…ched braces" (#14760)

* Revert "Fix parsing interpolated strings with unmatched braces (#14182)"

This reverts commit 947f2bb.

* Bump FSBuildVersion

---------

Co-authored-by: Adam Boniecki <[email protected]>
KevinRansom added a commit to KevinRansom/fsharp that referenced this pull request Feb 20, 2023
KevinRansom added a commit to KevinRansom/fsharp that referenced this pull request Feb 20, 2023
kant2002 pushed a commit to kant2002/fsharp that referenced this pull request Apr 1, 2023
…ched braces" (dotnet#14760)

* Revert "Fix parsing interpolated strings with unmatched braces (dotnet#14182)"

This reverts commit 947f2bb.

* Bump FSBuildVersion

---------

Co-authored-by: Adam Boniecki <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

In FSI, an invalid string literal can break further parsing of string in that interactive session
5 participants