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

Automatically respect .pylintrc files in subdirectories #442

Closed
thernstig opened this issue Dec 16, 2017 · 29 comments
Closed

Automatically respect .pylintrc files in subdirectories #442

thernstig opened this issue Dec 16, 2017 · 29 comments
Labels
area-editor-* User-facing catch-all area-linting feature-request Request for new features or functionality

Comments

@thernstig
Copy link

Environment data

VS Code version: 1.19.0
Python Extension version: 0.9.0 (14 December 2017)
Python Version: 3.6.1
OS and version: Windows 10 Home, Version 1703, OS-version 15063.483

Actual behavior

.pylintrc files in a directory in the workspace are not automatically found by pylint

Expected behavior

  • .pylintrc files found in a directory should be automatically respected for that folder and all sub-directories.
  • New .pylintrc files in sub-directories should take precedence over parent directories .pylintrc files

This is how it works when running the pylint command via terminal. .pylintrc files are automatically respected without supplying them specifically via the option --rcfile

Note that I am unsure if other options are respected as defined here
https://pylint.readthedocs.io/en/latest/user_guide/run.html#command-line-options

Steps to reproduce:

  • Create a .pylintrc file in a directory in the workspace and notice how the rules defined are not respected
@DonJayamanne DonJayamanne added awaiting 2-PR area-linting feature-request Request for new features or functionality labels Dec 17, 2017
@MikhailArkhipov MikhailArkhipov self-assigned this Feb 1, 2018
@MikhailArkhipov MikhailArkhipov added this to the February 2018 milestone Feb 1, 2018
@MikhailArkhipov
Copy link

d44386e

@thernstig
Copy link
Author

@MikhailArkhipov Was this supposed to be included in release 2018.3.0 (28 Mar 2018)? Because I have just downloaded that version and it does not seem to work? The Problems view still shows the pylint errors that should not be seen du to a .pylintrc int he same directory as the file being linted.

@MikhailArkhipov
Copy link

Can you attach the file and tell which errors are showing up?

@thernstig
Copy link
Author

Here is what my .pylintrc looks like

[MESSAGES CONTROL]
disable=
    missing-docstring,
    too-few-public-methods,
    no-self-use,
    unused-argument

[DESIGN]
# Maximum number of parents for a class (see R0901).
max-parents=15

Attached is a picture showing that VS code still gives errors.
problems
pythonversionvscode

@MikhailArkhipov
Copy link

OK, I can't replicate in a basic case. For example, empty .pylintrc

image

with your .pylintrc

image

@MikhailArkhipov
Copy link

What happens when you run pylint from command line on that file from the folder the file is in?

@thernstig
Copy link
Author

@MikhailArkhipov Could this in any way be related to that I run on Windows? Maybe a similar issue such as this? pylint-dev/pylint#1941

It works just fine running from command line:

(venv) ~\Desktop\projects\A [master]> pylint --rcfile=crawlers/.pylintrc crawlers
Using config file C:\Users\Moo\Desktop\projects\A\crawlers\.pylintrc

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

(venv) ~\Desktop\projects\A [master]> cd .\crawlers\
(venv) ~\Desktop\projects\A\crawlers [master]> pylint --rcfile=.pylintrc crawlers
Using config file C:\Users\Moo\Desktop\projects\A\crawlers\.pylintrc

--------------------------------------------------------------------
Your code has been rated at 10.00/10 (previous run: 10.00/10, +0.00)

@brettcannon brettcannon reopened this Apr 2, 2018
@brettcannon brettcannon modified the milestones: February 2018, April 2018 Apr 2, 2018
@brettcannon brettcannon added needs verification bug Issue identified by VS Code Team member as probable bug and removed needs PR feature-request Request for new features or functionality labels Apr 2, 2018
@MikhailArkhipov
Copy link

I use Windows too mostly. You are specifying pylintrc file location explicitly. Can you try running pylint from the file folder without --rcfile ?

@brettcannon brettcannon added info-needed Issue requires more information from poster and removed needs verification labels Apr 3, 2018
@thernstig
Copy link
Author

