Skip to content
New issue

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

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

Already on GitHub? # to your account

Internal checker for @utils.only_required_for_messages to verify that it's used when required and not used when not required #8623

Open
Pierre-Sassoulas opened this issue Apr 24, 2023 · 4 comments
Labels
Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@Pierre-Sassoulas
Copy link
Member

Pierre-Sassoulas commented Apr 24, 2023

@utils.only_required_for_messages should be working for token based checker and a check to verify that it's used when required and not used when not required.

Originally posted by @Pierre-Sassoulas in #8606 (review)

@Pierre-Sassoulas Pierre-Sassoulas added Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation labels Apr 24, 2023
@jacobtylerwalls
Copy link
Member

Would you be able to flesh this out a little more for me? I didn't follow the comment in the review.

@Pierre-Sassoulas
Copy link
Member Author

Pierre-Sassoulas commented Apr 25, 2023

Sure, sorry ! Let me create a functional test example:

from typing import TYPE_CHECKING

from astroid import nodes

from pylint.checkers import BaseChecker
from pylint.checkers.utils import only_required_for_messages

if TYPE_CHECKING:
    from pylint.lint import PyLinter


class AnnoyingCodeGolfChecker(BaseChecker):
    name = "annoying_code_golf"
    msgs = {
        "W0149": (
            "Used `while` loop",
            "while-used",
            "blablabla.",
        )
        "W0150": (
            "Used `for` loop",
            "for-used",
            "blablabla.",
        )
        "W0151": (
            "Used `assignment`",
            "assignment-used",
            "blablabla.",
        )
        "W0152": (
            "Ideas are hard to come by after a point.",
            "another-message",
            "blablabla.",
        )
    }

    @only_required_for_messages("while-used", "for-used") 
    def visit_while(self, node: nodes.While) -> None:  # pylint-suboptimization
        """'for-used' is never added in this function"""
        self.add_message("while-used", node=node)

    def visit_for(self, node: nodes.For) -> None:   # pylint-suboptimization
        """only_required_for_messages not used for this function"""
        self.add_message("for-used", node=node)

    @only_required_for_messages("assignment-used") 
    def visit_assignment(self, node: nodes.Assign) -> None:   # pylint-overoptimization
        """Another message than assignment used is created in this function. 
        
        Admittedly this one is going to be detected by the documentation tests
        because it will be impossible to raise 'another-message' when 
        'assignment-used' is disabled."""
        self.add_message("assignment-used", node=node)
        self._sneakily_add_a_message(node)

    def _sneakily_add_a_message(self, node):
        self.add_message("another-message", node=node)

@DanielNoord
Copy link
Collaborator

Don't we have this on CI? The Linux runner already tests if we have decorated all methods correctly. We don't check for over-usage, but that isn't a big issue imo.

@Pierre-Sassoulas
Copy link
Member Author

We also have some use of self.linter.is_message_enabled( (see example in the MR linked in the original issue text) because the only_required_for_messages decorator only works on visitor_x function.

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Enhancement ✨ Improvement to a component Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

3 participants