From addd2d86d59319e3e9564bc0416e6f58cc14e68c Mon Sep 17 00:00:00 2001 From: Jason White Date: Fri, 10 Mar 2017 19:10:54 -0500 Subject: [PATCH 1/3] add new check for multiple imports --- shopify_python/google_styleguide.py | 9 +++++++ .../shopify_python/test_google_styleguide.py | 26 ++++++++++++++++++- 2 files changed, 34 insertions(+), 1 deletion(-) diff --git a/shopify_python/google_styleguide.py b/shopify_python/google_styleguide.py index d32c9b3..57cb85b 100644 --- a/shopify_python/google_styleguide.py +++ b/shopify_python/google_styleguide.py @@ -63,6 +63,9 @@ 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."), + 'C2610': ('%s imports multiple items in one import statement', + 'multiple-import-items', + 'Separate imports into one item per line.') } options = ( @@ -99,6 +102,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 +140,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) + 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..e7b988f 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) + with self.assert_adds_code_messages(['multiple-import-items'], message): + self.walk(root) From febe3d807b0bad11075ad7d3298bc9d13259568f Mon Sep 17 00:00:00 2001 From: Jason White Date: Mon, 13 Mar 2017 17:00:22 -0400 Subject: [PATCH 2/3] better messaging for C6010 --- shopify_python/google_styleguide.py | 4 ++-- tests/shopify_python/test_google_styleguide.py | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/shopify_python/google_styleguide.py b/shopify_python/google_styleguide.py index 57cb85b..b454262 100644 --- a/shopify_python/google_styleguide.py +++ b/shopify_python/google_styleguide.py @@ -63,7 +63,7 @@ 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."), - 'C2610': ('%s imports multiple items in one import statement', + 'C6010': ('Multiple items imported from %(module)s in one import statement.', 'multiple-import-items', 'Separate imports into one item per line.') } @@ -143,7 +143,7 @@ def __import_full_path_only(self, node): # type: (astroid.ImportFrom) -> None 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) + 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 e7b988f..9788287 100644 --- a/tests/shopify_python/test_google_styleguide.py +++ b/tests/shopify_python/test_google_styleguide.py @@ -167,6 +167,6 @@ def test_multiple_from_imports(self): """) module2_node = root['module2'] - message = pylint.testutils.Message('multiple-import-items', node=module2_node) + 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) From 25c72b2f33924efc8bc9e73b41ff1de59f9ae2a2 Mon Sep 17 00:00:00 2001 From: Jason White Date: Tue, 14 Mar 2017 13:59:46 -0400 Subject: [PATCH 3/3] more helpful descriptors and help statements for C6010 --- shopify_python/google_styleguide.py | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/shopify_python/google_styleguide.py b/shopify_python/google_styleguide.py index b454262..96b47f6 100644 --- a/shopify_python/google_styleguide.py +++ b/shopify_python/google_styleguide.py @@ -63,9 +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': ('Multiple items imported from %(module)s in one import statement.', + 'C6010': ('Statement imports multiple items from %(module)s', 'multiple-import-items', - 'Separate imports into one item per line.') + 'Multiple imports usually result in noisy and potentially conflicting git diffs. To alleviate, ' + 'separate imports into one item per line.') } options = (