Make merge_strings key prefixing comment-aware

`merge_strings` prefixed keys with line-based regexes that assume the key, `=`, and value
are adjacent, so an inter-token `.strings` comment — `AppName /* c */ = WordPress;` or
`CFBundleName = WordPress /* trailing */;` — broke the match and the key was written to the
merged file without the prefix, while the `plutil`-derived bookkeeping still recorded it
*with* the prefix. The inconsistency could resurface the very collisions the prefix exists
to avoid (two files sharing a key behind a comment merged to a single colliding key).

Replace the regexes with `StringsFileValidationHelper.prefix_keys`, a comment-aware
tokenizer that reuses the same state machine `find_duplicated_keys` uses: it prefixes a key
wherever the tokenizer enters one, regardless of surrounding comments, and leaves
`key = value`-looking text inside a comment alone. Bookkeeping and rewriting now agree by
construction. Fixes the failing specs added in #742.

Author: jkmassel

CHANGELOG.md

@@ -17,7 +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]
+- `L10nHelper.merge_strings` now applies the key prefix via a comment-aware tokenizer, so it correctly prefixes unquoted keys containing `. - $ : /`, lines with an *unquoted* value (e.g. `CFBundleName = WordPress;`), and keys sitting behind an inter-token `.strings` comment (e.g. `CFBundleName /* note */ = WordPress;`) — matching the grammar `plutil` accepts. Previously the line-based matcher left those keys written to the merged file without the prefix while still bookkeeping them *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

@@ -99,23 +99,13 @@ def self.merge_strings(paths:, output_path:)
               all_keys_found += string_keys
 
               tmp_file.write("/* MARK: - #{File.basename(input_file)} */\n\n")
-              # 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 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
-                  # 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
+              # Add the prefix to every key. We tokenize via `StringsFileValidationHelper.prefix_keys` rather than
+              # matching keys with a line-based regex, so that keys are found regardless of where `.strings` comments
+              # sit (e.g. `CFBundleName /* note */ = WordPress;`) and `key = value`-looking text inside a comment is
+              # left alone — keeping the written keys consistent with the (`plutil`-derived) keys bookkept above.
+              lines = read_utf8_lines(input_file)
+              lines = Fastlane::Helper::Ios::StringsFileValidationHelper.prefix_keys(lines: lines, prefix: prefix)
+              lines.each { |line| tmp_file.write(line) }
               tmp_file.write("\n")
             end
             tmp_file.close # ensure we flush the content to disk

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

@@ -208,6 +208,51 @@ def self.scan_for_duplicate_keys(file:, assume_valid: false)
         rescue StandardError => e
           [:unscannable, e.message]
         end
+
+        # Rewrites `.strings` lines so every key carries `prefix`, leaving comments, values, whitespace and
+        # formatting untouched. A quoted key gets the prefix inside its quotes (`"key"` → `"<prefix>key"`); an
+        # unquoted key is wrapped in quotes (`key` → `"<prefix>key"`). Because it tokenizes the file the same way
+        # `find_duplicated_keys` does, it is comment-aware: a key sitting behind an inter-token comment (e.g.
+        # `key /* note */ = value;`) is still prefixed, and `key = value`-looking text *inside* a comment is left
+        # alone — a distinction a line-based regex can't reliably make.
+        #
+        # @param [Array<String>] lines The file's lines, already decoded to UTF-8 (e.g. via `L10nHelper.read_utf8_lines`).
+        # @param [String] prefix The prefix to insert before every key. A nil/empty prefix returns `lines` unchanged.
+        # @return [Array<String>] The rewritten lines.
+        def self.prefix_keys(lines:, prefix:)
+          return lines if prefix.nil? || prefix.empty?
+
+          state = State.new(context: :root, buffer: StringIO.new, in_escaped_ctx: false, found_key: nil, resume_context: :root)
+          lines.map do |line|
+            rewritten = +''
+            line.each_char do |c|
+              # Escaped characters only occur inside quoted strings or comments — never around a key boundary —
+              # so they're copied through verbatim (mirroring `find_duplicated_keys`' global escape handling).
+              if state.in_escaped_ctx || c == '\\'
+                state.in_escaped_ctx = !state.in_escaped_ctx
+                rewritten << c
+                next
+              end
+
+              previous_context = state.context
+              (_, transition) = TRANSITIONS[previous_context].find { |regex, _| c.match?(regex) } || [nil, nil]
+              raise "Invalid character `#{c}` found (current context: #{previous_context})" if transition.nil?
+
+              state.context = transition.is_a?(Proc) ? transition.call(state, c) : transition
+
+              if previous_context == :root && state.context == :in_quoted_key
+                rewritten << c << prefix # opening `"` of a quoted key — the prefix goes inside the quotes
+              elsif previous_context == :root && state.context == :in_unquoted_key
+                rewritten << '"' << prefix << c    # first char of an unquoted key — open a quote + prefix, then the char
+              elsif previous_context == :in_unquoted_key && state.context != :in_unquoted_key
+                rewritten << '"' << c              # the unquoted key just ended — close the quote, then the delimiter
+              else
+                rewritten << c
+              end
+            end
+            rewritten
+          end
+        end
       end
     end
   end