Bug 1321279 - Part 2 - Specify reference file in add_transforms.

Remove MergeContext.add_reference in favor of an additional argument to
MergeContext.add_transforms. This removes the redundancy of speficying
the destination path twice.

Author: stasm

fluent/migrate/context.py

@@ -2,6 +2,7 @@
 from __future__ import unicode_literals
 
 import os
+import sys
 import codecs
 import logging
 
@@ -109,22 +110,6 @@ def read_legacy_resource(self, path):
         # Transform the parsed result which is an iterator into a dict.
         return {entity.key: entity.val for entity in parser}
 
-    def add_reference(self, path, realpath=None):
-        """Add an FTL AST to this context's reference resources."""
-        fullpath = os.path.join(self.reference_dir, realpath or path)
-        try:
-            ast = self.read_ftl_resource(fullpath)
-        except IOError as err:
-            error_message = 'Missing reference file: {}'.format(fullpath)
-            logging.getLogger('migrate').error(error_message)
-            raise UnreadableReferenceError(error_message)
-        except UnicodeDecodeError as err:
-            error_message = 'Error reading file {}: {}'.format(fullpath, err)
-            logging.getLogger('migrate').error(error_message)
-            raise UnreadableReferenceError(error_message)
-        else:
-            self.reference_resources[path] = ast
-
     def maybe_add_localization(self, path):
         """Add a localization resource to migrate translations from.
 
@@ -150,8 +135,8 @@ def maybe_add_localization(self, path):
         else:
             self.localization_resources[path] = collection
 
-    def add_transforms(self, path, transforms):
-        """Define transforms for path.
+    def add_transforms(self, path, reference, transforms):
+        """Define transforms for path using reference as template.
 
         Each transform is an extended FTL node with `Transform` nodes as some
         values.  Transforms are stored in their lazy AST form until
@@ -166,6 +151,22 @@ def get_sources(acc, cur):
                 acc.add((cur.path, cur.key))
             return acc
 
+        refpath = os.path.join(self.reference_dir, reference)
+        try:
+            ast = self.read_ftl_resource(refpath)
+        except IOError as err:
+            error_message = 'Missing reference file: {}'.format(refpath)
+            logging.getLogger('migrate').error(error_message)
+            raise UnreadableReferenceError(error_message)
+        except UnicodeDecodeError as err:
+            error_message = 'Error reading file {}: {}'.format(refpath, err)
+            logging.getLogger('migrate').error(error_message)
+            raise UnreadableReferenceError(error_message)
+        else:
+            # The reference file will be used by the merge function as
+            # a template for serializing the merge results.
+            self.reference_resources[path] = ast
+
         for node in transforms:
             # Scan `node` for `Source` nodes and collect the information they
             # store into a set of dependencies.

tests/migrate/test_context.py

@@ -26,15 +26,14 @@ def setUp(self):
             localization_dir=here('fixtures/pl')
         )
 
-        self.ctx.add_reference('aboutDownloads.ftl')
         try:
             self.ctx.maybe_add_localization('aboutDownloads.dtd')
             self.ctx.maybe_add_localization('aboutDownloads.properties')
         except RuntimeError:
             self.skipTest('compare-locales required')
 
     def test_hardcoded_node(self):
-        self.ctx.add_transforms('aboutDownloads.ftl', [
+        self.ctx.add_transforms('aboutDownloads.ftl', 'aboutDownloads.ftl', [
             FTL.Message(
                 id=FTL.Identifier('about'),
                 value=FTL.Pattern([
@@ -59,7 +58,7 @@ def test_hardcoded_node(self):
         )
 
     def test_merge_single_message(self):
-        self.ctx.add_transforms('aboutDownloads.ftl', [
+        self.ctx.add_transforms('aboutDownloads.ftl', 'aboutDownloads.ftl', [
             FTL.Message(
                 id=FTL.Identifier('title'),
                 value=COPY(
@@ -85,7 +84,7 @@ def test_merge_single_message(self):
         )
 
     def test_merge_one_changeset(self):
-        self.ctx.add_transforms('aboutDownloads.ftl', [
+        self.ctx.add_transforms('aboutDownloads.ftl', 'aboutDownloads.ftl', [
             FTL.Message(
                 id=FTL.Identifier('title'),
                 value=COPY(
@@ -124,7 +123,7 @@ def test_merge_one_changeset(self):
         )
 
     def test_merge_two_changesets(self):
-        self.ctx.add_transforms('aboutDownloads.ftl', [
+        self.ctx.add_transforms('aboutDownloads.ftl', 'aboutDownloads.ftl', [
             FTL.Message(
                 id=FTL.Identifier('title'),
                 value=COPY(
@@ -177,7 +176,7 @@ def test_merge_two_changesets(self):
         self.assertDictEqual(merged_b, expected_b)
 
     def test_serialize_changeset(self):
-        self.ctx.add_transforms('aboutDownloads.ftl', [
+        self.ctx.add_transforms('aboutDownloads.ftl', 'aboutDownloads.ftl', [
             FTL.Message(
                 id=FTL.Identifier('title'),
                 value=COPY(
@@ -247,7 +246,7 @@ def tearDown(self):
 
     def test_missing_reference_file(self):
         with self.assertRaises(UnreadableReferenceError):
-            self.ctx.add_reference('missing.ftl')
+            self.ctx.add_transforms('some.ftl', 'missing.ftl', [])
 
 
 class TestIncompleteLocalization(unittest.TestCase):
@@ -261,13 +260,12 @@ def setUp(self):
             localization_dir=here('fixtures/pl')
         )
 
-        self.ctx.add_reference('toolbar.ftl')
         try:
             self.ctx.maybe_add_localization('browser.dtd')
         except RuntimeError:
             self.skipTest('compare-locales required')
 
-        self.ctx.add_transforms('toolbar.ftl', [
+        self.ctx.add_transforms('toolbar.ftl', 'toolbar.ftl', [
             FTL.Message(
                 id=FTL.Identifier('urlbar-textbox'),
                 attributes=[
@@ -323,7 +321,6 @@ def setUp(self):
             localization_dir=here('fixtures/pl')
         )
 
-        self.ctx.add_reference('privacy.ftl')
         try:
             self.ctx.maybe_add_localization('privacy.dtd')
         except RuntimeError:
@@ -334,7 +331,7 @@ def tearDown(self):
         logging.disable(logging.NOTSET)
 
     def test_existing_target_ftl_missing_string(self):
-        self.ctx.add_transforms('privacy.ftl', [
+        self.ctx.add_transforms('privacy.ftl', 'privacy.ftl', [
             FTL.Message(
                 id=FTL.Identifier('dnt-learn-more'),
                 value=COPY(
@@ -362,7 +359,7 @@ def test_existing_target_ftl_missing_string(self):
         )
 
     def test_existing_target_ftl_existing_string(self):
-        self.ctx.add_transforms('privacy.ftl', [
+        self.ctx.add_transforms('privacy.ftl', 'privacy.ftl', [
             FTL.Message(
                 id=FTL.Identifier('dnt-description'),
                 value=COPY(
@@ -401,7 +398,7 @@ def test_existing_target_ftl_existing_string(self):
         )
 
     def test_existing_target_ftl_with_all_messages(self):
-        self.ctx.add_transforms('privacy.ftl', [
+        self.ctx.add_transforms('privacy.ftl', 'privacy.ftl', [
             FTL.Message(
                 id=FTL.Identifier('dnt-description'),
                 value=COPY(
@@ -429,5 +426,4 @@ def test_add_ftl(self):
                 localization_dir=here('fixtures/pl')
             )
 
-            ctx.add_reference('privacy.ftl')
             ctx.maybe_add_localization('privacy.ftl')

tests/migrate/test_context_real_examples.py

@@ -27,14 +27,13 @@ def setUp(self):
             localization_dir=here('fixtures/pl')
         )
 
-        self.ctx.add_reference('aboutDownloads.ftl')
         try:
             self.ctx.maybe_add_localization('aboutDownloads.dtd')
             self.ctx.maybe_add_localization('aboutDownloads.properties')
         except RuntimeError:
             self.skipTest('compare-locales required')
 
-        self.ctx.add_transforms('aboutDownloads.ftl', [
+        self.ctx.add_transforms('aboutDownloads.ftl', 'aboutDownloads.ftl', [
             FTL.Message(
                 id=FTL.Identifier('title'),
                 value=COPY(
@@ -289,12 +288,11 @@ def setUp(self):
         )
 
         try:
-            self.ctx.add_reference('aboutDialog.ftl')
             self.ctx.maybe_add_localization('aboutDialog.dtd')
         except RuntimeError:
             self.skipTest('compare-locales required')
 
-        self.ctx.add_transforms('aboutDialog.ftl', [
+        self.ctx.add_transforms('aboutDialog.ftl', 'aboutDialog.ftl', [
             FTL.Message(
                 id=FTL.Identifier('update-failed'),
                 value=CONCAT(

tools/migrate/examples/about_dialog.py

@@ -9,10 +9,9 @@
 def migrate(ctx):
     """Migrate about:dialog, part {index}"""
 
-    ctx.add_reference('browser/about_dialog.ftl', realpath='about_dialog.ftl')
     ctx.maybe_add_localization('browser/chrome/browser/aboutDialog.dtd')
 
-    ctx.add_transforms('browser/about_dialog.ftl', [
+    ctx.add_transforms('browser/about_dialog.ftl', 'about_dialog.ftl', [
         FTL.Message(
             id=FTL.Identifier('update-failed'),
             value=CONCAT(

tools/migrate/examples/about_downloads.py

@@ -7,16 +7,12 @@
 def migrate(ctx):
     """Migrate about:download in Firefox for Android, part {index}"""
 
-    ctx.add_reference(
-        'mobile/about_downloads.ftl',
-        realpath='about_downloads.ftl'
-    )
     ctx.maybe_add_localization(
         'mobile/android/chrome/aboutDownloads.dtd')
     ctx.maybe_add_localization(
         'mobile/android/chrome/aboutDownloads.properties')
 
-    ctx.add_transforms('mobile/about_downloads.ftl', [
+    ctx.add_transforms('mobile/about_downloads.ftl', 'about_downloads.ftl', [
         FTL.Message(
             id=FTL.Identifier('title'),
             value=COPY(

tools/migrate/examples/bug_1291693.py

@@ -7,25 +7,12 @@
 def migrate(ctx):
     """Bug 1291693 - Migrate the menubar to FTL, part {index}"""
 
-    ctx.add_reference(
-        'browser/menubar.ftl',
-        realpath='menubar.ftl'
-    )
-    ctx.add_reference(
-        'browser/toolbar.ftl',
-        realpath='toolbar.ftl'
-    )
-    ctx.add_reference(
-        'browser/branding/official/brand.ftl',
-        realpath='brand.ftl'
-    )
-
     ctx.maybe_add_localization('browser/chrome/browser/browser.dtd')
     ctx.maybe_add_localization('browser/chrome/browser/browser.properties')
     ctx.maybe_add_localization('browser/branding/official/brand.dtd')
     ctx.maybe_add_localization('browser/branding/official/brand.properties')
 
-    ctx.add_transforms('browser/menubar.ftl', [
+    ctx.add_transforms('browser/menubar.ftl', 'menubar.ftl', [
         FTL.Message(
             id=FTL.Identifier('file-menu'),
             attributes=[
@@ -1795,7 +1782,7 @@ def migrate(ctx):
         ),
     ])
 
-    ctx.add_transforms('browser/toolbar.ftl', [
+    ctx.add_transforms('browser/toolbar.ftl', 'toolbar.ftl', [
         FTL.Message(
             id=FTL.Identifier('urlbar-textbox'),
             attributes=[
@@ -1889,7 +1876,7 @@ def migrate(ctx):
         ),
     ])
 
-    ctx.add_transforms('browser/branding/official/brand.ftl', [
+    ctx.add_transforms('browser/branding/official/brand.ftl', 'brand.ftl', [
         FTL.Message(
             id=FTL.Identifier('brand-shorter-name'),
             value=COPY(