From f6af12dc907e51aad33841db56edf6ba85927932 Mon Sep 17 00:00:00 2001 From: Jeff Zohrab Date: Wed, 18 Dec 2024 15:59:23 -0600 Subject: [PATCH 1/2] Remove duplicate code. --- pylint/lint/pylinter.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index cc4605b96a..ae8be4468b 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -323,8 +323,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.""" From 04e2ceef4f38fa2089109e32a3fb18cb48b3d73a Mon Sep 17 00:00:00 2001 From: Jeff Zohrab Date: Wed, 18 Dec 2024 16:03:35 -0600 Subject: [PATCH 2/2] Change _iterate_file_descrs to _get_file_items. Returning a materialized list instead of an iterator lets us do simple progress reporting in the future. As the list of FileItems is very lightweight, there shouldn't be a performance hit. --- pylint/lint/pylinter.py | 33 ++++++++++++++++----------------- tests/lint/unittest_lint.py | 2 +- tests/test_check_parallel.py | 10 +++++----- 3 files changed, 22 insertions(+), 23 deletions(-) diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index ae8be4468b..b22b9e312e 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -678,7 +678,7 @@ def check(self, files_or_modules: Sequence[str]) -> None: check_parallel( self, self.config.jobs, - self._iterate_file_descrs(files_or_modules), + self._get_file_items(files_or_modules), extra_packages_paths, ) sys.path = original_sys_path @@ -690,7 +690,7 @@ def check(self, files_or_modules: Sequence[str]) -> None: fileitems = self._get_file_descr_from_stdin(files_or_modules[0]) data: str | None = _read_stdin() else: - fileitems = self._iterate_file_descrs(files_or_modules) + fileitems = self._get_file_items(files_or_modules) data = None # The contextmanager also opens all checkers and sets up the PyLinter class @@ -703,7 +703,7 @@ def check(self, files_or_modules: Sequence[str]) -> None: self._lint_files(ast_per_fileitem, check_astroid_module) def _get_asts( - self, fileitems: Iterator[FileItem], data: str | None + self, fileitems: list[FileItem], data: str | None ) -> dict[FileItem, nodes.Module | None]: """Get the AST for all given FileItems.""" ast_per_fileitem: dict[FileItem, nodes.Module | None] = {} @@ -837,9 +837,9 @@ def _check_file( for msgid, line, args in spurious_messages: self.add_message(msgid, line, None, args) - def _get_file_descr_from_stdin(self, filepath: str) -> Iterator[FileItem]: - """Return file description (tuple of module name, file path, base name) from - given file path. + def _get_file_descr_from_stdin(self, filepath: str) -> list[FileItem]: + """Return single-entry list file description (tuple of module + name, file path, base name) from given file path. This method is used for creating suitable file description for _check_files when the source is standard input. @@ -850,7 +850,7 @@ def _get_file_descr_from_stdin(self, filepath: str) -> Iterator[FileItem]: self.config.ignore_patterns, self.config.ignore_paths, ): - return + return [] try: # Note that this function does not really perform an @@ -860,20 +860,19 @@ def _get_file_descr_from_stdin(self, filepath: str) -> Iterator[FileItem]: except ImportError: modname = os.path.splitext(os.path.basename(filepath))[0] - yield FileItem(modname, filepath, filepath) + return [FileItem(modname, filepath, filepath)] - def _iterate_file_descrs( - self, files_or_modules: Sequence[str] - ) -> Iterator[FileItem]: - """Return generator yielding file descriptions (tuples of module name, file + def _get_file_items(self, files_or_modules: Sequence[str]) -> list[FileItem]: + """Return list of file descriptions (tuples of module name, file path, base name). - The returned generator yield one item for each Python module that should be linted. + Each element in the list is a Python module that should be linted. """ - for descr in self._expand_files(files_or_modules).values(): - name, filepath, is_arg = descr["name"], descr["path"], descr["isarg"] - if self.should_analyze_file(name, filepath, is_argument=is_arg): - yield FileItem(name, filepath, descr["basename"]) + return [ + FileItem(d["name"], d["path"], d["basename"]) + for d in self._expand_files(files_or_modules).values() + if self.should_analyze_file(d["name"], d["path"], d["isarg"]) + ] def _expand_files( self, files_or_modules: Sequence[str] diff --git a/tests/lint/unittest_lint.py b/tests/lint/unittest_lint.py index 7ba8879e9a..bf310f6f43 100644 --- a/tests/lint/unittest_lint.py +++ b/tests/lint/unittest_lint.py @@ -1113,7 +1113,7 @@ def test_recursive_ignore(ignore_parameter: str, ignore_parameter_value: str) -> exit=False, ) - linted_files = run.linter._iterate_file_descrs( + linted_files = run.linter._get_file_items( tuple(run.linter._discover_files([join(REGRTEST_DATA_DIR, "directory")])) ) linted_file_paths = [file_item.filepath for file_item in linted_files] diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py index 6576e0ae9c..7cfca6ed07 100644 --- a/tests/test_check_parallel.py +++ b/tests/test_check_parallel.py @@ -353,7 +353,7 @@ def test_sequential_checkers_work(self) -> None: check_parallel( linter, jobs=1, - files=iter(single_file_container), + files=single_file_container, ) assert len(linter.get_checkers()) == 2, ( "We should only have the 'main' and 'sequential-checker' " @@ -422,7 +422,7 @@ def test_invoke_single_job(self) -> None: check_parallel( linter, jobs=1, - files=iter(single_file_container), + files=single_file_container, ) assert { @@ -523,7 +523,7 @@ 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(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" @@ -590,7 +590,7 @@ def test_map_reduce(self, num_files: int, num_jobs: int, num_checkers: int) -> N 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(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 @@ -624,7 +624,7 @@ def test_no_deadlock_due_to_initializer_error(self) -> None: check_parallel( linter, jobs=1, - files=iter(single_file_container), + files=single_file_container, # This will trigger an exception in the initializer for the parallel jobs # because arguments has to be an Iterable. extra_packages_paths=1, # type: ignore[arg-type]