summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDaniƫl van Noord <13665637+DanielNoord@users.noreply.github.com>2022-08-17 17:14:34 +0200
committerGitHub <noreply@github.com>2022-08-17 17:14:34 +0200
commita2c57ec06070b6d2485082e3a131a087f8d6d921 (patch)
treec0d01a231ee2c48772a4d9c84bcf855454bc74a5
parent39f0e30547f7d6de849c71ef6cb7fe1ec8b57ca0 (diff)
downloadpylint-git-a2c57ec06070b6d2485082e3a131a087f8d6d921.tar.gz
Refactor check() (#7288)
-rw-r--r--pylint/lint/pylinter.py136
-rw-r--r--tests/lint/test_pylinter.py25
-rw-r--r--tests/test_check_parallel.py12
3 files changed, 128 insertions, 45 deletions
diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py
index b83efc253..472b3b030 100644
--- a/pylint/lint/pylinter.py
+++ b/pylint/lint/pylinter.py
@@ -14,14 +14,14 @@ import tokenize
import traceback
import warnings
from collections import defaultdict
-from collections.abc import Callable, Iterable, Iterator, Sequence
+from collections.abc import Callable, Iterator, Sequence
from io import TextIOWrapper
from pathlib import Path
from re import Pattern
from typing import Any
import astroid
-from astroid import AstroidError, nodes
+from astroid import nodes
from pylint import checkers, exceptions, interfaces, reporters
from pylint.checkers.base_checker import BaseChecker
@@ -645,33 +645,63 @@ class PyLinter(
"Missing filename required for --from-stdin"
)
+ # TODO: Move the parallel invocation into step 5 of the checking process
+ if not self.config.from_stdin and self.config.jobs > 1:
+ original_sys_path = sys.path[:]
+ check_parallel(
+ self,
+ self.config.jobs,
+ self._iterate_file_descrs(files_or_modules),
+ files_or_modules, # this argument patches sys.path
+ )
+ sys.path = original_sys_path
+ return
+
# 3) Get all FileItems
with fix_import_path(files_or_modules):
if self.config.from_stdin:
fileitems = iter(
(self._get_file_descr_from_stdin(files_or_modules[0]),)
)
+ data: str | None = _read_stdin()
else:
fileitems = self._iterate_file_descrs(files_or_modules)
+ data = None
- if self.config.from_stdin:
+ # The contextmanager also opens all checkers and sets up the PyLinter class
+ with self._astroid_module_checker() as check_astroid_module:
with fix_import_path(files_or_modules):
- self._check_files(
- functools.partial(self.get_ast, data=_read_stdin()),
- fileitems,
+ # 4) Get the AST for each FileItem
+ ast_per_fileitem = self._get_asts(fileitems, data)
+
+ # 5) Lint each ast
+ self._lint_files(ast_per_fileitem, check_astroid_module)
+
+ def _get_asts(
+ self, fileitems: Iterator[FileItem], data: str | None
+ ) -> dict[FileItem, nodes.Module | None]:
+ """Get the AST for all given FileItems."""
+ ast_per_fileitem: dict[FileItem, nodes.Module | None] = {}
+
+ for fileitem in fileitems:
+ self.set_current_module(fileitem.name, fileitem.filepath)
+
+ try:
+ ast_per_fileitem[fileitem] = self.get_ast(
+ fileitem.filepath, fileitem.name, data
)
- elif self.config.jobs == 1:
- with fix_import_path(files_or_modules):
- self._check_files(self.get_ast, fileitems)
- else:
- original_sys_path = sys.path[:]
- check_parallel(
- self,
- self.config.jobs,
- fileitems,
- files_or_modules, # this argument patches sys.path
- )
- sys.path = original_sys_path
+ except astroid.AstroidBuildingError as ex:
+ template_path = prepare_crash_report(
+ ex, fileitem.filepath, self.crash_file_path
+ )
+ msg = get_fatal_error_message(fileitem.filepath, template_path)
+ self.add_message(
+ "astroid-error",
+ args=(fileitem.filepath, msg),
+ confidence=HIGH,
+ )
+
+ return ast_per_fileitem
def check_single_file(self, name: str, filepath: str, modname: str) -> None:
warnings.warn(
@@ -691,27 +721,61 @@ class PyLinter(
with self._astroid_module_checker() as check_astroid_module:
self._check_file(self.get_ast, check_astroid_module, file)
- def _check_files(
+ def _lint_files(
self,
- get_ast: GetAstProtocol,
- file_descrs: Iterable[FileItem],
+ ast_mapping: dict[FileItem, nodes.Module | None],
+ check_astroid_module: Callable[[nodes.Module], bool | None],
) -> None:
- """Check all files from file_descrs."""
- with self._astroid_module_checker() as check_astroid_module:
- for file in file_descrs:
- try:
- self._check_file(get_ast, check_astroid_module, file)
- except Exception as ex: # pylint: disable=broad-except
- template_path = prepare_crash_report(
- ex, file.filepath, self.crash_file_path
+ """Lint all AST modules from a mapping.."""
+ for fileitem, module in ast_mapping.items():
+ if module is None:
+ continue
+ try:
+ self._lint_file(fileitem, module, check_astroid_module)
+ except Exception as ex: # pylint: disable=broad-except
+ template_path = prepare_crash_report(
+ ex, fileitem.filepath, self.crash_file_path
+ )
+ msg = get_fatal_error_message(fileitem.filepath, template_path)
+ if isinstance(ex, astroid.AstroidError):
+ self.add_message(
+ "astroid-error", args=(fileitem.filepath, msg), confidence=HIGH
)
- msg = get_fatal_error_message(file.filepath, template_path)
- if isinstance(ex, AstroidError):
- self.add_message(
- "astroid-error", args=(file.filepath, msg), confidence=HIGH
- )
- else:
- self.add_message("fatal", args=msg, confidence=HIGH)
+ else:
+ self.add_message("fatal", args=msg, confidence=HIGH)
+
+ def _lint_file(
+ self,
+ file: FileItem,
+ module: nodes.Module,
+ check_astroid_module: Callable[[nodes.Module], bool | None],
+ ) -> None:
+ """Lint a file using the passed utility function check_astroid_module).
+
+ :param FileItem file: data about the file
+ :param nodes.Module module: the ast module to lint
+ :param Callable check_astroid_module: callable checking an AST taking the following arguments
+ - ast: AST of the module
+ :raises AstroidError: for any failures stemming from astroid
+ """
+ self.set_current_module(file.name, file.filepath)
+ self._ignore_file = False
+ self.file_state = FileState(file.modpath, self.msgs_store, module)
+ # fix the current file (if the source file was not available or
+ # if it's actually a c extension)
+ self.current_file = module.file
+
+ try:
+ check_astroid_module(module)
+ except Exception as e:
+ raise astroid.AstroidError from e
+
+ # warn about spurious inline messages handling
+ spurious_messages = self.file_state.iter_spurious_suppression_messages(
+ self.msgs_store
+ )
+ for msgid, line, args in spurious_messages:
+ self.add_message(msgid, line, None, args)
def _check_file(
self,
diff --git a/tests/lint/test_pylinter.py b/tests/lint/test_pylinter.py
index f982787d7..731af5b0a 100644
--- a/tests/lint/test_pylinter.py
+++ b/tests/lint/test_pylinter.py
@@ -3,10 +3,10 @@
# Copyright (c) https://github.com/PyCQA/pylint/blob/main/CONTRIBUTORS.txt
from typing import Any, NoReturn
+from unittest import mock
from unittest.mock import patch
import pytest
-from astroid import AstroidBuildingError
from py._path.local import LocalPath # type: ignore[import]
from pytest import CaptureFixture
@@ -15,7 +15,7 @@ from pylint.utils import FileState
def raise_exception(*args: Any, **kwargs: Any) -> NoReturn:
- raise AstroidBuildingError(modname="spam")
+ raise ValueError
@patch.object(FileState, "iter_spurious_suppression_messages", raise_exception)
@@ -32,12 +32,27 @@ def test_crash_in_file(
files = tmpdir.listdir()
assert len(files) == 1
assert "pylint-crash-20" in str(files[0])
- with open(files[0], encoding="utf8") as f:
- content = f.read()
- assert "Failed to import module spam." in content
+ assert any(m.symbol == "fatal" for m in linter.reporter.messages)
def test_check_deprecation(linter: PyLinter, recwarn):
linter.check("myfile.py")
msg = recwarn.pop()
assert "check function will only accept sequence" in str(msg)
+
+
+def test_crash_during_linting(
+ linter: PyLinter, capsys: CaptureFixture[str], tmpdir: LocalPath
+) -> None:
+ with mock.patch(
+ "pylint.lint.PyLinter.check_astroid_module", side_effect=RuntimeError
+ ):
+ linter.crash_file_path = str(tmpdir / "pylint-crash-%Y")
+ linter.check([__file__])
+ out, err = capsys.readouterr()
+ assert not out
+ assert not err
+ files = tmpdir.listdir()
+ assert len(files) == 1
+ assert "pylint-crash-20" in str(files[0])
+ assert any(m.symbol == "astroid-error" for m in linter.reporter.messages)
diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py
index 259c236d4..3b6d82e04 100644
--- a/tests/test_check_parallel.py
+++ b/tests/test_check_parallel.py
@@ -467,8 +467,10 @@ class TestCheckParallel:
# establish the baseline
assert (
linter.config.jobs == 1
- ), "jobs>1 are ignored when calling _check_files"
- linter._check_files(linter.get_ast, file_infos)
+ ), "jobs>1 are ignored when calling _lint_files"
+ ast_mapping = linter._get_asts(iter(file_infos), None)
+ with linter._astroid_module_checker() as check_astroid_module:
+ linter._lint_files(ast_mapping, check_astroid_module)
assert linter.msg_status == 0, "We should not fail the lint"
stats_single_proc = linter.stats
else:
@@ -534,8 +536,10 @@ class TestCheckParallel:
# establish the baseline
assert (
linter.config.jobs == 1
- ), "jobs>1 are ignored when calling _check_files"
- linter._check_files(linter.get_ast, file_infos)
+ ), "jobs>1 are ignored when calling _lint_files"
+ ast_mapping = linter._get_asts(iter(file_infos), None)
+ with linter._astroid_module_checker() as check_astroid_module:
+ linter._lint_files(ast_mapping, check_astroid_module)
stats_single_proc = linter.stats
else:
check_parallel(