bug 672031, add check for plural forms, r=flod
authorAxel Hecht <axel@pike.org>
Fri, 08 Dec 2017 17:14:38 +0100
changeset 397 470deccb6926ebffac19dcbdd144ee9edf0a5c60
parent 396 26f02b5a618aeb0577e3981a5207d462d148146b
child 398 caffef49b18c6028c885e1d2c82e8c660efd601d
push id136
push useraxel@mozilla.com
push dateWed, 13 Dec 2017 15:36:22 +0000
reviewersflod
bugs672031
bug 672031, add check for plural forms, r=flod Use the list of forms in plurals.py to check if there's an entry for each of them. Make those warnings now, as they're likely bugs, but not hard failures. The actual `pluralForm` string in intl.properties is also triggering the plural form logic, so exclude that. Given that we don't have the file name in the checker, just ignore strings with that id and a number as value. MozReview-Commit-ID: E9GkdLredY0
compare_locales/checks.py
compare_locales/tests/test_checks.py
--- a/compare_locales/checks.py
+++ b/compare_locales/checks.py
@@ -7,16 +7,17 @@ from collections import Counter
 from difflib import SequenceMatcher
 from xml import sax
 try:
     from cStringIO import StringIO
 except ImportError:
     from StringIO import StringIO
 
 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
     # if a check uses all reference entities, set this to True
     needs_reference = False
@@ -67,18 +68,22 @@ class PropertiesChecker(Checker):
 
     def check(self, refEnt, l10nEnt):
         '''Test for the different variable formats.
         '''
         refValue, l10nValue = refEnt.val, l10nEnt.val
         refSpecs = None
         # check for PluralForm.jsm stuff, should have the docs in the
         # comment
+        # That also includes intl.properties' pluralRule, so exclude
+        # entities with that key and values with just numbers
         if (refEnt.pre_comment
-                and 'Localization_and_Plurals' in refEnt.pre_comment.all):
+                and 'Localization_and_Plurals' in refEnt.pre_comment.all
+                and refEnt.key != 'pluralRule'
+                and not re.match(r'\d+$', refValue)):
             for msg_tuple in self.check_plural(refValue, l10nValue):
                 yield msg_tuple
             return
         # check for lost escapes
         raw_val = l10nEnt.raw_val
         for m in PropertiesEntity.escape.finditer(raw_val):
             if m.group('single') and \
                m.group('single') not in PropertiesEntity.known_escapes:
@@ -93,16 +98,27 @@ class PropertiesChecker(Checker):
             for t in self.checkPrintf(refSpecs, l10nValue):
                 yield t
             return
 
     def check_plural(self, refValue, l10nValue):
         '''Check for the stringbundle plurals logic.
         The common variable pattern is #1.
         '''
+        if self.locale in plurals.CATEGORIES_BY_LOCALE:
+            expected_forms = len(plurals.CATEGORIES_BY_LOCALE[self.locale])
+            found_forms = l10nValue.count(';') + 1
+            msg = 'expecting {} plurals, found {}'.format(
+                expected_forms,
+                found_forms
+            )
+            if expected_forms > found_forms:
+                yield ('warning', 0, msg, 'plural')
+            if expected_forms < found_forms:
+                yield ('warning', 0, msg, 'plural')
         pats = set(int(m.group(1)) for m in re.finditer('#([0-9]+)',
                                                         refValue))
         if len(pats) == 0:
             return
         lpats = set(int(m.group(1)) for m in re.finditer('#([0-9]+)',
                                                          l10nValue))
         if pats - lpats:
             yield ('warning', 0, 'not all variables used in l10n',
--- a/compare_locales/tests/test_checks.py
+++ b/compare_locales/tests/test_checks.py
@@ -85,16 +85,45 @@ downloadsTitleFiles=#1 file - Downloads;
 # See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
 # #1 number of files
 # example: 111 files - Downloads
 downloadsTitleFiles=#1 file - Downloads;#1 files - #2;#1 #3
 ''',
                    (('error', 0, 'unreplaced variables in l10n', 'plural'),))
 
 
+class TestPluralForms(BaseHelper):
+    file = File('foo.properties', 'foo.properties', locale='en-GB')
+    refContent = '''\
+# LOCALIZATION NOTE (downloadsTitleFiles): Semi-colon list of plural forms.
+# See: http://developer.mozilla.org/en/docs/Localization_and_Plurals
+# #1 number of files
+# example: 111 files - Downloads
+downloadsTitleFiles=#1 file;#1 files
+'''
+
+    def test_matching_forms(self):
+        self._test('''\
+downloadsTitleFiles=#1 fiiilee;#1 fiiilees
+''',
+                   tuple())
+
+    def test_lacking_forms(self):
+        self._test('''\
+downloadsTitleFiles=#1 fiiilee
+''',
+                   (('warning', 0, 'expecting 2 plurals, found 1', 'plural'),))
+
+    def test_excess_forms(self):
+        self._test('''\
+downloadsTitleFiles=#1 fiiilee;#1 fiiilees;#1 fiiilees
+''',
+                   (('warning', 0, 'expecting 2 plurals, found 3', 'plural'),))
+
+
 class TestDTDs(BaseHelper):
     file = File('foo.dtd', 'foo.dtd')
     refContent = '''<!ENTITY foo "This is &apos;good&apos;">
 <!ENTITY width "10ch">
 <!ENTITY style "width: 20ch; height: 280px;">
 <!ENTITY minStyle "min-height: 50em;">
 <!ENTITY ftd "0">
 <!ENTITY formatPercent "This is 100&#037; correct">