(venv) ~\Desktop\projects\A\crawlers [master]> pylint .
Using config file C:\Users\Moo\Desktop\projects\A\crawlers\.pylintrc
(venv) ~\Desktop\projects\A\crawlers [master]> pylint crawlers
Using config file C:\Users\Moo\Desktop\projects\A\crawlers\.pylintrc

------------------------------------
Your code has been rated at 10.00/10

@DonJayamanne
Copy link

@MikhailArkhipov
I think we need to run pylint using the file as the cwd. This way pylint can walk up the directory tree and pick the closest config file. Right now, the cwd is being set to the workspace directory.

We're doing this with the formatters today, for the same reason.

@thernstig
Copy link
Author

@MikhailArkhipov Any progress on this? :) It kind of died.

@brettcannon
Copy link
Member

@thernstig it's currently scheduled for our April release so this has not been forgotten (we're just really swamped with things ATM).

@MikhailArkhipov
Copy link

MikhailArkhipov commented Apr 17, 2018

Current behavior is not a bug. The extension have been running pylint from the workspace root for quite some time const cwd = this.getWorkspaceRootPath(document);.

Running pylint from the file folder would prevent it from finding pylintrc at the workspace root. Pylint does not walk up the tree in all cases. See

https://github.com/PyCQA/pylint/blob/975e08148c0faa79958b459303c47be1a2e1500a/pylint/config.py
https://pylint.readthedocs.io/en/latest/user_guide/run.html

  1. pylintrc in the current working directory
  2. .pylintrc in the current working directory
  3. If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a pylintrc file. This allows you to specify coding standards on a module-by-module basis. Of course, a directory is judged to be a Python module if it contains an init.py file.
  4. The file named by environment variable PYLINTRC
  5. if you have a home directory which isn’t /root:
  • .pylintrc in your home directory
  • .config/pylintrc in your home directory
  1. /etc/pylintrc

So changing cwd would be significant change in the linting behavior. One possibility is to run it from file folder AND supplying path to .pylintrc if it exists at the workspace root.

@DonJayamanne DonJayamanne added the feature-request Request for new features or functionality label Apr 17, 2018
@brettcannon brettcannon removed this from the May 2018 milestone Jun 6, 2018
@thernstig
Copy link
Author

@brettcannon Did this fell out and is not planned for any milestone? The issue has a few upvotes so I assume it is popular in some sense. :)

@thernstig
Copy link
Author

@MikhailArkhipov I just read your comment and I have a question about this part:

If the current working directory is in a Python module, Pylint searches up the hierarchy of Python modules until it finds a pylintrc file. This allows you to specify coding standards on a module-by-module basis. Of course, a directory is judged to be a Python module if it contains an __init__.py file.

I am not sure how to interpret this.

  1. First, how can a working directory be "in a Python module"? I am not sure how to read that. I would guess it means that if the current working directory contains a contains a Python module?
  2. Assuming I understand 1. correctly: Since Python 3.3+ a package does not need an __init__.py to be considered a package. Note that a package == module (at least Python interprets the type of an imported package).

I am most likely missing something. If I am not, then as I read it, if you run Pylint from the file folder it will eventually traverse upwards to find the .pylintrc in the root folder.

@brettcannon
Copy link
Member

@thernstig

  1. Read it as "if the current working directory has a Python module"

If someone wanted to come up with a PR to support running Pylint in the directory that the code being analyzed is in then we would be happy to review it, but right now this isn't a priority due to other work taking up our time.

@brettcannon brettcannon changed the title Automatically respect .pylintrc files Automatically respect .pylintrc files in subdirectories Dec 10, 2018
@gramster gramster added the area-editor-* User-facing catch-all label Oct 10, 2019
@a1291762
Copy link

I found this bug in VSCode, then found this bug report.

In my case, I have a folder (~/work/git) that is opened in vscode with a bunch of sub-directories (separate git repositories). One of these had a .pylintrc added to it. In theory, it's the same as what VSCode was using for linting. It was added so that our CI system could run pylint with those settings.

