Skip to content

Commit dd12758

Browse files
authored
Fix two multiprocessing issues (#10642)
1) Not possible to enable extension checks with `"default_enabled": False` Whenever `register_checker` is called it overwrites the message filter in case a message is disabled by default. Usually that isn't an issue as the checkers are first registered and the config loaded afterwards. https://github.com/pylint-dev/pylint/blob/6ce60320a4a369323650f2d4babd57955752e411/pylint/lint/pylinter.py#L495-L504 For multiple jobs the dynamic checkers are re-registered though which overwrites the previously loaded config. To resolve that, it's necessary to restore the message state by caching the list of enabled and disabled messages and reapplying it. https://github.com/pylint-dev/pylint/blob/6ce60320a4a369323650f2d4babd57955752e411/pylint/lint/parallel.py#L55-L58 Closes #10037 2) Warnings are emitted multiple times Whenever a checker get's registered, it's just appended to the list of checkers with the corresponding checker name. That's necessary to allow multiple modules to register the same checker name. https://github.com/pylint-dev/pylint/blob/6ce60320a4a369323650f2d4babd57955752e411/pylint/lint/pylinter.py#L495-L497 When the checker is now re-registered during the worker init, the "old" one is still active. Thus any message is actually printed twice. To resolve it, deregister the "old" dynamic checkers before loading them again.
1 parent 9ffd4b2 commit dd12758

File tree

7 files changed

+103
-3
lines changed

7 files changed

+103
-3
lines changed
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix enabling checks from extensions which are disabled by default if multiple jobs are used.
2+
3+
Closes #10037
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix duplicate messages for extension checks if multiple jobs are used.
2+
3+
Refs #10642

pylint/config/config_initialization.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66

