Merge "Remove log translation related check"

This commit is contained in:
Jenkins 2017-04-07 17:42:18 +00:00 committed by Gerrit Code Review
commit 2948a004d6
3 changed files with 0 additions and 170 deletions

View File

@ -12,11 +12,7 @@ Barbican Specific Commandments
- [B310] Check for improper use of logging format arguments.
- [B311] Use assertIsNone(...) instead of assertEqual(None, ...).
- [B312] Use assertTrue(...) rather than assertEqual(True, ...).
- [B313] Validate that debug level logs are not translated.
- [B314] str() and unicode() cannot be used on an exception. Remove or use six.text_type().
- [B315] Translated messages cannot be concatenated. String should be
included in translated message.
- [B316] Log messages, except debug ones, require translations!
- [B317] 'oslo_' should be used instead of 'oslo.'
- [B318] Must use a dict comprehension instead of a dict constructor
with a sequence of key-value pairs.
@ -25,31 +21,6 @@ Barbican Specific Commandments
- [B321] Use assertIsNotNone(...) rather than assertNotEqual(None, ...) or
assertIsNot(None, ...).
LOG Translations
----------------
LOG.debug messages will not get translated. Use ``_LI()`` for
``LOG.info``, ``_LW`` for ``LOG.warning``, ``_LE`` for ``LOG.error``
and ``LOG.exception``, and ``_LC()`` for ``LOG.critical``.
``_()`` is preferred for any user facing message, even if it is also
going to a log file. This ensures that the translated version of the
message will be available to the user.
The log marker functions (``_LI()``, ``_LW()``, ``_LE()``, and ``_LC()``)
must only be used when the message is only sent directly to the log.
Anytime that the message will be passed outside of the current context
(for example as part of an exception) the ``_()`` marker function
must be used.
A common pattern is to define a single message object and use it more
than once, for the log call and the exception. In that case, ``_()``
must be used because the message is going to appear in an exception that
may be presented to the user.
For more details about translations, see
http://docs.openstack.org/developer/oslo.i18n/guidelines.html
Creating Unit Tests
-------------------
For every new feature, unit tests should be created that both test and

View File

@ -34,26 +34,6 @@ Guidelines for writing new hacking checks
"""
UNDERSCORE_IMPORT_FILES = []
log_translation = re.compile(
r"(.)*LOG\.(audit|error|info|critical|exception)\(\s*('|\")")
log_translation_LC = re.compile(
r"(.)*LOG\.(critical)\(\s*(_\(|'|\")")
log_translation_LE = re.compile(
r"(.)*LOG\.(error|exception)\(\s*(_\(|'|\")")
log_translation_LI = re.compile(
r"(.)*LOG\.(info)\(\s*(_\(|'|\")")
log_translation_LW = re.compile(
r"(.)*LOG\.(warning|warn)\(\s*(_\(|'|\")")
translated_log = re.compile(
r"(.)*LOG\.(audit|error|info|warn|warning|critical|exception)"
"\(\s*_\(\s*('|\")")
string_translation = re.compile(r"[^_]*_\(\s*('|\")")
underscore_import_check = re.compile(r"(.)*import _$")
underscore_import_check_multi = re.compile(r"(.)*import (.)*_, (.)*")
# We need this for cases where they have created their own _ function.
custom_underscore_check = re.compile(r"(.)*_\s*=\s*(.)*")
oslo_namespace_imports = re.compile(r"from[\s]*oslo[.](.*)")
dict_constructor_with_list_copy_re = re.compile(r".*\bdict\((\[)?(\(|\[)")
assert_no_xrange_re = re.compile(r"\s*xrange\s*\(")
@ -109,23 +89,6 @@ class BaseASTChecker(ast.NodeVisitor):
return False
def no_translate_debug_logs(logical_line, filename):
"""Check for 'LOG.debug(u._('
As per our translation policy,
https://wiki.openstack.org/wiki/LoggingStandards#Log_Translation
we shouldn't translate debug level logs.
* This check assumes that 'LOG' is a logger.
* Use filename so we can start enforcing this in specific folders instead
of needing to do so all at once.
B313
"""
if logical_line.startswith("LOG.debug(u._("):
yield(0, "B313 Don't translate debug level logs")
class CheckLoggingFormatArgs(BaseASTChecker):
"""Check for improper use of logging format arguments.
@ -191,30 +154,6 @@ class CheckLoggingFormatArgs(BaseASTChecker):
return super(CheckLoggingFormatArgs, self).generic_visit(node)
def validate_log_translations(logical_line, physical_line, filename):
# Translations are not required in the test directories.
if ("barbican/tests" in filename or "functional" in filename):
return
if pep8.noqa(physical_line):
return
msg = "B316: LOG.critical messages require translations `_LC()`!"
if log_translation_LC.match(logical_line):
yield (0, msg)
msg = ("B316: LOG.error and LOG.exception messages require translations "
"`_LE()`!")
if log_translation_LE.match(logical_line):
yield (0, msg)
msg = "B316: LOG.info messages require translations `_LI()`!"
if log_translation_LI.match(logical_line):
yield (0, msg)
msg = "B316: LOG.warning messages require translations `_LW()`!"
if log_translation_LW.match(logical_line):
yield (0, msg)
msg = "B316: Log messages require translations!"
if log_translation.match(logical_line):
yield (0, msg)
class CheckForStrUnicodeExc(BaseASTChecker):
"""Checks for the use of str() or unicode() on an exception.
@ -262,28 +201,6 @@ class CheckForStrUnicodeExc(BaseASTChecker):
super(CheckForStrUnicodeExc, self).generic_visit(node)
class CheckForTransAdd(BaseASTChecker):
"""Checks for the use of concatenation on a translated string.
Translations should not be concatenated with other strings, but
should instead include the string being added to the translated
string to give the translators the most information.
"""
CHECK_DESC = ('B315 Translated messages cannot be concatenated. '
'String should be included in translated message.')
TRANS_FUNC = ['_', '_LI', '_LW', '_LE', '_LC']
def visit_BinOp(self, node):
if isinstance(node.op, ast.Add):
if self._check_call_names(node.left, self.TRANS_FUNC):
self.add_error(node.left)
elif self._check_call_names(node.right, self.TRANS_FUNC):
self.add_error(node.right)
super(CheckForTransAdd, self).generic_visit(node)
def check_oslo_namespace_imports(logical_line, physical_line, filename):
"""'oslo_' should be used instead of 'oslo.'
@ -365,11 +282,8 @@ def validate_assertIsNotNone(logical_line):
def factory(register):
register(validate_log_translations)
register(no_translate_debug_logs)
register(CheckForStrUnicodeExc)
register(CheckLoggingFormatArgs)
register(CheckForTransAdd)
register(check_oslo_namespace_imports)
register(dict_constructor_with_list_copy)
register(no_xrange)

