Skip to content

Commit c125862

Browse files
Recognize version guards however sys.version_info is spelled
is_sys_guard only matches the literal 'sys.version_info' attribute access, so an import guarded by 'from sys import version_info' still raised import-error when the guarded module was missing. PEP 810 forbids wrapping a lazy import in a try statement (the new invalid-lazy-import above), which leaves version guards as the pattern for version-dependent imports, so pylint had better recognize them reliably. import-error is only checked once an import has already failed, so the guard detection can afford to be thorough: the new zealous_is_sys_guard resolves the compared names instead of matching a spelling, and also recognizes aliased imports, sys.hexversion, six's PY2/PY3 flags however six is imported, comparisons putting the version on the right-hand side, and guards combined with 'and'/'or'/'not'. The pre-existing is_sys_guard call sites (no-name-in-module, ungrouped-imports, deprecated-module, used-before-assignment) run on every import whether it failed or not: they keep the cheap spelling check, a known tradeoff between cost and thoroughness.
1 parent e996ecd commit c125862

5 files changed

Lines changed: 166 additions & 2 deletions

File tree

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
Fixed a false positive for ``import-error`` when a failing import is guarded
2+
by a version check that does not spell out
3+
``sys.version_info`` literally: guards written ``from sys import
4+
version_info`` (possibly aliased), guards on an aliased ``sys`` import,
5+
``sys.hexversion``, and comparisons putting the version on the right-hand
6+
side are now recognized too.
7+
8+
Refs #11209

