Fix a slew of UTF-8/UTF-16 related issues - #4181
Conversation
|
This is more challenging than I expected. Another problem is that we, as I mentioned at some point, don't re-normalize split apart UTF-16 surrogates back together, so you can have "the same string" but two ways depending on how it's constructed. It's not impossible to fix this, but, it is very annoying. Ultimately this continues to be a problem with "what do template types mean". Are they describing JS strings? Are they UTF-8? Runes? Are they some abstract undefined concept? I tried my best in the last push, though |
|
Hi Jake Bailey (@jakebailey) , I saw your note about the difficulty with re-normalizing the split UTF-16 surrogates back together during inference. In my previous PR (#4144) that was closed in favor of this one, I used a localized workaround specifically for the template literal matching that might be useful here. By intercepting runes > 0xFFFF and splitting them using Go's utf16.EncodeRune directly within relater.go during the match step, I was able to get the inference to pass without needing to alter how the strings are stored or recombined globally. It bypassed the re-normalization trap by strictly mimicking the 2-byte evaluation at the AST node level. I know this PR is tackling a much broader WTF-8-esque refactor, so a localized fix might not fit the new architecture, but I wanted to drop a link to the logic here just in case it helps unblock that specific inference test case! |
|
I'll have to think about that, there's just more than just this one place... |
43f1bbc to
2923644
Compare
Store escaped lone surrogate code units losslessly using explicit surrogate byte sequences instead of converting them through Go runes and losing them as U+FFFD. Move the surrogate helpers into stringutil, teach the scanner and regexp parser to use the shared JS-string rune encoder and decoder, and ensure printer output always escapes lone surrogate code units at text boundaries. This fixes issue 1701, issue 3899, and the escaped-surrogate value corruption part of issue 4092.
Stop multiline comment emission from assuming that the byte before the next line start is the end of the line. That slices through multi-byte ECMAScript line separators such as U+2028 and U+2029 and can leave invalid UTF-8 in output. Scan each emitted comment line to the actual line-break rune before trimming and writing the text. This fixes issue 4119.
Mark synthesized template literal type spans with the existing no-ASCII-escaping emit flag so ordinary non-ASCII text is preserved in declaration emit. Required template literal escapes and lone surrogate escapes still go through the printer escape path. This responds to the review comment about emoji text inside template literal types in the issue 4071 coverage baseline.
2923644 to
af9daea
Compare
| func SurrogatePairToCodePoint(high rune, low rune) rune { | ||
| return (high-SurrogateHighStart)<<10 | (low - SurrogateLowStart) + SupplementaryStart | ||
| } |
There was a problem hiding this comment.
verified and I do not believe this is accurate
There was a problem hiding this comment.
Agreed (odd this was flagged "High") but can't we just use utf16.DecodeRune? Its implementation is the same (for valid inputs)
There was a problem hiding this comment.
IIRC because we do actually want it to run on invalid inputs (but I'll check)
There was a problem hiding this comment.
I tested, and we can actually use this, but I don't know how valuable it is when we already have to declare a bunch of other stuff that is normally unexported from utf16...
There was a problem hiding this comment.
I found a reasonable balance, pushed
The scanner only combined a high surrogate with a following low surrogate
when both used the plain \uXXXX form; any combination involving an extended
\u{...} escape was left as two un-combined lone surrogates. That gave
"\u{D83D}\u{DE00}" a different literal type from the identical-in-JS
"\uD83D\uDE00" and "😀", producing spurious TS2322 errors.
Combine a high surrogate with any following low-surrogate escape (\uLow or
\u{Low}) in string literals, mirroring how adjacent UTF-16 code units pair
in a JavaScript string. Regex behavior is unchanged: only plain \uHigh\uLow
combine in AnyUnicodeMode, and lone surrogates remain CESU-8 encoded.
decodeEntities wrote the decoded code point with strings.Builder.WriteRune, which replaces surrogate code points (0xD800-0xDFFF) with U+FFFD. A reference like � therefore emitted \uFFFD instead of the intended lone surrogate. Encode the decoded rune with stringutil.EncodeJSStringRune so lone surrogates survive as their CESU-8 sentinel and are re-escaped to \uXXXX on emit.
…ments
Template literal type values are assembled by concatenating separately
scanned fragments. A high surrogate ending one fragment and a low surrogate
beginning the next form a surrogate pair in the resulting JavaScript string,
exactly as "\uD83D" + "\uDE00" === "😀". The checker kept the two halves
as un-combined lone surrogates, so `${Hi}${Lo}` produced "\uD83D\uDE00"
instead of "😀", diverging from tsc and leaving invalid UTF-8 in the value.
Add stringutil.CombineSurrogatePairs, which merges adjacent high+low surrogate
sentinels into the supplementary code point they represent (with a fast path
that returns strings containing no sentinel unchanged), and apply it to each
concatenated value extracted in getTemplateLiteralType. Pairs separated by a
generic placeholder stay in distinct fragments and are correctly left alone.
Template literal type inference advanced one full code point at a time using utf8.DecodeRuneInString, which byte-breaks the invalid UTF-8 sentinels used to store lone surrogates. Use stringutil.DecodeJSStringRune so a lone-surrogate sentinel is pulled off whole, and recombine matched fragments via CombineSurrogatePairs.
If code elsewhere slices a string between the bytes of a lone-surrogate sentinel, the printer would emit the resulting stray bytes raw, producing invalid UTF-8 in the output. Detect a byte that does not begin a valid rune and escape it as the Unicode replacement character so emitted text is always well-formed.
af9daea to
f1f59d4
Compare
|
What's the issue for tracking intrinsic string mapping? |
|
Fixes #1701 (sorta)
Fixes #3899
Fixes #4092
Fixes #4119
Updates #4137
Fixes #4071
Each commit has its own description.