diff --git a/README.mdown b/README.mdown index fa99a506..d72e2842 100644 --- a/README.mdown +++ b/README.mdown @@ -240,6 +240,19 @@ issues. * `ignore` A comma separated list of error codes you wish to ignore. +#### pylint + +Uses [pylint](https://www.pylint.org/) to check python files. + +*Options* + +* `disable` A comma separated list of error codes you wish to ignore. Passed as + a command line option to pylint. +* `enable` A comma separated list of error codes you wish to include. Best if + used with `disable: "all"`. Passed as a command line option to + pylint +* `config` Provide a path to the pylintrc config file for pylint. + ### PHP #### PHPCS diff --git a/docker/python3.Dockerfile b/docker/python3.Dockerfile index 66c477bd..0ed5a59d 100644 --- a/docker/python3.Dockerfile +++ b/docker/python3.Dockerfile @@ -1,7 +1,9 @@ FROM python:3.6-alpine RUN mkdir /src \ - && mkdir /tool + && mkdir /tool \ + && apk add musl-dev gcc \ + && rm -rf /var/cache/apk/* COPY requirements-py3.txt /tool diff --git a/docker/requirements-py3.txt b/docker/requirements-py3.txt index ca68bc13..890f00c0 100644 --- a/docker/requirements-py3.txt +++ b/docker/requirements-py3.txt @@ -2,3 +2,4 @@ pep8>=1.7.0,<2.0.0 flake8>=3.3.0,<3.4.0 autopep8>=1.3.0,<2.0.0 black==18.6b4 +pylint>=2.3.1,<3.0.0 diff --git a/lintreview/tools/py3k.py b/lintreview/tools/py3k.py index 492051e9..139534e2 100644 --- a/lintreview/tools/py3k.py +++ b/lintreview/tools/py3k.py @@ -1,13 +1,14 @@ from __future__ import absolute_import -import os + import logging -import lintreview.docker as docker -from lintreview.tools import Tool, process_quickfix, stringify + +from lintreview.tools import stringify +from lintreview.tools.pylint import Pylint log = logging.getLogger(__name__) -class Py3k(Tool): +class Py3k(Pylint): """ $ pylint --py3k is a special mode for porting to python 3 which disables other pylint checkers. @@ -15,52 +16,17 @@ class Py3k(Tool): """ name = 'py3k' + accepted_options = ('ignore', ) + python_version = 'python2' - def check_dependencies(self): - """ - See if python image is available - """ - return docker.image_exists('python2') - - def match_file(self, filename): - base = os.path.basename(filename) - name, ext = os.path.splitext(base) - return ext == '.py' - - def process_files(self, files): - """ - Run code checks with pylint --py3k. - Only a single process is made for all files - to save resources. - """ - log.debug('Processing %s files with %s', files, self.name) - command = self.make_command(files) - output = docker.run('python2', command, self.base_path) - if not output: - log.debug('No py3k errors found.') - return False - - output = output.split("\n") - output = [line for line in output if not line.startswith("*********")] + def make_command(self, files): + self.check_options() - process_quickfix(self.problems, output, docker.strip_base) + command = self._base_command + command.append('--py3k') - def make_command(self, files): - msg_template = '{path}:{line}:{column}:{msg_id} {msg}' - command = [ - 'pylint', - '--py3k', - '--reports=n', - '--msg-template', - msg_template, - ] - accepted_options = ('ignore') if 'ignore' in self.options: command.extend(['-d', stringify(self.options['ignore'])]) - for option in self.options: - if option in accepted_options: - continue - log.warning('Set non-existent py3k option: %s', option) command.extend(files) return command diff --git a/lintreview/tools/pylint.py b/lintreview/tools/pylint.py new file mode 100644 index 00000000..3b80271c --- /dev/null +++ b/lintreview/tools/pylint.py @@ -0,0 +1,76 @@ +from __future__ import absolute_import + +import logging +import os + +import lintreview.docker as docker +from lintreview.tools import Tool, process_quickfix, python_image, stringify + +log = logging.getLogger(__name__) + + +class Pylint(Tool): + + name = 'pylint' + + accepted_options = ('python', 'disable', 'enable', 'config') + + @property + def _base_command(self): + return [ + 'pylint', + '--persistent=n', + '--reports=n', + '--score=n', + '--msg-template', + '{path}:{line}:{column}:{msg_id} {msg}' + ] + + @property + def python_version(self): + return python_image(self.options) + + def check_dependencies(self): + """ + See if python image is available + """ + return docker.image_exists(self.python_version) + + def match_file(self, filename): + base = os.path.basename(filename) + name, ext = os.path.splitext(base) + return ext == '.py' + + def process_files(self, files): + log.debug('Processing %s files with %s', files, self.name) + command = self.make_command(files) + output = docker.run(self.python_version, command, source_dir=self.base_path) + if not output: + log.debug('No %s errors found.', self.name) + return False + + output = output.split("\n") + output = [line for line in output if not line.startswith("*********")] + + process_quickfix(self.problems, output, docker.strip_base) + + def make_command(self, files): + self.check_options() + + command = self._base_command + + if self.options.get('disable'): + command.extend(['--disable', stringify(self.options['disable'])]) + + if self.options.get('enable'): + command.extend(['--enable', stringify(self.options['enable'])]) + + if self.options.get('config'): + command.extend(['--rcfile', docker.apply_base(stringify(self.options['config']))]) + + command.extend(files) + return command + + def check_options(self): + for unaccepted_option in set(self.options.keys()) - set(self.accepted_options): + log.warning('Set non-existent %s option: %s', self.name, unaccepted_option) diff --git a/tests/fixtures/pylint/has_errors.py b/tests/fixtures/pylint/has_errors.py new file mode 100644 index 00000000..23fe2306 --- /dev/null +++ b/tests/fixtures/pylint/has_errors.py @@ -0,0 +1,16 @@ +# Create unused imports +import os, re + +def thing(self): + return thing_two('arg1', + 'arg2') + + +def thing_two(arg1, arg2, arg3): + """A thinger for thinging but returning nothing + """ + result=arg1*arg2 + if result == arg1: + pass + elif result == '': + pass diff --git a/tests/fixtures/pylint/no_errors.py b/tests/fixtures/pylint/no_errors.py new file mode 100644 index 00000000..37388355 --- /dev/null +++ b/tests/fixtures/pylint/no_errors.py @@ -0,0 +1,12 @@ +"""Sample python file with no pylint errors. +used for testing the pylint.Tool +""" + +from __future__ import print_function + + +def does_something(thing): + """Function docstring + """ + print('hello') + return thing.buzz() diff --git a/tests/fixtures/pylint/sample_rcfile b/tests/fixtures/pylint/sample_rcfile new file mode 100644 index 00000000..ac6be2fa --- /dev/null +++ b/tests/fixtures/pylint/sample_rcfile @@ -0,0 +1,30 @@ +[MASTER] +load-plugins=pylint.extensions.emptystring + +[MESSAGES CONTROL] + +disable=W0611, + E1120, + W0613, + C0111, + C0330, + C0326, + C0410 + +[REPORTS] +# test that msg_template is not overwritten by the rcfile, otherwise +# lint-review won't know what to do with the messages +msg-template="{path}:{line}:{column}:{msg_id} foo" + +[FORMAT] + +# Maximum number of characters on a single line. +max-line-length=50 + +# Maximum number of lines in a module +max-module-lines=3 + +[DESIGN] + +# Maximum number of arguments for function / method +max-args=1 diff --git a/tests/tools/test_py3k.py b/tests/tools/test_py3k.py index 30e8e87a..7def0049 100644 --- a/tests/tools/test_py3k.py +++ b/tests/tools/test_py3k.py @@ -74,3 +74,7 @@ def test_process_files_two_files(self): Comment(fname, 11, 11, 'W1638 range built-in referenced when not iterating') ], problems) + + def test_py3k_ignore_py3(self): + self.tool = Py3k(self.problems, {'python': 3}) + eq_('python2', self.tool.python_version) diff --git a/tests/tools/test_pylint.py b/tests/tools/test_pylint.py new file mode 100644 index 00000000..654e59ce --- /dev/null +++ b/tests/tools/test_pylint.py @@ -0,0 +1,129 @@ +from __future__ import absolute_import + +from unittest import TestCase + +from nose.tools import eq_ +from tests import requires_image, root_dir + +from lintreview.review import Comment, Problems +from lintreview.tools.pylint import Pylint + + +class TestPylint(TestCase): + + fixtures = [ + 'tests/fixtures/pylint/no_errors.py', + 'tests/fixtures/pylint/has_errors.py', + ] + + def setUp(self): + self.problems = Problems() + self.tool = Pylint(self.problems, {}, root_dir) + + def test_match_file(self): + self.assertFalse(self.tool.match_file('test.php')) + self.assertFalse(self.tool.match_file('test.js')) + self.assertFalse(self.tool.match_file('dir/name/test.js')) + self.assertTrue(self.tool.match_file('test.py')) + self.assertTrue(self.tool.match_file('dir/name/test.py')) + + def test_process_files__one_file_pass(self): + self.tool.process_files([self.fixtures[0]]) + eq_([], self.problems.all(self.fixtures[0])) + + @requires_image('python2') + def test_process_files__one_file_fail(self): + self.tool.process_files([self.fixtures[1]]) + problems = self.problems.all(self.fixtures[1]) + eq_(7, len(problems)) + + fname = self.fixtures[1] + # pylint outputs are sorted by 'category', not line number + eq_([ + Comment(fname, 6, 6, "C0330 Wrong continued indentation (add 17 spaces)."), + Comment(fname, 12, 12, "C0326 Exactly one space required around assignment"), + Comment(fname, 1, 1, "C0111 Missing module docstring"), + Comment(fname, 2, 2, "C0410 Multiple imports on one line (os, re)\n" + "W0611 Unused import re\nW0611 Unused import os"), + Comment(fname, 4, 4, "C0111 Missing function docstring\nW0613 Unused argument 'self'"), + Comment(fname, 5, 5, "E1120 No value for argument 'arg3' in function call"), + Comment(fname, 9, 9, "W0613 Unused argument 'arg3'") + ], problems) + + @requires_image('python2') + def test_process_files_two_files(self): + self.tool.process_files(self.fixtures) + + eq_([], self.problems.all(self.fixtures[0])) + + problems = self.problems.all(self.fixtures[1]) + eq_(7, len(problems)) + expected = Comment(self.fixtures[1], 6, 6, + "C0330 Wrong continued indentation (add 17 spaces).") + eq_(expected, problems[0]) + + @requires_image('python3') + def test_process_files_two_files__python3(self): + self.tool.options['python'] = 3 + self.tool.process_files(self.fixtures) + + eq_([], self.problems.all(self.fixtures[0])) + + problems = self.problems.all(self.fixtures[1]) + eq_(7, len(problems)) + expected = Comment(self.fixtures[1], 6, 6, + "C0330 Wrong continued indentation (add 17 spaces).") + eq_(expected, problems[0]) + + @requires_image('python2') + def test_process_absolute_container_path(self): + fixtures = ['/src/' + path for path in self.fixtures] + self.tool.process_files(fixtures) + + eq_([], self.problems.all(self.fixtures[0])) + + problems = self.problems.all(self.fixtures[1]) + assert len(problems) >= 6 + + @requires_image('python2') + def test_process_files__disable(self): + options = { + 'disable': 'C,W0611,E1120', + } + self.tool = Pylint(self.problems, options, root_dir) + self.tool.process_files([self.fixtures[1]]) + problems = self.problems.all(self.fixtures[1]) + eq_([ + Comment(self.fixtures[1], 4, 4, "W0613 Unused argument 'self'"), + Comment(self.fixtures[1], 9, 9, "W0613 Unused argument 'arg3'"), + ], problems) + + @requires_image('python2') + def test_process_files__enable(self): + options = { + 'disable': 'C,W,E', + 'enable': 'W0613', + } + self.tool = Pylint(self.problems, options, root_dir) + self.tool.process_files([self.fixtures[1]]) + problems = self.problems.all(self.fixtures[1]) + eq_([ + Comment(self.fixtures[1], 4, 4, "W0613 Unused argument 'self'"), + Comment(self.fixtures[1], 9, 9, "W0613 Unused argument 'arg3'"), + ], problems) + + @requires_image('python2') + def test_process_files__config(self): + options = { + 'config': 'tests/fixtures/pylint/sample_rcfile', + } + self.tool = Pylint(self.problems, options, root_dir) + filename = self.fixtures[1] + self.tool.process_files([filename]) + problems = self.problems.all(self.fixtures[1]) + eq_([ + Comment(filename, 10, 10, "C0301 Line too long (51/50)"), + Comment(filename, 1, 1, "C0302 Too many lines in module (16/3)"), + Comment(filename, 9, 9, "R0913 Too many arguments (3/1)"), + Comment(filename, 15, 15, "C1901 Avoid comparisons to empty string"), + ], problems)