From afdc275f756c47d2aab65c030a9ccce749b108e6 Mon Sep 17 00:00:00 2001
From: Axel Hecht <l10n>
Date: Sat, 9 Jan 2016 17:13:13 +0100
Subject: [PATCH] Bug 1237678 - update compare-locales to version 1.0.
 r=mathjazz changeset: 300933:aa4e0275bb3c tag: tip user: Axel Hecht
 <axel@pike.org> summary: bug 1237678, update compare-locales to version 1.0,
 r=mathjazz

---
 .../compare_locales/__init__.py               |  2 +-
 .../compare-locales/compare_locales/checks.py | 28 +++++++--
 .../compare_locales/compare.py                | 10 +--
 .../compare_locales/tests/test_checks.py      | 62 ++++++++++++++++++-
 4 files changed, 89 insertions(+), 13 deletions(-)

diff --git a/python/compare-locales/compare_locales/__init__.py b/python/compare-locales/compare_locales/__init__.py
index ca1d7fe37e6f..346c93fc5729 100644
--- a/python/compare-locales/compare_locales/__init__.py
+++ b/python/compare-locales/compare_locales/__init__.py
@@ -1 +1 @@
-version = "0.10.1a"
+version = "1.0"
diff --git a/python/compare-locales/compare_locales/checks.py b/python/compare-locales/compare_locales/checks.py
index a359a88ae587..ee3bef03ddf6 100644
--- a/python/compare-locales/compare_locales/checks.py
+++ b/python/compare-locales/compare_locales/checks.py
@@ -240,6 +240,7 @@ class DTDChecker(Checker):
         # find entities the refValue references,
         # reusing markup from DTDParser.
         reflist = self.known_entities(refValue)
+        inContext = self.entities_for_value(refValue)
         entities = ''.join('<!ENTITY %s "">' % s for s in sorted(reflist))
         parser = sax.make_parser()
         parser.setFeature(sax.handler.feature_external_ges, False)
@@ -262,9 +263,6 @@ class DTDChecker(Checker):
         l10nlist = self.entities_for_value(l10nValue)
         missing = sorted(l10nlist - reflist)
         _entities = entities + ''.join('<!ENTITY %s "">' % s for s in missing)
-        warntmpl = u'Referencing unknown entity `%s`'
-        if reflist:
-            warntmpl += ' (%s known)' % ', '.join(sorted(reflist))
         if self.processContent is not None:
             self.texthandler.textcontent = ''
             parser.setContentHandler(self.texthandler)
@@ -295,9 +293,29 @@ class DTDChecker(Checker):
                     col -= len("<!DOCTYPE elem [")  # first line is DOCTYPE
             yield ('error', (lnr, col), ' '.join(e.args), 'xmlparse')
 
+        warntmpl = u'Referencing unknown entity `%s`'
+        if reflist:
+            if inContext:
+                elsewhere = reflist - inContext
+                warntmpl += ' (%s used in context' % \
+                    ', '.join(sorted(inContext))
+                if elsewhere:
+                    warntmpl += ', %s known)' % ', '.join(sorted(elsewhere))
+                else:
+                    warntmpl += ')'
+            else:
+                warntmpl += ' (%s known)' % ', '.join(sorted(reflist))
         for key in missing:
             yield ('warning', (0, 0), warntmpl % key.decode('utf-8'),
                    'xmlparse')
+        if inContext and l10nlist and l10nlist - inContext - set(missing):
+            mismatch = sorted(l10nlist - inContext - set(missing))
+            for key in mismatch:
+                yield ('warning', (0, 0),
+                       'Entity %s referenced, but %s used in context' % (
+                           key.decode('utf-8'),
+                           ', '.join(sorted(inContext))
+                ), 'xmlparse')
 
         # Number check
         if self.num.match(refValue) and not self.num.match(l10nValue):
