[ruby] Preserve source data and inherited plural output

lib/twine/formatters/abstract.rb

@@ -150,7 +150,7 @@ def format_section(section, lang)
 
       def format_definition(definition, lang)
         formatted_definition = [format_comment(definition, lang)]
-        if self.class::SUPPORTS_PLURAL && definition.is_plural?
+        if self.class::SUPPORTS_PLURAL && definition.plural_translation_for_lang(lang)
           formatted_definition << format_plural(definition, lang)
         else
           formatted_definition << format_key_value(definition, lang)

lib/twine/formatters/apple.rb

@@ -102,7 +102,7 @@ def format_value(value)
       end
 
       def should_include_definition(definition, lang)
-        return !definition.is_plural? && super
+        return !definition.plural_translation_for_lang(lang) && super
       end
     end
   end

lib/twine/formatters/apple_plural.rb

@@ -18,12 +18,12 @@ def default_file_name
       end
 
       def format_footer(lang)
-        footer = "</dict>\n</plist>"
+        "</dict>\n</plist>"
       end
 
       def format_file(lang)
         result = super
-        result += format_footer(lang)
+        result + format_footer(lang) if result
       end
 
       def format_header(lang)
@@ -157,7 +157,7 @@ def read(io, lang)
       end
 
       def should_include_definition(definition, lang)
-        return definition.is_plural? && definition.plural_translation_for_lang(lang)
+        return definition.plural_translation_for_lang(lang)
       end
     end
   end

lib/twine/formatters/gettext.rb

@@ -48,7 +48,7 @@ def read(io, lang)
       end
 
       def format_file(lang)
-        @default_lang = twine_file.language_codes[0]
+        @default_lang = @options[:developer_language] || twine_file.language_codes[0]
         result = super
         @default_lang = nil
         result
@@ -80,7 +80,7 @@ def format_key(key)
       end
 
       def format_base_translation(definition)
-        "msgid \"#{escape_quotes(definition.translations[@default_lang])}\"\n"
+        "msgid \"#{escape_quotes(definition.translation_for_lang(@default_lang))}\"\n"
       end
 
       def format_value(value)

lib/twine/output_processor.rb

@@ -2,6 +2,14 @@ module Twine
   module Processors
 
     class OutputProcessor
+      FALLBACK_MAPPING = {
+        'zh-CN' => 'zh-Hans',
+        'zh-TW' => 'zh-Hant',
+        'zh-HK' => 'zh-Hant',
+        'zh-MO' => 'zh-Hant'
+      }.freeze
+      REGIONAL_LANGUAGE = /\A([a-zA-Z]{2,3})-[a-zA-Z]+/
+
       def initialize(twine_file, options)
         @twine_file = twine_file
         @options = options
@@ -12,49 +20,51 @@ def default_language
       end
 
       def fallback_languages(language)
-        fallback_mapping = {
-          'zh-CN' => 'zh-Hans', # if we don't have a zh-CN translation, try zh-Hans before en
-          'zh-TW' => 'zh-Hant' # if we don't have a zh-TW translation, try zh-Hant before en
-        }
-
-        # Regional dialect fallbacks to generic language (for example: 'es-MX' to 'es' instead of default 'en').
-        if language.match(/([a-zA-Z]{2})-[a-zA-Z]+/)
-          generic_language = language.gsub(/([a-zA-Z])-[a-zA-Z]+/, '\1')
-        end
-
-        [fallback_mapping[language], generic_language, default_language].flatten.compact
+        match = REGIONAL_LANGUAGE.match(language)
+        [FALLBACK_MAPPING[language], match && match[1], default_language]
+          .compact.uniq.reject { |candidate| candidate == language }
       end
 
       def process(language)
         result = TwineFile.new
+        fallbacks = fallback_languages(language)
+        include_option = @options[:include]
+        tags = @options[:tags]
+        untagged = @options[:untagged]
 
         result.language_codes.concat @twine_file.language_codes
         @twine_file.sections.each do |section|
           new_section = TwineSection.new section.name
 
           section.definitions.each do |definition|
