Skip to content

Commit

Permalink
Only instantiate ProgressReporter once
Browse files Browse the repository at this point in the history
  • Loading branch information
DanielNoord committed Jan 16, 2025
1 parent d087105 commit a4489ba
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 16 deletions.
22 changes: 12 additions & 10 deletions pylint/lint/pylinter.py
Original file line number Diff line number Diff line change
Expand Up @@ -686,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 @@ -699,22 +701,22 @@ 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] = {}

# For progress reports, it would be nice to say "item 1 of N",
# but we can't get the total count of filenames without
# materializing the list, or creating a second iterator and
# iterating.
progress_reporter = ProgressReporter(self.verbose)
progress_reporter.start_get_asts()

for fileitem in fileitems:
Expand Down Expand Up @@ -752,10 +754,10 @@ 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 = ProgressReporter(self.verbose)
progress_reporter.start_linting(len(ast_mapping))
progress_reporter.start_linting()
for fileitem, module in ast_mapping.items():
progress_reporter.lint_file(fileitem.filepath)
if module is None:
Expand Down
4 changes: 2 additions & 2 deletions pylint/reporters/progress_reporters.py
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,10 @@ 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, ast_count: int) -> None:
self._ast_count = ast_count
def start_linting(self) -> None:
self._print_message(f"Linting {self._ast_count} modules.")

def lint_file(self, filename: str) -> None:
Expand Down
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

0 comments on commit a4489ba

Please sign in to comment.