Page MenuHomePhabricator

Exception pylint is an invalid linter
Closed, ResolvedPublic

Description

my configuration
$ cat .arclint

{
  "linters": {
    "pylint": {
      "type": "pylint",
      "include": "(\\.py$)",
      "exclude": "(^DB/)"
    }
  }
}

$ cat .arcconfig

{
    "lint.engine": "ArcanistConfigurationDrivenLintEngine",
    "lint.pylint.codes.error": "^(E|F).*",
    "lint.pylint.codes.warning": "^(W|R).*",
    "lint.pylint.codes.advice": "^C.*",
    "unit.engine": "NoseTestEngine"
}

From the latest pulled version of arcanist (7b61faa1927ece536edac2c81b2f8f35ac4ca4f6), running the following throws an exception
$ arc lint

Exception
Linter 'pylint' specifies invalid type 'pylint'. Available linters are: csharp, chmod, filename, phutil-xhpast, xhpast, csslint, gjslint, coffeelint, cppcheck, cpplint, flake8, golint, jshint, jsonlint, lessc, pep8, phpcs, puppet-lint, pyflakes, ruby, scala-sbt, generated, merge-conflict, nolint, phutil-library, script-and-regex, spelling, text, xml.
(Run with --trace for a full exception trace.)

I believe this is because src/lint/linter/ArcanistPyLintLinter.php is missing a getLinterConfigurationName function.
Indeed applying the following change fixes the problem:

diff --git a/src/lint/linter/ArcanistPyLintLinter.php b/src/lint/linter/ArcanistPyLintLinter.php
index 9ce9202..ae77d51 100644
--- a/src/lint/linter/ArcanistPyLintLinter.php
+++ b/src/lint/linter/ArcanistPyLintLinter.php
@@ -199,6 +199,10 @@ final class ArcanistPyLintLinter extends ArcanistLinter {
     return 'PyLint';
   }
 
+  public function getLinterConfigurationName() {
+    return 'pylint';
+  }
+
   private function getLinterVersion() {

If this is the correct solution could we please get this fixed on master?

Revisions and Commits

Event Timeline

trolas raised the priority of this task from to Unbreak Now!.
trolas updated the task description. (Show Details)
trolas added a subscriber: trolas.
epriestley lowered the priority of this task from Unbreak Now! to Normal.May 29 2014, 4:22 PM
joshuaspence added a subscriber: joshuaspence.
joshuaspence removed a subscriber: joshuaspence.
joshma added a subscriber: joshma.Aug 12 2014, 6:14 PM

any news on this? someone having the same problem?

Pylint hasn't been updated to support .arclint yet, and is sketchily written. It should be updated more comprehensively than by just adding a method to it, but doing so in a way that doesn't break backward compatibility will be difficult. This is hard to prioritize because it affects a relatively small number of users, there are several other python linters available (pyflakes, pep8, flake8), and testing and updating this class will be time consuming, and we'll likely have to break some of the really old options which will take further time to support.

jra3 added a subscriber: jra3.Aug 25 2014, 10:21 PM

I've been having the same issue myself. Sadly, pylint catches some useful stuff that the other 3 do not.

e.g. today I had some code like

from time import time # importing just this one function function
...
time.ctime() # oh hey! the time module has that function but that's not what we did! the time function masks the problem

and only pylint caught that one.

Pylint is recognised by a lot of people as a better python linter than the others, supports parsable output and catches more errors than flake8/pyflakes/pep8.
I'm definitely not an expert of php or this project but I am fairly confident that adding that method will allow us to start using pylint from arcanist rather than the less comprehensive flake8. I don't think it can break any existing functionality to do so.

I understand the reluctance to apply what seems to be a partial fix without reviewing the rest of the ArcanistPyLintLinter.php but I wonder how big of an impact it can have since as far as I understand it's not working at all at the moment.

The python "linting ecosystem' is one of those places where the only way to do a good job is "support all the things"

Pylint is not a better linter. Its a structural checking tool and utterly indispensable.

You could get away with not supporting pep8 or pyflakes, because you have flake8, but you cannot do the same with pylint.
I don't care if I have to write this myself because in addition to pyflakes, I want to add support for prospector and my own custom python check tools so I need to write code anyway, but can we get some guidance on this, is the code suggested by @trolas still correct, should I be using that to modify the existing pylint code to make my own linter?

avivey added a subscriber: avivey.Feb 9 2015, 6:09 PM

Would id make sense, in the Python community, to have flake8 also wrap Pylint?

How common is flake8 in the wild? How common is flake8 + Pylint?

(https://secure.phabricator.com/D10704 is a basic example for getting lint configuration from .arclint file. That + the patch above should get you ~75% there).

vrusinov added a subscriber: vrusinov.
joshuaspence closed this task as Resolved.May 28 2015, 9:12 PM
joshuaspence added a subscriber: joshuaspence.

This should have been resolved by modernizing ArcanistPylintLinter in D9109.