View File

@ -12,7 +12,6 @@
# License for the specific language governing permissions and limitations
# under the License.
import sys
import textwrap
import ddt
@ -57,16 +56,6 @@ class HackingTestCase(utils.BaseTestCase):
should pass.
"""
def test_no_translate_debug_logs(self):
self.assertEqual(1, len(list(checks.no_translate_debug_logs(
"LOG.debug(u._('foo'))", "barbican/scheduler/foo.py"))))
self.assertEqual(0, len(list(checks.no_translate_debug_logs(
"LOG.debug('foo')", "barbican/scheduler/foo.py"))))
self.assertEqual(0, len(list(checks.no_translate_debug_logs(
"LOG.info(_('foo'))", "barbican/scheduler/foo.py"))))
# We are patching pep8 so that only the check under test is actually
# installed.
@mock.patch('pep8._checks',
@ -187,50 +176,6 @@ class HackingTestCase(utils.BaseTestCase):
errors = [(8, 20, 'B314'), (8, 33, 'B314'), (9, 16, 'B314')]
self._assert_has_errors(code, checker, expected_errors=errors)
def test_trans_add(self):
checker = checks.CheckForTransAdd
code = """
def fake_tran(msg):
return msg
_ = fake_tran
_LI = _
_LW = _
_LE = _
_LC = _
def f(a, b):
msg = _('test') + 'add me'
msg = _LI('test') + 'add me'
msg = _LW('test') + 'add me'
msg = _LE('test') + 'add me'
msg = _LC('test') + 'add me'
msg = 'add to me' + _('test')
return msg
"""
# Python 3.4.0 introduced a change to the column calculation during AST
# parsing. This was reversed in Python 3.4.3, hence the version-based
# expected value calculation. See #1499743 for more background.
if sys.version_info < (3, 4, 0) or sys.version_info >= (3, 4, 3):
errors = [(13, 10, 'B315'), (14, 10, 'B315'), (15, 10, 'B315'),
(16, 10, 'B315'), (17, 10, 'B315'), (18, 24, 'B315')]
else:
errors = [(13, 11, 'B315'), (14, 13, 'B315'), (15, 13, 'B315'),
(16, 13, 'B315'), (17, 13, 'B315'), (18, 25, 'B315')]
self._assert_has_errors(code, checker, expected_errors=errors)
code = """
def f(a, b):
msg = 'test' + 'add me'
return msg
"""
errors = []
self._assert_has_errors(code, checker, expected_errors=errors)
def test_dict_constructor_with_list_copy(self):
self.assertEqual(1, len(list(checks.dict_constructor_with_list_copy(
" dict([(i, connect_info[i])"))))