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'], + ); +});