Fix pylintrc search #805
Fix pylintrc search #805
Conversation
Codecov Report
@@ Coverage Diff @@
## master #805 +/- ##
==========================================
+ Coverage 63.42% 63.64% +0.21%
==========================================
Files 258 258
Lines 11770 11807 +37
Branches 2082 2088 +6
==========================================
+ Hits 7465 7514 +49
+ Misses 4297 4285 -12
Partials 8 8
Continue to review full report at Codecov.
|
src/client/linters/pylint.ts
Outdated
| @@ -30,7 +30,7 @@ export class Pylint extends BaseLinter { | |||
| const settings = this.configService.getSettings(uri); | |||
| if (settings.linting.pylintUseMinimalCheckers | |||
| && this.info.linterArgs(uri).length === 0 | |||
| && !await Pylint.hasConfigurationFile(this.fileSystem, uri.fsPath, this.platformService)) { | |||
| && !await Pylint.hasConfigurationFile(this.fileSystem, this.getWorkspaceRootPath(document), this.platformService)) { | |||
There was a problem hiding this comment.
Its possible to have pylintrc next to the file itself, and not in the root directory. I don't think this would work under that scenario.
src/client/linters/pylint.ts
Outdated
| const uri = document.uri; | ||
| const settings = this.configService.getSettings(uri); | ||
| if (settings.linting.pylintUseMinimalCheckers | ||
| && this.info.linterArgs(uri).length === 0 | ||
| // Check pylintrc next to the file | ||
| && !await Pylint.hasConfigurationFile(this.fileSystem, path.dirname(uri.fsPath), this.platformService) |
There was a problem hiding this comment.
Does this work if we have a pylintrc in a sub directory? Not in workspace root, and not in same directory as pylint.
Workspace = c:\dev\project
Pylintrc = c:\dev\project\modules.pylintrc
Python file = c:\dev\project\modules\module1\helper.py
There was a problem hiding this comment.
If i'm not mistaken, pylint searces from current file path upwards.
There was a problem hiding this comment.
According to pylint code it only searches upwards within modules and breaks search if __init__.py is missing.
https://pylint.readthedocs.io/en/latest/user_guide/run.html#command-line-options
- pylintrc in the current working directory
- .pylintrc in the current working directory
- 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__.pyfile. - The file named by environment variable PYLINTRC
- if you have a home directory which isn’t /root:
a. .pylintrc in your home directory
b. .config/pylintrc in your home directory - /etc/pylintrc
which is what pylint code does.
def find_pylintrc():
...
curdir = os.path.abspath(os.getcwd())
while os.path.isfile(os.path.join(curdir, '__init__.py')):
curdir = os.path.abspath(os.path.join(curdir, '..'))
if os.path.isfile(os.path.join(curdir, 'pylintrc')):
return os.path.join(curdir, 'pylintrc')
if os.path.isfile(os.path.join(curdir, '.pylintrc')):
return os.path.join(curdir, '.pylintrc')
We do exactly this from the file location - but linter.run may change working directory to the root - this case was missing. Hence need to check file location and the root.
However, I don't mind relaxing the rule and search upwards in all cases, but it won't match pylint behavior.
There was a problem hiding this comment.
I guess there may be case when there is no config at the root, but there is one in B but not in C
root -> B -> C so we won't find it but pylint will. Let me relax the search.
Fixes #788
runchanges working folder to workspace root while config search searches from the file path.Fixes #786
Added handling of quote escapes