pylint-dev / pylint

It's not just a linter that annoys you!
https://pylint.readthedocs.io/en/latest/
GNU General Public License v2.0
5.32k stars 1.14k forks source link

Disable messages based on linters already installed locally #3517

Closed Pierre-Sassoulas closed 4 years ago

Pierre-Sassoulas commented 4 years ago

As seen in #3512 we want to disable message that aren't useful to the user. We can imagine that when another linter is installed with the same set of feature the user is using it. The following linter comes to mind:

Describe the solution you'd like

When we see that a linter is installed (by trying to import it API) we do not execute some checks. This speed pylint up and make the warning shown more relevant. Most of the work in order to know what check exactly to disable was done by @AWhetter in #3512 (for example bare-except is W0702 in pylint, and E722 in flake8 so we can disable W0702 if flake8 is installed)

We could add a warning to the user that some checks are automatically avoided and an option to disable the behavior. This feature would go well with default configurations.

PCManticore commented 4 years ago

Hey @Pierre-Sassoulas Thank you for opening the issue.

This is something that I don't agree in the suggested form. For personal projects I use both flake8, pylint and other tools such as mypy, since all of them catch things that the other tools do not catch. It's very possible that there are folks out there that try to use these tools in tandem as well, and this behaviour might be a bit surprising.

If this would be under a disabled-by-default flag, I wouldn't mind having it, but at this point it seems to me that we're trying to make pylint "friendlier" than it should, since other tools don't consider disabling their messages when pylint is detected. Disabling certain categories by default could also be an alternative, for instance disabling the format category regardless if black is installed or not.

The original issue for making pylint a bit more sane https://github.com/PyCQA/pylint/issues/746 suggested the addition of separate stages in which we could lump together various checks that are related via a given property (e.g. those that are a bit more advanced, but have a high yield of false positives can live under a particular stage that's not required for all users of pylint to consume). Rather than going the inference route of figuring out what other tools are installed and trying to be amenable to their existence, I would prefer we go to the separate stages route instead.

Pierre-Sassoulas commented 4 years ago

f this would be under a disabled-by-default flag,

Yeah I think the default needs to be very consensual and not have a single false positive. So a lot of things should be disabled by default.

I think we have this discussion because the configuration/option code need a very huge refactor, it does not seem to even use argparse. What I propose here likely requires a huge amount of rework before being easy to do. Easier option with argparse and configuration inheritance would make it something like :

if Configuration.disable_redondant_with_installed_linter()
    try:
        import black
        Configuration.inherits("formatting-handled-by-black")
    except ImportError:
       pass

Being friendly is admitedly easier if it cost you only 7 line of code :D

thejcannon commented 4 years ago

Something to consider is with tools like pre-commit which run each tool in isolation (helps avoid dependency conflicts) this feature would be moot. The user would still need to configure each tool.

I think having overlapping responsibilities from using multiple linters in tandem is a symptom of the problem that people need multiple linters. Instead of playing nicely with other tools, pylint should identify the gap and fill it.

As for linter-vs-style-formatter, my opinion would be that pylint stops worrying about style now that we have style tools (something that's echod in #746). pylint can focus on finding bugs/issues.

Pierre-Sassoulas commented 4 years ago

Good point, about pre-commit, I'm going to close this as indeed the problem would be best solved with being able to generate specific configuration compatible with tools easily (some of it was documented in #3647 ), and a better documentation around configuration file.