Prefix nested dict/array .strings values instead of failing soft

The previous fix cleared the `merge_strings` crash on a nested-dictionary
value by rescuing `prefix_keys` and copying the file through *unprefixed*.
That re-opened the exact write/bookkeep inconsistency [#741] closed: the keys
were bookkept with the prefix but written without it, so a same-named key in
another file collided in the merged output, went unreported, and was silently
collapsed by `plutil` — and downstream `ios_extract_keys_from_strings_files`,
which looks keys up by their prefixed name, would drop those translations.

Fix it at the root instead. Teach the shared `StringsFileValidationHelper`
tokenizer the container grammar via a `depth`-counted `:in_container_value`
context, plus `:in_container_quoted_string` and `:maybe_container_comment` so a
`}`/`;` hidden inside a quoted value or comment isn't read as structural.
Because `find_duplicated_keys` and `prefix_keys` share the `TRANSITIONS` table,
both now handle dictionary/array values: the outer key is prefixed and the
value copied verbatim, and inner keys are never recorded as top-level keys.

Also reorder `merge_strings` bookkeeping to run after the rewrite attempt, so
the surviving fail-soft backstop (for constructs still outside the grammar,
e.g. `<data>` values) bookkeeps its keys *unprefixed* to match what was
written — keeping the reported duplicates honest even there.

As a result `scan_for_duplicate_keys` returns `:scanned` instead of
`:unscannable` for nested-dict/array files, so `ios_lint_localizations`'
`check_duplicate_keys` now checks them too.

Author: jkmassel

CHANGELOG.md

@@ -16,9 +16,10 @@ _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]
+- `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` the scanner can tokenize (e.g. a `<data>` value); it now warns via `UI.important` and skips that file. [#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]
-- `L10nHelper.merge_strings` no longer crashes on a `:text` `.strings` file whose value is a nested dictionary (`"k" = { … };`) — valid input `plutil` accepts but the flat-`.strings` prefixing tokenizer can't rewrite. It now copies that file's lines through unprefixed with a `UI.important` warning instead of aborting the whole merge, mirroring the fail-soft behavior on the duplicate-key scanner path. [#750]
+- `StringsFileValidationHelper.find_duplicated_keys` (and `scan_for_duplicate_keys`) now tokenize dictionary- and array-valued entries (`"k" = { … };`, `"k" = ( … );`, nesting allowed), skipping the container body — so a `:text` file that uses them is scanned for duplicate *top-level* keys instead of returning `:unscannable`. [#750]
+- `L10nHelper.merge_strings` now prefixes the outer key of a dictionary/array-valued entry and copies the value through verbatim, instead of crashing on it — valid input `plutil` accepts that the flat-`.strings` tokenizer previously couldn't rewrite. If a file still holds a construct the tokenizer can't rewrite, its lines are copied through unprefixed with a `UI.important` warning (and its keys are bookkept unprefixed to match, so a genuine cross-file collision is still reported rather than silently collapsed) instead of aborting the whole merge. [#750]
 
 ### Internal Changes
 

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

@@ -78,9 +78,11 @@ def self.read_utf8_lines(file)
         # @note The method is able to handle input files which are using different encodings,
         #       guessing the encoding of each input file using the BOM (and defaulting to UTF8).
         #       The generated file will always be in utf-8, by convention.
-        # @note If a file's keys can't be prefixed (e.g. it holds a nested-dictionary value, `"k" = { … };`,
-        #       which `plutil` accepts but the flat-`.strings` tokenizer can't rewrite), its lines are copied
-        #       through unprefixed with a warning rather than aborting the whole merge.
+        # @note Dictionary- and array-valued entries (`"k" = { … };`, `"k" = ( … );`, nesting allowed) are
+        #       prefixed on their outer key with the value preserved verbatim. If a file still holds some
+        #       construct the tokenizer can't rewrite, its lines are copied through unprefixed with a warning
+        #       (and its keys are then bookkept unprefixed too, so the reported duplicates stay accurate)
+        #       rather than aborting the whole merge.
         #
         # @raise [RuntimeError] If one of the paths provided is not in text format (but XML or binary instead), or if any of the files are missing.
         #
@@ -97,27 +99,36 @@ def self.merge_strings(paths:, output_path:)
               raise "The file `#{input_file}` does not exist or is of unknown format." if fmt.nil?
               raise "The file `#{input_file}` is in #{fmt} format but we currently only support merging `.strings` files in text format." unless fmt == :text
 
-              string_keys = read_strings_file_as_hash(path: input_file).keys.map { |k| "#{prefix}#{k}" }
-              duplicates += (string_keys & all_keys_found) # Find duplicates using Array intersection, and add those to duplicates list
-              all_keys_found += string_keys
+              raw_keys = read_strings_file_as_hash(path: input_file).keys
 
               tmp_file.write("/* MARK: - #{File.basename(input_file)} */\n\n")
               # 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.
+              # left alone. It also handles dictionary/array values (`"k" = { … };`) — prefixing the outer key and
+              # copying the value verbatim.
               lines = read_utf8_lines(input_file)
+              applied_prefix = prefix
               begin
                 lines = Fastlane::Helper::Ios::StringsFileValidationHelper.prefix_keys(lines: lines, prefix: prefix)
               rescue StandardError => e
-                # `plutil` accepts flat-`.strings` constructs the tokenizer doesn't — most notably a nested-dictionary
-                # value (`"k" = { … };`), which parses fine (so the file clears the `:text` gate above) yet exposes no
-                # flat `key = value` for `prefix_keys` to rewrite, so it raises on the `{`. Fail soft: copy this file's
-                # lines through unprefixed rather than aborting the whole merge — mirroring the scanner path, where
+                # `plutil` may still accept a construct the tokenizer can't rewrite: it parses fine (so the file
+                # clears the `:text` gate above) yet `prefix_keys` raises on it. Fail soft: copy this file's lines
+                # through unprefixed rather than aborting the whole merge — mirroring the scanner path, where
                 # `scan_for_duplicate_keys` returns `:unscannable` instead of crashing the lane. `lines` is untouched
-                # by the raise (the assignment above never completes), so it still holds the original file contents.
+                # by the raise (the assignment above never completes), so it still holds the original file contents,
+                # and `applied_prefix` records that the keys went out *unprefixed* so the bookkeeping below matches.
+                applied_prefix = ''
                 UI.important("Could not add prefix `#{prefix}` to the keys in `#{input_file}` (#{e.message}); copying its lines through unprefixed.")
               end
+
+              # Bookkeep the keys as they were actually written — prefixed, or unprefixed on the fail-soft path.
+              # Doing this *after* the rewrite keeps the reported duplicates consistent with the merged file even
+              # when prefixing fell back, so a genuine collision is still surfaced rather than silently collapsed.
+              string_keys = raw_keys.map { |k| "#{applied_prefix}#{k}" }
+              duplicates += (string_keys & all_keys_found) # Find duplicates using Array intersection, and add those to duplicates list
+              all_keys_found += string_keys
+
               lines.each { |line| tmp_file.write(line) }
               tmp_file.write("\n")
             end

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

@@ -13,7 +13,8 @@ class StringsFileValidationHelper
         # `resume_context` holds the context to return to once a comment ends. Comments are valid not only at
         # the top level but also *between* the tokens of a statement (e.g. `"key" /* note */ = "value";`), so a
         # comment must resume the state it interrupted rather than always dropping back to `:root`.
-        State = Struct.new(:context, :buffer, :in_escaped_ctx, :found_key, :resume_context)
+        # `depth` tracks how deeply nested we are inside a container value (`{ … }` / `( … )`); see `:in_container_value`.
+        State = Struct.new(:context, :buffer, :in_escaped_ctx, :found_key, :resume_context, :depth)
 
         # Characters allowed in an *unquoted* string — a key or a value. Unquoted strings are valid
         # `.strings` syntax (the old-style ASCII property-list format) and are common in `InfoPlist.strings`
@@ -46,6 +47,31 @@ class StringsFileValidationHelper
           resume
         end
 
+        # A value can be a nested container — a dictionary `{ … }` or an array `( … )` — which `plutil` accepts
+        # (e.g. `"k" = { a = b; };` or `"k" = ( "a", "b" );`). We don't rewrite or record anything *inside* a
+        # container: its inner keys are not top-level keys, and `prefix_keys` copies the value through verbatim.
+        # We only need to find the matching close delimiter, so we just count nesting depth. `OPEN_CONTAINER`
+        # doubles as the entry transition from `:after_quoted_key_and_eq` and as the nested-open transition.
+        OPEN_CONTAINER = lambda do |state, _c|
+          state.depth += 1
+          :in_container_value
+        end
+
+        # Close one level of nesting. The value is only finished — and we go back to expecting the terminating
+        # `;` — once depth returns to 0; otherwise we're still inside an outer container.
+        CLOSE_CONTAINER = lambda do |state, _c|
+          state.depth -= 1
+          state.depth.zero? ? :after_quoted_value : :in_container_value
+        end
+
+        # A `/` inside a container may start a comment — whose body can contain `{ } ( ) ; "` that must NOT count
+        # toward nesting — or just be an ordinary value character (e.g. a path). Defer the decision one char,
+        # resuming the container in either case.
+        ENTER_CONTAINER_COMMENT = lambda do |state, _c|
+          state.resume_context = :in_container_value
+          :maybe_container_comment
+        end
+
         TRANSITIONS = {
           root: {
             /\s/u => :root,
@@ -120,6 +146,8 @@ class StringsFileValidationHelper
             '/' => ENTER_COMMENT_OR_VALUE,
             /\s/u => :after_quoted_key_and_eq,
             '"' => :in_quoted_value,
+            # A container value — a dictionary `{ … }` or an array `( … )`, which may nest (e.g. `"k" = { a = b; };`).
+            /[{(]/u => OPEN_CONTAINER,
             # An unquoted value, e.g. `CFBundleName = WordPress;` as used by `InfoPlist.strings`.
             UNQUOTED_STRING_CHARACTER => :in_unquoted_value
           },
@@ -139,6 +167,32 @@ class StringsFileValidationHelper
             '/' => ENTER_COMMENT,
             /\s/u => :after_quoted_value,
             ';' => :root
+          },
+          # Inside a container value (`{ … }` / `( … )`). We ignore the contents — inner keys aren't top-level
+          # keys and the value is copied verbatim by `prefix_keys` — and only track nesting so we can find the
+          # matching close. Quoted strings and comments are entered explicitly because their bodies can contain
+          # `{ } ( ) ;` that must not affect the depth count; everything else (`=`, `;`, `,`, whitespace, unquoted
+          # text, newlines) is consumed by the catch-all, which must stay LAST so the specific keys win first.
+          in_container_value: {
+            /[{(]/u => OPEN_CONTAINER,
+            /[})]/u => CLOSE_CONTAINER,
+            '"' => :in_container_quoted_string,
+            '/' => ENTER_CONTAINER_COMMENT,
+            /./mu => :in_container_value
+          },
+          # A quoted string inside a container. Skipped wholesale (its `{ } ( ) ; ,` are literal, not structural)
+          # until the closing quote returns us to the container. Escapes are handled globally (see the escape
+          # branch in `find_duplicated_keys`, whose allow-list includes this context).
+          in_container_quoted_string: {
+            '"' => :in_container_value,
+            /./mu => :in_container_quoted_string
+          },
+          # One char after a `/` inside a container: `*`/`/` confirm a comment (which resumes the container once
+          # it ends), anything else means the `/` was just a value character and we stay in the container.
+          maybe_container_comment: {
+            /\*/u => :in_block_comment,
+            '/' => :in_line_comment,
+            /./mu => :in_container_value
           }
         }.freeze
 
@@ -150,7 +204,7 @@ class StringsFileValidationHelper
         def self.find_duplicated_keys(file:)
           keys_with_lines = Hash.new { |h, k| h[k] = [] }
 
-          state = State.new(context: :root, buffer: StringIO.new, in_escaped_ctx: false, found_key: nil, resume_context: :root)
+          state = State.new(context: :root, buffer: StringIO.new, in_escaped_ctx: false, found_key: nil, resume_context: :root, depth: 0)
 
           # Using our `each_utf8_line` helper instead of `File.readlines` ensures we can also read files that are
           # encoded in UTF-16, yet process each of their lines as a UTF-8 string, so that `RegExp#match?` don't throw
@@ -161,7 +215,7 @@ def self.find_duplicated_keys(file:)
               # This is more straightforward than having to account for it in the `TRANSITIONS` table.
               if state.in_escaped_ctx || c == '\\'
                 # Just because we check for escaped characters at the global level, it doesn't mean we allow them in every context.
-                allowed_contexts_for_escaped_characters = %i[in_quoted_key in_quoted_value in_block_comment in_line_comment]
+                allowed_contexts_for_escaped_characters = %i[in_quoted_key in_quoted_value in_block_comment in_line_comment in_container_quoted_string]
                 raise "Found escaped character outside of allowed contexts on line #{line_no + 1} (current context: #{state.context})" unless allowed_contexts_for_escaped_characters.include?(state.context)
 
                 state.buffer.write(c) if state.context == :in_quoted_key
@@ -214,15 +268,17 @@ def self.scan_for_duplicate_keys(file:, assume_valid: false)
         # 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.
+        # alone — a distinction a line-based regex can't reliably make. It is likewise container-aware: a
+        # dictionary or array value (`"k" = { … };` / `"k" = ( … );`, nesting allowed) has only its outer key
+        # prefixed, with the value — including any keys *inside* it — copied through verbatim.
         #
         # @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)
+          state = State.new(context: :root, buffer: StringIO.new, in_escaped_ctx: false, found_key: nil, resume_context: :root, depth: 0)
           lines.map do |line|
             rewritten = +''
             line.each_char do |c|