Skip to content

Remove pointless check_messages decorators #6353

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 17, 2022
Merged

Remove pointless check_messages decorators #6353

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
93f0a94
Remove pointless ``check_messages`` decorators
DudeNr33 Apr 16, 2022
30d832c
Restore ``@check_messages`` and reference the new issue
DudeNr33 Apr 16, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 0 additions & 4 deletions pylint/checkers/imports.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

from pylint.checkers import BaseChecker, DeprecatedMixin
from pylint.checkers.utils import (
check_messages,
get_import_name,
is_from_fallback_block,
is_node_in_guarded_import_block,
Expand Down Expand Up @@ -420,7 +419,6 @@ def deprecated_modules(self):
"""Callback returning the deprecated modules."""
return self.linter.config.deprecated_modules

@check_messages(*MSGS)
def visit_import(self, node: nodes.Import) -> None:
"""Triggered when an import statement is seen."""
self._check_reimport(node)
Expand All @@ -446,7 +444,6 @@ def visit_import(self, node: nodes.Import) -> None:

self._add_imported_module(node, imported_module.name)

@check_messages(*MSGS)
def visit_importfrom(self, node: nodes.ImportFrom) -> None:
"""Triggered when a from statement is seen."""
basename = node.modname
Expand Down Expand Up @@ -474,7 +471,6 @@ def visit_importfrom(self, node: nodes.ImportFrom) -> None:
else:
self._add_imported_module(node, imported_module.name)

@check_messages(*MSGS)
def leave_module(self, node: nodes.Module) -> None:
# Check imports are grouped by category (standard, 3rd party, local)
std_imports, ext_imports, loc_imports = self._check_imports_order(node)
Expand Down
3 changes: 1 addition & 2 deletions pylint/checkers/logging.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@

from pylint import checkers, interfaces
from pylint.checkers import utils
from pylint.checkers.utils import check_messages, infer_all
from pylint.checkers.utils import infer_all

if TYPE_CHECKING:
from pylint.lint import PyLinter
Expand Down Expand Up @@ -180,7 +180,6 @@ def visit_import(self, node: nodes.Import) -> None:
if module in self._logging_modules:
self._logging_names.add(as_name or module)

@check_messages(*MSGS)
def visit_call(self, node: nodes.Call) -> None:
"""Checks calls to logging methods."""

Expand Down
1 change: 0 additions & 1 deletion pylint/checkers/strings.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,6 @@ def _check_interpolation(self, node: nodes.JoinedStr) -> None:
return
self.add_message("f-string-without-interpolation", node=node)

@check_messages(*MSGS)
def visit_call(self, node: nodes.Call) -> None:
func = utils.safe_infer(node.func)
if (
Expand Down
9 changes: 8 additions & 1 deletion pylint/checkers/typecheck.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1294,7 +1294,6 @@ def _check_isinstance_args(self, node):
self.add_message("isinstance-second-argument-not-valid-type", node=node)

# pylint: disable=too-many-branches,too-many-locals
@check_messages(*(list(MSGS.keys())))
Copy link
Collaborator

@DanielNoord DanielNoord Apr 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😅

def visit_call(self, node: nodes.Call) -> None:
"""Check that called functions/methods are inferred to callable objects,
and that passed arguments match the parameters in the inferred function.
Expand Down Expand Up @@ -1831,11 +1830,19 @@ def _check_unsupported_alternative_union_syntax(self, node: nodes.BinOp) -> None
self.add_message("unsupported-binary-operation", args=msg, node=node)
break

# pylint: disable-next=fixme
# TODO: This check was disabled (by adding the leading underscore)
# due to false positives several years ago - can we re-enable it?
# https://github.com/PyCQA/pylint/issues/6359
@check_messages("unsupported-binary-operation")
def _visit_binop(self, node: nodes.BinOp) -> None:
Copy link
Collaborator

@DanielNoord DanielNoord Apr 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you understand why these are underscored? They don't seem to be called in this file..

Copy link
Collaborator Author

@DudeNr33 DudeNr33 Apr 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I saw that the functional test for this message (unsupported-binary-operation) is always skipped, with the following comment:

Disable for unsupported-binary-operation

Unfortunately, this warning exhibits currently way too many false
positives, practically rendering this error useless. I can't fix
all of them until I release Pylint 1.5, so the most reasonable
choice for now is to disable it and reenable it as soon as we fix
those problems.

Copy link
Collaborator

@DanielNoord DanielNoord Apr 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh wow, we should probably follow up on that..

But then the decorator is "correct" we only need to de-underscore this function right? Perhaps we should keep the decorator and add a TODO to fix this?

Copy link
Collaborator Author

@DudeNr33 DudeNr33 Apr 16, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are right - if we plan to re-enable this check, the decorator should stay. Will fix this and add a TODO.

"""Detect TypeErrors for binary arithmetic operands."""
self._check_binop_errors(node)

# pylint: disable-next=fixme
# TODO: This check was disabled (by adding the leading underscore)
# due to false positives several years ago - can we re-enable it?
# https://github.com/PyCQA/pylint/issues/6359
@check_messages("unsupported-binary-operation")
def _visit_augassign(self, node: nodes.AugAssign) -> None:
"""Detect TypeErrors for augmented binary arithmetic operands."""
Expand Down