77
import sys
88
import warnings
9+
from copy import copy
910
from glob import glob
1011
from itertools import chain
1112
from pathlib import Path
@@ -58,8 +59,12 @@ def _config_initialization( # pylint: disable=too-many-statements
5859
exec(utils._unquote(config_data["init-hook"])) # pylint: disable=exec-used
5960

6061
# Load plugins if specified in the config file
62+
default_checkers = copy(linter._registered_checkers)
6163
if "load-plugins" in config_data:
6264
linter.load_plugin_modules(utils._splitstrip(config_data["load-plugins"]))
65+
linter._registered_dynamic_plugin_checkers = linter._registered_checkers.difference(
66+
default_checkers
67+
)
6368

6469
unrecognized_options_message = None
6570
# First we parse any options from a configuration file

pylint/lint/parallel.py

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -54,6 +54,12 @@ def _worker_initialize(
5454

5555
# Re-register dynamic plugins, since the pool does not have access to the
5656
# astroid module that existed when the linter was pickled.
57+
# Freeze register new messages to prevent overwriting enabled and disabled messaged
58+
# during dynamic plugin re-load.
59+
_worker_linter._freeze_register_msgs = True
60+
_worker_linter._deregister_checkers(
61+
_worker_linter._registered_dynamic_plugin_checkers
62+
)
5763
_worker_linter.load_plugin_modules(_worker_linter._dynamic_plugins, force=True)
5864
_worker_linter.load_plugin_configuration()
5965

pylint/lint/pylinter.py

Lines changed: 27 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
import traceback
1515
import warnings
1616
from collections import defaultdict
17-
from collections.abc import Callable, Iterable, Iterator, Sequence
17+
from collections.abc import Callable, Collection, Iterable, Iterator, Sequence
1818
from io import TextIOWrapper
1919
from pathlib import Path
2020
from re import Pattern
@@ -323,6 +323,16 @@ def __init__(
323323
"""Dictionary of registered and initialized checkers."""
324324
self._dynamic_plugins: dict[str, ModuleType | ModuleNotFoundError | bool] = {}
325325
"""Set of loaded plugin names."""
326+
self._registered_checkers: set[tuple[str, checkers.BaseChecker, int]] = set()
327+
"""Set of tuples with loaded checker name, reference to checker
328+
and checker object id.
329+
"""
330+
self._registered_dynamic_plugin_checkers: set[
331+
tuple[str, checkers.BaseChecker, int]
332+
] = set()
333+
"""Set of tuples with loaded dynamic plugin checker name, reference to
334+
checker and checker object id.
335+
"""
326336

327337
# Attributes related to stats
328338
self.stats = LinterStats()
@@ -356,6 +366,7 @@ def __init__(
356366
self.msgs_store = MessageDefinitionStore(self.config.py_version)
357367
self.msg_status = 0
358368
self._by_id_managed_msgs: list[ManagedMessage] = []
369+
self._freeze_register_msgs = False
359370

360371
# Attributes related to visiting files
361372
self.file_state = FileState("", self.msgs_store, is_base_filestate=True)
@@ -495,17 +506,30 @@ def report_order(self) -> list[BaseChecker]:
495506
def register_checker(self, checker: checkers.BaseChecker) -> None:
496507
"""This method auto registers the checker."""
497508
self._checkers[checker.name].append(checker)
509+
self._registered_checkers.add((checker.name, checker, id(checker)))
498510
for r_id, r_title, r_cb in checker.reports:
499511
self.register_report(r_id, r_title, r_cb, checker)
500-
if hasattr(checker, "msgs"):
512+
if not self._freeze_register_msgs and hasattr(checker, "msgs"):
501513
self.msgs_store.register_messages_from_checker(checker)
502514
for message in checker.messages:
503515
if not message.default_enabled:
504516
self.disable(message.msgid)
505517
# Register the checker, but disable all of its messages.
506-
if not getattr(checker, "enabled", True):
518+
if not (self._freeze_register_msgs or getattr(checker, "enabled", True)):
507519
self.disable(checker.name)
508520

521+
def _deregister_checkers(
522+
self, checker_collection: Collection[tuple[str, checkers.BaseChecker, int]]
523+
) -> None:
524+
"""De-registered a collection of checkers with its reports.
525+
526+
Leave messages in place as re-registering them is a no-op.
527+
"""
528+
for checker_name, checker, _ in checker_collection:
529+
self._checkers[checker_name].remove(checker)
530+
if checker.reports:
531+
self.deregister_reports(checker)
532+
509533
def enable_fail_on_messages(self) -> None:
510534
"""Enable 'fail on' msgs.
511535

pylint/reporters/reports_handler_mix_in.py

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -48,6 +48,11 @@ def register_report(
4848
reportid = reportid.upper()
4949
self._reports[checker].append((reportid, r_title, r_cb))
5050

51+
def deregister_reports(self, checker: BaseChecker) -> None:
52+
"""De-register all reports for a checker."""
53+
for r_id, r_title, r_cb in checker.reports:
54+
self._reports[checker].remove((r_id, r_title, r_cb))
55+
5156
def enable_report(self, reportid: str) -> None:
5257
"""Enable the report of the given id."""
5358
reportid = reportid.upper()

tests/test_check_parallel.py

Lines changed: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
import pylint.lint.parallel
2525
from pylint.checkers import BaseRawFileChecker
2626
from pylint.checkers.imports import ImportsChecker
27+
from pylint.config.config_initialization import _config_initialization
2728
from pylint.lint import PyLinter, augmented_sys_path
2829
from pylint.lint.parallel import _worker_check_single_file as worker_check_single_file
2930
from pylint.lint.parallel import _worker_initialize as worker_initialize
@@ -217,6 +218,59 @@ def test_worker_initialize_pickling(self) -> None:
217218
) as executor:
218219
executor.map(print, [1, 2])
219220

221+
def test_worker_initialize_custom_plugins(self) -> None:
222+
"""Test plugins are initialized (only once) and messages are set to
223+
enabled and disabled correctly, after the worker linter is initialized.
224+
"""
225+
linter = PyLinter(reporter=Reporter())
226+
linter.load_default_plugins()
227+
config_data = {
228+
"load-plugins": (
229+
"pylint.extensions.code_style,"
230+
"pylint.extensions.typing,"
231+
"pylint.checkers.raw_metrics," # Custom report
232+
),
233+
}
234+
config_args = [
235+
"--enable=consider-using-augmented-assign",
236+
"--disable=consider-alternative-union-syntax",
237+
]
238+
with patch(
239+
"pylint.config.config_file_parser._ConfigurationFileParser.parse_config_file",
240+
return_value=(config_data, config_args),
241+
):
242+
_config_initialization(linter, [])
243+
assert len(linter._checkers["code_style"]) == 1
244+
assert len(linter._checkers["typing"]) == 1
245+
assert len(linter._checkers["metrics"]) == 2
246+
old_metrics_checker = linter._checkers["metrics"][-1]
247+
assert len(linter._reports[old_metrics_checker]) == 2
248+
assert linter.is_message_enabled("consider-using-augmented-assign") is True
249+
assert ( # default disabled
250+
linter.is_message_enabled("prefer-typing-namedtuple") is False
251+
)
252+
assert linter.is_message_enabled("consider-alternative-union-syntax") is False
253+
254+
worker_initialize(linter=dill.dumps(linter))
255+
worker_linter = pylint.lint.parallel._worker_linter
256+
assert isinstance(worker_linter, PyLinter)
257+
assert len(worker_linter._registered_dynamic_plugin_checkers) == 3
258+
assert len(worker_linter._checkers["code_style"]) == 1
259+
assert len(worker_linter._checkers["typing"]) == 1
260+
assert len(worker_linter._checkers["metrics"]) == 2
261+
# The base checker overwrite __eq__ and __hash__ to only compare name and msgs.
262+
# Thus, while the ids for the metrics checker are different, they have the same
263+
# hash. That is used as key for the '_reports' dict.
264+
new_metrics_checker = worker_linter._checkers["metrics"][-1]
265+
assert id(old_metrics_checker) != id(new_metrics_checker)
266+
assert old_metrics_checker == new_metrics_checker
267+
assert len(worker_linter._reports[new_metrics_checker]) == 2
268+
assert linter.is_message_enabled("consider-using-augmented-assign") is True
269+
assert ( # default disabled
270+
linter.is_message_enabled("prefer-typing-namedtuple") is False
271+
)
272+
assert linter.is_message_enabled("consider-alternative-union-syntax") is False
273+
220274
def test_worker_check_single_file_uninitialised(self) -> None:
221275
pylint.lint.parallel._worker_linter = None
222276
with pytest.raises( # Objects that do not match the linter interface will fail

0 commit comments

Comments
 (0)