diff --git a/.github/workflows/ci-guards.yml b/.github/workflows/ci-guards.yml index f1198e845865..667c664d61ac 100644 --- a/.github/workflows/ci-guards.yml +++ b/.github/workflows/ci-guards.yml @@ -366,6 +366,7 @@ jobs: run: | python3 tests/test_localization_catalog.py python3 tests/test_localize_changes.py + python3 tests/test_localization_defaults.py - name: Test the PR media uploader if: ${{ matrix.group == 'preflight' }} diff --git a/docs/verification-receipts.md b/docs/verification-receipts.md index de52e8406fe0..d3bd265a424f 100644 --- a/docs/verification-receipts.md +++ b/docs/verification-receipts.md @@ -49,9 +49,9 @@ base ref to include committed branch changes since its merge-base with HEAD. `--affected` with `--swift-changed` to also parse changed Swift files, or use `--only` instead when you want to choose checks yourself. -Selection covers the twelve checks below. Each checker declares its file inputs +Selection covers the sixteen checks below. Each checker declares its file inputs in `CHECK_INPUTS` in `scripts/verify-local.py`; update those declarations when a -checker gains dependencies. Unknown paths select all twelve checks. Known prose +checker gains dependencies. Unknown paths select all sixteen checks. Known prose changes omit unrelated checks, while feature-flag expiry policy always runs because its result depends on today's date. This does not select native tests or the separate CI workflow guards. @@ -110,7 +110,7 @@ on PATH and runs `-frontend -parse -swift-version 5 -D DEBUG -enable-bare-slash- It does not resolve imports, expand macros, typecheck, compile, execute tests, or validate every conditional-compilation configuration. Use the intended toolchain; parsing with a newer compiler does not prove compatibility with an older one. -The default Linux CI recipe remains the ten portable checks below. +The default Linux CI recipe remains the sixteen portable checks below. Receipts record the parser version, exact argv, selected-file hashes before and after, selection origin/resolved base, and a separate `parsing` result. Missing @@ -133,10 +133,14 @@ it does not assert that the entire CI checkout equals the PR head. | --- | --- | | `xcstrings` | Localization catalog structure | | `localization` | macOS localization parity | +| `localization-defaults` | Swift `defaultValue` literals consume the same format arguments as their catalog `en` value | | `project-tests` | Five project normalizer unit tests at the demonstrated revision; counts are read from each execution | | `project` | Xcode project version pin and normalization | | `config-schema` | Embedded cmux.json schema matches its source | | `test-wiring-sync` | Test-wiring synchronization tool regression suite | +| `wire-app-sources` | App-source wiring tool regression suite | +| `ui-lab` | ui-lab harness directive regression suite | +| `ui-fuzzer` | UI fuzzer engine regression suite | | `launch-policy` | Generated Claude launch policy is current | | `test-wiring` | Every Swift test file belongs to the Xcode test target | | `package-groups` | Workspace Swift package grouping | @@ -145,7 +149,7 @@ it does not assert that the entire CI checkout equals the PR head. | `feature-flags` | Flag names, ownership, expiry, defaults, single evaluation and retired keys | Each failure prints a bounded diagnostic tail and an exact `--only` rerun command. -The default runs all twelve checks so one pass reveals independent failures. +The default runs all sixteen checks so one pass reveals independent failures. `--only` runs the named subset and says which checks actually ran; it does not infer affected tests from a diff. `--repo` targets another checkout. Each check has a 60-second deadline, adjustable with `--timeout`; Ctrl-C stops the active diff --git a/scripts/localization-default-mismatches.json b/scripts/localization-default-mismatches.json new file mode 100644 index 000000000000..fd81580be6cb --- /dev/null +++ b/scripts/localization-default-mismatches.json @@ -0,0 +1,5 @@ +{ + "cloudTree.error.renameTerminalUnavailable": "Swift passes the terminal id to String(format:) but the catalog copy has no %@ and is terminal-scoped on a path that renames any tab kind; needs a copy decision (#15864).", + "remoteDaemon.upload.installFailedWithDetail": "Catalog copy ends in ': %@' but the Swift call site neither includes the specifier in its defaultValue nor formats the detail; fix the call site, then remove this entry (#15864).", + "remoteDaemon.bootstrap.removeCorruptFailedWithDetail": "Catalog copy ends in ': %@' but the Swift call site neither includes the specifier in its defaultValue nor formats the detail; fix the call site, then remove this entry (#15864)." +} diff --git a/scripts/localization_defaults.py b/scripts/localization_defaults.py new file mode 100644 index 000000000000..2a36c031d119 --- /dev/null +++ b/scripts/localization_defaults.py @@ -0,0 +1,157 @@ +#!/usr/bin/env python3 +"""Compare Swift `defaultValue` literals with the catalog English they localize (Python 3.9+). + +`localization_catalog.py check` takes its baseline from the catalog itself: every +locale is compared with the entry's own `en` value. A catalog `en` value that lost +or gained a format specifier relative to the Swift `defaultValue` therefore passes, +because every locale agrees with `en` and nothing reads the Swift source. At +runtime `String(format:)` then silently drops the argument, or reads past the +argument list. This check reads the Swift source and compares the two signatures. + +Known mismatches that are waiting on a copy decision live in +scripts/localization-default-mismatches.json, keyed by catalog key with a reason. +An entry that no longer mismatches is an error, so the list only shrinks. +""" + +from __future__ import annotations + +import argparse +import importlib.util +import json +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parent.parent +ALLOWLIST = "scripts/localization-default-mismatches.json" +# Test targets and fixtures use catalog keys with their own default text on +# purpose; vendored and generated trees are not cmux copy. +SKIP_DIRS = frozenset(( + ".build", ".git", "DerivedData", "Examples", "Prototypes", "Tests", "artifacts", + "cmuxCLITestSupport", "cmuxCLITests", "cmuxTests", "cmuxUITests", "experiments", + "node_modules", "scripts", "tests", "tests_v2", "vendor", +)) + + +def load_changes_module(): + spec = importlib.util.spec_from_file_location("cmux_localize_changes", ROOT / "scripts/localize_changes.py") + if spec is None or spec.loader is None: + raise RuntimeError("unable to load scripts/localize_changes.py") + module = importlib.util.module_from_spec(spec) + sys.modules[spec.name] = module + spec.loader.exec_module(module) + return module + + +CHANGES = load_changes_module() +CATALOG = CHANGES.CATALOG + + +def swift_files(root: Path) -> list[Path]: + return sorted(path for path in root.rglob("*.swift") + if SKIP_DIRS.isdisjoint(path.relative_to(root).parts[:-1])) + + +def swift_defaults(root: Path) -> dict[str, tuple[str, str]]: + """Map each key to (defaultValue, Swift path). + + Call sites that disagree about a key's default are dropped, as are calls the + shared extractor cannot read (interpolated or unsupported literals); both are + already reported by localize_changes.py, and neither yields one signature. + """ + defaults: dict[str, tuple[str, str]] = {} + conflicts: set[str] = set() + for path in swift_files(root): + relative = path.relative_to(root).as_posix() + messages, _ = CHANGES.parse_swift_messages(relative, path.read_text(encoding="utf-8"), conflicts=conflicts) + for key, message in messages.items(): + previous = defaults.get(key) + if previous is not None and previous[0] != message.source: + conflicts.add(key) + else: + defaults[key] = (message.source, relative) + for key in conflicts: + defaults.pop(key, None) + return defaults + + +def catalog_index(root: Path) -> dict[str, list[tuple[str, object]]]: + index: dict[str, list[tuple[str, object]]] = {} + for path in CATALOG.discover(root): + for entry in CATALOG.catalog_entries(path.read_text(encoding="utf-8")): + index.setdefault(entry.key, []).append((path.relative_to(root).as_posix(), entry.value)) + return index + + +def load_allowlist(path: Path) -> dict[str, str]: + if not path.is_file(): + return {} + data = json.loads(path.read_text(encoding="utf-8")) + if not isinstance(data, dict) or not all(isinstance(reason, str) and reason.strip() for reason in data.values()): + raise ValueError(f"{path}: expected an object mapping catalog keys to a non-empty reason") + return data + + +def arguments(text: str) -> list[tuple[int, str]]: + """The (argument, specifier) pairs a format string consumes, in argument order.""" + return sorted(CATALOG.signature(text)) + + +def check(root: Path, allowlist: dict[str, str]) -> tuple[list[str], int]: + """Return (errors, comparison count); a key carried by several catalogs is compared with each.""" + errors: list[str] = [] + index = catalog_index(root) + compared = 0 + still_mismatched: set[str] = set() + for key, (default, swift_path) in sorted(swift_defaults(root).items()): + catalogs = [] + for catalog_path, entry in index.get(key, []): + try: + catalogs.append((catalog_path, arguments(CATALOG.source(entry)))) + except ValueError: + continue # localization_catalog.py check already names a malformed entry + try: + expected = arguments(default) + except ValueError: + continue # e.g. a %#@name@ substitution in Swift text; localize_changes.py owns that review + for catalog_path, actual in catalogs: + compared += 1 + if expected == actual: + continue + if key in allowlist: + still_mismatched.add(key) + continue + # The same key can live in more than one product's catalog with + # different copy; name the siblings so the fix lands in the right one. + siblings = "".join(f"; {other} carries {sibling!r}" for other, sibling in catalogs if other != catalog_path) + errors.append( + f"{catalog_path}:{key}: catalog en placeholders {actual!r} != " + f"Swift defaultValue {expected!r} ({swift_path}){siblings}" + ) + for key in sorted(set(allowlist) - still_mismatched): + errors.append(f"{ALLOWLIST}: {key} no longer mismatches its Swift defaultValue; remove the entry") + return errors, compared + + +def main(argv: list[str] | None = None) -> int: + parser = argparse.ArgumentParser(description=__doc__, formatter_class=argparse.RawDescriptionHelpFormatter) + parser.add_argument("--root", type=Path, default=ROOT) + parser.add_argument("--allowlist", type=Path, help=f"default: /{ALLOWLIST}") + parser.add_argument("--limit", type=int, default=40, help="maximum printed diagnostics; 0 prints all") + args = parser.parse_args(argv) + if args.limit < 0: + parser.error("limit must be nonnegative") + root = args.root.resolve() + errors, compared = check(root, load_allowlist(args.allowlist or root / ALLOWLIST)) + for error in errors[:args.limit or None]: + print(error, file=sys.stderr) + noun = "mismatch" if len(errors) == 1 else "mismatches" + print(f"{compared} Swift defaultValue/catalog en comparisons: {len(errors)} {noun}") + return int(bool(errors)) + + +if __name__ == "__main__": + try: + raise SystemExit(main()) + except (OSError, ValueError, KeyError, TypeError) as error: + print(f"error: {error}", file=sys.stderr) + raise SystemExit(2) diff --git a/scripts/localize_changes.py b/scripts/localize_changes.py index 7d7782f1e947..68c550d4c188 100644 --- a/scripts/localize_changes.py +++ b/scripts/localize_changes.py @@ -201,10 +201,18 @@ def swift_call_suffix(text: str, start: int) -> str: return "" -def parse_swift_messages(path: str, text: str) -> tuple[dict[str, SwiftMessage], list[str]]: +def parse_swift_messages(path: str, text: str, *, + conflicts: set[str] | None = None) -> tuple[dict[str, SwiftMessage], list[str]]: + """Map each key to its one message; keys whose call sites disagree are dropped. + + Pass ``conflicts`` to learn which keys were dropped: they are only named in + ``attention`` otherwise, and a caller merging several files must not let one + file's single default stand in for another file's disagreement. + """ messages: dict[str, SwiftMessage] = {} attention: list[str] = [] - conflicts: set[str] = set() + if conflicts is None: + conflicts = set() handled = 0 for match in SWIFT_CALL.finditer(text): suffix = swift_call_suffix(text, match.end()) diff --git a/scripts/verify-local.py b/scripts/verify-local.py index f4adb25888e8..8bbd1abbc4fe 100644 --- a/scripts/verify-local.py +++ b/scripts/verify-local.py @@ -25,6 +25,7 @@ CHECKS = ( ("xcstrings", "static_analysis", "XCStrings structure", ["python3", "scripts/lint-xcstrings.py"]), ("localization", "static_analysis", "Localization parity", ["python3", "scripts/localization_catalog.py", "check"]), + ("localization-defaults", "static_analysis", "Swift defaultValue parity", ["python3", "scripts/localization_defaults.py"]), ("project-tests", "tests", "Project normalizer tests", ["python3", "tests/test_normalize_pbxproj.py"]), ("project", "static_analysis", "Xcode project normalization and version", ["bash", "scripts/check-pbxproj.sh"]), ("config-schema", "static_analysis", "Embedded cmux.json schema", ["python3", "scripts/generate-cmux-config-schema.py", "--check"]), @@ -48,6 +49,9 @@ "xcstrings": ("*.xcstrings",), "localization": ("*.xcstrings", "scripts/localization-allowed-omissions.json", "scripts/localization-plurals.json"), + "localization-defaults": ("*.xcstrings", "Sources/*", "Packages/*", "CLI/*", "ios/*", "TunnelExtension/*", + "scripts/localization_catalog.py", "scripts/localize_changes.py", + "scripts/localization-default-mismatches.json"), "project-tests": ("scripts/normalize-pbxproj.py", "scripts/check-pbxproj-group-membership.py"), "project": ("scripts/normalize-pbxproj.py", "scripts/check-pbxproj-group-membership.py", "cmux.xcodeproj/project.pbxproj", diff --git a/tests/test-execution.toml b/tests/test-execution.toml index 9c30c16d5664..78d78ff01dd8 100644 --- a/tests/test-execution.toml +++ b/tests/test-execution.toml @@ -773,6 +773,10 @@ lane = "linux-guard" path = "tests/test_localize_changes.py" lane = "linux-guard" +[[test]] +path = "tests/test_localization_defaults.py" +lane = "linux-guard" + [[test]] path = "tests/test_package_resolved_policy_remote_inputs.py" lane = "linux-guard" diff --git a/tests/test_localization_defaults.py b/tests/test_localization_defaults.py new file mode 100644 index 000000000000..e3bb3352f94c --- /dev/null +++ b/tests/test_localization_defaults.py @@ -0,0 +1,163 @@ +#!/usr/bin/env python3 + +import contextlib +import importlib.util +import io +import json +import sys +import tempfile +import unittest +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +SPEC = importlib.util.spec_from_file_location("localization_defaults", ROOT / "scripts/localization_defaults.py") +MODULE = importlib.util.module_from_spec(SPEC) +assert SPEC.loader is not None +sys.modules[SPEC.name] = MODULE +SPEC.loader.exec_module(MODULE) + + +def unit(value): + return {"stringUnit": {"state": "translated", "value": value}} + + +def swift_call(key, default): + return f'let _ = String(localized: "{key}", defaultValue: "{default}")\n' + + +class Fixture: + """A throwaway checkout: one catalog under Resources, Swift wherever the test puts it.""" + + def __init__(self): + self.directory = tempfile.TemporaryDirectory() + self.root = Path(self.directory.name) + + def catalog(self, entries, path="Resources/Localizable.xcstrings"): + strings = {key: {"localizations": {"en": unit(english), "ja": unit(japanese)}} + for key, (english, japanese) in entries.items()} + target = self.root / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(json.dumps({"sourceLanguage": "en", "strings": strings, "version": "1.0"}, + ensure_ascii=False), encoding="utf-8") + + def swift(self, text, path="Sources/App.swift"): + target = self.root / path + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(text, encoding="utf-8") + + def allowlist(self, entries): + target = self.root / MODULE.ALLOWLIST + target.parent.mkdir(parents=True, exist_ok=True) + target.write_text(json.dumps(entries), encoding="utf-8") + + def check(self): + return MODULE.check(self.root, MODULE.load_allowlist(self.root / MODULE.ALLOWLIST)) + + +class LocalizationDefaultsTests(unittest.TestCase): + def setUp(self): + self.fixture = Fixture() + self.addCleanup(self.fixture.directory.cleanup) + + def test_catalog_that_drops_a_default_argument_is_reported(self): + self.fixture.catalog({"rename.unavailable": ("The state could not be refreshed.", "状態を更新できませんでした。")}) + self.fixture.swift(swift_call("rename.unavailable", "The state for %@ is unavailable.")) + errors, compared = self.fixture.check() + self.assertEqual(compared, 1) + self.assertEqual(errors, [ + "Resources/Localizable.xcstrings:rename.unavailable: catalog en placeholders [] != " + "Swift defaultValue [(1, '@')] (Sources/App.swift)" + ]) + + def test_catalog_that_adds_an_argument_is_reported(self): + self.fixture.catalog({"install.failed": ("failed to install: %@", "インストールできませんでした：%@")}) + self.fixture.swift(swift_call("install.failed", "failed to install")) + errors, _ = self.fixture.check() + self.assertEqual(len(errors), 1) + self.assertIn("catalog en placeholders [(1, '@')] != Swift defaultValue []", errors[0]) + + def test_same_arguments_in_another_order_or_spelling_match(self): + self.fixture.catalog({ + "move": ("Move %2$@ to %1$@", "%1$@ に %2$@ を移動"), + "count": ("%lld items in %@", "%@ 内の %lld 項目"), + }) + self.fixture.swift(swift_call("move", "Move %2$@ to %1$@") + swift_call("count", "%lld items in %@")) + self.assertEqual(self.fixture.check(), ([], 2)) + + def test_calls_without_one_readable_default_are_skipped(self): + self.fixture.catalog({ + "interpolated": ("Hello %@", "こんにちは %@"), + "conflicting": ("Open %@", "%@ を開く"), + "key.only": ("Close %@", "%@ を閉じる"), + }) + self.fixture.swift( + 'let _ = String(localized: "interpolated", defaultValue: "Hello \\(name)")\n' + + swift_call("conflicting", "Open") + + 'let _ = String(localized: "key.only")\n', + "Sources/A.swift", + ) + self.fixture.swift(swift_call("conflicting", "Open %@"), "Sources/B.swift") + self.assertEqual(self.fixture.check(), ([], 0)) + + def test_a_conflict_inside_one_file_is_not_settled_by_another_file(self): + self.fixture.catalog({"conflicting": ("Open %@", "%@ を開く")}) + self.fixture.swift(swift_call("conflicting", "Open") + swift_call("conflicting", "Open %@"), "Sources/A.swift") + self.fixture.swift(swift_call("conflicting", "Open"), "Sources/B.swift") + self.assertEqual(self.fixture.check(), ([], 0)) + + def test_mismatch_names_the_other_catalogs_carrying_the_key(self): + self.fixture.catalog({"shared": ("Open %@", "%@ を開く")}) + self.fixture.catalog({"shared": ("Open", "開く")}, "Packages/macOS/Pkg/Sources/Pkg/Localizable.xcstrings") + self.fixture.swift(swift_call("shared", "Open %@"), "Packages/macOS/Pkg/Sources/Pkg/View.swift") + errors, compared = self.fixture.check() + self.assertEqual(compared, 2) + self.assertEqual(errors, [ + "Packages/macOS/Pkg/Sources/Pkg/Localizable.xcstrings:shared: catalog en placeholders [] != " + "Swift defaultValue [(1, '@')] (Packages/macOS/Pkg/Sources/Pkg/View.swift)" + "; Resources/Localizable.xcstrings carries [(1, '@')]" + ]) + + def test_a_default_the_signature_parser_rejects_is_skipped_not_fatal(self): + self.fixture.catalog({"items": ("%lld items", "%lld 項目"), "ok": ("Open %@", "%@ を開く")}) + self.fixture.swift(swift_call("items", "%#@count@ items") + swift_call("ok", "Open %@")) + self.assertEqual(self.fixture.check(), ([], 1)) + + def test_test_targets_and_vendored_trees_are_not_scanned(self): + self.fixture.catalog({"age": ("%lld minutes", "%lld 分")}) + for path in ("cmuxTests/AgeTests.swift", "Packages/macOS/Pkg/Tests/PkgTests/T.swift", "vendor/Dep/Sources/D.swift"): + self.fixture.swift(swift_call("age", "a few minutes"), path) + self.assertEqual(self.fixture.check(), ([], 0)) + + def test_allowlisted_mismatch_passes_and_a_stale_entry_fails(self): + self.fixture.catalog({ + "known": ("The state could not be refreshed.", "状態を更新できませんでした。"), + "fixed": ("Open %@", "%@ を開く"), + }) + self.fixture.swift(swift_call("known", "The state for %@ is unavailable.") + swift_call("fixed", "Open %@")) + self.fixture.allowlist({"known": "waiting on a copy decision", "fixed": "already fixed"}) + errors, _ = self.fixture.check() + self.assertEqual(errors, [ + f"{MODULE.ALLOWLIST}: fixed no longer mismatches its Swift defaultValue; remove the entry" + ]) + + def test_allowlist_entries_need_a_reason(self): + self.fixture.allowlist({"known": ""}) + with self.assertRaises(ValueError): + MODULE.load_allowlist(self.fixture.root / MODULE.ALLOWLIST) + + def test_main_reports_the_count_and_exit_status(self): + self.fixture.catalog({"ok": ("Open %@", "%@ を開く"), "bad": ("Close", "閉じる")}) + self.fixture.swift(swift_call("ok", "Open %@") + swift_call("bad", "Close %@")) + stdout, stderr = io.StringIO(), io.StringIO() + with contextlib.redirect_stdout(stdout), contextlib.redirect_stderr(stderr): + status = MODULE.main(["--root", str(self.fixture.root)]) + self.assertEqual(status, 1) + self.assertIn("Resources/Localizable.xcstrings:bad:", stderr.getvalue()) + self.assertEqual(stdout.getvalue(), "2 Swift defaultValue/catalog en comparisons: 1 mismatch\n") + (self.fixture.root / "Sources/App.swift").write_text(swift_call("ok", "Open %@") + swift_call("bad", "Close")) + with contextlib.redirect_stdout(io.StringIO()): + self.assertEqual(MODULE.main(["--root", str(self.fixture.root)]), 0) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_localize_changes.py b/tests/test_localize_changes.py index 052330d56832..da5661a338d8 100644 --- a/tests/test_localize_changes.py +++ b/tests/test_localize_changes.py @@ -51,6 +51,18 @@ def write_catalog(root: Path, strings: dict) -> Path: class LocalizeChangesTests(unittest.TestCase): + def test_parse_swift_messages_can_report_the_keys_it_dropped_for_conflicts(self): + text = ( + 'String(localized: "same", defaultValue: "Open")\n' + 'String(localized: "same", defaultValue: "Open %@")\n' + 'String(localized: "other", defaultValue: "Close")\n' + ) + conflicts = set() + messages, attention = MODULE.parse_swift_messages("Sources/View.swift", text, conflicts=conflicts) + self.assertEqual(sorted(messages), ["other"]) + self.assertEqual(conflicts, {"same"}) + self.assertTrue(any("multiple default values" in line for line in attention)) + def test_key_only_call_cannot_steal_next_default(self): messages, attention = MODULE.parse_swift_messages( "Sources/View.swift", diff --git a/tests/test_verify_local.py b/tests/test_verify_local.py index d1a794a8ca2f..808d611f1024 100644 --- a/tests/test_verify_local.py +++ b/tests/test_verify_local.py @@ -370,7 +370,7 @@ def test_catalog_selects_both_consumers_and_time_sensitive_check(self): (repo / "Resources").mkdir() (repo / "Resources" / "odd name\nstrings.xcstrings").write_text("{}") selected, evidence = verify.affected_checks(repo, "HEAD") - self.assertEqual(selected, ["xcstrings", "localization", "feature-flags"]) + self.assertEqual(selected, ["xcstrings", "localization", "localization-defaults", "feature-flags"]) self.assertEqual(evidence["paths"], ["Resources/odd name\nstrings.xcstrings"]) self.assertIn("time_sensitive", evidence["reasons"]["feature-flags"])