diff --git a/doc/whatsnew/fragments/10037.bugfix b/doc/whatsnew/fragments/10037.bugfix new file mode 100644 index 0000000000..e1877acc63 --- /dev/null +++ b/doc/whatsnew/fragments/10037.bugfix @@ -0,0 +1,3 @@ +Fix enabling checks from extensions which are disabled by default if multiple jobs are used. + +Closes #10037 diff --git a/doc/whatsnew/fragments/10642.bugfix b/doc/whatsnew/fragments/10642.bugfix new file mode 100644 index 0000000000..459d8635cc --- /dev/null +++ b/doc/whatsnew/fragments/10642.bugfix @@ -0,0 +1,3 @@ +Fix duplicate messages for extension checks if multiple jobs are used. + +Refs #10642 diff --git a/pylint/config/config_initialization.py b/pylint/config/config_initialization.py index a0210d47ac..fe823d4af9 100644 --- a/pylint/config/config_initialization.py +++ b/pylint/config/config_initialization.py @@ -6,6 +6,7 @@ import sys import warnings +from copy import copy from glob import glob from itertools import chain from pathlib import Path @@ -58,8 +59,12 @@ def _config_initialization( # pylint: disable=too-many-statements exec(utils._unquote(config_data["init-hook"])) # pylint: disable=exec-used # Load plugins if specified in the config file + default_checkers = copy(linter._registered_checkers) if "load-plugins" in config_data: linter.load_plugin_modules(utils._splitstrip(config_data["load-plugins"])) + linter._registered_dynamic_plugin_checkers = linter._registered_checkers.difference( + default_checkers + ) unrecognized_options_message = None # First we parse any options from a configuration file diff --git a/pylint/lint/parallel.py b/pylint/lint/parallel.py index af381494c3..c4cbaa83cd 100644 --- a/pylint/lint/parallel.py +++ b/pylint/lint/parallel.py @@ -54,6 +54,12 @@ def _worker_initialize( # Re-register dynamic plugins, since the pool does not have access to the # astroid module that existed when the linter was pickled. + # Freeze register new messages to prevent overwriting enabled and disabled messaged + # during dynamic plugin re-load. + _worker_linter._freeze_register_msgs = True + _worker_linter._deregister_checkers( + _worker_linter._registered_dynamic_plugin_checkers + ) _worker_linter.load_plugin_modules(_worker_linter._dynamic_plugins, force=True) _worker_linter.load_plugin_configuration() diff --git a/pylint/lint/pylinter.py b/pylint/lint/pylinter.py index 59dac6b77a..4fe45c0bfe 100644 --- a/pylint/lint/pylinter.py +++ b/pylint/lint/pylinter.py @@ -14,7 +14,7 @@ import traceback import warnings from collections import defaultdict -from collections.abc import Callable, Iterable, Iterator, Sequence +from collections.abc import Callable, Collection, Iterable, Iterator, Sequence from io import TextIOWrapper from pathlib import Path from re import Pattern @@ -323,6 +323,16 @@ def __init__( """Dictionary of registered and initialized checkers.""" self._dynamic_plugins: dict[str, ModuleType | ModuleNotFoundError | bool] = {} """Set of loaded plugin names.""" + self._registered_checkers: set[tuple[str, checkers.BaseChecker, int]] = set() + """Set of tuples with loaded checker name, reference to checker + and checker object id. + """ + self._registered_dynamic_plugin_checkers: set[ + tuple[str, checkers.BaseChecker, int] + ] = set() + """Set of tuples with loaded dynamic plugin checker name, reference to + checker and checker object id. + """ # Attributes related to stats self.stats = LinterStats() @@ -356,6 +366,7 @@ def __init__( self.msgs_store = MessageDefinitionStore(self.config.py_version) self.msg_status = 0 self._by_id_managed_msgs: list[ManagedMessage] = [] + self._freeze_register_msgs = False # Attributes related to visiting files self.file_state = FileState("", self.msgs_store, is_base_filestate=True) @@ -495,17 +506,30 @@ def report_order(self) -> list[BaseChecker]: def register_checker(self, checker: checkers.BaseChecker) -> None: """This method auto registers the checker.""" self._checkers[checker.name].append(checker) + self._registered_checkers.add((checker.name, checker, id(checker))) for r_id, r_title, r_cb in checker.reports: self.register_report(r_id, r_title, r_cb, checker) - if hasattr(checker, "msgs"): + if not self._freeze_register_msgs and hasattr(checker, "msgs"): self.msgs_store.register_messages_from_checker(checker) for message in checker.messages: if not message.default_enabled: self.disable(message.msgid) # Register the checker, but disable all of its messages. - if not getattr(checker, "enabled", True): + if not (self._freeze_register_msgs or getattr(checker, "enabled", True)): self.disable(checker.name) + def _deregister_checkers( + self, checker_collection: Collection[tuple[str, checkers.BaseChecker, int]] + ) -> None: + """De-registered a collection of checkers with its reports. + + Leave messages in place as re-registering them is a no-op. + """ + for checker_name, checker, _ in checker_collection: + self._checkers[checker_name].remove(checker) + if checker.reports: + self.deregister_reports(checker) + def enable_fail_on_messages(self) -> None: """Enable 'fail on' msgs. diff --git a/pylint/reporters/reports_handler_mix_in.py b/pylint/reporters/reports_handler_mix_in.py index 071879ca1e..a4d3a683b1 100644 --- a/pylint/reporters/reports_handler_mix_in.py +++ b/pylint/reporters/reports_handler_mix_in.py @@ -48,6 +48,11 @@ def register_report( reportid = reportid.upper() self._reports[checker].append((reportid, r_title, r_cb)) + def deregister_reports(self, checker: BaseChecker) -> None: + """De-register all reports for a checker.""" + for r_id, r_title, r_cb in checker.reports: + self._reports[checker].remove((r_id, r_title, r_cb)) + def enable_report(self, reportid: str) -> None: """Enable the report of the given id.""" reportid = reportid.upper() diff --git a/tests/test_check_parallel.py b/tests/test_check_parallel.py index c46da09ef5..91ed4738f8 100644 --- a/tests/test_check_parallel.py +++ b/tests/test_check_parallel.py @@ -24,6 +24,7 @@ import pylint.lint.parallel from pylint.checkers import BaseRawFileChecker from pylint.checkers.imports import ImportsChecker +from pylint.config.config_initialization import _config_initialization from pylint.lint import PyLinter, augmented_sys_path from pylint.lint.parallel import _worker_check_single_file as worker_check_single_file from pylint.lint.parallel import _worker_initialize as worker_initialize @@ -217,6 +218,59 @@ def test_worker_initialize_pickling(self) -> None: ) as executor: executor.map(print, [1, 2]) + def test_worker_initialize_custom_plugins(self) -> None: + """Test plugins are initialized (only once) and messages are set to + enabled and disabled correctly, after the worker linter is initialized. + """ + linter = PyLinter(reporter=Reporter()) + linter.load_default_plugins() + config_data = { + "load-plugins": ( + "pylint.extensions.code_style," + "pylint.extensions.typing," + "pylint.checkers.raw_metrics," # Custom report + ), + } + config_args = [ + "--enable=consider-using-augmented-assign", + "--disable=consider-alternative-union-syntax", + ] + with patch( + "pylint.config.config_file_parser._ConfigurationFileParser.parse_config_file", + return_value=(config_data, config_args), + ): + _config_initialization(linter, []) + assert len(linter._checkers["code_style"]) == 1 + assert len(linter._checkers["typing"]) == 1 + assert len(linter._checkers["metrics"]) == 2 + old_metrics_checker = linter._checkers["metrics"][-1] + assert len(linter._reports[old_metrics_checker]) == 2 + assert linter.is_message_enabled("consider-using-augmented-assign") is True + assert ( # default disabled + linter.is_message_enabled("prefer-typing-namedtuple") is False + ) + assert linter.is_message_enabled("consider-alternative-union-syntax") is False + + worker_initialize(linter=dill.dumps(linter)) + worker_linter = pylint.lint.parallel._worker_linter + assert isinstance(worker_linter, PyLinter) + assert len(worker_linter._registered_dynamic_plugin_checkers) == 3 + assert len(worker_linter._checkers["code_style"]) == 1 + assert len(worker_linter._checkers["typing"]) == 1 + assert len(worker_linter._checkers["metrics"]) == 2 + # The base checker overwrite __eq__ and __hash__ to only compare name and msgs. + # Thus, while the ids for the metrics checker are different, they have the same + # hash. That is used as key for the '_reports' dict. + new_metrics_checker = worker_linter._checkers["metrics"][-1] + assert id(old_metrics_checker) != id(new_metrics_checker) + assert old_metrics_checker == new_metrics_checker + assert len(worker_linter._reports[new_metrics_checker]) == 2 + assert linter.is_message_enabled("consider-using-augmented-assign") is True + assert ( # default disabled + linter.is_message_enabled("prefer-typing-namedtuple") is False + ) + assert linter.is_message_enabled("consider-alternative-union-syntax") is False + def test_worker_check_single_file_uninitialised(self) -> None: pylint.lint.parallel._worker_linter = None with pytest.raises( # Objects that do not match the linter interface will fail