Skip to content

Commit

Permalink
Ignore import errors if in guarded import block (#4702)
Browse files Browse the repository at this point in the history
* Ignore import errors if in guarded import block
* Use new astroid helper methods
  • Loading branch information
cdce8p authored Jul 19, 2021
1 parent 6a16625 commit bf493bb
Show file tree
Hide file tree
Showing 7 changed files with 77 additions and 25 deletions.
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,12 @@ Release date: TBA

Closes #4698

* Don't emit ``import-error``, ``no-name-in-module``, and ``ungrouped-imports``
for imports guarded by ``sys.version_info`` or ``typing.TYPE_CHECKING``.

Closes #3285
Closes #3382

* Fix ``invalid-overridden-method`` with nested property

Closes #4368
Expand Down
34 changes: 16 additions & 18 deletions pylint/checkers/imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@
import os
import sys
from distutils import sysconfig
from typing import Dict, List, Union
from typing import Dict, List, Set, Union

import astroid

Expand All @@ -58,6 +58,7 @@
check_messages,
get_import_name,
is_from_fallback_block,
is_node_in_guarded_import_block,
node_ignores_exception,
)
from pylint.exceptions import EmptyReportError
Expand Down Expand Up @@ -119,18 +120,10 @@ def _ignore_import_failure(node, modname, ignored_modules):
if submodule in ignored_modules:
return True

# ignore import failure if guarded by `sys.version_info` test
if isinstance(node.parent, astroid.If) and isinstance(
node.parent.test, astroid.Compare
):
value = node.parent.test.left
if isinstance(value, astroid.Subscript):
value = value.value
if (
isinstance(value, astroid.Attribute)
and value.as_string() == "sys.version_info"
):
return True
if is_node_in_guarded_import_block(node):
# Ignore import failure if part of guarded import block
# I.e. `sys.version_info` or `typing.TYPE_CHECKING`
return True

return node_ignores_exception(node, ImportError)

Expand Down Expand Up @@ -556,25 +549,30 @@ def visit_importfrom(self, node):
self._add_imported_module(node, imported_module.name)

@check_messages(*MSGS)
def leave_module(self, node):
def leave_module(self, node: astroid.Module) -> None:
# Check imports are grouped by category (standard, 3rd party, local)
std_imports, ext_imports, loc_imports = self._check_imports_order(node)

# Check that imports are grouped by package within a given category
met_import = set() # set for 'import x' style
met_from = set() # set for 'from x import y' style
met_import: Set[str] = set() # set for 'import x' style
met_from: Set[str] = set() # set for 'from x import y' style
current_package = None
for import_node, import_name in std_imports + ext_imports + loc_imports:
if not self.linter.is_message_enabled(
"ungrouped-imports", import_node.fromlineno
):
continue
if isinstance(import_node, astroid.node_classes.ImportFrom):
if isinstance(import_node, astroid.ImportFrom):
met = met_from
else:
met = met_import
package, _, _ = import_name.partition(".")
if current_package and current_package != package and package in met:
if (
current_package
and current_package != package
and package in met
and is_node_in_guarded_import_block(import_node) is False
):
self.add_message("ungrouped-imports", node=import_node, args=package)
current_package = package
met.add(package)
Expand Down
9 changes: 9 additions & 0 deletions pylint/checkers/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -1552,3 +1552,12 @@ def get_import_name(
modname, level=importnode.level
)
return modname


def is_node_in_guarded_import_block(node: astroid.NodeNG) -> bool:
"""Return True if node is part for guarded if block.
I.e. `sys.version_info` or `typing.TYPE_CHECKING`
"""
return isinstance(node.parent, astroid.If) and (
node.parent.is_sys_guard() or node.parent.is_typing_guard()
)
12 changes: 10 additions & 2 deletions pylint/checkers/variables.py
Original file line number Diff line number Diff line change
Expand Up @@ -1173,12 +1173,16 @@ def visit_name(self, node):
self.add_message("undefined-variable", args=name, node=node)

@utils.check_messages("no-name-in-module")
def visit_import(self, node):
def visit_import(self, node: astroid.Import) -> None:
"""check modules attribute accesses"""
if not self._analyse_fallback_blocks and utils.is_from_fallback_block(node):
# No need to verify this, since ImportError is already
# handled by the client code.
return
if utils.is_node_in_guarded_import_block(node) is True:
# Don't verify import if part of guarded import block
# I.e. `sys.version_info` or `typing.TYPE_CHECKING`
return

for name, _ in node.names:
parts = name.split(".")
Expand All @@ -1191,12 +1195,16 @@ def visit_import(self, node):
self._check_module_attrs(node, module, parts[1:])

@utils.check_messages("no-name-in-module")
def visit_importfrom(self, node):
def visit_importfrom(self, node: astroid.ImportFrom) -> None:
"""check modules attribute accesses"""
if not self._analyse_fallback_blocks and utils.is_from_fallback_block(node):
# No need to verify this, since ImportError is already
# handled by the client code.
return
if utils.is_node_in_guarded_import_block(node) is True:
# Don't verify import if part of guarded import block
# I.e. `sys.version_info` or `typing.TYPE_CHECKING`
return

name_parts = node.modname.split(".")
try:
Expand Down
32 changes: 27 additions & 5 deletions tests/functional/i/import_error.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,14 +24,36 @@
except ImportError:
pass

# pylint: disable=no-name-in-module
from functional.s.syntax_error import toto # [syntax-error]

# Don't emit import-error if guarded behind `sys.version_info`
from functional.s.syntax_error import toto # [no-name-in-module,syntax-error]


# Don't emit `import-error` or `no-name-in-module`
# if guarded behind `sys.version_info` or `typing.TYPE_CHECKING`
import sys
import typing
import typing as tp # pylint: disable=reimported
from typing import TYPE_CHECKING


if sys.version_info >= (3, 9):
import zoneinfo
import some_module
from some_module import some_class
else:
import some_module_alt

if sys.version_info[:2] >= (3, 9):
import zoneinfo
import some_module
else:
import some_module_alt


if typing.TYPE_CHECKING:
import stub_import

if tp.TYPE_CHECKING:
import stub_import

if TYPE_CHECKING:
import stub_import
from stub_import import stub_class
1 change: 1 addition & 0 deletions tests/functional/i/import_error.txt
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import-error:3:0::Unable to import 'totally_missing'
import-error:16:4::Unable to import 'maybe_missing_2'
no-name-in-module:28:0::No name 'syntax_error' in module 'functional.s'
syntax-error:28:0::Cannot import 'functional.s.syntax_error' due to syntax error 'invalid syntax (<unknown>, line 1)'
8 changes: 8 additions & 0 deletions tests/functional/u/ungrouped_imports.py
Original file line number Diff line number Diff line change
Expand Up @@ -24,3 +24,11 @@
import unittest
from unittest import TestCase
from unittest.mock import MagicMock


# https://github.com/PyCQA/pylint/issues/3382
# Imports in a `if TYPE_CHECKING` block should not trigger `ungrouped-imports`
from typing import TYPE_CHECKING
if TYPE_CHECKING:
import re
from typing import List

0 comments on commit bf493bb

Please sign in to comment.