-            next unless definition.matches_tags?(@options[:tags], @options[:untagged])
+            next unless definition.matches_tags?(tags, untagged)
 
             value = definition.translation_for_lang(language)
+            has_translation = !value.nil?
 
-            next if value && @options[:include] == :untranslated
+            next if has_translation && include_option == :untranslated
 
-            if value.nil? && @options[:include] != :translated
-              value = definition.translation_for_lang(fallback_languages(language))
+            if !has_translation && include_option != :translated
+              value = definition.translation_for_lang(fallbacks)
             end
 
             next unless value
 
             new_definition = definition.dup
             new_definition.translations[language] = value
 
-            if definition.is_plural?
-              # If definition is plural, but no translation found -> create
-              # Then check 'other' key
-              if !(new_definition.plural_translations[language] ||= {}).key? 'other'
-                new_definition.plural_translations[language]['other'] = value
+            forms = definition.plural_translation_for_lang(language)
+            if forms.nil? && !has_translation && include_option != :translated
+              fallbacks.each do |fallback|
+                forms = definition.plural_translation_for_lang(fallback)
+                break unless forms.nil?
               end
             end
+            if forms
+              forms['other'] = value unless forms.key?('other')
+              new_definition.plural_translations[language] = forms
+              new_definition.translations[language] = forms['other']
+            end
 
             new_section.definitions << new_definition
             result.definitions_by_key[new_definition.key] = new_definition

lib/twine/runner.rb

@@ -236,6 +236,7 @@ def consume_localization_archive
     end
 
     def validate_twine_file
+      @twine_file.validate_references
       total_definitions = 0
       all_keys = Set.new
       duplicate_keys = Set.new

lib/twine/twine_file.rb

@@ -58,12 +58,25 @@ def translation_for_lang(lang)
 
     def plural_translation_for_lang(lang)
       if @plural_translations.has_key? lang
-        @plural_translations[lang].dup.sort_by { |key,_| TwineDefinition::PLURAL_KEYS.index(key) }.to_h
+        @plural_translations[lang].sort_by { |key,_| TwineDefinition::PLURAL_KEYS.index(key) }.to_h
+      elsif !@translations.key?(lang) && reference
+        # An explicit singular value overrides inherited plural forms.
+        reference.plural_translation_for_lang(lang)
       end
     end
 
     def is_plural?
-      !@plural_translations.empty?
+      !@plural_translations.empty? || (reference ? reference.is_plural? : false)
+    end
+
+    private
+
+    def initialize_copy(other)
+      super
+      # Output processing adds fallbacks, so each copy owns its mutable maps.
+      @translations = other.translations.dup
+      @plural_translations = other.plural_translations.transform_values(&:dup)
+      @tags = other.tags.dup if other.tags
     end
   end
 
@@ -194,10 +207,30 @@ def read(path)
         end
       end
 
-      # resolve_references
       @definitions_by_key.each do |key, definition|
         next unless definition.reference_key
-        definition.reference = @definitions_by_key[definition.reference_key]
+        definition.reference = @definitions_by_key.fetch(definition.reference_key) do
+          raise Twine::Error, "Unknown reference '#{definition.reference_key}' in [#{key}]"
+        end
+      end
+      validate_references
+    end
+
+    # Check the graph once before inherited lookups traverse it.
+    def validate_references
+      finished = {}
+      @definitions_by_key.each_value do |definition|
+        current = definition
+        path = {}
+        while current && !finished.key?(current)
+          raise Twine::Error, "Circular reference at [#{current.key}]" if path.key?(current)
+          if current.reference_key && current.reference.nil?
+            raise Twine::Error, "Unknown reference '#{current.reference_key}' in [#{current.key}]"
+          end
+          path[current] = true
+          current = current.reference
+        end
+        finished.merge!(path)
       end
     end
 

test/test_reference_generation.rb

