From e78295cb819036067e1489b0b534916eea657399 Mon Sep 17 00:00:00 2001 From: NekoPunch Date: Fri, 4 Sep 2026 22:20:39 -0700 Subject: [PATCH] chore(ci): ratchet locale hygiene A new UI locale must only mean a new UiCatalog key. Branching on a locale literal, defaulting the locale parameter, or sniffing CJK in a payload all compile cleanly and render the wrong language instead. Count those per file against the base commit and fail on growth, so existing debt shrinks without any ledger to maintain. The rules accept both quote styles, since biome leaves apps/desktop and packages/ui unformatted, and count per match rather than per line. The step needs only node built-ins, so it runs with the install-free gates. Generated-by: Claude Code Claude-Session: https://claude.ai/code/session_01AqdSkg56F2x55wEGRWvzcB --- .github/workflows/ci.yml | 15 +++ package.json | 1 + scripts/check-locale-hygiene.mjs | 152 ++++++++++++++++++++++++++ scripts/check-locale-hygiene.test.mjs | 80 ++++++++++++++ 4 files changed, 248 insertions(+) create mode 100644 scripts/check-locale-hygiene.mjs create mode 100644 scripts/check-locale-hygiene.test.mjs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 31c1978380..a34d0ea26d 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -134,6 +134,21 @@ jobs: - name: Check ASF source headers run: npm run check:asf-headers + # Locale hygiene ratchet (node built-ins only, so it runs install-free): a new UI locale must only ever mean a new + # UiCatalog key, so locale-literal branches, silent locale defaults, + # and payload sniffing may shrink but never grow. + - name: Check locale hygiene + if: steps.plan.outputs.code == 'true' + env: + BASE_SHA: ${{ github.event_name == 'push' && github.event.before || github.event.pull_request.base.sha }} + run: | + node --test scripts/check-locale-hygiene.test.mjs + if [[ -n "$BASE_SHA" && ! "$BASE_SHA" =~ ^0+$ ]]; then + npm run check:locale-hygiene -- --base "$BASE_SHA" + else + npm run check:locale-hygiene + fi + # Everything below this line may need an installed toolchain, so each # step names the selections it belongs to. `setup-node` itself is # unconditional: it costs seconds on a runner the job is already holding, diff --git a/package.json b/package.json index d16d6d4b72..43c10a5ee5 100644 --- a/package.json +++ b/package.json @@ -60,6 +60,7 @@ "release:cli:eval": "node scripts/release-cli-eval-package.mjs", "check:app-shell-hooks": "node scripts/check-app-shell-hooks.mjs", "check:tui-copy": "node scripts/check-tui-copy.mjs", + "check:locale-hygiene": "node scripts/check-locale-hygiene.mjs", "check:asf-headers": "node scripts/asf-license-headers.mjs check", "write:asf-headers": "node scripts/asf-license-headers.mjs write", "release:asf:source": "node scripts/asf-source-release.mjs create", diff --git a/scripts/check-locale-hygiene.mjs b/scripts/check-locale-hygiene.mjs new file mode 100644 index 0000000000..461d719860 --- /dev/null +++ b/scripts/check-locale-hygiene.mjs @@ -0,0 +1,152 @@ +#!/usr/bin/env node +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +// Ratchet for locale hygiene. Every rule below is a place where adding a +// locale to UI_LOCALES compiles cleanly but silently renders the wrong +// language, because the code branches on a locale literal instead of +// indexing a `UiCatalog`. Only files the diff touches are scanned, so each +// (file, rule) count may shrink but never grow; there is no ledger. + +import { execFileSync } from 'node:child_process'; +import { readFileSync } from 'node:fs'; +import { join } from 'node:path'; +import { fileURLToPath, pathToFileURL } from 'node:url'; + +const repoRoot = fileURLToPath(new URL('..', import.meta.url)); + +export const SCOPE = ['apps/desktop/src', 'packages/core/src', 'packages/ui/src']; + +// Both quote styles: biome leaves apps/desktop and packages/ui unformatted, so +// `"en"` is as permanent there as `'en'`. Every pattern is global so one line +// with two hits counts two. +export const RULES = { + // `locale === 'zh-CN' ? a : b` — a fourth locale falls into `b` unnoticed. + // Narrower than check-tui-copy's AST `locale-branch` (which also sees + // `switch (locale)` and if-statements) but runs install-free over the + // desktop, core, and ui trees; the two rules are deliberately distinct. + 'locale-literal-compare': + /\blocale(?:\.[A-Za-z]+)?\s*(?:===|!==)\s*['"](?:zh(?:-CN|-TW)?|en)['"]|\blocale\.startsWith\(['"]zh['"]\)/gu, + // `locale: UiLocale = 'zh-CN'` — a caller that forgets the argument gets one language. + 'silent-locale-default': /(? count > (base[rule] ?? 0)) + .map(([rule, count]) => ({ + path, + rule, + base: base[rule] ?? 0, + current: count, + lines: hits.filter((hit) => hit.rule === rule).map((hit) => `${hit.line}: ${hit.text}`), + })); +} + +function git(args) { + return execFileSync('git', args, { cwd: repoRoot, encoding: 'utf8', maxBuffer: 1 << 28 }); +} + +// Working tree against base, so uncommitted edits are checked too. The 30% +// similarity floor still pairs a file that was rewritten while it moved. The +// diff is not pathspec-limited: a file moved into scope from outside would +// otherwise show as added and lose its base. +function changedFiles(base) { + return git(['diff', '-M30%', '--name-status', '--diff-filter=AMR', base]) + .split('\n') + .filter(Boolean) + .map((line) => { + const [status, from, to] = line.split('\t'); + return { path: to ?? from, basePath: status === 'A' ? undefined : from }; + }) + .filter(({ path }) => inScope(path) && /\.tsx?$/u.test(path) && !EXCLUDED.test(path)); +} + +function inScope(path) { + return SCOPE.some((dir) => path.startsWith(`${dir}/`)); +} + +function resolveBase(argv) { + const index = argv.indexOf('--base'); + if (index >= 0 && argv[index + 1]) return argv[index + 1]; + try { + return git(['merge-base', 'HEAD', 'origin/main']).trim(); + } catch { + return undefined; + } +} + +function main(argv) { + const base = resolveBase(argv); + if (!base) { + console.error('Locale hygiene check failed: pass --base or fetch origin/main.'); + return 1; + } + const violations = changedFiles(base).flatMap(({ path, basePath }) => + compare( + path, + basePath ? git(['show', `${base}:${basePath}`]) : '', + readFileSync(join(repoRoot, path), 'utf8'), + ), + ); + if (violations.length === 0) { + console.log('Locale hygiene check passed.'); + return 0; + } + console.error( + 'Locale hygiene check failed: new locale branches were added. Index a UiCatalog instead.', + ); + for (const violation of violations) { + console.error( + `- ${violation.path}: ${violation.rule} ${violation.base} -> ${violation.current}`, + ); + for (const line of violation.lines) console.error(` ${line}`); + } + return 1; +} + +if (process.argv[1] && import.meta.url === pathToFileURL(process.argv[1]).href) { + process.exitCode = main(process.argv.slice(2)); +} diff --git a/scripts/check-locale-hygiene.test.mjs b/scripts/check-locale-hygiene.test.mjs new file mode 100644 index 0000000000..5c8338fd62 --- /dev/null +++ b/scripts/check-locale-hygiene.test.mjs @@ -0,0 +1,80 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +import assert from 'node:assert/strict'; +import test from 'node:test'; +import { compare, scanSource } from './check-locale-hygiene.mjs'; + +function rules(source) { + return scanSource(source).map(({ rule }) => rule); +} + +test('flags locale literal comparisons in either quote style', () => { + assert.deepEqual(rules("const label = locale === 'zh-CN' ? '启用' : 'Enable';"), [ + 'locale-literal-compare', + ]); + assert.deepEqual(rules("if (input.locale !== 'en') return zh;"), ['locale-literal-compare']); + assert.deepEqual(rules("const zh = locale.startsWith('zh');"), ['locale-literal-compare']); + assert.deepEqual(rules('if (locale === "en") return en;'), ['locale-literal-compare']); +}); + +test('counts every match on a line, not the line', () => { + assert.deepEqual( + rules( + "const closeLabel = input.locale === 'zh-CN' ? '关闭' : input.locale === 'zh-TW' ? '關閉' : 'Close';", + ), + ['locale-literal-compare', 'locale-literal-compare'], + ); +}); + +test('flags silent locale defaults and payload sniffing', () => { + assert.deepEqual( + rules("export function f(error: unknown, locale: UiLocale = 'zh-CN'): string {"), + ['silent-locale-default'], + ); + assert.deepEqual(rules(' locale: UiLocale = "en",'), ['silent-locale-default']); + assert.deepEqual(rules('if (/[\\u4e00-\\u9fff]/.test(raw)) return raw;'), ['cjk-sniff']); + assert.deepEqual(rules('if (/[\\u4E00-\\u9FFF]/.test(raw)) return raw;'), ['cjk-sniff']); + assert.deepEqual(rules(" '凭据已保存': '憑證已儲存',"), ['string-keyed-translation']); + assert.deepEqual(rules(' "凭据已保存": "憑證已儲存",'), ['string-keyed-translation']); +}); + +test('ignores catalog indexing', () => { + assert.deepEqual( + rules( + "const copy = COPY[locale]; const zhCn = { 'zh-CN': '启用', 'zh-TW': '啟用', en: 'Enable' };", + ), + [], + ); + assert.deepEqual(rules('export function f(error: unknown, locale: UiLocale): string {'), []); + assert.deepEqual(rules(" let locale: UiLocale = 'en';"), []); +}); + +test('compare fails only on growth per rule', () => { + const one = "locale === 'zh-CN'"; + const two = `${one}\n${one}`; + assert.deepEqual(compare('a.ts', one, one), []); + assert.deepEqual(compare('a.ts', two, one), []); + assert.deepEqual( + compare('a.ts', one, `${two}\n/[\\u4e00-\\u9fff]/`).map( + ({ rule, base, current }) => `${rule} ${base}->${current}`, + ), + ['locale-literal-compare 1->2', 'cjk-sniff 0->1'], + ); +});