Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix abs path pylint #248

Merged
merged 11 commits into from
May 23, 2018
Merged
11 changes: 5 additions & 6 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,12 +10,11 @@ python:
- "3.6-dev"
- "3.7-dev"
install:
- "pip install nose coverage coveralls"
- "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 nose coverage coveralls mock"
- "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 ."
script:
Expand Down
97 changes: 59 additions & 38 deletions prospector/tools/pylint/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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)
Expand All @@ -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 = []
Expand All @@ -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):
Expand All @@ -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 = [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:
Expand Down Expand Up @@ -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

Expand All @@ -165,21 +176,23 @@ 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:
linter.reset_options()
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
Expand All @@ -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 = []
Expand All @@ -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)
Expand Down
9 changes: 3 additions & 6 deletions setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,16 @@
_PACKAGES = find_packages(exclude=["*.tests", "*.tests.*", "tests.*", "tests"])

_INSTALL_REQUIRES = [
'pylint>=1.5.6',
'pylint-celery>=0.3',
'pylint-django>=0.7.2',
'pylint-flask>=0.3',
'pylint<2.0.0,>=1.5.6',
'pylint-plugin-utils>=0.2.6',
'pylint-common>=0.2.5',
'requirements-detector>=0.4.1',
'setoptconf>=0.2.0',
'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',
]
Expand Down
Empty file added tests/tools/__init__.py
Empty file.
Empty file added tests/tools/pylint/__init__.py
Empty file.
31 changes: 31 additions & 0 deletions tests/tools/pylint/test_pylint_tool.py
Original file line number Diff line number Diff line change
@@ -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)])
Empty file.
Empty file.
2 changes: 1 addition & 1 deletion tox.ini
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,5 @@ skip_missing_interpreters = true
[testenv]
deps =
nose
Django
py27: mock
commands = nosetests tests