add new check for multiple imports

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."""
 

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)