From 50d30a8c4986002d365e788f4e7ee23a8296bbf0 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 25 Aug 2026 01:35:06 +0000 Subject: [PATCH] fix(i18n,scripts): hold every inline defaultValue to the spelling the fallback resolves MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `check:i18n-keys`' `default-value-drift` class pins an inline default byte-identical to its `en` row, and objectui#3512 holds `en` to the one placeholder spelling `createSafeTranslation`'s `fallbackT` interpolates — so most inline defaults are covered transitively. Re-measured on this tree, 66 of 1003 are not: 3 literals on dynamic keys, 63 written as a computed expression. Class 7, `unresolvable-default-spelling`, puts #3512's rule beside `holesOf()` and applies it to the text an inline default carries — the folded sentence, or a template literal's static segments. Judged over every inline default (977) rather than only the residue (37 with readable text, 24 with none), so the count is a live control instead of a set that could silently empty; the CLI exits non-zero below 500. Single-brace holes and JSX braces are out of range by construction: the rule is handed a literal's TEXT, never source. Closing the residue at the source reaches only 5 sites, and the measurement behind that is the load-bearing part. react-i18next's not-ready `t` returns `options.defaultValue` VERBATIM — it does not interpolate. So at a call site bound to a bare `useObjectTranslation()`, `` `Signed in as ${user.email}` `` is the only form that renders correctly with no provider, and rewriting it to `'Signed in as {{email}}'` would put literal braces in front of the user — the exact defect this family of cards exists to prevent. `objectBulkActionDispatch.test.tsx` fails on precisely that substitution. The 5 that are safe: four whose default carries no hole (the record-form submit button, which also stops falling back to `Save` where the pack renders `Update`; the context-selector package label; the approvals separator) and one behind a `createSafeTranslation` hook, whose fallback does interpolate. Part of #4905 Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_019b5UBNMtTzKbVtZZGvFuxe --- .changeset/4905-inline-default-value-pin.md | 34 ++ packages/app-shell/src/console/AppContent.tsx | 4 +- .../app-shell/src/layout/ContextSelectors.tsx | 2 +- .../app-shell/src/utils/approverIdentity.ts | 2 +- packages/plugin-detail/src/DetailSection.tsx | 2 +- .../check-i18n-call-site-keys.test.ts | 154 ++++++++- scripts/check-i18n-call-site-keys.mjs | 292 +++++++++++++++++- 7 files changed, 478 insertions(+), 12 deletions(-) create mode 100644 .changeset/4905-inline-default-value-pin.md diff --git a/.changeset/4905-inline-default-value-pin.md b/.changeset/4905-inline-default-value-pin.md new file mode 100644 index 0000000000..266fe1250d --- /dev/null +++ b/.changeset/4905-inline-default-value-pin.md @@ -0,0 +1,34 @@ +--- +'@object-ui/app-shell': patch +'@object-ui/plugin-detail': patch +--- + +Inline `t(key, { defaultValue })` strings are now held to the one placeholder spelling a +provider-less host can resolve, and five of them are pinned to the pack value for the first +time (objectui#4905). + +`check:i18n-keys`' `default-value-drift` class pins an inline default byte-identical to its +`en` row, and objectui#3512 holds `en` to the one spelling `createSafeTranslation`'s +`fallbackT` interpolates — so most inline defaults were covered transitively. Re-measured +on this tree, 66 of 1003 were not: three literal defaults on dynamic keys, and 63 written +as a computed expression. A new `unresolvable-default-spelling` class in +`scripts/check-i18n-call-site-keys.mjs` now judges the text every inline default carries — +the folded sentence, or a template literal's static segments — so `{{ name }}`, +`{{count, number}}`, `{{- name}}` and `$t(key)` are refused wherever they are written, +rather than only inside a copy table. + +Four call sites gain a real pin because their default carries no placeholder at all: the +record-form submit button now falls back to `Update`/`Create` (the pack's wording) instead +of `Save`/`Create` via a nested `t('common.save')`, the context selector's package label +and the approvals separator now state their literal. One more (`detail.showEmptyFields`) +is behind a `createSafeTranslation` hook, whose fallback does interpolate, so it can safely +say what the pack says. + +The other 61 are deliberately left computed, and the measurement behind that is the useful +part: react-i18next's not-ready `t` returns `options.defaultValue` **verbatim, without +interpolating it**. At a call site bound to a bare `useObjectTranslation()`, a default +written as `` `Signed in as ${user.email}` `` is therefore the only form that renders +correctly with no provider — rewriting it to `'Signed in as {{email}}'` would put literal +braces in front of the user, which is the exact defect this family of cards exists to +prevent. Those sites keep their template literals and are covered by the spelling class +instead. diff --git a/packages/app-shell/src/console/AppContent.tsx b/packages/app-shell/src/console/AppContent.tsx index 6e6dbc75f2..1188be9606 100644 --- a/packages/app-shell/src/console/AppContent.tsx +++ b/packages/app-shell/src/console/AppContent.tsx @@ -1099,8 +1099,8 @@ export function AppContent({ extraRoutes, extraRoutesNoApp }: AppContentProps = showSubmit: true, showCancel: true, submitText: editingRecord - ? t('form.update', { defaultValue: t('common.save', { defaultValue: 'Save' }) }) - : t('form.create', { defaultValue: t('common.create', { defaultValue: 'Create' }) }), + ? t('form.update', { defaultValue: 'Update' }) + : t('form.create', { defaultValue: 'Create' }), cancelText: t('common.cancel'), }} dataSource={dataSource} diff --git a/packages/app-shell/src/layout/ContextSelectors.tsx b/packages/app-shell/src/layout/ContextSelectors.tsx index 3bb4d880e7..d17fc1bee9 100644 --- a/packages/app-shell/src/layout/ContextSelectors.tsx +++ b/packages/app-shell/src/layout/ContextSelectors.tsx @@ -405,7 +405,7 @@ function SelectorControl({ const Icon = getIcon(def.icon); const rawLabel = resolveKeyedI18nLabel(def.label as any, t) || def.id; const label = rawLabel === 'Package' - ? (t?.('common.package', { defaultValue: rawLabel }) ?? rawLabel) + ? (t?.('common.package', { defaultValue: 'Package' }) ?? rawLabel) : rawLabel; const placeholder = t?.('actionDialog.selectPlaceholder', { label, diff --git a/packages/app-shell/src/utils/approverIdentity.ts b/packages/app-shell/src/utils/approverIdentity.ts index b91470b489..1dbf27e07f 100644 --- a/packages/app-shell/src/utils/approverIdentity.ts +++ b/packages/app-shell/src/utils/approverIdentity.ts @@ -362,7 +362,7 @@ export function approverCopyFrom( const str = (out: unknown, fallbackText: string): string => typeof out === 'string' && out ? out : fallbackText; const separator = str( - t('approvalsInbox.approverNameSeparator', { defaultValue: DEFAULT_NAME_SEPARATOR }), + t('approvalsInbox.approverNameSeparator', { defaultValue: ', ' }), DEFAULT_NAME_SEPARATOR, ); return { diff --git a/packages/plugin-detail/src/DetailSection.tsx b/packages/plugin-detail/src/DetailSection.tsx index ef4e194ab2..f6082e8e80 100644 --- a/packages/plugin-detail/src/DetailSection.tsx +++ b/packages/plugin-detail/src/DetailSection.tsx @@ -490,7 +490,7 @@ export const DetailSection: React.FC = ({ )} {showEmptyOverride ? t('detail.hideEmptyFields', { defaultValue: 'Hide empty fields' }) - : t('detail.showEmptyFields', { count: emptyCount, defaultValue: `Show ${emptyCount} empty field${emptyCount === 1 ? '' : 's'}` })} + : t('detail.showEmptyFields', { count: emptyCount, defaultValue: 'Show {{count}} empty fields' })} )} diff --git a/scripts/__tests__/check-i18n-call-site-keys.test.ts b/scripts/__tests__/check-i18n-call-site-keys.test.ts index 0b65d23f7f..5da1c834b7 100644 --- a/scripts/__tests__/check-i18n-call-site-keys.test.ts +++ b/scripts/__tests__/check-i18n-call-site-keys.test.ts @@ -16,6 +16,7 @@ import { PACK_HOOK, readVocabulary, RESERVED_OPTION_NAMES, + unresolvableSpellings, } from '../check-i18n-call-site-keys.mjs'; /** @@ -1065,11 +1066,162 @@ export const A = (name: string, label: string, n: number) => { path.join(repoRoot, 'packages/app-shell/src/layout/ContextSelectors.tsx'), 'utf8', ); - expect(selectors).toContain("t?.('common.package', { defaultValue: rawLabel }) ?? rawLabel"); + // Pinned on the OPTIONAL CALL and the `??` OPERAND — the two things this + // rule abstained on — and deliberately not on the options object between + // them. objectui#4905 rewrote both calls' `defaultValue` ARGUMENT (to the + // `en` value, so class 3 pins them where it previously could not), which is + // a different rule acting on a different position; a marker spanning both + // made that fix read as this rule's regression. + expect(selectors).toContain("t?.('common.package'"); + expect(selectors).toContain(') ?? rawLabel'); expect(selectors).toContain('}) ?? `Select ${label}…`'); }); }); +describe('an inline defaultValue spells its holes the one way the fallback resolves (objectui#4905)', () => { + /** + * Class 7. `fallbackT` interpolates with an exact literal needle, so it + * resolves `{{name}}` and nothing else; i18next also accepts `{{ name }}`, + * `{{count, number}}`, `{{- name}}` and `$t(key)`. The divergence is visible + * only WITHOUT a provider, which is the one host nobody watches. + * + * objectui#3512 gated this rule over the copy TABLES and said in writing that + * inline defaults were out of its scope, because finding one means classifying + * the call site — the walk this file owns. The residue it recorded is what + * objectui#4905 closed: most of it at the SOURCE (call sites rewritten so the + * drift rule pins them), the rest here. + */ + function spellingOf(root: string): string[] { + return analyze(root, { families: [] }) + .findings.filter((f: { reason: string }) => f.reason === 'unresolvable-default-spelling') + .map((f: { detail: string; actual: string }) => `${f.detail}: ${f.actual}`) + .sort(); + } + + const withDefault = (expression: string) => ({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (name: string) => { const { t } = useObjectTranslation(); return t('interp.greet', { name, defaultValue: ${expression} }); }; +`, + }); + + it('the rule itself: canonical passes, each of the four i18next-only spellings fails', () => { + // A unit pin on the predicate, so the four dialects are named once in a + // place that does not depend on the walk finding them. + expect(unresolvableSpellings('Hello {{name}}')).toEqual([]); + expect(unresolvableSpellings('Hello {{ name }}')).toHaveLength(1); + expect(unresolvableSpellings('Deleted {{count, number}} rows')).toHaveLength(1); + expect(unresolvableSpellings('Hello {{- name}}')).toHaveLength(1); + expect(unresolvableSpellings('Hello $t(common.save)')).toHaveLength(1); + expect(unresolvableSpellings('Hello {{user.name}}')).toHaveLength(1); + expect(unresolvableSpellings('{{a}} and {{b}}')).toEqual([]); + }); + + it('is silent when the default spells its hole the way the fallback reads it', () => { + const { findings, counters } = analyze(repoWith(withDefault("'Hello {{name}}'")), { families: [] }); + expect(findings).toEqual([]); + expect(counters.spellingJudgedDefaults).toBe(1); + }); + + it.each([ + ["'Hello {{ name }}'", 'whitespace inside the braces'], + ["'Hello {{name, upper}}'", 'an i18next format spec'], + ["'Hello {{- name}}'", 'the {{- x}} unescape prefix'], + ["'Hello $t(common.save)'", 'i18next nesting'], + ])('RED on %s', (expression, fragment) => { + const found = spellingOf(repoWith(withDefault(expression))); + expect(found).toHaveLength(1); + expect(found[0]).toContain(fragment); + }); + + it('reaches a COMPUTED default, which class 3 structurally cannot judge', () => { + // The residue the card is about: a template literal is not a comparable + // sentence, so `default-value-drift` counts it and moves on — but its + // literal SEGMENTS are text the fallback renders verbatim. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (name: string, n: number) => { + const { t } = useObjectTranslation(); + return t('interp.both', { name, n, defaultValue: \`Hello \${name}, you have {{ n }} messages\` }); +}; +`, + }); + const { counters } = analyze(root, { families: [] }); + expect(counters.computedDefaultValues).toBe(1); + expect(spellingOf(root)).toEqual(['interp.both: "{{ n }}" — whitespace inside the braces; the fallback resolves only {{name}}']); + }); + + it('reaches a default on a DYNAMIC key, which no transitive pin can cover', () => { + // No single `en` value exists, so there is nothing for class 3 to compare + // and nothing #3512 covers transitively. The spelling is still judged. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (which: string, name: string) => { + const { t } = useObjectTranslation(); + return t(\`common.\${which}\`, { name, defaultValue: 'Hello {{ name }}' }); +}; +`, + }); + expect(spellingOf(root)).toEqual(['(dynamic key): "{{ name }}" — whitespace inside the braces; the fallback resolves only {{name}}']); + }); + + it('a hole straddling a substitution is reported, not silently joined into a valid one', () => { + // Joining the segments would invent an adjacency the runtime never + // produces: at runtime the substitution sits between the braces, so + // neither interpolator can resolve it. Reading it as `{{name}}` would be + // the false green. + const found = spellingOf(repoWith(withDefault('`{{ ${name} }}`'))); + expect(found).toHaveLength(1); + expect(found[0]).toContain('unterminated `{{`'); + }); + + it('counts, and does not judge, a default with no readable text at all', () => { + const root = repoWith(withDefault('name')); + const { findings, counters } = analyze(root, { families: [] }); + expect(findings).toEqual([]); + expect(counters.opaqueDefaultText).toBe(1); + expect(counters.spellingJudgedDefaults).toBe(0); + }); + + it('leaves single-brace holes alone — objectui#4135 spells a downstream fill that way', () => { + expect(spellingOf(repoWith(withDefault("'Hello {name}'")))).toEqual([]); + expect(unresolvableSpellings('Resend in {seconds}s')).toEqual([]); + }); + + it('leaves JSX object-literal braces out of range by construction', () => { + // `style={{ opacity: 0 }}` is `{{` that is syntax, not copy. The rule is + // handed the TEXT of a literal, never source, so a JSX brace cannot reach + // it — excluded by where the rule looks, not by an allow-list. + const root = repoWith({ + 'packages/i18n/src/locales/en.ts': EN_FIXTURE, + 'packages/x/src/A.tsx': `import { useObjectTranslation } from '${I18N_PKG}'; +export const A = (name: string) => { + const { t } = useObjectTranslation(); + return {t('interp.greet', { name, defaultValue: 'Hello {{name}}' })}; +}; +`, + }); + expect(spellingOf(root)).toEqual([]); + }); + + it('main carries no unresolvable spelling, which is why this rule has no baseline', () => { + expect(spellingOf(repoRoot)).toEqual([]); + }); + + it('and that green is not vacuous — it is measured over hundreds of real defaults', () => { + // The trap this rule's own card named: a coverage number that looks like + // success and can be reached by judging nothing. The CLI exits non-zero + // below 500; this pins the same floor where the counter is readable. + const { counters } = analyze(repoRoot); + expect(counters.spellingJudgedDefaults).toBeGreaterThan(500); + // The residue route C could not reach is still IN the judged set, not + // quietly dropped from it. + expect(counters.spellingJudgedResidueDefaults).toBeGreaterThan(0); + }); +}); + describe('a declared dynamic family is checked MEMBER by member (objectui#4964)', () => { /** * The class the prefix rule structurally cannot reach: the head resolves, so diff --git a/scripts/check-i18n-call-site-keys.mjs b/scripts/check-i18n-call-site-keys.mjs index cbd4297018..7ef52af2e0 100644 --- a/scripts/check-i18n-call-site-keys.mjs +++ b/scripts/check-i18n-call-site-keys.mjs @@ -5,7 +5,9 @@ * must say the same thing the pack does (objectui#3810) — and the interpolation * arguments the call site passes must be exactly the holes the `en` value has * to receive them (objectui#3845) — and the OTHER spelling of a fallback, - * `t(key) || 'English'`, must not exist at all (objectui#4117). + * `t(key) || 'English'`, must not exist at all (objectui#4117) — and whatever + * text that inline default carries must spell its placeholders the one way the + * provider-less fallback can resolve them (objectui#4905). * * Run: node scripts/check-i18n-call-site-keys.mjs (also `pnpm check:i18n-keys`) * Exit: 0 = every in-scope call-site key resolves (or is baselined), no inline @@ -292,6 +294,58 @@ * 112 checked member keys), and a missing translation is a gap to surface, not * something to invent pack entries for. * + * 7. `unresolvable-default-spelling` (objectui#4905) — an inline `defaultValue` + * spells a placeholder in one of the four dialects i18next accepts and + * `createSafeTranslation`'s `fallbackT` does not (`{{ name }}`, + * `{{count, number}}`, `{{- name}}`, `$t(key)`). WITH a provider the pack + * value wins and nothing is visible; WITHOUT one the braces reach the user. + * + * This is objectui#3512's rule, and the reason it is HERE rather than there + * is the whole of objectui#4905. That card gated the three copy TABLE + * surfaces and left inline defaults out in writing, because a `defaultValue` + * is a call-site OPTION rather than a table: finding one means resolving + * which `t` is in scope at that position and reading the call's arguments — + * the classifier this file already is, and a second independently-rotting + * copy of it anywhere else. The residue #3512 recorded was 3 "not + * comparable" plus 62 computed inline defaults with no transitive pin. + * + * Closing that residue at the SOURCE — rewriting a computed default as the + * `en` value so class 3 pins it — turns out to reach only 5 of the 66 sites + * measured on this tree, and the reason is worth stating because it is the + * opposite of what the card assumed. THREE things can render an inline + * default, and only two of them interpolate it: + * + * 1. i18next, with a provider. It interpolates — but the pack value wins, + * so the default never renders at all. Moot. + * 2. `createSafeTranslation`'s `fallbackT`. It interpolates, with the exact + * literal needle this class is named for. SAFE to rewrite. + * 3. react-i18next's not-ready `t`, which is what a bare + * `useObjectTranslation()` yields when no i18next instance is + * initialised. Measured in `react-i18next/dist/es/useTranslation.js`: + * `notReadyT` returns `options.defaultValue` VERBATIM — it does not + * interpolate at all. + * + * So at a `useObjectTranslation` call site, a default written as + * `` `Signed in as ${user.email}` `` is not an unpinned near-miss to be + * tidied into `'Signed in as {{email}}'` — the template literal is the only + * one of the two that renders correctly there, and "fixing" it would put + * literal braces in front of a user on exactly the provider-less host this + * whole family of cards is about. `objectBulkActionDispatch.test.tsx` fails + * on precisely that substitution, which is how it was found. + * + * Hence only 5 rewrites: four whose default carries no hole (nothing to + * interpolate, so all three renderers agree) and one behind a + * `createSafeTranslation` hook. The rest stay computed, and this class is + * what covers them. + * + * Judged over EVERY inline default, not just the residue. A pinned default is + * byte-equal to an `en` value #3512 already holds to this rule, so those + * verdicts are green twice over — which is the point: it makes the judged + * count a live control (hundreds, guarded), instead of a rule whose whole + * subject is three strings that could silently become zero. HARD from day + * one, like classes 3-5: the first full run found 0 violations, so there is + * no debt for a ratchet to hold, and 0 stops being luck. + * * ## Dynamic keys: the explicit policy * * A key that is not a string literal cannot be resolved statically. Those call @@ -457,6 +511,95 @@ export function holesOf(value) { return names; } +/** + * A `{{…}}` pair and its contents. `[^{}]*` deliberately: a placeholder never + * nests braces, and refusing to cross one keeps an unterminated `{{` from + * swallowing the rest of the sentence into a bogus "placeholder". + */ +const DOUBLE_BRACE = /\{\{([^{}]*)\}\}/g; + +/** Every `{{` occurrence, matched or not — the balance check's other half. */ +const DOUBLE_BRACE_OPEN = /\{\{/g; + +/** + * The one placeholder spelling `createSafeTranslation`'s `fallbackT` resolves. + * The option name comes from `Object.entries(options)` and is spliced into the + * needle raw (``value.split(`{{${k}}}`)``), so the accepted name is exactly a + * bare identifier: no whitespace, no format spec, no `-` prefix, no keypath. + */ +const CANONICAL_HOLE_NAME = /^[A-Za-z0-9_]+$/; + +/** i18next's nesting syntax. The fallback has no notion of it at all. */ +const NESTING_MARKER = '$t('; + +/** Why one placeholder is not something `fallbackT` can resolve. */ +function unresolvableReason(inner) { + if (inner !== inner.trim()) return 'whitespace inside the braces'; + if (inner.startsWith('-')) return 'the {{- x}} unescape prefix'; + if (inner.includes(',')) return 'an i18next format spec'; + if (inner.includes('.')) return 'a dotted/keyed placeholder path'; + return 'a non-identifier placeholder name'; +} + +/** + * Placeholder spellings the provider-less fallback cannot resolve + * (objectui#4905, class 7 — the rule is objectui#3512's, applied to the one + * copy surface that card left out). + * + * `fallbackT` interpolates with an EXACT literal needle, so it recognises + * `{{name}}` and nothing else. i18next — which serves the SAME string whenever + * an `I18nProvider` is mounted — additionally recognises `{{ name }}`, + * `{{count, number}}`, `{{- name}}` and `$t(otherKey)`. Those four render + * correctly through the provider and leak literal braces without one, which is + * a divergence only a provider-less host ever sees. + * + * Returns one human-readable violation per offending placeholder, and `[]` for + * text both paths render identically. + * + * ## What is structurally out of range, and why that matters + * + * - **Single braces.** `{shown}` / `{seconds}` is objectui#4135's spelling for + * a hole filled DOWNSTREAM of `t()`. Only the inside of a `{{…}}` pair is + * ever inspected, so a single-brace hole cannot reach a verdict here — + * excluded by where the rule looks, not by an allow-list that could rot. + * - **JSX object literals.** `style={{ opacity: 0 }}` is `{{` that is syntax, + * not copy. This rule never greps source text: it is handed the TEXT of a + * string literal or the literal segments of a template, and a JSX brace is + * not inside either. + * + * ## The sibling copy, named rather than hidden + * + * `packages/i18n/src/__tests__/fallback-placeholder-spelling-3512.test.ts` + * carries the same rule as `placeholderViolations`, over the ten locale packs + * and the 31+3 defaults TABLES. This copy exists because that gate is a vitest + * suite reading copy tables and this one is a node script reading call-site + * ARGUMENTS — the walk that finds a `defaultValue` is the classifier this file + * already owns, and rebuilding it there was rejected on objectui#4905. The + * self-test pins this copy against all four i18next-only spellings and both + * out-of-range classes, so the two can only drift by someone editing one and + * not the other with both self-tests in front of them. + */ +export function unresolvableSpellings(value) { + const out = []; + const regions = [...value.matchAll(DOUBLE_BRACE)]; + for (const region of regions) { + const inner = region[1]; + if (CANONICAL_HOLE_NAME.test(inner)) continue; + out.push(`${JSON.stringify(region[0])} — ${unresolvableReason(inner)}; the fallback resolves only {{name}}`); + } + // An unterminated `{{` renders as literal braces on BOTH paths, so it is not + // an i18next divergence — but it is never intentional copy, and the regions + // above cannot report what they did not match. A hole straddling a template + // substitution (`` `{{ ${name} }}` ``) lands here, which is the one shape + // neither interpolator can resolve. + const opens = (value.match(DOUBLE_BRACE_OPEN) ?? []).length; + if (opens > regions.length) out.push('an unterminated `{{` with no closing `}}`'); + if (value.includes(NESTING_MARKER)) { + out.push('`$t(` — i18next nesting, which the fallback emits verbatim'); + } + return out; +} + /** * `t` bindings that do NOT resolve against the locale packs. Every entry is a * decision with a reason; an imported `t` from anywhere else is a hard error @@ -870,6 +1013,40 @@ function staticString(node, source) { return null; } +/** + * Every piece of an expression that is STATIC TEXT, or `null` when none of it + * is (objectui#4905). + * + * `staticString` above answers "is this whole expression one readable string", + * which is what the drift rule needs — it compares a sentence. The spelling + * rule asks something weaker and therefore reaches further: a template literal + * is not a readable sentence, but its literal SEGMENTS are text a placeholder + * can be misspelled in, and that text renders verbatim on a provider-less host. + * `` `Uploading… ({{ pct }}%)` `` is `null` to `staticString` and two segments + * here, and the second is where the defect lives. + * + * The segments are judged one by one rather than joined, because joining them + * would invent adjacencies the runtime never produces: the substitution between + * two segments becomes arbitrary text at runtime, so a `{{` in one and a `}}` + * in the next is not a placeholder either interpolator can resolve, and reading + * it as one would be the false green. + */ +function staticTextSegments(node, source) { + const inner = unwrapExpression(node); + if (!inner) return null; + if (ts.isStringLiteral(inner) || ts.isNoSubstitutionTemplateLiteral(inner)) return [inner.text]; + if (ts.isTemplateExpression(inner)) { + return [inner.head.text, ...inner.templateSpans.map((span) => span.literal.text)]; + } + if (ts.isBinaryExpression(inner) && inner.operatorToken.kind === ts.SyntaxKind.PlusToken) { + const left = staticTextSegments(inner.left, source); + const right = staticTextSegments(inner.right, source); + if (left === null && right === null) return null; + return [...(left ?? []), ...(right ?? [])]; + } + return null; +} + /** * Dotted leaf paths of `packages/i18n/src/locales/en.ts`, read from its AST, * plus the leaf VALUES — the strings the app actually renders. @@ -1182,6 +1359,11 @@ function literalKeysOf(argument, source) { * `{ present: true, text: null }` — written, but computed (a template with a * substitution, a variable, a ternary). Not * comparable, so it is counted, never failed. + * + * `segments` is the same expression read for STATIC TEXT rather than for a + * whole sentence (objectui#4905): `null` when nothing in it is readable text, + * otherwise every literal piece. A computed default is `text: null` and can + * still carry segments — that is the surface class 7 judges and class 3 cannot. */ function inlineDefaultValue(node, source) { for (const argument of node.arguments.slice(1)) { @@ -1192,10 +1374,14 @@ function inlineDefaultValue(node, source) { const name = ts.isIdentifier(property.name) || ts.isStringLiteral(property.name) ? property.name.text : null; if (name !== 'defaultValue') continue; - return { present: true, text: staticString(property.initializer, source) }; + return { + present: true, + text: staticString(property.initializer, source), + segments: staticTextSegments(property.initializer, source), + }; } } - return { present: false, text: null }; + return { present: false, text: null, segments: null }; } /** @@ -1414,6 +1600,9 @@ export function analyze(root, /** @type {{ families?: DynamicKeyFamily[] }} */ { matchingDefaultValues: 0, computedDefaultValues: 0, unjudgedDefaultValues: 0, + spellingJudgedDefaults: 0, + spellingJudgedResidueDefaults: 0, + opaqueDefaultText: 0, judgedInterpolation: 0, unjudgedInterpolation: 0, opaqueOptions: 0, @@ -1578,6 +1767,42 @@ export function analyze(root, /** @type {{ families?: DynamicKeyFamily[] }} */ { } } + // objectui#4905 — class 7. Whatever the drift rule decided above, the + // TEXT this default carries is text `fallbackT` may be asked to + // render, and `fallbackT` resolves exactly one placeholder spelling. + // Judged on every inline default, not only the ones drift leaves + // unpinned: a pinned default is byte-equal to its `en` value and + // objectui#3512 holds `en` to the same rule, so those come back green + // twice over — which is what makes the count a live control on this + // rule rather than a set of three strings nobody would notice + // emptying. + if (inlineDefault.present) { + // The folded sentence when there is one, else the literal pieces of + // a template — never both, so a `+`-concatenated default is judged + // whole rather than once per operand. + const subjects = inlineDefault.text !== null ? [inlineDefault.text] : (inlineDefault.segments ?? []); + const pinnedByDrift = inlineDefault.text !== null && enValue !== undefined; + if (subjects.length === 0) { + // A bare runtime value (`defaultValue: label`). There is no text + // to spell, and saying so is not the same as saying it is fine. + counters.opaqueDefaultText += 1; + } else { + counters.spellingJudgedDefaults += 1; + if (!pinnedByDrift) counters.spellingJudgedResidueDefaults += 1; + for (const subject of subjects) { + for (const violation of unresolvableSpellings(subject)) { + findings.push({ + reason: 'unresolvable-default-spelling', + ...at, + detail: key ?? '(dynamic key)', + expected: subject, + actual: violation, + }); + } + } + } + } + // objectui#3845 — what this call site passes must be what the `en` // value has holes for, in both directions. Judged on the same // preconditions as class 3, plus a readable option-name set. @@ -1891,6 +2116,19 @@ const HINTS = { ' EXTERNALLY_INTERPOLATED_HOLES with the file that does the substitution — and note that' + ' those keys must still NOT be passed the argument, or i18next consumes the hole before the' + ' consumer gets to see it.', + 'unresolvable-default-spelling': + 'This inline `defaultValue` spells a placeholder in a dialect only i18next understands' + + ' (objectui#4905). `createSafeTranslation`\'s `fallbackT` interpolates with an EXACT literal' + + ' needle — ``value.split(`{{${k}}}`)`` — so `{{name}}` is the only spelling it resolves,' + + ' while i18next also accepts `{{ name }}`, `{{count, number}}`, `{{- name}}` and `$t(key)`.' + + ' The consequence is invisible where we usually look: WITH an `I18nProvider` the pack value' + + ' wins and this string never renders at all; without one it renders and the braces reach the' + + ' user verbatim. Fix it at the CALL SITE by respelling the hole as `{{name}}` — the' + + ' maintainer\'s objectui#4135 ruling is that `{{x}}` is exclusively i18next-bound copy, so' + + ' teaching the fallback more dialects is not the fix. A hole this component fills ITSELF,' + + ' downstream of `t()`, is spelled with SINGLE braces (`{x}`) and is out of this rule\'s range' + + ' by construction. Same rule, same reasons, over the copy TABLES:' + + ' `packages/i18n/src/__tests__/fallback-placeholder-spelling-3512.test.ts`.', 'dead-sibling-fallback': 'The key EXISTS in `en`, and this fallback is written as the call\'s SIBLING' + ' (`t(key) || \'English\'`) rather than as an argument — so it is dead on every path, not just' + @@ -1927,6 +2165,21 @@ if (invokedDirectly) { process.exit(1); } + // The same guard for class 7's own subject (objectui#4905). The rule above is + // silent on a tree with no inline defaults in it, and silent is exactly how a + // broken `inlineDefaultValue` or `staticTextSegments` would read — so the + // spelling verdict asserts it had something to judge, rather than inheriting + // the key-count guard's word for it. + if (counters.spellingJudgedDefaults < 500) { + console.error( + `The inline-default spelling scan collapsed: ${counters.spellingJudgedDefaults} default(s) with readable` + + ` text, ${counters.opaqueDefaultText} without. Expected hundreds — this repo carries` + + ' roughly a thousand inline defaults, so a number this small means the reader stopped' + + ' reading them and the spelling rule is passing on an empty set.', + ); + process.exit(1); + } + const { unexpected, stale } = applyBaseline(findings, readBaseline(root)); console.log( @@ -1941,6 +2194,11 @@ if (invokedDirectly) { `(${counters.matchingDefaultValues} match their en value, ${counters.unjudgedDefaultValues} not comparable), ` + `${counters.computedDefaultValues} computed (report-only).`, ); + console.log( + `Inline default spelling: ${counters.spellingJudgedDefaults} default(s) carry readable text and are held to ` + + `the one placeholder spelling the provider-less fallback resolves — ${counters.spellingJudgedResidueDefaults} ` + + `of them on call sites the drift rule cannot pin, ${counters.opaqueDefaultText} with no readable text at all.`, + ); console.log( `Interpolation parity: ${counters.judgedInterpolation} call sites compared against their en value's holes, ` + `${counters.unjudgedInterpolation} with no single comparable en value, ${counters.opaqueOptions} with an ` + @@ -1967,6 +2225,7 @@ if (invokedDirectly) { const drift = unexpected.filter((finding) => finding.reason === 'default-value-drift'); const parity = unexpected.filter((finding) => finding.reason === 'interpolation-parity'); const siblings = unexpected.filter((finding) => finding.reason === 'dead-sibling-fallback'); + const spelling = unexpected.filter((finding) => finding.reason === 'unresolvable-default-spelling'); // objectui#4964's classes read on their own too: they are all about a template // family whose HEAD resolves, which is precisely the case the two key classes // above declare out of scope. @@ -1979,7 +2238,12 @@ if (invokedDirectly) { 'empty-vocabulary', ]); const families = unexpected.filter((finding) => FAMILY_CLASSES.has(finding.reason)); - const VALUE_CLASSES = new Set(['default-value-drift', 'interpolation-parity', 'dead-sibling-fallback']); + const VALUE_CLASSES = new Set([ + 'default-value-drift', + 'interpolation-parity', + 'dead-sibling-fallback', + 'unresolvable-default-spelling', + ]); const keyFindings = unexpected.filter( (finding) => !VALUE_CLASSES.has(finding.reason) && !FAMILY_CLASSES.has(finding.reason), ); @@ -1988,8 +2252,9 @@ if (invokedDirectly) { console.log( `Every in-scope call-site key resolves against the en pack (${enKeyCount} keys), every` + ' literal inline defaultValue matches the value the pack serves, every call site passes' + - ' exactly the arguments that value has holes for, no call site carries a literal' + - ' fallback beside itself, and every dynamic key family either checks its members' + + ' exactly the arguments that value has holes for, every inline defaultValue spells its' + + ' placeholders the one way the provider-less fallback resolves, no call site carries a' + + ' literal fallback beside itself, and every dynamic key family either checks its members' + ' against a declared vocabulary or says in writing why it has none.', ); process.exit(0); @@ -2051,6 +2316,21 @@ if (invokedDirectly) { } } + if (spelling.length > 0) { + const distinct = new Set(spelling.map((finding) => `${finding.file}:${finding.line}`)); + console.error( + `\n${spelling.length} placeholder${spelling.length === 1 ? '' : 's'} in an inline defaultValue ` + + `cannot be resolved by the provider-less fallback (${distinct.size} call ` + + `site${distinct.size === 1 ? '' : 's'}) — with a provider i18next renders ` + + 'them correctly, so the braces reach the user only where nobody is looking:', + ); + for (const finding of spelling) { + console.error(` ${finding.file}:${finding.line}:${finding.column} [${finding.reason}] ${finding.detail}`); + console.error(` default text: ${quote(finding.expected)}`); + console.error(` ${finding.actual}`); + } + } + if (families.length > 0) { console.error( `\n${families.length} dynamic-family finding${families.length === 1 ? '' : 's'} — the head resolves, so the` +