@@ -0,0 +1,175 @@
+require 'twine_test'
+
+class TestReferenceGeneration < TwineTest
+  PLURALS = "[count]\nen:one = %d bridge\nen:other = %d bridges\n" \
+            "fr:one = %d pont\nfr:other = %d ponts\n" \
+            "[middle]\nref = count\n[alias]\nref = middle\n"
+
+  def read_twine(text)
+    path = File.join(@output_dir, 'strings.txt')
+    File.write(path, text)
+    result = Twine::TwineFile.new
+    result.read(path)
+    result
+  end
+
+  def format_file(twine_file, formatter_type, language, options = {})
+    formatter = formatter_type.new
+    formatter.twine_file = twine_file
+    formatter.options = { include: :all }.merge(options)
+    formatter.format_file(language)
+  end
+
+  def test_generated_fallbacks_do_not_mutate_source_or_depend_on_order
+    base = "[base]\nen = Bridge\n"
+    derived = "[alias]\nref = base\nen = Pedestrian Bridge\n"
+    [base + derived, derived + base].each do |text|
+      twine_file = read_twine(text)
+      before = Marshal.dump(twine_file)
+      processor = Twine::Processors::OutputProcessor.new(twine_file, include: :all)
+      result = processor.process('fr-CA')
+      assert_equal 'Pedestrian Bridge', result.definitions_by_key['alias'].translations['fr-CA']
+      assert_equal before, Marshal.dump(twine_file)
+      assert_equal result.definitions_by_key['alias'].translations, processor.process('fr-CA').definitions_by_key['alias'].translations
+    end
+  end
+
+  def test_copied_plural_maps_are_independent
+    twine_file = read_twine(PLURALS)
+    source = twine_file.definitions_by_key['count']
+    result = Twine::Processors::OutputProcessor.new(twine_file, {}).process('fr')
+    result.definitions_by_key['count'].plural_translations['fr']['one'] = 'changed'
+    assert_equal '%d pont', source.plural_translations['fr']['one']
+  end
+
+  def test_plural_completion_does_not_add_source_forms
+    twine_file = read_twine(
+      "[count]\nen:one = %d bridge\nen:other = %d bridges\nfr:one = %d pont\n"
+    )
+    result = Twine::Processors::OutputProcessor.new(twine_file, {}).process('fr')
+    assert_equal({ 'one' => '%d pont', 'other' => '%d bridges' },
+                 result.definitions_by_key['count'].plural_translations['fr'])
+    assert_equal({ 'one' => '%d pont' }, twine_file.definitions_by_key['count'].plural_translations['fr'])
+  end
+
+  def test_android_plural_aliases_keep_all_forms
+    %w(fr fr-CA).each do |language|
+      twine_file = read_twine(PLURALS)
+      output = format_file(twine_file, Twine::Formatters::Android, language)
+      root = REXML::Document.new(output).root
+      %w(count middle alias).each do |key|
+        plural = root.elements["plurals[@name='#{key}']"]
+        refute_nil plural
+        assert_equal '%d pont', plural.elements["item[@quantity='one']"].text
+        assert_equal '%d ponts', plural.elements["item[@quantity='other']"].text
+      end
+      assert_empty twine_file.definitions_by_key['alias'].plural_translations
+    end
+  end
+
+  def test_apple_routes_plural_aliases_to_stringsdict
+    twine_file = read_twine(PLURALS)
+    regular = format_file(twine_file, Twine::Formatters::Apple, 'fr-CA')
+    assert_empty regular.strip
+    output = format_file(twine_file, Twine::Formatters::ApplePlural, 'fr-CA')
+    root = REXML::Document.new(output).elements['plist/dict']
+    keys = root.elements.to_a('key')
+    assert_equal %w(count middle alias), keys.map(&:text)
+    keys.each do |key|
+      forms = key.next_element.elements['dict']
+      values = forms.elements.to_a('key').to_h { |entry| [entry.text, entry.next_element.text] }
+      assert_equal '%d pont', values['one']
+      assert_equal '%d ponts', values['other']
+    end
+  end
+
+  def test_singular_values_override_inherited_plural_type
+    [:all, :translated].each do |include_option|
+      ['Passerelle', ''].each do |value|
+        twine_file = read_twine(PLURALS + "fr = #{value}\n")
+        output = format_file(twine_file, Twine::Formatters::Android, 'fr', include: include_option)
+        root = REXML::Document.new(output).root
+        node = root.elements["string[@name='alias']"]
+        refute_nil node
+        assert_equal value, node.text.to_s
+        assert_nil root.elements["plurals[@name='alias']"]
+      end
+    end
+  end
+
+  def test_empty_values_count_as_translated
+    [:all, :translated, :untranslated].each do |include_option|
+      twine_file = read_twine("[base]\nen = Bridge\nfr = \n[alias]\nref = base\n")
+      result = Twine::Processors::OutputProcessor.new(twine_file, include: include_option).process('fr')
+      if include_option == :untranslated
+        assert_empty result.definitions_by_key
+      else
+        assert_equal %w(base alias), result.definitions_by_key.keys
+        result.definitions_by_key.each_value { |definition| assert_equal '', definition.translations['fr'] }
+      end
+    end
+  end
+
+  def test_chinese_and_three_letter_fallbacks
+    expected = {
+      'zh-CN' => %w(zh-Hans zh en),
+      'zh-HK' => %w(zh-Hant zh en),
+      'zh-MO' => %w(zh-Hant zh en),
+      'yue-HK' => %w(yue en),
+      'yue-MO' => %w(yue en),
+      'ast-ES' => %w(ast en),
+      'en' => []
+    }
+    twine_file = read_twine("[base]\nen = Bridge\nzh-Hant = 橋\n")
+    processor = Twine::Processors::OutputProcessor.new(twine_file, {})
+    expected.each { |language, fallbacks| assert_equal fallbacks, processor.fallback_languages(language) }
+    result = processor.process('zh-HK')
+    assert_equal '橋', result.definitions_by_key['base'].translations['zh-HK']
+  end
+
+  def test_gettext_resolves_inherited_developer_text
+    twine_file = read_twine(
+      "[base]\nen = Bridge\nfr = Pont\n[middle]\nref = base\n[alias]\nref = middle\n"
+    )
+    output = format_file(twine_file, Twine::Formatters::Gettext, 'fr')
+    assert_includes output, "msgctxt \"alias\"\nmsgid \"Bridge\"\nmsgstr \"Pont\""
+  end
+
+  def test_gettext_honors_developer_language_option
+    twine_file = read_twine("[base]\nen = Bridge\nfr = Pont\n")
+    output = format_file(twine_file, Twine::Formatters::Gettext, 'en', developer_language: 'fr')
+    assert_includes output, "msgid \"Pont\"\nmsgstr \"Bridge\""
+  end
+
+  def test_empty_plural_output_is_skippable
+    assert_nil format_file(Twine::TwineFile.new, Twine::Formatters::ApplePlural, 'fr')
+  end
+
+  def test_read_rejects_dangling_and_circular_references
+    cases = [
+      ["[alias]\nen = Alias\nref = absent\n", "Unknown reference 'absent'"],
+      ["[alias]\nen = Alias\nref = alias\n", 'Circular reference'],
+      ["[a]\nref = b\n[b]\nref = a\n", 'Circular reference']
+    ]
+    cases.each do |text, message|
+      error = assert_raises(Twine::Error) { read_twine(text) }
+      assert_includes error.message, message
+    end
+  end
+
+  def test_forward_and_shared_references_are_valid
+    twine_file = read_twine(
+      "[alias]\nref = middle\n[other]\nref = middle\n[middle]\nref = base\n[base]\nen = Bridge\n"
+    )
+    assert_equal 'Bridge', twine_file.definitions_by_key['alias'].translation_for_lang('en')
+    assert_equal 'Bridge', twine_file.definitions_by_key['other'].translation_for_lang('en')
+  end
+
+  def test_validation_rejects_a_mutated_cycle
+    twine_file = read_twine("[base]\nen = Bridge\n[alias]\nref = base\n")
+    definition = twine_file.definitions_by_key['alias']
+    definition.reference = definition
+    error = assert_raises(Twine::Error) { Twine::Runner.new({}, twine_file).validate_twine_file }
+    assert_includes error.message, 'Circular reference'
+  end
+end