From 4f40a68defe773572c1fe41d7d39f60f61fb2632 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 20 Apr 2018 11:37:40 -0300 Subject: [PATCH 1/8] Use os.path.split instead of bare split to avoid problems with abs paths --- prospector/tools/pylint/__init__.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/prospector/tools/pylint/__init__.py b/prospector/tools/pylint/__init__.py index b569d466..01052e5f 100644 --- a/prospector/tools/pylint/__init__.py +++ b/prospector/tools/pylint/__init__.py @@ -111,7 +111,7 @@ def configure(self, prospector_config, found_files): # create a list of packages, but don't include packages which are # subpackages of others as checks will be duplicated - packages = [p.split(os.path.sep) for p in found_files.iter_package_paths(abspath=False)] + packages = [os.path.split(p) for p in found_files.iter_package_paths(abspath=False)] packages.sort(key=len) check_paths = set() for package in packages: From b440774cc4b173955f32044de1e15800d69cf1dc Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 20 Apr 2018 11:38:02 -0300 Subject: [PATCH 2/8] Add pyyaml dependency to deps on tox --- tox.ini | 1 + 1 file changed, 1 insertion(+) diff --git a/tox.ini b/tox.ini index 7f423f96..1b10dd28 100644 --- a/tox.ini +++ b/tox.ini @@ -7,5 +7,6 @@ skip_missing_interpreters = true deps = nose Django + pyyaml commands = nosetests tests From 53bf9250d0a6f6a8a320bd51d19121af80695834 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 20 Apr 2018 12:12:20 -0300 Subject: [PATCH 3/8] Update pre-commit configuration --- .pre-commit-config.yaml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 747a44c8..3966971d 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,5 +1,5 @@ - repo: git@github.com:pre-commit/pre-commit-hooks - sha: e306ff3b7d0d9a6fc7d128ef9ca2e0b6e6e76e8f + sha: v0.9.2 hooks: - id: trailing-whitespace - id: end-of-file-fixer @@ -12,6 +12,6 @@ - --ignore=E126 - id: check-added-large-files - repo: git://github.com/FalconSocial/pre-commit-python-sorter - sha: ec01d99f48a0dabb2ebbb2675139e2cc0fe2aa93 + sha: b57843b0b874df1d16eb0bef00b868792cb245c2 hooks: - id: python-import-sorter From f866efc762c323779d283457abb86271b514d957 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 20 Apr 2018 15:14:34 -0300 Subject: [PATCH 4/8] prospector should be agnostic of pylint plugins that depend on frameworks --- setup.py | 3 --- tox.ini | 2 -- 2 files changed, 5 deletions(-) diff --git a/setup.py b/setup.py index fcb7fba7..58ebc122 100644 --- a/setup.py +++ b/setup.py @@ -19,9 +19,6 @@ _INSTALL_REQUIRES = [ 'pylint>=1.5.6', - 'pylint-celery>=0.3', - 'pylint-django>=0.7.2', - 'pylint-flask>=0.3', 'pylint-plugin-utils>=0.2.6', 'pylint-common>=0.2.5', 'requirements-detector>=0.4.1', diff --git a/tox.ini b/tox.ini index 1b10dd28..90191ff8 100644 --- a/tox.ini +++ b/tox.ini @@ -6,7 +6,5 @@ skip_missing_interpreters = true [testenv] deps = nose - Django - pyyaml commands = nosetests tests From 747945c5c3cf54bc90cca5dfba01b3e95508d13a Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 18 May 2018 13:33:05 -0300 Subject: [PATCH 5/8] Add reason to add pyyaml git dependency directly into travis.yml --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index ca006ff1..2536b8d8 100644 --- a/.travis.yml +++ b/.travis.yml @@ -16,6 +16,7 @@ install: - "pip install git+https://github.com/landscapeio/pylint-celery.git@develop" - "pip install git+https://github.com/landscapeio/pylint-django.git@develop" - "pip install git+https://github.com/landscapeio/requirements-detector.git@develop" + # reason: https://github.com/yaml/pyyaml/issues/126 - "pip install git+https://github.com/yaml/pyyaml.git@master" - "pip install --editable ." script: From 6522182e370c18c8fba5172b93766a81c52a4b9d Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 18 May 2018 14:22:54 -0300 Subject: [PATCH 6/8] Properly format pylint init file --- prospector/tools/pylint/__init__.py | 97 ++++++++++++++++++----------- 1 file changed, 59 insertions(+), 38 deletions(-) diff --git a/prospector/tools/pylint/__init__.py b/prospector/tools/pylint/__init__.py index 01052e5f..d98192b8 100644 --- a/prospector/tools/pylint/__init__.py +++ b/prospector/tools/pylint/__init__.py @@ -11,8 +11,8 @@ from prospector.tools.pylint.indent_checker import IndentChecker from prospector.tools.pylint.linter import ProspectorLinter - -_UNUSED_WILDCARD_IMPORT_RE = re.compile(r'^Unused import (.*) from wildcard import$') +_UNUSED_WILDCARD_IMPORT_RE = re.compile( + r'^Unused import (.*) from wildcard import$') class PylintTool(ToolBase): @@ -57,19 +57,29 @@ def _prospector_configure(self, prospector_config, linter): # The warnings about disabling warnings are useful for figuring out # with other tools to suppress messages from. For example, an unused - # import which is disabled with 'pylint disable=unused-import' will still - # generate an 'FL0001' unused import warning from pyflakes. Using the - # information from these messages, we can figure out what was disabled. - linter.disable('locally-disabled') # notification about disabling a message - linter.disable('locally-enabled') # notification about enabling a message - linter.enable('file-ignored') # notification about disabling an entire file - linter.enable('suppressed-message') # notification about a message being supressed - linter.disable('useless-suppression') # notification about message supressed which was not raised - linter.disable('deprecated-pragma') # notification about use of deprecated 'pragma' option - - # disable the 'mixed indentation' warning, since it actually will only allow - # the indentation specified in the pylint configuration file; we replace it - # instead with our own version which is more lenient and configurable + # import which is disabled with 'pylint disable=unused-import' will + # still generate an 'FL0001' unused import warning from pyflakes. + # Using the information from these messages, we can figure out what + # was disabled. + linter.disable( + 'locally-disabled') # notification about disabling a message + linter.disable( + 'locally-enabled') # notification about enabling a message + linter.enable( + 'file-ignored') # notification about disabling an entire file + linter.enable('suppressed-message' + ) # notification about a message being supressed + linter.disable( + 'useless-suppression' + ) # notification about message supressed which was not raised + linter.disable( + 'deprecated-pragma' + ) # notification about use of deprecated 'pragma' option + + # disable the 'mixed indentation' warning, since it actually will only + # allow the indentation specified in the pylint configuration file; we + # replace it instead with our own version which is more lenient and + # configurable linter.disable('mixed-indentation') indent_checker = IndentChecker(linter) linter.register_checker(indent_checker) @@ -85,12 +95,7 @@ def _prospector_configure(self, prospector_config, linter): def _error_message(self, filepath, message): location = Location(filepath, None, None, 0, 0) - return Message( - 'prospector', - 'config-problem', - location, - message - ) + return Message('prospector', 'config-problem', location, message) def _pylintrc_configure(self, pylintrc, linter): errors = [] @@ -101,7 +106,9 @@ def _pylintrc_configure(self, pylintrc, linter): try: linter.load_plugin_modules([plugin]) except ImportError: - errors.append(self._error_message(pylintrc, "Could not load plugin %s" % plugin)) + errors.append( + self._error_message( + pylintrc, "Could not load plugin %s" % plugin)) return errors def configure(self, prospector_config, found_files): @@ -111,7 +118,10 @@ def configure(self, prospector_config, found_files): # create a list of packages, but don't include packages which are # subpackages of others as checks will be duplicated - packages = [os.path.split(p) for p in found_files.iter_package_paths(abspath=False)] + packages = [ + os.path.split(p) + for p in found_files.iter_package_paths(abspath=False) + ] packages.sort(key=len) check_paths = set() for package in packages: @@ -156,7 +166,8 @@ def configure(self, prospector_config, found_files): if pylintrc is None: pylintrc = find_pylintrc() if pylintrc is None: - pylintrc_path = os.path.join(prospector_config.workdir, '.pylintrc') + pylintrc_path = os.path.join(prospector_config.workdir, + '.pylintrc') if os.path.exists(pylintrc_path): pylintrc = pylintrc_path @@ -165,7 +176,8 @@ def configure(self, prospector_config, found_files): configured_by = pylintrc ext_found = True - self._args = linter.load_command_line_configuration(check_paths) + self._args = linter.load_command_line_configuration( + check_paths) config_messages += self._pylintrc_configure(pylintrc, linter) if not ext_found: @@ -173,13 +185,14 @@ def configure(self, prospector_config, found_files): self._args = linter.load_command_line_configuration(check_paths) self._prospector_configure(prospector_config, linter) - # Pylint 1.4 introduced the idea of explicitly specifying which C-extensions - # to load. This is because doing so allows them to execute any code whatsoever, - # which is considered to be unsafe. The following option turns off this, allowing - # any extension to load again, since any setup.py can execute arbitrary code and - # the safety gained through this approach seems minimal. Leaving it on means - # that the inference engine cannot do much inference on modules with C-extensions - # which is a bit useless. + # Pylint 1.4 introduced the idea of explicitly specifying which + # C-extensions to load. This is because doing so allows them to + # execute any code whatsoever, which is considered to be unsafe. + # The following option turns off this, allowing any extension to + # load again, since any setup.py can execute arbitrary code and + # the safety gained through this approach seems minimal. Leaving + # it on means that the inference engine cannot do much inference + # on modules with C-extensions which is a bit useless. linter.set_option('unsafe-load-any-extension', True) # we don't want similarity reports right now @@ -196,7 +209,8 @@ def configure(self, prospector_config, found_files): def _combine_w0614(self, messages): """ For the "unused import from wildcard import" messages, - we want to combine all warnings about the same line into a single message. + we want to combine all warnings about the same line into + a single message. """ by_loc = defaultdict(list) out = [] @@ -210,19 +224,26 @@ def _combine_w0614(self, messages): for location, message_list in by_loc.items(): names = [] for msg in message_list: - names.append(_UNUSED_WILDCARD_IMPORT_RE.match(msg.message).group(1)) + names.append( + _UNUSED_WILDCARD_IMPORT_RE.match(msg.message).group(1)) - msgtxt = 'Unused imports from wildcard import: %s' % ', '.join(names) - combined_message = Message('pylint', 'unused-wildcard-import', location, msgtxt) + msgtxt = 'Unused imports from wildcard import: %s' % ', '.join( + names) + combined_message = Message('pylint', 'unused-wildcard-import', + location, msgtxt) out.append(combined_message) return out def combine(self, messages): """ - Some error messages are repeated, causing many errors where only one is strictly necessary. + Combine repeated messages. + + Some error messages are repeated, causing many errors where + only one is strictly necessary. - For example, having a wildcard import will result in one 'Unused Import' warning for every unused import. + For example, having a wildcard import will result in one + 'Unused Import' warning for every unused import. This method will combine these into a single warning. """ combined = self._combine_w0614(messages) From e66b1a21f6429daf9a827c84fb37e67074e30bc3 Mon Sep 17 00:00:00 2001 From: Carlos Coelho Date: Fri, 18 May 2018 18:03:26 -0300 Subject: [PATCH 7/8] Add test for path split and regression for old path behavior Mock argv before calling ProspectorConfig in tests --- .travis.yml | 2 +- tests/tools/__init__.py | 0 tests/tools/pylint/__init__.py | 0 tests/tools/pylint/test_pylint_tool.py | 31 +++++++++++++++++++++++++ tests/tools/pylint/testpath/__init__.py | 0 tests/tools/pylint/testpath/testfile.py | 0 tox.ini | 1 + 7 files changed, 33 insertions(+), 1 deletion(-) create mode 100644 tests/tools/__init__.py create mode 100644 tests/tools/pylint/__init__.py create mode 100644 tests/tools/pylint/test_pylint_tool.py create mode 100644 tests/tools/pylint/testpath/__init__.py create mode 100644 tests/tools/pylint/testpath/testfile.py diff --git a/.travis.yml b/.travis.yml index 2536b8d8..b236c5dc 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,7 +10,7 @@ python: - "3.6-dev" - "3.7-dev" install: - - "pip install nose coverage coveralls" + - "pip install nose coverage coveralls mock" - "pip install git+https://github.com/landscapeio/pylint-plugin-utils.git@develop" - "pip install git+https://github.com/landscapeio/pylint-common.git@develop" - "pip install git+https://github.com/landscapeio/pylint-celery.git@develop" diff --git a/tests/tools/__init__.py b/tests/tools/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/tools/pylint/__init__.py b/tests/tools/pylint/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/tools/pylint/test_pylint_tool.py b/tests/tools/pylint/test_pylint_tool.py new file mode 100644 index 00000000..aabf5776 --- /dev/null +++ b/tests/tools/pylint/test_pylint_tool.py @@ -0,0 +1,31 @@ +# -*- coding: utf-8 -*- +import os +import sys +from unittest import TestCase + +from prospector.config import ProspectorConfig +from prospector.finder import find_python +from prospector.tools.pylint import PylintTool + +if sys.version_info >= (3, 0): + from unittest.mock import patch +else: + from mock import patch + + +class TestPylintTool(TestCase): + def setUp(self): + with patch('sys.argv', ['']): + self.config = ProspectorConfig() + self.pylint_tool = PylintTool() + + def test_absolute_path_is_computed_correctly(self): + root = os.path.join(os.path.dirname(__file__), 'testpath', 'test.py') + root_sep_split = root.split(os.path.sep) + root_os_split = os.path.split(root) + found_files = find_python([], [root], explicit_file_mode=True) + self.pylint_tool.configure(self.config, found_files) + self.assertNotEqual(self.pylint_tool._args, + [os.path.join(*root_sep_split)]) + self.assertEqual(self.pylint_tool._args, + [os.path.join(*root_os_split)]) diff --git a/tests/tools/pylint/testpath/__init__.py b/tests/tools/pylint/testpath/__init__.py new file mode 100644 index 00000000..e69de29b diff --git a/tests/tools/pylint/testpath/testfile.py b/tests/tools/pylint/testpath/testfile.py new file mode 100644 index 00000000..e69de29b diff --git a/tox.ini b/tox.ini index 30b5feb9..8a0f3795 100644 --- a/tox.ini +++ b/tox.ini @@ -6,4 +6,5 @@ skip_missing_interpreters = true [testenv] deps = nose + py27: mock commands = nosetests tests From 12e14364ee3900400b64a81096c4aff9d6579847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Co=C3=AAlho?= Date: Tue, 22 May 2018 23:37:13 -0300 Subject: [PATCH 8/8] Update requirements MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Signed-off-by: Carlos Coêlho --- .travis.yml | 8 +++----- setup.py | 6 +++--- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/.travis.yml b/.travis.yml index b236c5dc..477ab2cf 100644 --- a/.travis.yml +++ b/.travis.yml @@ -11,11 +11,9 @@ python: - "3.7-dev" install: - "pip install nose coverage coveralls mock" - - "pip install git+https://github.com/landscapeio/pylint-plugin-utils.git@develop" - - "pip install git+https://github.com/landscapeio/pylint-common.git@develop" - - "pip install git+https://github.com/landscapeio/pylint-celery.git@develop" - - "pip install git+https://github.com/landscapeio/pylint-django.git@develop" - - "pip install git+https://github.com/landscapeio/requirements-detector.git@develop" + - "pip install git+https://github.com/landscapeio/pylint-plugin-utils.git@master" + - "pip install git+https://github.com/landscapeio/pylint-common.git@master" + - "pip install git+https://github.com/landscapeio/requirements-detector.git@master" # reason: https://github.com/yaml/pyyaml/issues/126 - "pip install git+https://github.com/yaml/pyyaml.git@master" - "pip install --editable ." diff --git a/setup.py b/setup.py index 9765f1a9..45c99242 100644 --- a/setup.py +++ b/setup.py @@ -18,7 +18,7 @@ _PACKAGES = find_packages(exclude=["*.tests", "*.tests.*", "tests.*", "tests"]) _INSTALL_REQUIRES = [ - 'pylint>=1.5.6', + 'pylint<2.0.0,>=1.5.6', 'pylint-plugin-utils>=0.2.6', 'pylint-common>=0.2.5', 'requirements-detector>=0.4.1', @@ -26,8 +26,8 @@ 'dodgy>=0.1.9', 'pyyaml', 'mccabe>=0.5.0', - 'pyflakes>=0.8.1', - 'pycodestyle<2.4.0', + 'pyflakes<2.0.0,>=0.8.1', + 'pycodestyle<2.4.0,>=2.0.0', 'pep8-naming>=0.3.3', 'pydocstyle>=2.0.0', ]