pylint
pylint copied to clipboard
pylint-dev
Do not lint ignored file on stdin
Type of Changes
| Type | |
|---|---|
| ✓ | :bug: Bug fix |
Description
This PR adds a check to the see whether the filename passed with the --from-stdin option is ignored. The input will not be linted if it is. A corresponding test is also included.
Closes #4354
Pull Request Test Coverage Report for Build 2977710138
- 5 of 5 (100.0%) changed or added relevant lines in 1 file are covered.
- No unchanged relevant lines lost coverage.
- Overall coverage increased (+0.0003%) to 95.319%
| Totals | |
|---|---|
| Change from base Build 2977378583: | 0.0003% |
| Covered Lines: | 16942 |
| Relevant Lines: | 17774 |
💛 - Coveralls
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉
This comment was generated for commit 30d85dfbc529efd0a2a7f351632710e31229f1c7
![github-actions[bot] avatar](https://avatars.githubusercontent.com/in/15368?v=4 "Published by a pub.dev verified publisher"){kind=small}
@Pierre-Sassoulas @cblessing24 Would we be okay with merging this without the refactor? This is breaking the pylint plugin for VS Code, see https://github.com/microsoft/vscode-pylint/issues/169
I think it make sense to offer "first-class" support for the extension as a large part of our user base is likely using VS Code. We could potentially get this in 2.15.1.
Right, but this is technical debt and if this kind of treatment isn't DRY we're going to have a bad time in the long term. We already have a bad time now, this issue is clearly a problem of not treating all sources the same way. Often new contributors come, fix an issue they want to fix but are not expert in the codebase so they do not know that there's two places to modify, tests only the one they changed but not the other and we get inconsistencies (new issues are opened, new contributor come, etc.). I think this is a vicious circle.
I'm not saying we should not create this technical debt but we should make sure we think about it.
@cblessing24 I'm quite familiar with this code. Would you be okay with me taking a stab at this refactor?
My proposed refactor is actually quite small.
See: https://github.com/PyCQA/pylint/pull/6528/files
In that PR we already extracted the ignore checking into a separate function out of _expand_modules so that we could re-use it.
However, I don't think there is a good single place to call this. In _expand_modules we need to call it halfway through the discovery because we need to determine whether we should continue in certain directories. We do this while iterating so there is not really a good way to extract that from the full process.
For from-stdin we don't want to "expand_modules" as we create a FileItem based on an import that astroids import system gives us. By entering into expand_modules we would incur a lot of side effects that can only cause further issue with the only benefit of one less call to _is_ignored_file.
The third and final call is done for the --recursive option which has a similar issue as expand_modules. It is called while iterating so we can't really extract more than maths already did by creating a separate function for it.
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉
This comment was generated for commit 9da9b5d44bac02e9c7dded54cfda3c1dde1667bd