bug 1385234, add reference entity list to checkers after creation, r=jotes
Switch the API on getChecker to allow the reference entity list to be
avoided. Instead add a flag on the Checker class that asks for it
to be set.
This allows external users to inspect this, and use compare-locales
checks more efficiently.
MozReview-Commit-ID: 6oTbfJxVKBo
--- a/compare_locales/checks.py
+++ b/compare_locales/checks.py
@@ -13,36 +13,45 @@ except ImportError:
from compare_locales.parser import DTDParser, PropertiesEntity
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
@classmethod
def use(cls, file):
return cls.pattern.match(file.file)
def __init__(self, extra_tests):
self.extra_tests = extra_tests
+ self.reference = None
def check(self, refEnt, l10nEnt):
'''Given the reference and localized Entities, performs checks.
This is a generator yielding tuples of
- "warning" or "error", depending on what should be reported,
- tuple of line, column info for the error within the string
- description string to be shown in the report
'''
if True:
raise NotImplementedError("Need to subclass")
yield ("error", (0, 0), "This is an example error", "example")
+ def set_reference(self, reference):
+ '''Set the reference entities.
+ Only do this if self.needs_reference is True.
+ '''
+ self.reference = reference
+
class PrintfException(Exception):
def __init__(self, msg, pos):
self.pos = pos
self.msg = msg
class PropertiesChecker(Checker):
@@ -183,29 +192,29 @@ class DTDChecker(Checker):
The code tries to parse until it doesn't find any unresolved entities
anymore. If it finds one, it tries to grab the key, and adds an empty
<!ENTITY key ""> definition to the header.
Also checks for some CSS and number heuristics in the values.
"""
pattern = re.compile('.*\.dtd$')
+ needs_reference = True # to cast a wider net for known entity references
eref = re.compile('&(%s);' % DTDParser.Name)
tmpl = '''<!DOCTYPE elem [%s]>
<elem>%s</elem>
'''
xmllist = set(('amp', 'lt', 'gt', 'apos', 'quot'))
- def __init__(self, extra_tests, reference):
+ def __init__(self, extra_tests):
super(DTDChecker, self).__init__(extra_tests)
self.processContent = False
if self.extra_tests is not None and 'android-dtd' in self.extra_tests:
self.processContent = True
- self.reference = reference
self.__known_entities = None
def known_entities(self, refValue):
if self.__known_entities is None and self.reference is not None:
self.__known_entities = set()
for ent in self.reference:
self.__known_entities.update(self.entities_for_value(ent.val))
return self.__known_entities if self.__known_entities is not None \
@@ -475,16 +484,16 @@ class FluentChecker(Checker):
for attr in l10n_entry.attributes
if attr.id.name in obsolete_attr_names
]
for attr in obsolete_attrs:
yield ('error', attr.span.start,
'Obsolete attribute: ' + attr.id.name, 'fluent')
-def getChecker(file, reference=None, extra_tests=None):
+def getChecker(file, extra_tests=None):
if PropertiesChecker.use(file):
return PropertiesChecker(extra_tests)
if DTDChecker.use(file):
- return DTDChecker(extra_tests, reference)
+ return DTDChecker(extra_tests)
if FluentChecker.use(file):
return FluentChecker(extra_tests)
return None
--- a/compare_locales/compare.py
+++ b/compare_locales/compare.py
@@ -463,18 +463,19 @@ class ContentComparer:
ar = AddRemove()
ar.set_left(e.key for e in ref_entities)
ar.set_right(e.key for e in l10n_entities)
report = missing = obsolete = changed = unchanged = keys = 0
missing_w = changed_w = unchanged_w = 0 # word stats
missings = []
skips = []
- checker = getChecker(l10n,
- reference=ref_entities, extra_tests=extra_tests)
+ checker = getChecker(l10n, extra_tests=extra_tests)
+ if checker and checker.needs_reference:
+ checker.set_reference(ref_entities)
for msg in p.findDuplicates(ref_entities):
self.notify('warning', l10n, msg)
for msg in p.findDuplicates(l10n_entities):
self.notify('error', l10n, msg)
for action, entity_id in ar:
if action == 'delete':
# missing entity
if isinstance(ref_entities[ref_map[entity_id]], parser.Junk):
--- a/compare_locales/tests/test_checks.py
+++ b/compare_locales/tests/test_checks.py
@@ -14,29 +14,25 @@ class BaseHelper(unittest.TestCase):
file = None
refContent = None
def setUp(self):
p = getParser(self.file.file)
p.readContents(self.refContent)
self.refList, self.refMap = p.parse()
- def _test(self, content, refWarnOrErrors, with_ref_file=False):
+ def _test(self, content, refWarnOrErrors):
p = getParser(self.file.file)
p.readContents(content)
l10n = [e for e in p]
assert len(l10n) == 1
l10n = l10n[0]
- if with_ref_file:
- kwargs = {
- 'reference': self.refList
- }
- else:
- kwargs = {}
- checker = getChecker(self.file, **kwargs)
+ checker = getChecker(self.file)
+ if checker.needs_reference:
+ checker.set_reference(self.refList)
ref = self.refList[self.refMap[l10n.key]]
found = tuple(checker.check(ref, l10n))
self.assertEqual(found, refWarnOrErrors)
class TestProperties(BaseHelper):
file = File('foo.properties', 'foo.properties')
refContent = '''some = value
@@ -179,57 +175,51 @@ 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)
+ self._test('''<!ENTITY ent.start "Mit &brandShorterName;">''', tuple())
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)
+ 'xmlparse'),))
def testAcross(self):
self._test('''<!ENTITY ent.end "Mit &brandShorterName;">''',
- tuple(),
- with_ref_file=True)
+ tuple())
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)
+ tuple())
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)
+ 'xmlparse'),))
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)
+ 'xmlparse'),))
class TestAndroid(unittest.TestCase):
"""Test Android checker
Make sure we're hitting our extra rules only if
we're passing in a DTD file in the embedding/android module.
"""
@@ -377,17 +367,18 @@ class TestAndroid(unittest.TestCase):
self.assertEqual(tuple(checker.check(ref, l10n)),
())
def test_entities_across_dtd(self):
f = File("browser/strings.dtd", "strings.dtd", "browser")
p = getParser(f.file)
p.readContents('<!ENTITY other "some &good.ref;">')
ref = p.parse()
- checker = getChecker(f, reference=ref[0])
+ checker = getChecker(f)
+ checker.set_reference(ref[0])
# good string
ref = self.getDTDEntity("plain string")
l10n = self.getDTDEntity("plain localized string")
self.assertEqual(tuple(checker.check(ref, l10n)),
())
# dtd warning
ref = self.getDTDEntity("plain string")
l10n = self.getDTDEntity("plain localized string &ref;")