fix(zh): don't consume a leading Chinese numeral as a word boundary - #653
Open
Jaybhade wants to merge 1 commit into
Open
fix(zh): don't consume a leading Chinese numeral as a word boundary#653Jaybhade wants to merge 1 commit into
Jaybhade wants to merge 1 commit into
Conversation
The inherited patternLeftBoundary() is `(\W|^)`, and CJK characters satisfy `\W`. When a Chinese expression starts at the very beginning of the text, the alternation prefers `\W`, consumes the first character, and the parser then matches a shorter expression with a different value: 十一月十日 (Nov 10) -> 一月十日 (Jan 10) 十二月十日 (Dec 10) -> 二月十日 (Feb 10) 二十天前 (20 days ago) -> 十天前 (10 days ago) 十五天前 (15 days ago) -> 五天前 (5 days ago) 二十天後 (in 20 days) -> 十天後 (in 10 days) These are silent wrong answers rather than parse failures. The same expressions resolve correctly once they are preceded by a space or embedded in a sentence, which is why this went unnoticed. ZHHansTimeExpressionParser and ZHHantTimeExpressionParser already avoid this by returning an empty capturing group from patternLeftBoundary(). Apply that same override to the date, ago and deadline parsers, keeping zh.hans and zh.hant symmetric.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The problem
Chinese date expressions that start at the very beginning of the text silently resolve to the wrong date. With
refDate = 2012-08-10:result.text十一月十日一月十日十二月十日二月十日二十天前十天前十五天前五天前二十天後十天後十五天後五天後This affects both
zh.hansandzh.hant. These are silent wrong answers, not parse failures, so callers get a plausible-looking date that is off by up to ten months.Why
AbstractParserWithWordBoundaryCheckingprependspatternLeftBoundary(), which defaults to:JavaScript's
\Wis[^A-Za-z0-9_], so every CJK character satisfies\W. At index 0 the alternation tries\Wbefore^, consumes the leading character, and the parser then matches whatever still parses from index 1 — a shorter expression with a different value.十月十日(Oct 10) is unaffected only by luck: dropping十leaves月十日, which can't match, so the engine backtracks to^. But dropping the十of十一月leaves the perfectly parseable一月, so no backtracking happens.The same inputs parse correctly once they're preceded by a space or embedded in a sentence — the preceding character then serves as the boundary and nothing is eaten. That's why this hasn't shown up in normal use:
The fix
ZHHansTimeExpressionParserandZHHantTimeExpressionParseralready solve exactly this, with a comment naming the cause:This PR applies that same established override to the three parser families that still inherit the default — date, ago and deadline — in both script variants, keeping
zh.hansandzh.hantsymmetric:ZHHansDateParser/ZHHantDateParserZHHansAgoFormatParser/ZHHantAgoFormatParserZHHansDeadlineFormatParser/ZHHantDeadlineFormatParserThe remaining zh parsers (casual, weekday, relation-weekday) don't need it: their patterns begin with non-numeric characters (
今,明,星期, …), so dropping the first character never leaves a parseable remainder.Considered and rejected: fixing the base class
Reordering the default boundary to
(^|\\W)also fixes all six cases with a one-line change, and the whole suite stays green — but a differential run over a 1,416-entry corpus showed it changesresult.text/result.indexfor other locales, because the boundary character gets absorbed into the match:textbeforetextafter(Monday)Monday)(Monday),MondayMonday,MondayNot worth the cross-locale blast radius, so I kept the change scoped to the zh parsers, consistent with the existing precedent.
Verification
npm test— 146 suites / 680 tests green (674 before, +6 new).(Monday),-5 days ago,@3pm) shows only the 12 intended zh corrections changed; no other locale is affected.prettier --checkclean;tsc -p tsconfig.build.jsonclean.Tests follow the conventions in
AGENTS.md: an explicitrefDateon every case, assertions on bothresult.indexandresult.text, and matching Simplified/Traditional coverage. Each new block also keeps a control case (十月十日,十天前,十天後) so a future regression in the opposite direction is caught too.One behaviour worth flagging
For
ZHHans/HantDateParser, an input with leading whitespace now reports that whitespace insideresult.text:That's inherent to
return "()"when a pattern can begin with\s*, and it already happens onmasterfor the time-expression parser that uses the same override (" 十点"→text: " 十点"). I kept the fix consistent with that precedent rather than special-casing it, but happy to trim the leading whitespace in the date parser's pattern if you'd preferresult.textstay tight.npm run eslintfails on this branch and onmasteralike (ESLint 9 expectseslint.config.js; reported in #606) — unrelated to this change, and CI runsnpm run test.