As per this bug, VSCode is not using that .pylintrc but what I'm seeing is that VSCode isn't even using it's default pylint settings either. As a result, I am seeing extra pylint warnings and a heap of pylint infos.

If I remove the .pylintrc, the extra warnings/infos go away.
If I copy the .pylintrc to ~/work/git, the extra warnings/infos go away.

Luckily, I don't want extra pylint stuff, just the standard VSCode rules running over everything, so I'm able to copy the file to ~/work/git to work around the bug.

Version: 1.40.0
Commit: 86405ea23e3937316009fc27c9361deee66ffbf5
Date: 2019-11-06T18:14:08.920Z
Electron: 6.1.2
Chrome: 76.0.3809.146
Node.js: 12.4.0
V8: 7.6.303.31-electron.0
OS: Linux x64 4.15.0-66-generic snap

@SheepDomination
Copy link

I'd suggest to use @JamesMessinger solution, atleast your workspace will find the .pylintrc file and so pylint will lint any files within your workspace automatically; then again you can do it manually though the terminal.

@mgasner
Copy link

mgasner commented May 19, 2022

This is surprising behavior! Especially in larger repositories or monorepos, it's likely that different pylint standards will be applied to different parts of the directory tree or even to submodules. For example, you might want to enforce the existence of module-level docstrings in a library, but not in test files.

@github-actions github-actions bot removed the needs PR label Aug 9, 2022
@karrtikr karrtikr added the needs PR Ready to be worked on label Aug 9, 2022
@luctowers
Copy link

luctowers commented Aug 27, 2022

One possibility is to run it from file folder AND supplying path to .pylintrc if it exists at the workspace root.

This still seems like a pretty significant change from the current linting behaviour. And also pretty arbitrary. Because pylint doesn't just get its configuration from .pylintrc eg. pylintrc (without dot), pyproject.toml, setup.cfg and a million different rules. Pylint configuration is a mess honestly.

I think a good compromise would just be adding a boolean setting for the extension that tells it whether or not to run the linting in file's directory or the workspace directory. This would also be useful for other linters like flake8 which have similar issues in this extension.

Thoughts?

@luctowers
Copy link

Not to mention that pylint for some reason produces different linting decisions for its import ordering depending on which directory you call it from, regardless of any configuration you provide it. Which is fun. I think it is probably best to keep current behaviour, and put this enhancment behind a setting imo.

@ssbarnea
Copy link

I face a similar problem with our own linter and we decided to not allow more than one configuration file. There is no good way to fix the problem of multiple config files, especially in the context of partial or live linting in LS style.

I would recommend a very different path here: making the multi-configuration a warning in the two tools that support it, basically deprecating it. I would support that and probably it is an easier way for everyone.

Regarding the config location, it should be up to the tool (linter) itsel to be able to report it, so the LS would even need to know where it is stored. I do not think that we need UI features for editing linter own settings, just the on/off.

@karthiknadig
Copy link
Member

The pre-release version of pylint extension https://marketplace.visualstudio.com/items?itemName=ms-python.pylint has a setting pylint.cwd that you can set to ${fileDirname}. This runs pylint in the parent directory for the file. this should allow pylint config search to pick up .pylintrc.

@github-actions github-actions bot removed the needs PR Ready to be worked on label Oct 26, 2023
@thernstig
Copy link
Author

I have now gone Ruff instead 😄

@karthiknadig
Copy link
Member

karthiknadig commented Oct 27, 2023

@thernstig ruff is a multi-linter, import sorted, and now formatter implemend in rust with python wrapper package for it. The ruff extension is built on top of the python tools template we provide for the community to contribute extensions for their favorite tools. ruff-lsp the underlying python package for this is based on lsprotocol another package we create for the community to implement wrappers for tools that can integrate with any IDE not limited vscode.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 27, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-editor-* User-facing catch-all area-linting feature-request Request for new features or functionality
Projects
None yet
Development

No branches or pull requests