From 28c8014ff05b569ed229b4cb2f77edb5869934ba Mon Sep 17 00:00:00 2001 From: Vivek Jain Date: Fri, 29 Jul 2016 23:08:22 +0530 Subject: [PATCH] Add hacking rule for explicit import of _ function Change-Id: Id0c1610f0b6353a43341f704e0b4b00bd04828bb Closes-bug: #1490194 --- HACKING.rst | 1 + magnum/hacking/checks.py | 29 +++++++++++++++++++++++++++++ magnum/tests/unit/test_hacking.py | 29 +++++++++++++++++++++++++++++ 3 files changed, 59 insertions(+) diff --git a/HACKING.rst b/HACKING.rst index 148ac1ba51..0a20239699 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -25,4 +25,5 @@ Magnum Specific Commandments with a sequence of key-value pairs. - [M338] Use assertIn/NotIn(A, B) rather than assertEqual(A in B, True/False). - [M339] Don't use xrange() +- [M340] Check for explicit import of the _ function. - [M352] LOG.warn is deprecated. Enforce use of LOG.warning. diff --git a/magnum/hacking/checks.py b/magnum/hacking/checks.py index f3e641b2a7..e5a5536ad1 100644 --- a/magnum/hacking/checks.py +++ b/magnum/hacking/checks.py @@ -29,6 +29,7 @@ Guidelines for writing new hacking checks - Add test cases for each new rule to magnum/tests/unit/test_hacking.py """ +UNDERSCORE_IMPORT_FILES = [] mutable_default_args = re.compile(r"^\s*def .+\((.+=\{\}|.+=\[\])") assert_equal_in_end_with_true_or_false_re = re.compile( @@ -55,6 +56,12 @@ assert_true_isinstance_re = re.compile( dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)") assert_xrange_re = re.compile( r"\s*xrange\s*\(") +custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*") +underscore_import_check = re.compile(r"(.)*import _(.)*") +translated_log = re.compile( + r"(.)*LOG\.(audit|error|info|critical|exception)" + "\(\s*_\(\s*('|\")") +string_translation = re.compile(r"[^_]*_\(\s*('|\")") def assert_equal_none(logical_line): @@ -175,6 +182,27 @@ def no_log_warn(logical_line): yield (0, msg) +def check_explicit_underscore_import(logical_line, filename): + """Check for explicit import of the _ function + + We need to ensure that any files that are using the _() function + to translate logs are explicitly importing the _ function. We + can't trust unit test to catch whether the import has been + added so we need to check for it here. + """ + + # Build a list of the files that have _ imported. No further + # checking needed once it is found. + if filename in UNDERSCORE_IMPORT_FILES: + pass + elif (underscore_import_check.match(logical_line) or + custom_underscore_check.match(logical_line)): + UNDERSCORE_IMPORT_FILES.append(filename) + elif (translated_log.match(logical_line) or + string_translation.match(logical_line)): + yield(0, "M340: Found use of _() without explicit import of _ !") + + def factory(register): register(no_mutable_default_args) register(assert_equal_none) @@ -187,3 +215,4 @@ def factory(register): register(dict_constructor_with_list_copy) register(no_xrange) register(no_log_warn) + register(check_explicit_underscore_import) diff --git a/magnum/tests/unit/test_hacking.py b/magnum/tests/unit/test_hacking.py index ea40ee86cb..e014b2ff94 100644 --- a/magnum/tests/unit/test_hacking.py +++ b/magnum/tests/unit/test_hacking.py @@ -258,3 +258,32 @@ class HackingTestCase(base.TestCase): self.assertEqual(0, len(list(checks.dict_constructor_with_list_copy( " self._render_dict(xml, data_el, data.__dict__)")))) + + def test_check_explicit_underscore_import(self): + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "LOG.info(_('My info message'))", + "magnum/tests/other_files.py"))), 1) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "msg = _('My message')", + "magnum/tests/other_files.py"))), 1) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "from magnum.i18n import _", + "magnum/tests/other_files.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "LOG.info(_('My info message'))", + "magnum/tests/other_files.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "msg = _('My message')", + "magnum/tests/other_files.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "from magnum.i18n import _, _LW", + "magnum/tests/other_files2.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "msg = _('My message')", + "magnum/tests/other_files2.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "_ = translations.ugettext", + "magnum/tests/other_files3.py"))), 0) + self.assertEqual(len(list(checks.check_explicit_underscore_import( + "msg = _('My message')", + "magnum/tests/other_files3.py"))), 0)