Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 4 commits into from
Apr 20, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@ _None_
* Add the option for `an_localize_libs` to provide a `source_id` for each library being merged.
If provided, that identifier will be added as an `a8c-src-lib` XML attribute to the `<string>` nodes being updated with strings from said library.
This can be useful to help identify where each string come from in the resulting, merged `strings.xml`. [#351]
* Add the option for `an_localize_libs` to set the `tools:ignore="UnusedResources"` XML attribute for each string being merged from a library. [#354]

### Bug Fixes

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,12 @@ def self.details

def self.available_options
libs_hash_description = <<~KEYS
- `:library`: The library display name
- `:strings_path`: The path to the `strings.xml` file of the library
- `:exclusions`: An optional `Array` of string keys to exclude from merging
- `:library`: The library display name.
- `:strings_path`: The path to the `strings.xml` file of the library.
- `:exclusions`: An optional `Array` of string keys to exclude from merging.
- `:source_id`: An optional `String` which will be added as the `a8c-src-lib` XML attribute
to strings coming from this library, to help identify their source in the merged file.
- `:add_ignore_attr`: If set to true, will add `tools:ignore="UnusedResources"` to merged strings.
KEYS
[
FastlaneCore::ConfigItem.new(key: :app_strings_path,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,20 @@ def self.skip_string_by_exclusion_list(library, string_name)
end
end

# Adds the appropriate XML attributes to an XML `<string>` node according to library configuration
def self.add_xml_attributes!(string_node, library)
if library[:add_ignore_attr] == true
existing_ignores = (string_node['tools:ignore'] || '').split(',')
existing_ignores.append('UnusedResources') unless existing_ignores.include?('UnusedResources')
string_node['tools:ignore'] = existing_ignores.join(',')
end
string_node[LIB_SOURCE_XML_ATTR] = library[:source_id] unless library[:source_id].nil?
end

# Merge string_line into main_string
def self.merge_string(main_strings, library, string_line)
string_name = string_line.attr('name')
string_content = string_line.content
lib_src_id = library[:source_id]

# Skip strings in the exclusions list
return :skipped if skip_string_by_exclusion_list(library, string_name)
Expand All @@ -61,14 +70,14 @@ def self.merge_string(main_strings, library, string_line)
else
# It has the tools:ignore flag, so update the content without touching the other attributes
this_string.content = string_content
this_string[LIB_SOURCE_XML_ATTR] = lib_src_id unless lib_src_id.nil?
add_xml_attributes!(this_string, library)
return result
end
end
end

# String not found, or removed because needing update and not in the exclusion list: add to the main file
string_line[LIB_SOURCE_XML_ATTR] = lib_src_id unless lib_src_id.nil?
add_xml_attributes!(string_line, library)
main_strings.xpath('//string').last().add_next_sibling("\n#{' ' * 4}#{string_line.to_xml().strip}")
return result
end
Expand Down Expand Up @@ -103,8 +112,14 @@ def self.verify_string(main_strings, library, string_line)
# @param [Hash] library Hash describing the library to merge. The Hash should contain the following keys:
# - `:library`: The human readable name of the library, used to display in console messages
# - `:strings_path`: The path to the strings.xml file of the library to merge into the main one
# - `:exclusions`: An array of strings keys to exclude during merge. Any of those keys from the library's `strings.xml` will be skipped and won't be merged into the main one.
# - `:exclusions`: An array of strings keys to exclude during merge. Any of those keys from the
# library's `strings.xml` will be skipped and won't be merged into the main one.
# - `:source_id`: An optional `String` which will be added as the `a8c-src-lib` XML attribute
# to strings coming from this library, to help identify their source in the merged file.
# - `:add_ignore_attr`: If set to `true`, will add `tools:ignore="UnusedResources"` to merged strings.
#
# @return [Boolean] True if at least one string from the library has been added to (or has updated) the main strings file.
#
def self.merge_lib(main, library)
UI.message("Merging #{library[:library]} strings into #{main}")
main_strings = File.open(main) { |f| Nokogiri::XML(f, nil, Encoding::UTF_8.to_s) }
Expand Down
259 changes: 217 additions & 42 deletions spec/an_localize_libs_action_spec.rb
Original file line number Diff line number Diff line change
@@ -1,48 +1,223 @@
require 'spec_helper'

describe Fastlane::Actions::AnLocalizeLibsAction do
# This test is more of a way of ensuring `run_described_fastlane_action` handles array of hashes properly than a comprehensive test for the `an_localize_libs_action` action.
#
# Please consider expanding this test if you'll find yourself working on its action.
it 'merges the strings from the given array into the given main strings file' do
in_tmp_dir do |tmp_dir|
app_strings_path = File.join(tmp_dir, 'app.xml')
File.write(app_strings_path, android_xml_with_content('<string name="a_string">test from app</string>'))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
File.write(lib1_strings_path, android_xml_with_content('<string name="a_lib1_string">test from lib 1</string>'))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
File.write(lib2_strings_path, android_xml_with_content('<string name="a_lib2_string">test from lib 2</string>'))

run_described_fastlane_action(
app_strings_path: app_strings_path,
libs_strings_path: [
{ library: 'lib_1', strings_path: lib1_strings_path, exclusions: [] },
{ library: 'lib_2', strings_path: lib2_strings_path, exclusions: [] },
]
)

expected = <<~XML
<string name="a_string">test from app</string>
<string name="a_lib1_string">test from lib 1</string>
<string name="a_lib2_string">test from lib 2</string>
XML
expect(File.read(app_strings_path)).to eq(android_xml_with_content(expected))
end
def android_xml_with_lines(lines)
# I couldn't find a way to interpolate a multiline string preserving its indentation in the heredoc below, so I hacked the following transformation of the input that adds the desired indentation to all lines.
#
# The desired indentation is 4 spaces to stay aligned with the production code applies when merging the XMLs.
indented_content = lines.map { |l| " #{l.chomp}" }.join("\n")

return <<~XML
<?xml version="1.0" encoding="UTF-8"?>
<resources xmlns:tools="http://schemas.android.com/tools">
#{indented_content}
</resources>
XML
end
end

def android_xml_with_content(content)
# I couldn't find a way to interpolate a multiline string preserving its indentation in the heredoc below, so I hacked the following transformation of the input that adds the desired indentation to all lines.
#
# The desired indentation is 4 spaces to stay aligned with the production code applies when merging the XMLs.
indented_content = content.chomp.lines.map { |l| " #{l}" }.join

return <<~XML
<?xml version="1.0" encoding="UTF-8"?>
<resources xmlns:tools="http://schemas.android.com/tools">
#{indented_content}
</resources>
XML
def write_android_xml(path, lines)
File.write(path, android_xml_with_lines(lines))
end

describe 'merges the strings from the given array into the given main strings file' do
it 'handles simple XMLs with no duplicates nor attributes' do
in_tmp_dir do |tmp_dir|
app_strings_path = File.join(tmp_dir, 'app.xml')
File.write(app_strings_path, android_xml_with_lines(['<string name="a_string">test from app</string>']))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
File.write(lib1_strings_path, android_xml_with_lines(['<string name="a_lib1_string">test from lib 1</string>']))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
File.write(lib2_strings_path, android_xml_with_lines(['<string name="a_lib2_string">test from lib 2</string>']))

run_described_fastlane_action(
app_strings_path: app_strings_path,
libs_strings_path: [
{ library: 'lib_1', strings_path: lib1_strings_path, exclusions: [] },
{ library: 'lib_2', strings_path: lib2_strings_path, exclusions: [] },
]
)

expected = [
'<string name="a_string">test from app</string>',
'<string name="a_lib1_string">test from lib 1</string>',
'<string name="a_lib2_string">test from lib 2</string>',
]
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
end
end

it 'keeps app value if content_override is used' do
in_tmp_dir do |tmp_dir|
app_strings_path = File.join(tmp_dir, 'app.xml')
app_xml_lines = [
'<string name="override-true" content_override="true">from app override-true</string>',
'<string name="override-false" content_override="false">from app override-false</string>',
'<string name="override-missing">from app override-missing</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib_strings_path = File.join(tmp_dir, 'lib.xml')
lib_xml_lines = [
'<string name="override-true">from lib override-true</string>',
'<string name="override-false">from lib override-false</string>',
'<string name="override-missing">from lib override-missing</string>',
]
File.write(lib_strings_path, android_xml_with_lines(lib_xml_lines))

run_described_fastlane_action(
app_strings_path: app_strings_path,
libs_strings_path: [
{ library: 'lib', strings_path: lib_strings_path, exclusions: [] },
]
)

expected = [
'<string name="override-true" content_override="true">from app override-true</string>',
'', '', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

'<string name="override-false">from lib override-false</string>',
'<string name="override-missing">from lib override-missing</string>',
]
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
end
end

it 'adds a8c-lib-src attribute if provided' do
in_tmp_dir do |tmp_dir|
app_strings_path = File.join(tmp_dir, 'app.xml')
app_xml_lines = [
'<string name="override-true" content_override="true">from app override-true</string>',
'<string name="override-missing">from app override-missing</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
lib1_xml_lines = [
'<string name="override-true">from lib1 override-true</string>',
'<string name="override-missing">from lib1 override-missing</string>',
'<string name="lib1-key">Key only present in lib1</string>',
]
File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
lib2_xml_lines = [
'<string name="override-true">from lib2 override-true</string>',
'<string name="override-missing">from lib2 override-missing</string>',
'<string name="lib2-key">Key only present in lib2</string>',
]
File.write(lib2_strings_path, android_xml_with_lines(lib2_xml_lines))

run_described_fastlane_action(
app_strings_path: app_strings_path,
libs_strings_path: [
{ library: 'lib1', strings_path: lib1_strings_path, source_id: 'lib1-id' },
{ library: 'lib2', strings_path: lib2_strings_path, source_id: 'lib2-id' },
]
)

expected = [
'<string name="override-true" content_override="true">from app override-true</string>',
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
'<string name="lib1-key" a8c-src-lib="lib1-id">Key only present in lib1</string>',
'<string name="override-missing" a8c-src-lib="lib2-id">from lib2 override-missing</string>',
'<string name="lib2-key" a8c-src-lib="lib2-id">Key only present in lib2</string>',
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
]
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
end
end

it 'adds tools:ignore attribute when requested' do
in_tmp_dir do |tmp_dir|
app_strings_path = File.join(tmp_dir, 'app.xml')
app_xml_lines = [
'<string name="override-true" content_override="true">from app, override true</string>',
'<string name="ignore-unused" tools:ignore="UnusedResources">from app, tools:ignore="UnusedResources"</string>',
'<string name="ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">from app, tools:ignore mix</string>',
'<string name="ignore-x-y" tools:ignore="X,Y">from app, tools:ignore mix</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib1_strings_path = File.join(tmp_dir, 'lib1.xml')
lib1_xml_lines = [
'<string name="override-true">from lib1, override true</string>',
'<string name="lib1-key">Key only present in lib1, no extra attribute</string>',
'<string name="lib1-ignore-unused" tools:ignore="UnusedResources">Key only present in lib1, with tools:ignore attribute</string>',
'<string name="lib1-ignore-x-y" tools:ignore="X,Y">Key only present in lib1, with tools:ignore attribute x,y</string>',
'<string name="lib1-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">Key only present in lib1, with tools:ignore attribute x,y</string>',
]
File.write(lib1_strings_path, android_xml_with_lines(lib1_xml_lines))

lib2_strings_path = File.join(tmp_dir, 'lib2.xml')
lib2_xml_lines = [
'<string name="override-true">from lib2, override true</string>',
'<string name="lib2-key">Key only present in lib2, no extra attribute</string>',
'<string name="lib2-ignore-unused" tools:ignore="UnusedResources">Key only present in lib2, with tools:ignore attribute</string>',
'<string name="lib2-ignore-x-y" tools:ignore="X,Y">Key only present in lib2, with tools:ignore attribute x,y</string>',
'<string name="lib2-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">Key only present in lib2, with tools:ignore attribute x,y</string>',
]
File.write(lib2_strings_path, android_xml_with_lines(lib2_xml_lines))

run_described_fastlane_action(
app_strings_path: app_strings_path,
libs_strings_path: [
{ library: 'lib1', strings_path: lib1_strings_path, source_id: 'lib1', add_ignore_attr: true },
{ library: 'lib2', strings_path: lib2_strings_path, source_id: 'lib2' },
]
)

expected = [
'<string name="override-true" content_override="true">from app, override true</string>',
'<string name="ignore-unused" tools:ignore="UnusedResources">from app, tools:ignore="UnusedResources"</string>',
'<string name="ignore-x-unused-y" tools:ignore="X,UnusedResources,Y">from app, tools:ignore mix</string>',
'<string name="ignore-x-y" tools:ignore="X,Y">from app, tools:ignore mix</string>',
'<string name="lib1-key" tools:ignore="UnusedResources" a8c-src-lib="lib1">Key only present in lib1, no extra attribute</string>',
'<string name="lib1-ignore-unused" tools:ignore="UnusedResources" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute</string>',
'<string name="lib1-ignore-x-y" tools:ignore="X,Y,UnusedResources" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute x,y</string>',
'<string name="lib1-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y" a8c-src-lib="lib1">Key only present in lib1, with tools:ignore attribute x,y</string>',
'<string name="lib2-key" a8c-src-lib="lib2">Key only present in lib2, no extra attribute</string>',
'<string name="lib2-ignore-unused" tools:ignore="UnusedResources" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute</string>',
'<string name="lib2-ignore-x-y" tools:ignore="X,Y" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute x,y</string>',
'<string name="lib2-ignore-x-unused-y" tools:ignore="X,UnusedResources,Y" a8c-src-lib="lib2">Key only present in lib2, with tools:ignore attribute x,y</string>',
]
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
end
end

it 'handles exclusions list per library' do
in_tmp_dir do |tmp_dir|
app_strings_path = File.join(tmp_dir, 'app.xml')
app_xml_lines = [
'<string name="override-true" content_override="true">from app override-true</string>',
'<string name="override-false" content_override="false">from app override-false</string>',
'<string name="override-missing">from app override-missing</string>',
]
File.write(app_strings_path, android_xml_with_lines(app_xml_lines))

lib_strings_path = File.join(tmp_dir, 'lib.xml')
lib_xml_lines = [
'<string name="override-true">from lib override-true</string>',
'<string name="override-false">from lib override-false</string>',
'<string name="override-missing">from lib override-missing</string>',
]
File.write(lib_strings_path, android_xml_with_lines(lib_xml_lines))

run_described_fastlane_action(
app_strings_path: app_strings_path,
libs_strings_path: [
{ library: 'lib', strings_path: lib_strings_path, exclusions: ['override-missing'] },
]
)

expected = [
'<string name="override-true" content_override="true">from app override-true</string>',
'', # FIXME: Current implementation adds empty lines; we should get rid of those at some point
'<string name="override-missing">from app override-missing</string>',
'<string name="override-false">from lib override-false</string>',
]
expect(File.read(app_strings_path)).to eq(android_xml_with_lines(expected))
end
end
end
end