diff --git a/HACKING.rst b/HACKING.rst index 3995a9ba08..34c79b1e21 100644 --- a/HACKING.rst +++ b/HACKING.rst @@ -38,6 +38,7 @@ Nova Specific Commandments - [N323] Ensure that the _() function is explicitly imported to ensure proper translations. - [N324] Ensure that jsonutils.%(fun)s must be used instead of json.%(fun)s - [N325] str() cannot be used on an exception. Remove use or use six.text_type() +- [N326] Translated messages cannot be concatenated. String should be included in translated message. Creating Unit Tests ------------------- diff --git a/nova/api/metadata/vendordata_json.py b/nova/api/metadata/vendordata_json.py index ee8a938784..a167d16e56 100644 --- a/nova/api/metadata/vendordata_json.py +++ b/nova/api/metadata/vendordata_json.py @@ -44,13 +44,15 @@ class JsonFileVendorData(base.VendorDataDriver): data = jsonutils.load(fp) except IOError as e: if e.errno == errno.ENOENT: - LOG.warn(logprefix + _LW("file does not exist")) + LOG.warn(_LW("%(logprefix)sfile does not exist"), + {'logprefix': logprefix}) else: - LOG.warn(logprefix + _LW("Unexpected IOError when " - "reading")) + LOG.warn(_LW("%(logprefix)unexpected IOError when " + "reading"), {'logprefix': logprefix}) raise e except ValueError: - LOG.warn(logprefix + _LW("failed to load json")) + LOG.warn(_LW("%(logprefix)sfailed to load json"), + {'logprefix': logprefix}) raise self._data = data diff --git a/nova/exception.py b/nova/exception.py index 05c6ecef9e..c0e233bab7 100644 --- a/nova/exception.py +++ b/nova/exception.py @@ -292,7 +292,7 @@ class InvalidVolume(Invalid): class InvalidVolumeAccessMode(Invalid): - msg_fmt = _("Invalid volume access mode") + ": %(access_mode)s" + msg_fmt = _("Invalid volume access mode: %(access_mode)s") class InvalidMetadata(Invalid): diff --git a/nova/hacking/checks.py b/nova/hacking/checks.py index a22f6318f2..c6e929402e 100644 --- a/nova/hacking/checks.py +++ b/nova/hacking/checks.py @@ -102,6 +102,13 @@ class BaseASTChecker(ast.NodeVisitor): error = (node.lineno, node.col_offset, message, self.__class__) self._errors.append(error) + def _check_call_names(self, call_node, names): + if isinstance(call_node, ast.Call): + if isinstance(call_node.func, ast.Name): + if call_node.func.id in names: + return True + return False + def import_no_db_in_virt(logical_line, filename): """Check for db calls from nova/virt @@ -367,16 +374,37 @@ class CheckForStrExc(BaseASTChecker): super(CheckForStrExc, self).generic_visit(node) def visit_Call(self, node): - if isinstance(node.func, ast.Name): - if node.func.id == 'str': - if node not in self.already_checked: - self.already_checked.append(node) - if isinstance(node.args[0], ast.Name): - if node.args[0].id in self.name: - self.add_error(node.args[0]) + if self._check_call_names(node, ['str']): + if node not in self.already_checked: + self.already_checked.append(node) + if isinstance(node.args[0], ast.Name): + if node.args[0].id in self.name: + self.add_error(node.args[0]) super(CheckForStrExc, 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 = ('N326 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 factory(register): register(import_no_db_in_virt) register(no_db_session_in_public_api) @@ -396,3 +424,4 @@ def factory(register): register(check_explicit_underscore_import) register(use_jsonutils) register(CheckForStrExc) + register(CheckForTransAdd) diff --git a/nova/tests/compute/test_keypairs.py b/nova/tests/compute/test_keypairs.py index 97f8373620..8c923beb29 100644 --- a/nova/tests/compute/test_keypairs.py +++ b/nova/tests/compute/test_keypairs.py @@ -120,7 +120,7 @@ class CreateImportSharedTestMixIn(object): self.assertEqual(expected_message, unicode(exc)) def assertInvalidKeypair(self, expected_message, name): - msg = _('Keypair data is invalid') + ': ' + expected_message + msg = _('Keypair data is invalid: %s') % expected_message self.assertKeyNameRaises(exception.InvalidKeypair, msg, name) def test_name_too_short(self): diff --git a/nova/tests/test_hacking.py b/nova/tests/test_hacking.py index 205357a1b1..f42af18472 100644 --- a/nova/tests/test_hacking.py +++ b/nova/tests/test_hacking.py @@ -301,3 +301,39 @@ class HackingTestCase(test.NoDBTestCase): """ errors = [(8, 20, 'N325'), (8, 29, 'N325')] 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 + """ + errors = [(13, 10, 'N326'), (14, 10, 'N326'), (15, 10, 'N326'), + (16, 10, 'N326'), (17, 10, 'N326'), (18, 24, 'N326')] + 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)