Allow auto-addition of tools:ignore="UnusedResources" when merging Android strings

[AliSoftware]

This PR adds the ability for the an_localize_libs action (which merges strings.xml files from libraries into the main one in order to send their strings to translation) to automatically add tools:ignore="UnusedResources" to the merged strings when appropriate. — as discussed in Slack and Zoom with @ParaskP7

Merging strings from binary libraries

When merging strings.xml files from binary dependencies (typically precompiled and downloaded from S3), since the linter is unable to detect when a string it sees merged in the app's strings.xml is used in the binary library we pulled it from, we need to add tools:ignore="UnusedResources" to that merged string, to ensure lint won't complain about it.

Note: If we want to detect strings that are actually unused even in the binary libraries themselves, we would rely on running lint in the repos of the binary libraries themselves, while this PR is only about adding tools:ignore="UnusedResources" on the strings after they are merged in the app's strings.xml, not from a point of view of the libraries.

Merging strings from local modules

When merging strings.xml files from local modules (like image-editor or WordPress-Editor), because we currently have the checkDependencies true option enabled for lint in the WordPress/build.gradle, that means that running lint on the WordPress project will detect when strings merged in the strings.xml of the app that are in fact used by the module, and will then not consider them unused.

So for that case, the linter complaining that a string merged from one of our local module is unused would be a genuine warning that would indeed warrant manual investigation(†)
Thus, for those local modules, and at least as long as we have checkDependencies true set, we should NOT make our tooling add tools:ignore="UnusedResources" automatically for all the strings merged from those modules.

(†) A warning in such a case would not always necessarily mean that the string is indeed unused and could be deleted blindly from that module without priori investigation, because that module could itself declare that string… to override the value of one of its transitive dependency… so we should still investigate such cases manually, and decide to either delete the genuinely unused string from the module, or add tools:ignore="UnusedResources" to that string manually if we consider it's expected.

Solution

This PR allows the caller of an_localize_libs to specify, for each lib, if it should add the tools:ignore="UnusedResources" attribute automatically when merging the libs' strings or not — via the new add_ignore_attr: true option.

The implementation made an_localize_libs smart enough to also support cases where a tools:ignore attribute would already exist on the string being merged, either in the app's strings.xml or in the lib's one, and merge the UnusedResources value with the existing value of the attribute.

To Test

• Read the unit tests (specs) carefully to ensure that the expected = […] values I used in each of the unit test would indeed correspond to what we really expect from such a merge and configuration of the action
• Ensure that bundle exec rspec passes (this one is already run by CI, so checking that CI is green should be enough)

[AliSoftware]

@mokagio CI seems to fail on this error, which is not relateed to the changes from this PR, but rather on the helper to find duplicated strings that we recently merged:

1) FastlaneCore::Helper::Ios::StringsFileValidationHelper when there are no duplicated keys returns an empty array
--
  | Failure/Error: raise "Invalid character `#{c}` found on line #{line_no + 1}, col #{col_no + 1}" if next_context.nil?
  |  
  | RuntimeError:
  | Invalid character `I` found on line 21, col 1

No idea why it would fail in this PR (and on my Mac)… but didn't in your PR adding that helper and those specs in the first place… See also my comment on #342 (comment)

Can I let you take a closer look during the week? (I'll be AFK until next Tuesday)

[AliSoftware]

@mokagio CI seems to fail on this error, which is not related to the changes from this PR, but rather on the helper to find duplicated strings that we recently merged:

1) FastlaneCore::Helper::Ios::StringsFileValidationHelper when there are no duplicated keys returns an empty array
--
  | Failure/Error: raise "Invalid character `#{c}` found on line #{line_no + 1}, col #{col_no + 1}" if next_context.nil?
  |  
  | RuntimeError:
  | Invalid character `I` found on line 21, col 1

No idea why it would fail in this PR (and on my Mac)… but didn't in your PR adding that helper and those specs in the first place… See also my comment on #342 (comment)

Can I let you take a closer look during the week? (I'll be AFK until next Tuesday)

Ended up tackling this on my own today via #357

[mokagio]

LGTM 👍

Once the unit tests fix from #357 will land in this PR, CI will be green and this will be merge-able.

@AliSoftware since I didn't get to address the test failure and the current merge conflicts are due to my work with #356, I'd be happy to return the favor and solve them here. Let me know.

[mokagio]

I'm glad to see you took on my suggestion to expand the test 🙇‍♂️

[AliSoftware]

@AliSoftware since I didn't get to address the test failure and the current merge conflicts are due to my work with #356, I'd be happy to return the favor and solve them here. Let me know.

I'll gladly take you up on that offer 🙂

[AliSoftware]

@AliSoftware since I didn't get to address the test failure and the current merge conflicts are due to my work with #356, I'd be happy to return the favor and solve them here. Let me know.

I'll gladly take you up on that offer 🙂

Change of plans, I ended up doing the conflict resolution myself as it was otherwise blocking me to move forward with working on #358 (which had to be built on top of this one to avoid conflicts between the 2 PRs making different improvements but on the same action and specs)

[AliSoftware]

This will actually already be addressed by #358 (which builds on top of this PR) 🎉