Prefix unquoted keys and values in merge_strings

`L10nHelper.merge_strings` rewrites each key to add the caller's prefix, but its line
matcher only handled `[A-Z0-9_]` keys with a *quoted* value. An unquoted key
containing `. - $ : /`, or any line with an *unquoted* value (`CFBundleName = WordPress;`),
was written to the merged file without the prefix while still being bookkept *with* it
— an inconsistency that can resurface the collisions the prefix avoids and break
downstream key extraction.

The matcher now accepts the full unquoted-string character set `plutil` allows. The
unquoted-value case keys on the fact that an unquoted string can't contain spaces, so
it won't mistake comment prose (`and = a red herring ...`) for a key/value pair — a
case the fixtures deliberately guard.

Author: jkmassel

CHANGELOG.md

@@ -17,6 +17,7 @@ _None_
 - `StringsFileValidationHelper.find_duplicated_keys` now parses *unquoted* keys and values (valid `.strings` syntax, common in `InfoPlist.strings`) instead of raising `Invalid character`, matching the character set `plutil` accepts for unquoted strings (alphanumerics plus `_ . - $ : /`). This lets `ios_lint_localizations`' `check_duplicate_keys` work on `InfoPlist.strings`-style files. [#741]
 - `StringsFileValidationHelper.find_duplicated_keys` now also accepts comments placed *between* the tokens of a statement (e.g. `"key" /* note */ = "value";`), which `plutil` allows, instead of raising `Invalid character` on the `/`. [#741]
 - `ios_lint_localizations`' `check_duplicate_keys` no longer crashes the lane on a file that parses as a property list but isn't a flat `.strings` (e.g. a nested-dictionary value); it now warns via `UI.important` and skips that file. [#741]
+- `L10nHelper.merge_strings` now applies the key prefix to unquoted keys containing `. - $ : /` and to lines with an *unquoted* value (e.g. `CFBundleName = WordPress;`), matching the unquoted-string grammar `plutil` accepts. Previously those keys were written to the merged file without the prefix while still being bookkept *with* it, leaving the output inconsistent with the reported keys (which could resurface the very collisions the prefix avoids and break downstream key extraction). [#741]
 
 ### Internal Changes
 

lib/fastlane/plugin/wpmreleasetoolkit/helper/ios/ios_l10n_helper.rb

@@ -102,9 +102,17 @@ def self.merge_strings(paths:, output_path:)
               # Read line-by-line to reduce memory footprint during content copy
               read_utf8_lines(input_file).each do |line|
                 unless prefix.nil? || prefix.empty?
-                  # The `/u` modifier on the RegExps is to make them UTF-8
+                  # The `/u` modifier on the RegExps is to make them UTF-8.
+                  # The unquoted-key character set below matches what `plutil` accepts (alphanumerics plus
+                  # `_ . - $ : /`, as in `StringsFileValidationHelper::UNQUOTED_STRING_CHARACTER`) so that keys
+                  # containing `. - $ : /` (e.g. reverse-DNS keys) are prefixed like any other.
                   line.gsub!(/^(\s*")/u, "\\1#{prefix}") # Lines starting with a quote are considered to be start of a key; add prefix right after the quote
-                  line.gsub!(/^(\s*)([A-Z0-9_]+)(\s*=\s*")/ui, "\\1\"#{prefix}\\2\"\\3") # Lines starting with an identifier followed by a '=' are considered to be an unquoted key (typical in InfoPlist.strings files for example)
+                  # Lines starting with an unquoted key followed by a `=` and a *quoted* value (typical in `InfoPlist.strings`).
+                  line.gsub!(%r{^(\s*)([a-zA-Z0-9_.$:/-]+)(\s*=\s*")}u, "\\1\"#{prefix}\\2\"\\3")
+                  # Lines starting with an unquoted key followed by a `=` and an *unquoted* value (e.g. `CFBundleName = WordPress;`).
+                  # The value is required to be a single unquoted token ending in `;` — an unquoted string can't contain spaces, so
+                  # this won't mistake comment prose like `and = a red herring for when…` (spaces in the "value") for a key/value pair.
+                  line.gsub!(%r{^(\s*)([a-zA-Z0-9_.$:/-]+)(\s*=\s*)([a-zA-Z0-9_.$:/-]+\s*;)}u, "\\1\"#{prefix}\\2\"\\3\\4")
                 end
                 tmp_file.write(line)
               end

spec/ios_l10n_helper_spec.rb

@@ -100,6 +100,28 @@ def file_encoding(path)
       end
     end
 
+    it 'prefixes unquoted keys with unquoted values and keys containing `. - $ : /`' do
+      # Regression: prefixing must cover the full unquoted-string grammar `plutil` accepts — unquoted *values*
+      # (e.g. `CFBundleName = WordPress;`) and keys containing `. - $ : /` — so the written file stays consistent
+      # with the prefixed keys we report. (Previously only `[A-Z0-9_]` keys with a *quoted* value were prefixed,
+      # leaving these written without the prefix and resurfacing the very collisions the prefix avoids.)
+      content = <<~STRINGS
+        CFBundleName = WordPress;
+        com.automattic.app-id = "X";
+        "QuotedKey" = "Y";
+      STRINGS
+      Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir|
+        input_file = File.join(tmp_dir, 'InfoPlist.strings')
+        File.write(input_file, content)
+        output_file = File.join(tmp_dir, 'output.strings')
+        described_class.merge_strings(paths: { input_file => 'pfx.' }, output_path: output_file)
+        merged = File.read(output_file)
+        expect(merged).to include('"pfx.CFBundleName" = WordPress;')
+        expect(merged).to include('"pfx.com.automattic.app-id" = "X";')
+        expect(merged).to include('"pfx.QuotedKey" = "Y";')
+      end
+    end
+
     it 'returns duplicate keys found' do
       paths = { fixture('Localizable-utf16.strings') => nil, fixture('non-latin-utf16.strings') => nil }
       Dir.mktmpdir('a8c-release-toolkit-l10n-helper-tests-') do |tmp_dir|