Add invalid-lazy-import for PEP 810 imports Python rejects - #11209
Add invalid-lazy-import for PEP 810 imports Python rejects#11209Pierre-Sassoulas wants to merge 2 commits into
invalid-lazy-import for PEP 810 imports Python rejects#11209Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #11209 +/- ##
=======================================
Coverage 96.41% 96.41%
=======================================
Files 178 178
Lines 20068 20099 +31
=======================================
+ Hits 19348 19379 +31
Misses 720 720
🚀 New features to boost your workflow:
|
This comment has been minimized.
This comment has been minimized.
d684d05 to
c125862
Compare
This comment has been minimized.
This comment has been minimized.
c125862 to
1ac3d1a
Compare
This comment has been minimized.
This comment has been minimized.
The false positives fixed in the previous commits are not py-version
dependent: the element of a comprehension is not an assignment target, and
'dict([*pairs for pairs in nested])' is not equivalent to a key/value dict
comprehension, whichever version the code targets. What does depend on
py-version is whether the syntax exists at all, and 'unsupported_version'
is where pylint says so, next to f-strings, the walrus operator, 'except*'
and PEP 695.
'using-comprehension-unpacking-in-unsupported-version' (W2607) reports
'[*element for element in elements]' and '{**element for element in
elements}' when py-version still includes a version that cannot compile
them.
The documentation example cannot be parsed before 3.15, so
'doc/test_messages_documentation.py' learns to skip an example declaring
the version it needs in a 'min_pyver' file, the example asks for the newest
grammar in 'doc/data/ruff.toml', and black is excluded from it because it
cannot check that it preserved the meaning of code the interpreter it runs
on cannot parse. pylint-dev#11209 adds the same 'min_pyver' support for its own
example, so whichever of the two lands second drops the duplicate.
Refs pylint-dev#10982
292b4db to
9ba9f73
Compare
This comment has been minimized.
This comment has been minimized.
9ba9f73 to
db8d774
Compare
This comment has been minimized.
This comment has been minimized.
db8d774 to
20b8788
Compare
This comment has been minimized.
This comment has been minimized.
The false positives fixed in the previous commits are not py-version
dependent: the element of a comprehension is not an assignment target, and
'dict([*pairs for pairs in nested])' is not equivalent to a key/value dict
comprehension, whichever version the code targets. What does depend on
py-version is whether the syntax exists at all, and 'unsupported_version'
is where pylint says so, next to f-strings, the walrus operator, 'except*'
and PEP 695.
'using-comprehension-unpacking-in-unsupported-version' (W2607) reports
'[*element for element in elements]' and '{**element for element in
elements}' when py-version still includes a version that cannot compile
them.
The documentation example cannot be parsed before 3.15, so
'doc/test_messages_documentation.py' learns to skip an example declaring
the version it needs in a 'min_pyver' file, the example asks for the newest
grammar in 'doc/data/ruff.toml', and black is excluded from it because it
cannot check that it preserved the meaning of code the interpreter it runs
on cannot parse. #11209 adds the same 'min_pyver' support for its own
example, so whichever of the two lands second drops the duplicate.
Refs #10982
DanielNoord
left a comment
There was a problem hiding this comment.
Couple of questions, but already looks pretty good. Needs a rebase though
| assert isinstance(guard, nodes.If) | ||
| assert utils.zealous_is_sys_guard(guard) is True, guard.as_string() | ||
|
|
||
| for not_a_guard in code[10:]: |
There was a problem hiding this comment.
Probably better to split this test up at the 10th index, that makes it a lot easier to see what is being tested.
| return modname | ||
|
|
||
|
|
||
| def is_lazy_import(node: nodes.Import | nodes.ImportFrom) -> bool: |
| return False | ||
|
|
||
|
|
||
| def zealous_is_sys_guard(node: nodes.If) -> bool: |
There was a problem hiding this comment.
Is this really that much slower that we can't always call this improved version?
There was a problem hiding this comment.
Yeah, I was wondering that too. Plus we need corectness more than we need pylint being 1% faster.
There was a problem hiding this comment.
We have a lot of those compromise with corectness against performance, for numpy in particular if it's not imported as np. or numpy we sometime don't bother. Or for aliases. Going all in on handling this might be what differentiate pylint tbh.
There was a problem hiding this comment.
I would vote to just always be zealous in this particular case
A lazy import is only valid at module level, outside any try statement, and never as a wildcard import. Python raises a SyntaxError for the other positions, but 'ast' parses them, so pylint sees a normal module and has to report them itself. 'astroid' 4.3.0 renamed 'Import.lazy' to 'Import.is_lazy' and pylint accepts 'astroid>=4.2.0b5,<=4.3', so the new 'is_lazy_import' helper asks for either spelling. Reading one of them directly would raise an 'AttributeError' on every import statement for half of the supported range, and the test suite pins 4.2.0b5, so CI could not see it. The documentation examples for the new message can only be parsed by Python 3.15, so the message documentation test now skips an example that declares a 'min_pyver'.
20b8788 to
3a395af
Compare
'invalid-lazy-import' says where Python forbids a lazy import; it says nothing about whether the interpreter the code targets knows the syntax at all. That is what 'unsupported_version' is for, next to f-strings, the walrus operator, 'except*' and PEP 695. 'using-lazy-import-in-unsupported-version' (W2608) reports 'lazy import module' when py-version still includes a version that cannot compile it. The documentation example reuses the 'min_pyver' skip added for 'invalid-lazy-import', and asks for the newest grammar in 'doc/data/ruff.toml' like its 'good.py' does. W2607 is already taken by 'using-comprehension-unpacking-in-unsupported-version', which reports the PEP 798 unpacking the same way. Refs pylint-dev#10982
This comment has been minimized.
This comment has been minimized.
3a395af to
5a68efa
Compare
|
🤖 According to the primer, this change has no effect on the checked open source code. 🤖🎉 This comment was generated for commit 5a68efa |
|
Need to merge #11293 first. |
Type of Changes
Description
Adds
invalid-lazy-import(E0403) andusing-lazy-import-in-unsupported-version(W2608), split out of #10983 so that PR stays limited to making pylint work on
Python 3.15 rather than growing new checks.
The version-guard work that was here — and that @DanielNoord reviewed — moved to
its own PR, #11293: it is about
is_sys_guard, not about PEP 810.invalid-lazy-importPEP 810 only allows a
lazyimport at module level, outside of anytrystatement, and never as a wildcard import. Python rejects the other positions
when it compiles the module, but
ast.parseaccepts them all, so astroidbuilds a perfectly ordinary module and pylint had nothing to say about code that
cannot run.
All seven positions CPython rejects are reported:
lazy importinside a function orasyncfunctionlazy importinside a class bodylazy importin atrybody,except,elseorfinally, includingexcept*lazy from x import *None of the legal positions are flagged:
if,for,while,withandmatchat module level are all accepted. I checked the classification against CPython
3.15.0b1 by compiling each snippet, and ruff's parser independently agrees on the
same set — it rejects
lazy import jsoninsidetrywith the same reasoning.The message carries
minversion=(3, 15), so it cannot fire on interpreters wherethe syntax does not exist.
astroid4.3.0 renamedImport.lazytoImport.is_lazyand pylint acceptsastroid>=4.2.0b5,<=4.3, so anis_lazy_importhelper asks for either spelling.Reading one of them directly raises an
AttributeErroron every import statementfor half of the supported range, and the test suite pins 4.2.0b5, so CI cannot
see it. The helper carries a
TODOto readnode.is_lazydirectly once the pinrequires 4.3, which #11234 does.
using-lazy-import-in-unsupported-versioninvalid-lazy-importsays where Python forbids a lazy import; it says nothingabout whether the interpreter the code targets knows the syntax at all. That is
what
unsupported_versionis for, next to f-strings, the walrus operator,except*and PEP 695. W2608 reportslazy import modulewhenpy-versionstillincludes a version that cannot compile it.
Documentation examples on 3.15-only syntax
The examples for a message about 3.15 syntax cannot be parsed by an older
interpreter, and
doc/test_messages_documentation.pylints every example withwhatever interpreter is running — CI uses
DEFAULT_PYTHON. An example directorymay now declare a
min_pyverfile and be skipped below that version.That gate is deliberately about parsing, not about whether the message applies.
A message that merely does not apply to every
py-versionalready has a way tosay so: pin
py-versionin the example's ownpylintrc, the wayboolean-datetimedoes. My first attempt keyed the skip on the message's ownminversion/maxversionmetadata instead, which was wrong — pylint comparesthose against the configured
py-version, not the interpreter, and it silentlyskipped
boolean-datetimeandreturn-arg-in-generator, which had been passing.The examples are also excluded from
ruff-doc, which targets py310 and cannotparse them.
black26.5.1 handles PEP 810 natively, soblack-docneeds nothing.Testing
tests/anddoc/test_messages_documentation.pyare green on 3.15.0b1 and on3.13, where the two new examples skip, at each commit. One failure on 3.15 is
pre-existing on
mainand fixed by #10983, not by this PR: theunspecified-encodingexample raises nothing on a 3.15 interpreter becausePEP 686 gives that message
maxversion=(3, 15).Refs #10982