From 92bf21f29118c64682effa16559d287969fbbbe1 Mon Sep 17 00:00:00 2001 From: Oguz Kocer Date: Fri, 17 Jul 2026 18:46:58 -0400 Subject: [PATCH 1/7] Add failing tests: placeholder gate rejects valid reordered human translations MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TranslationValidator compares a source's positional and sequential specifier views independently, so it only permits reordering when the source is ALREADY positional. A non-positional source (`%@ … %@`) whose translation reorders via positional specifiers (`%2$@ … %1$@`) is rejected — even though that is the standard, Apple-documented iOS reordering mechanism and is correct at runtime (`String(format:)` honors positional specifiers regardless of the source shape). The human-translation gate added for the String Catalog fold (`CatalogStrings.trusted_human`, `PluralStrings.human_forms_for`) inherits this, so it rejects and downgrades valid, currently-shipping human translations to machine/English. This is not hypothetical: the committed translated `.strings` contain 23 key-as-source strings across 34 locales (513 cells) that positionalize a bare-`%@` English source this way, e.g. ar "%@ of %@ used on your site" => "%1$@ من %2$@ على موقعك". Two failing tests pin the correct behavior: - translation_validator_test.rb — the root cause at the validator. - catalog_strings_helper_test.rb — the user-facing downgrade in the fold. The fix belongs in TranslationValidator (shared by AI, plurals, and regular human strings): treat a non-positional source as implicitly numbered 1..N by appearance order and accept a fully-positional candidate whose index→type map matches. Existing tests already keep genuine breakage (sequential flip, type/ count change) rejected, so a correct fix cannot just drop the distinction. --- fastlane/lanes/catalog_strings_helper_test.rb | 15 +++++++++++++++ fastlane/lanes/translation_validator_test.rb | 18 ++++++++++++++++++ 2 files changed, 33 insertions(+) diff --git a/fastlane/lanes/catalog_strings_helper_test.rb b/fastlane/lanes/catalog_strings_helper_test.rb index 8f093402c1f4..b8fb8ec3a203 100644 --- a/fastlane/lanes/catalog_strings_helper_test.rb +++ b/fastlane/lanes/catalog_strings_helper_test.rb @@ -73,6 +73,21 @@ def test_human_translation_is_used_and_marked_translated assert_equal({ 'state' => 'translated', 'value' => 'Enregistrer' }, cell(cat, 'a', 'fr')) end + # A human translation that reorders a non-positional source (%@ … %@) via positional specifiers (%2$@ … %1$@) + # is valid at runtime — String(format:) honors positional specifiers regardless of the source's shape — and is + # the standard iOS reordering pattern for RTL / word-order-differing locales. It must ship as `translated`, NOT + # be rejected by the placeholder gate and downgraded to machine/English. This is the user-facing half of the + # TranslationValidator regression (see translation_validator_test.rb): 23 key-as-source strings across 34 + # locales (513 currently-shipping cells) do exactly this, e.g. ar "%@ of %@ used on your site" folds to + # "%1$@ من %2$@ على موقعك". With no AI tier here, the rejected human currently degrades to English (needs_review). + def test_reordered_human_translation_of_a_non_positional_source_is_kept_as_translated + cat = catalog('a' => entry('%@ of %@ used')) + capture_stderr { fold(cat, translations: { 'fr' => { 'a' => '%2$@ sur %1$@ utilisés' } }) } + + assert_equal({ 'state' => 'translated', 'value' => '%2$@ sur %1$@ utilisés' }, cell(cat, 'a', 'fr'), + 'a reordered human translation must ship as translated, not be downgraded') + end + # A whitespace-only GlotPress value is not a real translation: it must not ship as `translated` — the key # should still reach the model and land as `needs_review`, same as if GlotPress had no value at all. def test_whitespace_only_human_value_is_treated_as_untranslated diff --git a/fastlane/lanes/translation_validator_test.rb b/fastlane/lanes/translation_validator_test.rb index 8cbd91a2f38f..e6298bdbd7c1 100644 --- a/fastlane/lanes/translation_validator_test.rb +++ b/fastlane/lanes/translation_validator_test.rb @@ -23,6 +23,24 @@ def test_positional_type_change_is_rejected refute V.placeholders_match?('%1$@ posts', '%1$d posts') end + # A NON-positional source (%@ … %@) whose translation reorders via positional specifiers (%2$@ … %1$@) is the + # standard, Apple-documented iOS way to reorder arguments for target grammar: String(format:) honors positional + # specifiers regardless of the source's shape, and each specifier still reads an argument of the same type. So + # this must be ACCEPTED, exactly like the already-positional reorder above. The gate currently rejects it, + # because it compares the source's positional/sequential views independently and a bare-%@ source has an empty + # positional map. This is not academic: real GlotPress data has 23 key-as-source strings across 34 locales + # (513 shipping cells) that positionalize a bare-%@ English source this way — e.g. the Arabic + # "%@ of %@ used on your site" => "%1$@ من %2$@ على موقعك". The human-translation gate this catalog work adds + # (CatalogStrings.trusted_human, PluralStrings.human_forms_for) rejects every one of them and downgrades a + # valid, currently-shipping human translation to machine/English. + # + # (Genuine breakage stays rejected by the existing tests: a sequential flip `%@: %d` => `%d : %@` and a type + # change are still caught, so a correct fix cannot simply ignore the positional/sequential distinction.) + def test_positionalizing_a_non_positional_source_to_reorder_is_allowed + assert V.placeholders_match?('%@ - %@', '%2$@ - %1$@') # two objects, reordered + assert V.placeholders_match?('%@. %d posts.', '%1$@. %2$d posts.') # object + int, positionalized in place + end + def test_sequential_order_must_be_preserved refute V.placeholders_match?('%@: %d', '%d : %@') # flipped non-positional args assert V.placeholders_match?('%@: %d', 'Total %@: %d') # same order, prose changed From fa71e74197a064976126efb83c9647c9b7c59bff Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Sat, 18 Jul 2026 14:12:29 -0600 Subject: [PATCH 2/7] Fix placeholder gate: accept positional reorder of a non-positional source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit TranslationValidator compared a source's positional and sequential specifier views independently, so it only permitted reordering when the source was ALREADY positional. A non-positional source (`%@ … %@`) whose translation reorders via positional specifiers (`%2$@ … %1$@`) was rejected — even though that is the standard, Apple-documented iOS mechanism and is correct at runtime, because `String(format:)` honors positional specifiers regardless of the source's shape. Normalize a purely-sequential signature into the implicit 1..N positional map its bare specifiers already carry (`%@ %d` => `{1 => object, 2 => int}`) before comparing, applied to both source and candidate. Once both sides are index→type maps, `%@ - %@` and `%2$@ - %1$@` compare equal and the reorder is accepted. Genuine breakage stays rejected: a sequential flip `%@: %d` => `%d : %@` yields `{1 => object, 2 => int}` vs `{1 => int, 2 => object}` — a disagreement on which index owns which type — and type/length/count changes are unaffected. A signature that already uses positional specifiers, or one that mixes bare and positional specifiers (malformed), is left unchanged, so mixed strings still require an exact structural match. Fixes the two failing tests added in the previous commit and propagates to all three gate consumers (AI, plurals, regular human strings) via the shared validator. All 7 fastlane/lanes suites pass (113 tests); rubocop clean. --- fastlane/lanes/translation_validator.rb | 32 ++++++++++++++++++++----- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/fastlane/lanes/translation_validator.rb b/fastlane/lanes/translation_validator.rb index 29b9e9c887fa..a9c1ecab99c8 100644 --- a/fastlane/lanes/translation_validator.rb +++ b/fastlane/lanes/translation_validator.rb @@ -3,10 +3,11 @@ # Format-specifier safety gate for machine-translated strings. # # The one correctness invariant for a translated `.strings` / `.xcstrings` value: it must preserve the -# source's printf / NSString format ARGUMENTS exactly — same count, same types, and (for positional -# `%1$@` specifiers) the same index→type mapping. The surrounding prose is free to change; the argument -# contract is not. Break it and the app reads the wrong vararg off the stack — a crash or garbage at -# runtime, in a locale the author can't read and CI can't catch. +# source's printf / NSString format ARGUMENTS exactly — same count, same types, and the same index→type +# mapping (bare specifiers are numbered by appearance order, so a translation may reorder them with +# explicit `%2$@ … %1$@` specifiers — the standard iOS mechanism). The surrounding prose is free to +# change; the argument contract is not. Break it and the app reads the wrong vararg off the stack — a +# crash or garbage at runtime, in a locale the author can't read and CI can't catch. # # This is deliberately plain Ruby with no dependencies, so it can gate EVERY machine translation before it # is written and be unit-tested directly. It's the floor under the `human ?? AI ?? English` resolution in @@ -60,8 +61,8 @@ def placeholders_match?(source, candidate) # nil when the contract is preserved; otherwise a short human-readable reason (for logging which AI cells # were rejected and why). def mismatch_reason(source, candidate) - src = signature(source) - cand = signature(candidate) + src = implied_positional(signature(source)) + cand = implied_positional(signature(candidate)) if src.positional != cand.positional "positional placeholders differ (source: #{describe_positional(src.positional)}; " \ @@ -88,6 +89,25 @@ def signature(str) Signature.new(positional, sequential) end + # A purely-sequential signature rewritten with the implicit 1..N indices its bare specifiers already + # carry (`%@ %d` binds argument 1 then 2, so it becomes `{1 => object, 2 => int}`). This is what lets a + # translation reorder a NON-positional source via explicit positional specifiers: once both sides are + # expressed as index→type maps, `%@ - %@` and `%2$@ - %1$@` compare equal — the reorder is honored at + # runtime by `String(format:)` regardless of the source's shape. A genuine sequential flip still fails, + # because `%@: %d` → `{1 => object, 2 => int}` and `%d : %@` → `{1 => int, 2 => object}` disagree on + # which index owns which type. A signature that already uses positional specifiers is returned + # unchanged; so is one that MIXES bare and positional specifiers (malformed per Apple's docs), so a + # mixed string still requires an exact structural match and can't accidentally validate against a clean + # map. + def implied_positional(sig) + return sig unless sig.positional.empty? && !sig.sequential.empty? + + positional = {} + sig.sequential.each_with_index { |token, i| positional[i + 1] = token } + Signature.new(positional, []) + end + private_class_method :implied_positional + # Yields each format-specifier MatchData in appearance order. Scans forward from the end of each match, so # adjacent specifiers (`%d%@`) and specifiers embedded in text are all found. def each_specifier(str) From 8148a3bb73b4c251414c4e7fa635a3a4c230ee74 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:20:18 -0600 Subject: [PATCH 3/7] Add failing tests: gate and coverage compare miss format-argument edge cases Three currently-red tests documenting real defects found auditing the placeholder gate and the build-free coverage compare: - A candidate that binds the same positional index to two different types (%1$@ %1$d) is accepted: signature() collapses it last-wins, and this PR's implied_positional widened the reach to bare-specifier sources (%d), which regressed from reject to accept. - Dynamic field width/precision (%*d, %.*f) consume an extra int vararg the signature never counts, so %d and %*d reduce to the same signature. - coverage_gap conflates keys that differ only in argument type (%d days vs %@ days), masking a genuinely dropped key behind a same-prose sibling. --- fastlane/lanes/catalog_helper_test.rb | 12 ++++++++++ fastlane/lanes/translation_validator_test.rb | 25 ++++++++++++++++++++ 2 files changed, 37 insertions(+) diff --git a/fastlane/lanes/catalog_helper_test.rb b/fastlane/lanes/catalog_helper_test.rb index 02e4470777ad..20abe66f577b 100644 --- a/fastlane/lanes/catalog_helper_test.rb +++ b/fastlane/lanes/catalog_helper_test.rb @@ -46,3 +46,15 @@ def test_reports_every_reworded_key assert_equal %w[b c], reworded(strings, { 'a' => 'One', 'b' => 'TWO', 'c' => 'THREE' }).sort end end + +# Failing test: gap surfaced by the pipeline-breaker audit (currently RED). `canonical` replaces every format +# specifier with a single sentinel regardless of type/index, so two keys that differ ONLY in argument type collapse +# to the same coverage cell. That masks a genuinely-dropped key: if the build-free extraction loses "%d days" +# (e.g. a same-basename .stringsdata overwrite — the exact regression this gate exists to catch), the surviving +# "%@ days" makes coverage_gap report no gap even though an int-vs-object contract was lost. +class CatalogHelperCoverageGapTest < Minitest::Test + def test_distinct_type_sibling_keys_are_not_conflated + # "%d days" (int) and "%@ days" (object) are real, distinct en.lproj keys; dropping either is a real hole. + assert_equal ['%d days'], CatalogHelper.coverage_gap(['%d days', '%@ days'], ['%@ days']) + end +end diff --git a/fastlane/lanes/translation_validator_test.rb b/fastlane/lanes/translation_validator_test.rb index e6298bdbd7c1..bbc9c3a955e3 100644 --- a/fastlane/lanes/translation_validator_test.rb +++ b/fastlane/lanes/translation_validator_test.rb @@ -74,4 +74,29 @@ def test_mismatch_reason_is_descriptive assert_nil V.mismatch_reason('%1$@ invited %2$@', '%2$@ a invité %1$@') end + + # --- Failing tests: gaps surfaced by the pipeline-breaker audit (currently RED; each documents a real defect + # where the gate violates its own "same count, same types, same index→type mapping" invariant) ----------- + + # A candidate that binds the SAME positional index to two different types (%1$@ then %1$d) changes the argument + # contract: at runtime String(format:) reads argument 1 as an OBJECT for %1$@, but the source only ever supplies + # an int there — a wrong-vararg read (crash / garbage). The gate must reject it. It currently does NOT, because + # signature() stores specifiers in a Hash keyed by index with plain assignment, so %1$d silently OVERWRITES + # %1$@ at index 1 (last-wins), collapsing the type conflict to {1 => int}. This PR's implied_positional widened + # the blast radius: a bare-specifier source like `%d` now normalizes to {1 => int} and matches the collapsed + # candidate, so the first case below regressed from reject (pre-PR) to accept. + def test_duplicate_positional_index_with_conflicting_type_is_rejected + refute V.placeholders_match?('%d', '%1$@ %1$d') # regressed by implied_positional (rejected pre-PR) + refute V.placeholders_match?('%1$d words', '%1$@ %1$d words') # pre-existing last-wins collapse at index 1 + end + + # Dynamic field width / precision (`*`) consumes an EXTRA int vararg before the value: `%*d` reads a width int + # then the int value (two args); `%.*f` reads a precision int then the double (two args). The gate matches the + # `*` in the FORMAT_SPECIFIER regex but never emits an argument token for it, so `%d` and `%*d` (and `%.*f` vs + # `%.2f`) reduce to the identical signature — an arg-count mismatch it must catch per its own "same count" + # invariant. Both currently accepted. No shipping source uses `%*` today, so this is latent, not live. + def test_dynamic_width_precision_star_is_counted + refute V.placeholders_match?('%d posts', '%*d posts') # candidate reads one int past what the source supplies + refute V.placeholders_match?('Value: %.*f', 'Value: %.2f') # source consumes precision-int + double; candidate one + end end From e3092296aa4a91277c74aef41c1d69baa3f374fe Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:21:01 -0600 Subject: [PATCH 4/7] Fix placeholder gate: reject a positional index reused with a conflicting type MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit signature() stored positional specifiers in a Hash keyed by index with plain assignment, so a second reference to the same index (`%1$@` then `%1$d`) silently overwrote the first, collapsing an object/int conflict to one token. A candidate that references argument 1 as both an object and an int passes an int to a `%@` conversion at runtime — a wrong-vararg read. This PR's implied_positional widened the reach: a bare-specifier source like `%d` now normalizes to {1 => int} and matched the collapsed candidate, regressing that case from reject to accept. Record a conflicting reuse as a distinct token so it can never equal a well-formed source's single type, while still allowing a consistent reuse (`%1$d … %1$d`, which legally reads one argument twice). --- fastlane/lanes/translation_validator.rb | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/fastlane/lanes/translation_validator.rb b/fastlane/lanes/translation_validator.rb index a9c1ecab99c8..ce558187ed88 100644 --- a/fastlane/lanes/translation_validator.rb +++ b/fastlane/lanes/translation_validator.rb @@ -81,7 +81,13 @@ def signature(str) token = "#{match[:length]}:#{TYPE_CLASS.fetch(match[:conv], match[:conv])}" if match[:position] - positional[match[:position].to_i] = token + index = match[:position].to_i + # A positional index MAY be referenced more than once, but only with a consistent type (`%1$d … %1$d` is + # legal — it reads argument 1 twice). Two DIFFERENT types at one index is malformed: it passes an int to a + # `%@` conversion (or vice versa) at runtime. Record the conflict as a distinct token that can never equal a + # well-formed source's single type, rather than silently overwriting last-wins. + existing = positional[index] + positional[index] = existing && existing != token ? "conflict(#{[existing, token].sort.join(', ')})" : token else sequential << token end From 4a7ee39d5beb82fe304cfce2e6a144c7e27b49f6 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:21:43 -0600 Subject: [PATCH 5/7] Fix placeholder gate: count dynamic field width/precision arguments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A `*` in a specifier's field width or precision (`%*d`, `%.*f`) consumes its own int argument before the value, so `%d` and `%*d` are not interchangeable. The FORMAT_SPECIFIER regex matched the `*` but signature() emitted no token for it, so the two reduced to the same signature and a translation could add or drop a dynamic-width/precision argument undetected — reading one vararg past what the call site supplies. Emit an int argument for each `*`, in appearance order before the value. --- fastlane/lanes/translation_validator.rb | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/fastlane/lanes/translation_validator.rb b/fastlane/lanes/translation_validator.rb index ce558187ed88..4a9f53e294c4 100644 --- a/fastlane/lanes/translation_validator.rb +++ b/fastlane/lanes/translation_validator.rb @@ -24,8 +24,8 @@ module TranslationValidator % # leading percent (?:(?\d+)\$)? # optional positional index: 1$, 2$, … [\#0\-+']* # flags (NOT space — see note above) - (?:\d+|\*)? # field width - (?:\.(?:\d+|\*))? # precision + (?\d+|\*)? # field width (`*` = a dynamic-width int argument) + (?:\.(?\d+|\*))? # precision (`*` = a dynamic-precision int argument) (?hh|h|ll|l|L|q|z|t|j)? # length modifier (?[@dDiuUxXoOfFeEgGaAcCsSpn%]) # conversion /x @@ -79,6 +79,12 @@ def signature(str) each_specifier(str.to_s) do |match| next if match[:conv] == '%' # literal %% — not an argument + # Dynamic field width/precision each consume their OWN int argument, bound sequentially before the value + # (`%*d` reads a width int then the value; `%.*f` a precision int then the value). Count them, so `%d` and + # `%*d` — which are NOT interchangeable — no longer reduce to the same signature. + sequential << ':int' if match[:width] == '*' + sequential << ':int' if match[:precision] == '*' + token = "#{match[:length]}:#{TYPE_CLASS.fetch(match[:conv], match[:conv])}" if match[:position] index = match[:position].to_i From f031a7538b243d9c8e83ab2f1db853cd687fa5e2 Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:28:10 -0600 Subject: [PATCH 6/7] Fix coverage compare: keep argument type when canonicalizing keys MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit canonical() replaced every format specifier with one sentinel regardless of type, so keys differing only in argument type (`%d days` vs `%@ days`) collapsed to the same coverage cell — a genuinely-dropped key was masked by a same-prose sibling of a different type, defeating the same-basename overwrite regression the coverage gate exists to catch. Strip only the positional index (the sole difference between the genstrings source-form `%li` and the xcstringstool-normalized `%1$li` for the same source literal), so `%li` and `%1$li` still compare equal while `%d` and `%@` stay distinct. --- fastlane/lanes/catalog_helper.rb | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/fastlane/lanes/catalog_helper.rb b/fastlane/lanes/catalog_helper.rb index 449fa2ab1915..32b2a63db2ae 100644 --- a/fastlane/lanes/catalog_helper.rb +++ b/fastlane/lanes/catalog_helper.rb @@ -24,9 +24,13 @@ def coverage_gap(reference, catalog_keys) reference.reject { |key| catalog_canonical.include?(canonical(key)) } end - # Collapse format specifiers to a single token so source-form (%li) and normalized (%1$li) compare equal. + # Strip the positional index from each format specifier so a source-form specifier (%li) and its normalized form + # (%1$li) compare equal, while specifiers of a DIFFERENT argument type stay distinct. The positional `N$` prefix + # is the only thing that differs between the two extraction paths (genstrings vs xcstringstool) for the same + # source literal; collapsing every specifier to one token — as this did before — conflated `%d days` with + # `%@ days` and masked a genuinely-dropped key behind a same-prose sibling of a different type. def canonical(key) - key.gsub(FORMAT_SPECIFIER, "\u0001") + key.gsub(FORMAT_SPECIFIER) { |specifier| specifier.sub(/\A%\d+\$/, '%') } end # --- immutable-key enforcement ------------------------------------------------------------------------- From b96a1cc429911950e2bb441098d8a8d1725752db Mon Sep 17 00:00:00 2001 From: Jeremy Massel <1123407+jkmassel@users.noreply.github.com> Date: Sat, 18 Jul 2026 15:40:46 -0600 Subject: [PATCH 7/7] Satisfy RuboCop: extract signature helpers and merge the catalog test class Danger's RuboCop pass flagged two offenses from the previous commits: - Metrics/AbcSize: signature() exceeded the ABC limit once it gained the positional-conflict and dynamic-width handling. Extract record_specifier and add_positional so signature() is a thin loop again; behavior unchanged. - Style/OneClassPerFile: the coverage_gap test added a second top-level class to catalog_helper_test.rb. Fold it into the file's single test class, renamed CatalogHelperTest since it now covers both reword detection and the coverage compare. --- fastlane/lanes/catalog_helper_test.rb | 20 ++++------ fastlane/lanes/translation_validator.rb | 52 ++++++++++++++----------- 2 files changed, 38 insertions(+), 34 deletions(-) diff --git a/fastlane/lanes/catalog_helper_test.rb b/fastlane/lanes/catalog_helper_test.rb index 20abe66f577b..10b0bedcea1e 100644 --- a/fastlane/lanes/catalog_helper_test.rb +++ b/fastlane/lanes/catalog_helper_test.rb @@ -6,9 +6,9 @@ require 'minitest/autorun' require_relative 'catalog_helper' -# Exercises the reword detector: an explicit-key string whose stored English differs from a fresh extraction is -# flagged; key-as-source, variations/plural, unchanged, and removed keys are not. -class CatalogHelperRewordedKeysTest < Minitest::Test +# Exercises the reword detector (an explicit-key string whose stored English differs from a fresh extraction is +# flagged; key-as-source, variations/plural, unchanged, and removed keys are not) and the coverage compare. +class CatalogHelperTest < Minitest::Test def en(value) { 'localizations' => { 'en' => { 'stringUnit' => { 'state' => 'translated', 'value' => value } } } } end @@ -45,16 +45,12 @@ def test_reports_every_reworded_key strings = { 'a' => en('One'), 'b' => en('Two'), 'c' => en('Three') } assert_equal %w[b c], reworded(strings, { 'a' => 'One', 'b' => 'TWO', 'c' => 'THREE' }).sort end -end -# Failing test: gap surfaced by the pipeline-breaker audit (currently RED). `canonical` replaces every format -# specifier with a single sentinel regardless of type/index, so two keys that differ ONLY in argument type collapse -# to the same coverage cell. That masks a genuinely-dropped key: if the build-free extraction loses "%d days" -# (e.g. a same-basename .stringsdata overwrite — the exact regression this gate exists to catch), the surviving -# "%@ days" makes coverage_gap report no gap even though an int-vs-object contract was lost. -class CatalogHelperCoverageGapTest < Minitest::Test - def test_distinct_type_sibling_keys_are_not_conflated - # "%d days" (int) and "%@ days" (object) are real, distinct en.lproj keys; dropping either is a real hole. + # coverage_gap must not conflate keys that differ only in argument TYPE. `canonical` used to replace every format + # specifier with one sentinel regardless of type/index, so if the build-free extraction loses "%d days" (e.g. a + # same-basename .stringsdata overwrite — the exact regression this gate exists to catch), the surviving "%@ days" + # masked it. "%d days" (int) and "%@ days" (object) are real, distinct en.lproj keys; dropping either is a hole. + def test_coverage_gap_does_not_conflate_distinct_type_sibling_keys assert_equal ['%d days'], CatalogHelper.coverage_gap(['%d days', '%@ days'], ['%@ days']) end end diff --git a/fastlane/lanes/translation_validator.rb b/fastlane/lanes/translation_validator.rb index 4a9f53e294c4..853359d4ab71 100644 --- a/fastlane/lanes/translation_validator.rb +++ b/fastlane/lanes/translation_validator.rb @@ -76,31 +76,39 @@ def mismatch_reason(source, candidate) def signature(str) positional = {} sequential = [] - each_specifier(str.to_s) do |match| - next if match[:conv] == '%' # literal %% — not an argument - - # Dynamic field width/precision each consume their OWN int argument, bound sequentially before the value - # (`%*d` reads a width int then the value; `%.*f` a precision int then the value). Count them, so `%d` and - # `%*d` — which are NOT interchangeable — no longer reduce to the same signature. - sequential << ':int' if match[:width] == '*' - sequential << ':int' if match[:precision] == '*' - - token = "#{match[:length]}:#{TYPE_CLASS.fetch(match[:conv], match[:conv])}" - if match[:position] - index = match[:position].to_i - # A positional index MAY be referenced more than once, but only with a consistent type (`%1$d … %1$d` is - # legal — it reads argument 1 twice). Two DIFFERENT types at one index is malformed: it passes an int to a - # `%@` conversion (or vice versa) at runtime. Record the conflict as a distinct token that can never equal a - # well-formed source's single type, rather than silently overwriting last-wins. - existing = positional[index] - positional[index] = existing && existing != token ? "conflict(#{[existing, token].sort.join(', ')})" : token - else - sequential << token - end - end + each_specifier(str.to_s) { |match| record_specifier(match, positional, sequential) } Signature.new(positional, sequential) end + # Fold one matched specifier into the running positional/sequential views (mutated in place). `%%` is a literal + # percent, consuming no argument. Dynamic field width/precision each consume their OWN int argument, bound + # sequentially before the value (`%*d` = width int then value; `%.*f` = precision int then value), so counting + # them keeps `%d` and `%*d` — which are NOT interchangeable — from reducing to the same signature. + def record_specifier(match, positional, sequential) + return if match[:conv] == '%' + + sequential << ':int' if match[:width] == '*' + sequential << ':int' if match[:precision] == '*' + + token = "#{match[:length]}:#{TYPE_CLASS.fetch(match[:conv], match[:conv])}" + if match[:position] + add_positional(positional, match[:position].to_i, token) + else + sequential << token + end + end + private_class_method :record_specifier + + # Record a positional specifier's type at its index. A positional index MAY be referenced more than once, but only + # with a CONSISTENT type (`%1$d … %1$d` is legal — it reads argument 1 twice). Two different types at one index is + # malformed — it passes an int to a `%@` conversion (or vice versa) at runtime — so store the conflict as a + # distinct token that can never equal a well-formed source's single type, rather than overwriting last-wins. + def add_positional(positional, index, token) + existing = positional[index] + positional[index] = existing && existing != token ? "conflict(#{[existing, token].sort.join(', ')})" : token + end + private_class_method :add_positional + # A purely-sequential signature rewritten with the implicit 1..N indices its bare specifiers already # carry (`%@ %d` binds argument 1 then 2, so it becomes `{1 => object, 2 => int}`). This is what lets a # translation reorder a NON-positional source via explicit positional specifiers: once both sides are