Skip to content

Commit 81a3ae3

Browse files
committed
Don't treat * dynamic width/precision as a fixed specifier
`%*d` / `%.*f` consume an extra `int` argument (the width/precision) that the one-slot-per-specifier signature model can't represent, so the regex matched them but undercounted — making `%d` and `%*d` compare as compatible. Concretely, changing a base string from `%.1f km` to `%.*f km` shifts the call site from one argument to two, yet the check would wave it through, so every unchanged translation would render the precision where the value belongs. Stop matching `*` in the width/precision, leaving such a specifier unrecognized: a change to or from a `*` form now surfaces as a difference and gets flagged rather than silently passing. Fixed width/precision/length specifiers are unaffected.
1 parent d2e4bc3 commit 81a3ae3

2 files changed

Lines changed: 23 additions & 1 deletion

File tree

lib/fastlane/plugin/wpmreleasetoolkit/helper/string_placeholders_helper.rb

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,7 +38,12 @@ module StringPlaceholdersHelper
3838
# flags, width, precision, an optional length modifier, then the conversion
3939
# character. `%%` (a literal percent) is matched too so it can be explicitly
4040
# skipped rather than mistaken for a placeholder.
41-
SPECIFIER = /%(?<position>\d+\$)?[-+ 0#]*(?:\d+|\*)?(?:\.(?:\d+|\*))?(?:hh|h|ll|l|q|L|z|t|j)?(?<conversion>[@diouxXeEfgGaAcCsSpn%])/
41+
#
42+
# `*` dynamic width/precision (`%*d`, `%.*f`) is deliberately NOT matched: it consumes an extra
43+
# `int` argument that this one-slot-per-specifier model can't represent. Leaving it unrecognized
44+
# is the safe choice — rather than silently treating `%*d` as compatible with `%d`, a change to
45+
# or from a `*` specifier surfaces as a difference and gets flagged.
46+
SPECIFIER = /%(?<position>\d+\$)?[-+ 0#]*\d*(?:\.\d+)?(?:hh|h|ll|l|q|L|z|t|j)?(?<conversion>[@diouxXeEfgGaAcCsSpn%])/
4247

4348
# A canonical signature of the placeholders in a string value.
4449
#

spec/string_placeholders_helper_spec.rb

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,16 @@
107107
expect(described_class.placeholder_signature('%@ and %@')).to eq('1:object,2:object')
108108
end
109109
end
110+
111+
context 'with `*` dynamic width or precision' do
112+
# `%*d`/`%.*f` consume an extra `int` (the width/precision) that the one-slot-per-specifier
113+
# model can't represent, so they are intentionally left unrecognized rather than silently
114+
# treated as compatible with a fixed-width/precision specifier.
115+
it 'does not recognize a `*` specifier (so it cannot masquerade as its fixed counterpart)' do
116+
expect(described_class.placeholder_signature('%.*f km')).to eq('')
117+
expect(described_class.placeholder_signature('%*d items')).to eq('')
118+
end
119+
end
110120
end
111121

112122
describe '.placeholders_compatible?' do
@@ -141,6 +151,13 @@
141151
it 'treats adding a genuinely new positional argument as incompatible' do
142152
expect(described_class.placeholders_compatible?('%1$@', '%1$@ in %2$@')).to be(false)
143153
end
154+
155+
it 'flags a change between a fixed and a `*` dynamic width/precision specifier' do
156+
# `%.1f km` consumes one float; `%.*f km` consumes an int (precision) + a float. Treating them
157+
# as compatible would let an existing translation render the precision where the value belongs.
158+
expect(described_class.placeholders_compatible?('%.1f km', '%.*f km')).to be(false)
159+
expect(described_class.placeholders_compatible?('%5d items', '%*d items')).to be(false)
160+
end
144161
end
145162

146163
describe '.incompatible_placeholder_changes' do

0 commit comments

Comments
 (0)