Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Ignore import errors if in guarded import block #4702

Merged
merged 3 commits into from
Jul 19, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions ChangeLog
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,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:
cdce8p marked this conversation as resolved.
Show resolved Hide resolved
"""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