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

Should pylint be equivalent to pylint . ? #5701

Open
Pierre-Sassoulas opened this issue Jan 19, 2022 · 12 comments · May be fixed by #9072
Open

Should pylint be equivalent to pylint . ? #5701

Pierre-Sassoulas opened this issue Jan 19, 2022 · 12 comments · May be fixed by #9072
Labels
Blocker 🙅 Blocks the next release Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Milestone

Comments

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Jan 19, 2022

Current problem

Right now pylint is displaying the help if called without argument. We could also default to the current directory i.e. pylint would be equivalent to pylint ..

Desired solution

If you want pylint to be equivalent to pylint . then react to this post with 👍, if not react with 👎. Most popular solution makes it to pylint 3.0.

Additional context

Tools like flake8 or pytest default to current directory, mypy or isort do not.

More generally, if a tool can work without argument like ls or pwd it does, if it can't because it makes no sense without arguments then it shows the help like mv, or scp, so I'm personally in favor as I think linting the current directory make sense, but I did not find the rational for mypy or isort to not use the current directory as default.

Original proposition done in #352 (comment), created because of #5682 (comment)

@DanielNoord
Copy link
Collaborator

To explain why I'm against this change:

  1. This is a large breaking change as it changes the behaviour of how pylint is invoked. The benefit is that you wouldn't need to type " .", which is minimal in my opinion and therefore does not warrant such a large breaking change.
  2. Arguably, this is also incorrect as when a user invokes pylint without any directories are files as argument we don't know what we're supposed to lint. We can make the assumption that it should be the current directory, like this proposal does, but I don't think we should make assumptions like that.

I prefer the approach mypy takes which supports a files option in the configuration file which, when set, then allows invoking mypy without any other arguments. In that case the files argument is already set in the configuration.

Benefits of this approach are that it would be non breaking (the behaviour would only change when files is set, which would only be so for user that have updated their configuration) and not make any assumptions about users' intentions.
The only additional effort required by the user is to set that files option once.

@orSolocate
Copy link
Contributor

@DanielNoord just to make sure I understand your suggestion:
You suggest to add a new optional entry called files or files_default in setup.cfg under [options] and set the . there? something like:

[options]
files = .:

This way when the user invoke pylint with no args it would be equivalent to pylint .?
and if the user wants to run specific files or directory he can without breaking old behavior?

@DanielNoord
Copy link
Collaborator

Yes. If you specify the files option on the command line it would overwrite whatever is in setup.cfg as it has higher priority. You could specify project/subdir if you're only ready to run pylint over a sub directory.
Technically we already support the files option: the arguments passed on the command line that do not fit in any other option are read as files, but we do not support adding it to your config file.

@Avasam
Copy link

Avasam commented Feb 12, 2022

This is what I'm doing at the moment for pylint to work on my entire project:
pylint --score=n --output-format=colorized $(git ls-files 'backend/*.py')

@kdeldycke
Copy link

Looks like pylint --recursive=y . now does the trick since v2.13.0. See: #352.

@yaleman
Copy link

yaleman commented May 5, 2022

I'd support it, if I could figure out a way to exclude directories like .venv/ - despite "documentation" and fixes, I've just spent an hour trying to figure out how to exclude that directory, without success.

@Pierre-Sassoulas
Copy link
Member Author

Pierre-Sassoulas commented May 5, 2022

There's an ignore option and ignore-patterns option. First search result for "pylint exclude directory" : https://stackoverflow.com/questions/2503717/. Please open a new issue to tell us how can we make the documentation better :) (Edit: #6471 needs to be fixed)

@smac89
Copy link

smac89 commented Aug 5, 2022

@Pierre-Sassoulas actually the option you need is --ignore-paths .

@yaleman Try --ignore-paths='.venv'

@DanielNoord
Copy link
Collaborator

I have a solution for this in #7496 that is non-breaking. Anybody who is looking for this feature is more than welcome to test it out and give feedback. This is crucial behaviour so I'm very wary of any backward incompatible changes, ironing those out before release is crucial.

@ssbarnea
Copy link
Contributor

ssbarnea commented Dec 1, 2022

In absence of configuration, pylint and pylint . should do the same. Still, when configuration exists, . should override whichever default value was configured inside the config.

Another example is that pylint without arg should allow it to find its config in parent directories and run from there.

PS. I maintain several linters.

@Pierre-Sassoulas Pierre-Sassoulas unpinned this issue Dec 11, 2022
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0, 3.0.0b1 Mar 9, 2023
@Pierre-Sassoulas Pierre-Sassoulas added Needs PR This issue is accepted, sufficiently specified and now needs an implementation and removed Needs decision 🔒 Needs a decision before implemention or rejection labels Mar 9, 2023
@Pierre-Sassoulas
Copy link
Member Author

Pierre-Sassoulas commented Mar 9, 2023

With 85% in favor (45 👍 , 8 👎 as of 2023-03-09 after this issue being pinned for 14 months), pylint will be equivalent to pylint . in 3.0.0.

@Pierre-Sassoulas Pierre-Sassoulas added Blocker 🙅 Blocks the next release and removed Discussion 🤔 labels Aug 3, 2023
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 25, 2023
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 25, 2023
Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 25, 2023
@Pierre-Sassoulas Pierre-Sassoulas linked a pull request Sep 25, 2023 that will close this issue
@Pierre-Sassoulas Pierre-Sassoulas modified the milestones: 3.0.0b1, 4.0.0 Sep 30, 2023
@Pierre-Sassoulas
Copy link
Member Author

Moving milestone to 4.0.0 because we want to release a new version of pylint on time for python 3.12 release on 2023-10-02 and supporting python 3.12 in a 2.X branch is not reasonable.

Pierre-Sassoulas added a commit to Pierre-Sassoulas/pylint that referenced this issue Sep 30, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Blocker 🙅 Blocks the next release Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants