Skip to content

Commit

Permalink
Issue 1554: add progress reporting (#10178)
Browse files Browse the repository at this point in the history
  • Loading branch information
jzohrab authored Jan 17, 2025
1 parent 80f2946 commit 649c875
Show file tree
Hide file tree
Showing 5 changed files with 123 additions and 10 deletions.
3 changes: 2 additions & 1 deletion pylint/config/config_initialization.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@
from pylint.lint import PyLinter


def _config_initialization(
def _config_initialization( # pylint: disable=too-many-statements
linter: PyLinter,
args_list: list[str],
reporter: reporters.BaseReporter | reporters.MultiReporter | None = None,
Expand All @@ -33,6 +33,7 @@ def _config_initialization(
"""Parse all available options, read config files and command line arguments and
set options accordingly.
"""
linter.verbose = verbose_mode
config_file = Path(config_file) if config_file else None

# Set the current module to the configuration file
Expand Down
24 changes: 19 additions & 5 deletions pylint/lint/pylinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
)
from pylint.message import Message, MessageDefinition, MessageDefinitionStore
from pylint.reporters.base_reporter import BaseReporter
from pylint.reporters.progress_reporters import ProgressReporter
from pylint.reporters.text import TextReporter
from pylint.reporters.ureports import nodes as report_nodes
from pylint.typing import (
Expand Down Expand Up @@ -323,8 +324,7 @@ def __init__(
self.option_groups_descs[opt_group[0]] = opt_group[1]
self._option_groups: tuple[tuple[str, str], ...] = (
*option_groups,
("Messages control", "Options controlling analysis messages"),
("Reports", "Options related to output formatting and reporting"),
*PyLinter.option_groups_descs.items(),
)
self.fail_on_symbols: list[str] = []
"""List of message symbols on which pylint should fail, set by --fail-on."""
Expand Down Expand Up @@ -354,6 +354,7 @@ def __init__(
self.current_file: str | None = None
self._ignore_file = False
self._ignore_paths: list[Pattern[str]] = []
self.verbose = False

self.register_checker(self)

Expand Down Expand Up @@ -685,6 +686,8 @@ def check(self, files_or_modules: Sequence[str]) -> None:
sys.path = original_sys_path
return

progress_reporter = ProgressReporter(self.verbose)

# 1) Get all FileItems
with augmented_sys_path(extra_packages_paths):
if self.config.from_stdin:
Expand All @@ -698,18 +701,26 @@ def check(self, files_or_modules: Sequence[str]) -> None:
with augmented_sys_path(extra_packages_paths):
with self._astroid_module_checker() as check_astroid_module:
# 2) Get the AST for each FileItem
ast_per_fileitem = self._get_asts(fileitems, data)
ast_per_fileitem = self._get_asts(fileitems, data, progress_reporter)

# 3) Lint each ast
self._lint_files(ast_per_fileitem, check_astroid_module)
self._lint_files(
ast_per_fileitem, check_astroid_module, progress_reporter
)

def _get_asts(
self, fileitems: Iterator[FileItem], data: str | None
self,
fileitems: Iterator[FileItem],
data: str | None,
progress_reporter: ProgressReporter,
) -> dict[FileItem, nodes.Module | None]:
"""Get the AST for all given FileItems."""
ast_per_fileitem: dict[FileItem, nodes.Module | None] = {}

progress_reporter.start_get_asts()

for fileitem in fileitems:
progress_reporter.get_ast_for_file(fileitem.filepath)
self.set_current_module(fileitem.name, fileitem.filepath)

try:
Expand Down Expand Up @@ -743,9 +754,12 @@ def _lint_files(
self,
ast_mapping: dict[FileItem, nodes.Module | None],
check_astroid_module: Callable[[nodes.Module], bool | None],
progress_reporter: ProgressReporter,
) -> None:
"""Lint all AST modules from a mapping.."""
progress_reporter.start_linting()
for fileitem, module in ast_mapping.items():
progress_reporter.lint_file(fileitem.filepath)
if module is None:
continue
try:
Expand Down
31 changes: 31 additions & 0 deletions pylint/reporters/progress_reporters.py
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Licensed under the GPL: https://www.gnu.org/licenses/old-licenses/gpl-2.0.html
# For details: https://github.com/pylint-dev/pylint/blob/main/LICENSE
# Copyright (c) https://github.com/pylint-dev/pylint/blob/main/CONTRIBUTORS.txt


class ProgressReporter:
"""Progress reporter."""

def __init__(self, is_verbose: bool = True) -> None:
self._is_verbose = is_verbose
self._ast_count = 0
self._lint_counter = 0

def start_get_asts(self) -> None:
self._print_message("Get ASTs.")

def get_ast_for_file(self, filename: str) -> None:
self._ast_count += 1
self._print_message(f"AST for {filename}")

def start_linting(self) -> None:
self._print_message(f"Linting {self._ast_count} modules.")

def lint_file(self, filename: str) -> None:
self._lint_counter += 1
self._print_message(f"{filename} ({self._lint_counter} of {self._ast_count})")

def _print_message(self, msg: str) -> None:
"""Display progress message."""
if self._is_verbose:
print(msg, flush=True)
21 changes: 17 additions & 4 deletions tests/test_check_parallel.py
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
from pylint.lint.parallel import _worker_check_single_file as worker_check_single_file
from pylint.lint.parallel import _worker_initialize as worker_initialize
from pylint.lint.parallel import check_parallel
from pylint.reporters.progress_reporters import ProgressReporter
from pylint.testutils import GenericTestReporter as Reporter
from pylint.testutils.utils import _test_cwd
from pylint.typing import FileItem
Expand Down Expand Up @@ -506,6 +507,8 @@ def test_compare_workers_to_single_proc(

file_infos = _gen_file_datas(num_files)

progress_reporter = ProgressReporter(is_verbose=False)

# Loop for single-proc and mult-proc, so we can ensure the same linter-config
for do_single_proc in range(2):
linter = PyLinter(reporter=Reporter())
Expand All @@ -523,9 +526,13 @@ def test_compare_workers_to_single_proc(
assert (
linter.config.jobs == 1
), "jobs>1 are ignored when calling _lint_files"
ast_mapping = linter._get_asts(iter(file_infos), None)
ast_mapping = linter._get_asts(
iter(file_infos), None, progress_reporter
)
with linter._astroid_module_checker() as check_astroid_module:
linter._lint_files(ast_mapping, check_astroid_module)
linter._lint_files(
ast_mapping, check_astroid_module, progress_reporter
)
assert linter.msg_status == 0, "We should not fail the lint"
stats_single_proc = linter.stats
else:
Expand Down Expand Up @@ -585,14 +592,20 @@ def test_map_reduce(self, num_files: int, num_jobs: int, num_checkers: int) -> N
if num_checkers > 2:
linter.register_checker(ThirdParallelTestChecker(linter))

progress_reporter = ProgressReporter(is_verbose=False)

if do_single_proc:
# establish the baseline
assert (
linter.config.jobs == 1
), "jobs>1 are ignored when calling _lint_files"
ast_mapping = linter._get_asts(iter(file_infos), None)
ast_mapping = linter._get_asts(
iter(file_infos), None, progress_reporter
)
with linter._astroid_module_checker() as check_astroid_module:
linter._lint_files(ast_mapping, check_astroid_module)
linter._lint_files(
ast_mapping, check_astroid_module, progress_reporter
)
stats_single_proc = linter.stats
else:
check_parallel(
Expand Down
54 changes: 54 additions & 0 deletions tests/test_self.py
Original file line number Diff line number Diff line change
Expand Up @@ -341,6 +341,60 @@ def test_wrong_import_position_when_others_disabled(self) -> None:
actual_output = actual_output[actual_output.find("\n") :]
assert self._clean_paths(expected_output.strip()) == actual_output.strip()

def test_progress_reporting(self) -> None:
module1 = join(HERE, "regrtest_data", "import_something.py")
module2 = join(HERE, "regrtest_data", "wrong_import_position.py")
args = [
module2,
module1,
"--disable=all",
"--enable=wrong-import-position",
"--verbose",
"-rn",
"-sn",
]
out = StringIO()
self._run_pylint(args, out=out)
actual_output = self._clean_paths(out.getvalue().strip())

expected_output = textwrap.dedent(
f"""
Using config file pylint/testutils/testing_pylintrc
Get ASTs.
AST for {module2}
AST for {module1}
Linting 2 modules.
{module2} (1 of 2)
************* Module wrong_import_position
{module2}:11:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
{module1} (2 of 2)
"""
)
assert self._clean_paths(expected_output.strip()) == actual_output.strip()

def test_progress_reporting_not_shown_if_not_verbose(self) -> None:
module1 = join(HERE, "regrtest_data", "import_something.py")
module2 = join(HERE, "regrtest_data", "wrong_import_position.py")
args = [
module2,
module1,
"--disable=all",
"--enable=wrong-import-position",
"-rn",
"-sn",
]
out = StringIO()
self._run_pylint(args, out=out)
actual_output = self._clean_paths(out.getvalue().strip())

expected_output = textwrap.dedent(
f"""
************* Module wrong_import_position
{module2}:11:0: C0413: Import "import os" should be placed at the top of the module (wrong-import-position)
"""
)
assert self._clean_paths(expected_output.strip()) == actual_output.strip()

def test_type_annotation_names(self) -> None:
"""Test resetting the `_type_annotation_names` list to `[]` when leaving a module.
Expand Down

0 comments on commit 649c875

Please sign in to comment.