@@ -371,8 +389,8 @@ class PrincessAndroid(DTDChecker):
     def use(cls, file):
         """Use this Checker only for DTD files in embedding/android."""
         return (file.module in ("embedding/android",
-                                "mobile/android/base")
-                and cls.pattern.match(file.file))
+                                "mobile/android/base") and
+                cls.pattern.match(file.file))
 
     def processContent(self, val):
         """Actual check code.
diff --git a/python/compare-locales/compare_locales/compare.py b/python/compare-locales/compare_locales/compare.py
index 65acf1269480..602c94d4c796 100644
--- a/python/compare-locales/compare_locales/compare.py
+++ b/python/compare-locales/compare_locales/compare.py
@@ -276,8 +276,8 @@ class Observer(object):
                          for k in ('changed', 'unchanged', 'report', 'missing',
                                    'missingInFiles')
                          if k in summary])
-            rate = (('changed' in summary and summary['changed'] * 100)
-                    or 0) / total
+            rate = (('changed' in summary and summary['changed'] * 100) or
+                    0) / total
             item.update((k, summary.get(k, 0))
                         for k in ('changed', 'unchanged'))
             item.update((k, summary[k])
@@ -349,8 +349,8 @@ class Observer(object):
                          if k in summary])
             rate = 0
             if total:
-                rate = (('changed' in summary and summary['changed'] * 100)
-                        or 0) / total
+                rate = (('changed' in summary and summary['changed'] * 100) or
+                        0) / total
             out.append('%d%% of entries changed' % rate)
         return '\n'.join(map(tostr, self.details.getContent()) + out)
 
@@ -608,7 +608,7 @@ def compareApp(app, other_observer=None, merge_stage=None, clobber=False):
                 locale_merge = merge_stage.format(ab_CD=localization.locale)
                 comparer.set_merge_stage(locale_merge)
                 if clobber:
-                    # if clobber on, remove the stage for the module if it exists
+                    # if clobber, remove the stage for the module if it exists
                     clobberdir = os.path.join(locale_merge, module)
                     if os.path.exists(clobberdir):
                         shutil.rmtree(clobberdir)
diff --git a/python/compare-locales/compare_locales/tests/test_checks.py b/python/compare-locales/compare_locales/tests/test_checks.py
index 8ea934e9456d..b995d43f9000 100644
--- a/python/compare-locales/compare_locales/tests/test_checks.py
+++ b/python/compare-locales/compare_locales/tests/test_checks.py
@@ -19,13 +19,19 @@ class BaseHelper(unittest.TestCase):
         p.readContents(self.refContent)
         self.refList, self.refMap = p.parse()
 
-    def _test(self, content, refWarnOrErrors):
+    def _test(self, content, refWarnOrErrors, with_ref_file=False):
         p = getParser(self.file.file)
         p.readContents(content)
         l10n = [e for e in p]
         assert len(l10n) == 1
         l10n = l10n[0]
-        checker = getChecker(self.file)
+        if with_ref_file:
+            kwargs = {
+                'reference': self.refList
+            }
+        else:
+            kwargs = {}
+        checker = getChecker(self.file, **kwargs)
         ref = self.refList[self.refMap[l10n.key]]
         found = tuple(checker.check(ref, l10n))
         self.assertEqual(found, refWarnOrErrors)
@@ -169,6 +175,58 @@ stuff">
         self._test('''<!ENTITY ftd "0">''', tuple())
 
 
+class TestEntitiesInDTDs(BaseHelper):
+    file = File('foo.dtd', 'foo.dtd')
+    refContent = '''<!ENTITY short "This is &brandShortName;">
+<!ENTITY shorter "This is &brandShorterName;">
+<!ENTITY ent.start "Using &brandShorterName; start to">
+<!ENTITY ent.end " end">
+'''
+
+    def testOK(self):
+        self._test('''<!ENTITY ent.start "Mit &brandShorterName;">''', tuple(),
+                   with_ref_file=True)
+
+    def testMismatch(self):
+        self._test('''<!ENTITY ent.start "Mit &brandShortName;">''',
+                   (('warning', (0, 0),
+                     'Entity brandShortName referenced, '
+                     'but brandShorterName used in context',
+                     'xmlparse'),),
+                   with_ref_file=True)
+
+    def testAcross(self):
+        self._test('''<!ENTITY ent.end "Mit &brandShorterName;">''',
+                   tuple(),
+                   with_ref_file=True)
+
+    def testAcrossWithMismatch(self):
+        '''If we could tell that ent.start and ent.end are one string,
+        we should warn. Sadly, we can't, so this goes without warning.'''
+        self._test('''<!ENTITY ent.end "Mit &brandShortName;">''',
+                   tuple(),
+                   with_ref_file=True)
+
+    def testUnknownWithRef(self):
+        self._test('''<!ENTITY ent.start "Mit &foopy;">''',
+                   (('warning',
+                     (0, 0),
+                     'Referencing unknown entity `foopy` '
+                     '(brandShorterName used in context, '
+                     'brandShortName known)',
+                     'xmlparse'),),
+                   with_ref_file=True)
+
+    def testUnknown(self):
+        self._test('''<!ENTITY ent.end "Mit &foopy;">''',
+                   (('warning',
+                     (0, 0),
+                     'Referencing unknown entity `foopy`'
+                     ' (brandShortName, brandShorterName known)',
+                     'xmlparse'),),
+                   with_ref_file=True)
+
+
 class TestAndroid(unittest.TestCase):
     """Test Android checker