pylint/checkers/imports.py

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@
3131
is_module_ignored,
3232
is_sys_guard,
3333
node_ignores_exception,
34+
zealous_is_sys_guard,
3435
)
3536
from pylint.constants import MAX_NUMBER_OF_IMPORT_SHOWN
3637
from pylint.exceptions import EmptyReportError
@@ -148,10 +149,11 @@ def _ignore_import_failure(
148149
return True
149150

150151
# Ignore import failure if part of guarded import block
151-
# I.e. `sys.version_info` or `typing.TYPE_CHECKING`
152+
# I.e. `sys.version_info` or `typing.TYPE_CHECKING`. The import already
153+
# failed, so this runs rarely and can afford the zealous guard detection.
152154
if in_type_checking_block(node):
153155
return True
154-
if isinstance(node.parent, nodes.If) and is_sys_guard(node.parent):
156+
if isinstance(node.parent, nodes.If) and zealous_is_sys_guard(node.parent):
155157
return True
156158

157159
return node_ignores_exception(node, ImportError)

pylint/checkers/utils.py

Lines changed: 79 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1864,6 +1864,85 @@ def is_sys_guard(node: nodes.If) -> bool:
18641864
return False
18651865

18661866

1867+
def zealous_is_sys_guard(node: nodes.If) -> bool:
1868+
"""Return True if IF stmt is a version guard, however it is spelled.
1869+
1870+
A more thorough (and more expensive) version of ``is_sys_guard``: it
1871+
resolves names instead of matching the exact ``sys.version_info`` spelling,
1872+
so it also recognizes guards written ``from sys import version_info``,
1873+
guards on aliased imports, ``sys.hexversion``, and comparisons putting the
1874+
version on the right-hand side. Reserve it for checks that run rarely,
1875+
e.g. once an import has already failed.
1876+
"""
1877+
if is_sys_guard(node):
1878+
return True
1879+
return _is_version_guard_test(node.test)
1880+
1881+
1882+
def _is_version_guard_test(test: nodes.NodeNG) -> bool:
1883+
match test:
1884+
case nodes.BoolOp():
1885+
return any(_is_version_guard_test(value) for value in test.values)
1886+
case nodes.UnaryOp(op="not"):
1887+
return _is_version_guard_test(test.operand)
1888+
case nodes.Compare():
1889+
operands = [test.left] + [operand for _, operand in test.ops]
1890+
return any(_is_version_info_value(operand) for operand in operands)
1891+
case _:
1892+
# A bare truthiness guard: only six's version flags qualify.
1893+
return _is_six_version_flag(test)
1894+
1895+
1896+
def _is_version_info_value(value: nodes.NodeNG) -> bool:
1897+
match value:
1898+
case nodes.Subscript():
1899+
return _is_version_info_value(value.value)
1900+
case nodes.Attribute(attrname="version_info" | "hexversion"):
1901+
return _resolves_to_module(value.expr, "sys")
1902+
case nodes.Attribute():
1903+
# e.g. the `major` in `version_info.major`
1904+
return _is_version_info_value(value.expr)
1905+
case nodes.Name():
1906+
return _binds_module_member(
1907+
value, "sys", frozenset({"version_info", "hexversion"})
1908+
)
1909+
case _:
1910+
return False
1911+
1912+
1913+
def _is_six_version_flag(test: nodes.NodeNG) -> bool:
1914+
match test:
1915+
case nodes.Attribute(attrname="PY2" | "PY3"):
1916+
return _resolves_to_module(test.expr, "six")
1917+
case nodes.Name():
1918+
return _binds_module_member(test, "six", frozenset({"PY2", "PY3"}))
1919+
case _:
1920+
return False
1921+
1922+
1923+
def _resolves_to_module(expr: nodes.NodeNG, modname: str) -> bool:
1924+
match safe_infer(expr):
1925+
case nodes.Module(name=name) if name == modname:
1926+
return True
1927+
return False
1928+
1929+
1930+
def _binds_module_member(
1931+
name: nodes.Name, modname: str, members: frozenset[str]
1932+
) -> bool:
1933+
"""Whether the name is bound by a ``from modname import <member>``."""
1934+
_, assignments = name.lookup(name.name)
1935+
return any(
1936+
isinstance(assignment, nodes.ImportFrom)
1937+
and assignment.modname == modname
1938+
and any(
1939+
real in members and (alias or real) == name.name
1940+
for real, alias in assignment.names
1941+
)
1942+
for assignment in assignments
1943+
)
1944+
1945+
18671946
def _is_node_in_same_scope(
18681947
candidate: nodes.NodeNG, node_scope: nodes.LocalsDictNodeNG
18691948
) -> bool:

tests/checkers/unittest_utils.py

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,66 @@ def test_if_sys_guard() -> None:
385385
assert utils.is_sys_guard(code[5]) is False
386386

387387

388+
def test_if_zealous_sys_guard() -> None:
389+
code = astroid.extract_node("""
390+
import sys
391+
import sys as system
392+
from sys import version_info
393+
from sys import version_info as vi
394+
from sys import hexversion
395+
from six import PY2
396+
from collections import OrderedDict as version_info_fake
397+
398+
if version_info >= (3, 8): #@
399+
pass
400+
401+
if vi[:2] >= (3, 8): #@
402+
pass
403+
404+
if system.version_info >= (3, 8): #@
405+
pass
406+
407+
if sys.version_info.major >= 3: #@
408+
pass
409+
410+
if (3, 8) <= sys.version_info: #@
411+
pass
412+
413+
if hexversion >= 0x030800F0: #@
414+
pass
415+
416+
if sys.version_info >= (3, 8) and sys.platform == "linux": #@
417+
pass
418+
419+
if not sys.version_info >= (3, 8): #@
420+
pass
421+
422+
if PY2: #@
423+
pass
424+
425+
if sys.version_info > (3, 8): #@
426+
pass
427+
428+
if sys.some_other_function > (3, 8): #@
429+
pass
430+
431+
if version_info_fake >= (3, 8): #@
432+
pass
433+
434+
if sys.platform == "linux": #@
435+
pass
436+
""")
437+
assert isinstance(code, list) and len(code) == 13
438+
439+
for guard in code[:10]:
440+
assert isinstance(guard, nodes.If)
441+
assert utils.zealous_is_sys_guard(guard) is True, guard.as_string()
442+
443+
for not_a_guard in code[10:]:
444+
assert isinstance(not_a_guard, nodes.If)
445+
assert utils.zealous_is_sys_guard(not_a_guard) is False, not_a_guard.as_string()
446+
447+
388448
def test_if_typing_guard() -> None:
389449
code = astroid.extract_node("""
390450
import typing

tests/functional/i/import_error.py

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -114,3 +114,18 @@
114114
with contextlib.suppress(ImportError):
115115
with contextlib.suppress(TypeError):
116116
import foo2
117+
118+
# Version guards are recognized however `sys.version_info` is spelled
119+
from sys import version_info
120+
121+
if version_info >= (3, 9):
122+
import some_module
123+
from some_module import some_class
124+
else:
125+
import some_module_alt
126+
127+
if (3, 9) <= version_info:
128+
import some_module
129+
130+
if version_info[:2] >= (3, 9):
131+
import some_module

0 commit comments

Comments
 (0)