fix: correctly handle even numbers of backslashes before closing string quote#427
Conversation
| // An even number of backslashes before the closing quote means the backslashes | ||
| // escape each other (e.g. `\\` = one literal backslash), so the quote is NOT escaped. | ||
| // The previous implementation only checked the immediately preceding character, | ||
| // causing it to incorrectly treat the closing quote as escaped. |
There was a problem hiding this comment.
Please remove the new comments from the test files
There was a problem hiding this comment.
Done — removed the comments from both test files in 769ea9d.
| // Checks if the quote was escaped by counting consecutive preceding backslashes. | ||
| // An even number of backslashes means they cancel each other out (e.g. `\\` is a | ||
| // literal backslash), so the quote is NOT escaped. An odd number means the quote | ||
| // IS escaped (e.g. `\'`). |
There was a problem hiding this comment.
Wouldn't checking the immediate next character (and consuming it) if it's also a backslash work as well?
There was a problem hiding this comment.
Yes, that's cleaner — switched to exactly that in 769ea9d: when a backslash is encountered, both it and the following character are consumed (pos += 2), so an escaped quote never terminates the scan and a literal backslash can't swallow the closing quote. Behavior is identical (all snapshots unchanged, 235 tests pass).
|
@chatman-media I'll need you to rebase this to latest |
…ng quote
The `skipString` function in the tokenizer checked only the immediately
preceding character to determine if a closing quote was escaped. This
caused it to treat `'\\'` (a string ending with a literal backslash) as
an unclosed string, because the closing quote's predecessor is a `\`.
The correct rule is to count all consecutive backslashes before the
quote: an even count means they escape each other (literal backslashes),
so the quote terminates the string; an odd count means the last
backslash escapes the quote.
Fixes resolution errors thrown when a class constructor has a string
default parameter value ending with an even number of backslashes, e.g.:
class MyService {
constructor(dep1, path = 'C:\\Windows\\', dep2) {}
}
924b4a6 to
5056017
Compare
|
Done — rebased onto the latest |
|
Released as v13.0.5, thanks! |
Problem
The
skipStringfunction insrc/function-tokenizer.tsused a one-character look-behind to decide whether a closing quote is escaped:This check is incorrect when an even number of backslashes precedes the closing quote. For example, the source string
'test\\'represents the JS valuetest\(a string ending with one literal backslash). Here the source characters are' t e s t \ \ '. Because the character immediately before the closing'is a backslash, the old code treats the quote as escaped and runs past the end of the string, ultimately throwing:This manifests in practice whenever a class constructor has a parameter with a string default value whose final character is a backslash (e.g. Windows filesystem paths):
Fix
Count all consecutive backslashes before the quote. An even count means each pair cancels out (literal backslash characters), so the quote is the end of the string. An odd count means the last backslash is an escape for the quote, so the string continues.
Tests
can skip strings ending with an even number of backslashes) covering 2-backslash and 4-backslash cases.string default ending with even backslashes) that directly exercises the real-world failure scenario, including a check that single-backslash escaped quotes still work correctly.All 235 tests pass.