Refactor check() (#7288)

pylint/lint/pylinter.py

@@ -14,14 +14,14 @@
 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 @@ def check(self, files_or_modules: Sequence[str] | str) -> None:
                     "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 @@ def check_single_file_item(self, file: FileItem) -> None:
         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,

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 @@
 
 
 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)

tests/test_check_parallel.py

@@ -467,8 +467,10 @@ def test_compare_workers_to_single_proc(self, num_files, num_jobs, num_checkers)
                 # 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 @@ def test_map_reduce(self, num_files, num_jobs, num_checkers):
                 # 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(