diff --git a/shopify_python/google_styleguide.py b/shopify_python/google_styleguide.py index d32c9b3..96b47f6 100644 --- a/shopify_python/google_styleguide.py +++ b/shopify_python/google_styleguide.py @@ -63,6 +63,10 @@ class GoogleStyleGuideChecker(checkers.BaseChecker): 'finally-too-long', "The larger the 'finally' body size, the more likely that an exception will be raised during " "resource cleanup activities."), + 'C6010': ('Statement imports multiple items from %(module)s', + 'multiple-import-items', + 'Multiple imports usually result in noisy and potentially conflicting git diffs. To alleviate, ' + 'separate imports into one item per line.') } options = ( @@ -99,6 +103,7 @@ def visit_tryfinally(self, node): # type: (astroid.TryFinally) -> None def visit_importfrom(self, node): # type: (astroid.ImportFrom) -> None self.__import_modules_only(node) self.__import_full_path_only(node) + self.__limit_one_import(node) def visit_raise(self, node): # type: (astroid.Raise) -> None self.__dont_use_archaic_raise_syntax(node) @@ -136,6 +141,11 @@ def __import_full_path_only(self, node): # type: (astroid.ImportFrom) -> None for child_module in self.__get_module_names(node): self.add_message('import-full-path', node=node, args={'module': child_module}) + def __limit_one_import(self, node): # type: (astroid.ImportFrom) -> None + """Only one item imported per line.""" + if len(node.names) > 1: + self.add_message('multiple-import-items', node=node, args={'module': node.modname}) + def __avoid_global_variables(self, node): # type: (astroid.Assign) -> None """Avoid global variables.""" diff --git a/tests/shopify_python/test_google_styleguide.py b/tests/shopify_python/test_google_styleguide.py index 2decadb..9788287 100644 --- a/tests/shopify_python/test_google_styleguide.py +++ b/tests/shopify_python/test_google_styleguide.py @@ -1,3 +1,4 @@ +import contextlib import sys import astroid @@ -11,6 +12,16 @@ class TestGoogleStyleGuideChecker(pylint.testutils.CheckerTestCase): CHECKER_CLASS = google_styleguide.GoogleStyleGuideChecker + @contextlib.contextmanager + def assert_adds_code_messages(self, codes, *messages): + """Asserts that the checker received the list of messages, including only messages with the given codes.""" + yield + got = [message for message in self.linter.release_messages() if message[0] in codes] + msg = ('Expected messages did not match actual.\n' + 'Expected:\n%s\nGot:\n%s' % ('\n'.join(repr(m) for m in messages), + '\n'.join(repr(m) for m in got))) + self.assertEqual(list(messages), got, msg) + def test_importing_function_fails(self): root = astroid.builder.parse(""" from os.path import join @@ -50,7 +61,8 @@ def test_importing_relatively_fails(self): from . import string from .. import string, os """) - with self.assertAddsMessages( + with self.assert_adds_code_messages( + ['import-full-path'], pylint.testutils.Message('import-full-path', node=root.body[0], args={'module': '.string'}), pylint.testutils.Message('import-full-path', node=root.body[1], args={'module': '.string'}), pylint.testutils.Message('import-full-path', node=root.body[1], args={'module': '.os'}), @@ -146,3 +158,15 @@ def test_try_exc_fnly_complexity(self): pylint.testutils.Message('except-too-long', node=try_except.handlers[0], args={'found': 39}), ): self.walk(root) + + def test_multiple_from_imports(self): + root = astroid.builder.parse(""" + import sys + from package.module import module1, module2 + from other_package import other_module as F + """) + + module2_node = root['module2'] + message = pylint.testutils.Message('multiple-import-items', node=module2_node, args={'module': 'package.module'}) + with self.assert_adds_code_messages(['multiple-import-items'], message): + self.walk(root)