Bug 1376747 - Check for message references that don't match en-US. r=Pike
authorStaś Małolepszy <stas@mozilla.com>
Thu, 18 Jan 2018 12:03:08 +0100
changeset 410 229648855a49091fb5fbb9b1162f2313e18388b0
parent 407 2f8dd53809786eb017de6ec6cfbbbf967a385fc2
child 411 47656ee19b7624bee053576e1121f0c8fe917057
push id139
push useraxel@mozilla.com
push dateThu, 18 Jan 2018 20:10:22 +0000
reviewersPike
bugs1376747
Bug 1376747 - Check for message references that don't match en-US. r=Pike MozReview-Commit-ID: Kv7jj3BiAwg
compare_locales/checks.py
compare_locales/tests/test_merge.py
--- a/compare_locales/checks.py
+++ b/compare_locales/checks.py
@@ -6,16 +6,18 @@ import re
 from collections import Counter
 from difflib import SequenceMatcher
 from xml import sax
 try:
     from cStringIO import StringIO
 except ImportError:
     from StringIO import StringIO
 
+from fluent.syntax import ast as ftl
+
 from compare_locales.parser import DTDParser, PropertiesEntity
 from compare_locales import plurals
 
 
 class Checker(object):
     '''Abstract class to implement checks per file type.
     '''
     pattern = None
@@ -452,26 +454,61 @@ class DTDChecker(Checker):
                 yield ('error', m.end(0)+offset, msg, 'android')
 
 
 class FluentChecker(Checker):
     '''Tests to run on Fluent (FTL) files.
     '''
     pattern = re.compile('.*\.ftl')
 
+    def find_message_references(self, entry):
+        refs = {}
+
+        def collect_message_references(node):
+            if isinstance(node, ftl.MessageReference):
+                # The key is the name of the referenced message and it will
+                # be used in set algebra to find missing and obsolete
+                # references. The value is the node itself and its span
+                # will be used to pinpoint the error.
+                refs[node.id.name] = node
+            # BaseNode.traverse expects this function to return the node.
+            return node
+
+        entry.traverse(collect_message_references)
+        return refs
+
     def check(self, refEnt, l10nEnt):
         ref_entry = refEnt.entry
         l10n_entry = l10nEnt.entry
         # verify that values match, either both have a value or none
         if ref_entry.value is not None and l10n_entry.value is None:
             yield ('error', 0, 'Missing value', 'fluent')
         if ref_entry.value is None and l10n_entry.value is not None:
             offset = l10n_entry.value.span.start - l10n_entry.span.start
             yield ('error', offset, 'Obsolete value', 'fluent')
 
+        # verify that message references are the same
+        ref_msg_refs = self.find_message_references(ref_entry)
+        l10n_msg_refs = self.find_message_references(l10n_entry)
+
+        # create unique sets of message names referenced in both entries
+        ref_msg_refs_names = set(ref_msg_refs.keys())
+        l10n_msg_refs_names = set(l10n_msg_refs.keys())
+
+        missing_msg_ref_names = ref_msg_refs_names - l10n_msg_refs_names
+        for msg_name in missing_msg_ref_names:
+            yield ('warning', 0, 'Missing message reference: ' + msg_name,
+                   'fluent')
+
+        obsolete_msg_ref_names = l10n_msg_refs_names - ref_msg_refs_names
+        for msg_name in obsolete_msg_ref_names:
+            pos = l10n_msg_refs[msg_name].span.start - l10n_entry.span.start
+            yield ('warning', pos, 'Obsolete message reference: ' + msg_name,
+                   'fluent')
+
         # verify that we're having the same set of attributes
         ref_attr_names = set((attr.id.name for attr in ref_entry.attributes))
         ref_pos = dict((attr.id.name, i)
                        for i, attr in enumerate(ref_entry.attributes))
         l10n_attr_counts = \
             Counter(attr.id.name for attr in l10n_entry.attributes)
         l10n_attr_names = set(l10n_attr_counts)
         l10n_pos = dict((attr.id.name, i)
--- a/compare_locales/tests/test_merge.py
+++ b/compare_locales/tests/test_merge.py
@@ -512,16 +512,102 @@ eff = lEff {
         merged_foo = merged_entities[merged_map['foo']]
 
         # foo should be l10n
         p.readFile(self.l10n)
         l10n_entities, l10n_map = p.parse()
         l10n_foo = l10n_entities[l10n_map['foo']]
         self.assertTrue(merged_foo.equals(l10n_foo))
 
+    def testMatchingReferences(self):
+        self.reference("""\
+foo = Reference { bar }
+""")
+        self.localized("""\
+foo = Localized { bar }
+""")
+        cc = ContentComparer([Observer()])
+        cc.compare(File(self.ref, "en-reference.ftl", ""),
+                   File(self.l10n, "l10n.ftl", ""),
+                   mozpath.join(self.tmp, "merge", "l10n.ftl"))
+
+        self.assertDictEqual(
+            cc.observers[0].toJSON(),
+            {
+                'details': {},
+                'summary': {
+                    None: {
+                        'changed': 1,
+                        'changed_w': 1
+                    }
+                }
+            }
+        )
+
+        # validate merge results
+        mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
+        self.assert_(not os.path.exists(mergepath))
+
+    def testMismatchingReferences(self):
+        self.reference("""\
+foo = Reference { bar }
+bar = Reference { baz }
+baz = Reference
+""")
+        self.localized("""\
+foo = Localized { qux }
+bar = Localized
+baz = Localized { qux }
+""")
+        cc = ContentComparer([Observer()])
+        cc.compare(File(self.ref, "en-reference.ftl", ""),
+                   File(self.l10n, "l10n.ftl", ""),
+                   mozpath.join(self.tmp, "merge", "l10n.ftl"))
+
+        self.assertDictEqual(
+            cc.observers[0].toJSON(),
+            {
+                'details': {
+                    'l10n.ftl': [
+                            {
+                                'warning':
+                                    u'Missing message reference: bar '
+                                    u'at line 1, column 1 for foo'
+                            },
+                            {
+                                'warning':
+                                    u'Obsolete message reference: qux '
+                                    u'at line 1, column 19 for foo'
+                            },
+                            {
+                                'warning':
+                                    u'Missing message reference: baz '
+                                    u'at line 2, column 1 for bar'
+                            },
+                            {
+                                'warning':
+                                    u'Obsolete message reference: qux '
+                                    u'at line 3, column 19 for baz'
+                            },
+                    ],
+                },
+                'summary': {
+                    None: {
+                        'changed': 3,
+                        'changed_w': 3,
+                        'warnings': 4
+                    }
+                }
+            }
+        )
+
+        # validate merge results
+        mergepath = mozpath.join(self.tmp, "merge", "l10n.ftl")
+        self.assert_(not os.path.exists(mergepath))
+
     def testMismatchingAttributes(self):
         self.reference("""
 foo = Foo
 bar = Bar
   .tender = Attribute value
 eff = Eff
 """)
         self.localized("""\