Skip to content
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.

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

Weak or improper support for overloads #5712

Closed
flisboac opened this issue Jan 24, 2022 · 2 comments
Closed

Weak or improper support for overloads #5712

flisboac opened this issue Jan 24, 2022 · 2 comments
Labels
Duplicate 🐫 Duplicate of an already existing issue False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@flisboac
Copy link

Bug description

As requested by @DanielNoord , I'm creating this issue with some of the findings I commented about on another issue.

It seems like pylint either:

  1. does not apply the correct set of overloads to the usage of an overloaded method; or
  2. completely ignores overloads altogether.

Consider the following base code:

"""whatever"""

# Disabling irrelevant checks
# pylint: disable=too-few-public-methods

from abc import ABCMeta, abstractmethod
from dataclasses import dataclass
from typing import Optional, Union
from typing_extensions import overload, TypedDict


@dataclass
class JobDescriptor:
    """whatever"""
    name: str
    location: str


class JobDescriptorData(TypedDict):
    """whatever"""
    name: str
    location: str


JobDescriptorLike = Union[JobDescriptor, JobDescriptorData]


@dataclass
class RunJobResult:
    """whatever"""
    success: bool


class Service(metaclass=ABCMeta):
    """whatever"""
    @overload
    def run_job(self, descriptor: JobDescriptorLike) -> RunJobResult: ...

    @overload
    def run_job(self, *, name: str, location: str) -> RunJobResult: ...

    @abstractmethod
    def run_job(
        self,
        descriptor: Optional[JobDescriptorLike] = None,
        *,
        name: Optional[str] = None,
        location: Optional[str] = None,
    ) -> RunJobResult:
        """whatever"""


class DefaultService(Service):
    """whatever"""

    # SHOULD BE OKAY, but flags "arguments-differ" for `run_job`. Here's the message:
    #
    # > test_pylint.py:60:4: W0221: Number of parameters was 2 in 'Service.run_job'
    # > and is now 4 in overridden 'DefaultService.run_job' method (arguments-differ)
    def run_job(
        self,
        descriptor: Optional[JobDescriptor] = None,
        *,
        name: Optional[str] = None,
        location: Optional[str] = None,
    ) -> RunJobResult:
        """whatever"""

Now, suppose I'm gonna use Service.run_job in some capacity. The comments expose the behaviour I expect pylint to have, including some comparison with mypy (but could be another linter; I used it as means to comparison, just to confirm if I was mistaken or not):

def main(service: Service):
    """whatever"""
    # This should be ALLOWED.
    # Accepted by both mypy and pylint, which is what I'd expect.
    service.run_job(JobDescriptor("job_1", "/a/b/c"))
    service.run_job(name="job_1", location="/a/b/c")
    service.run_job({"name": "job_1", "location": "/a/b/c"})
    data: JobDescriptorData = {"name": "job_1", "location": "/a/b/c"}
    service.run_job(data)

    # This should be DISALLOWED, but pylint does not flag.
    # mypy properly complains.
    # Why did pylint fail to see that the dict is not of the correct shape?
    service.run_job({})
    service.run_job({"property_not_declared": "shall_not_pass"})

    # DISALLOWED, but pylint does not flag.
    # mypy properly complains.
    # Why?
    # There's no signature on which an int is a valid first argument.
    # Using "/," (instead of, or with, "*,") yields no difference.
    service.run_job(1)

    # DISALLOWED, but pylint does not flag.
    # mypy properly complains.
    # Why?
    # There's no signature on which you can specify both `descriptor`
    # and `name`.
    # The @abstractmethod-marked function in Service shouldn't be a
    # signature candidate, because it refers to the implementation.
    service.run_job(JobDescriptor("a", "b"), name="c")

Note that the problem here is the absence of output for some pretty obvious mistakes.

Configuration

# defaults; no custom config whatsoever

Command used

pylint -- test_pylint.py

Pylint output

************* Module test_pylint
test_pylint.py:60:4: W0221: Number of parameters was 2 in 'Service.run_job' and is now 4 in overridden 'DefaultService.run_job' method (arguments-differ)

------------------------------------------------------------------
Your code has been rated at 9.68/10 (previous run: 9.35/10, +0.32)

Expected behavior

Please, see the comments on main function's implementation.

Pylint version

pylint 2.12.2
astroid 2.9.0
Python 3.8.5 (tags/v3.8.5:580fbb0, Jul 20 2020, 15:57:54) [MSC v.1924 64 bit (AMD64)]

OS / Environment

Microsoft Windows 10 Enterprise (v. 10.0.18363 build 18363)

Additional dependencies

No response

@flisboac flisboac added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 24, 2022
@DanielNoord DanielNoord added False Negative 🦋 No message is emitted but something is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jan 24, 2022
@DanielNoord
Copy link
Collaborator

Note that the arguments-differ issue is tracked in #5264. I think we should revisit this issue after that has been fixed to see what remains of the various issues mentioned here.

@Pierre-Sassoulas Pierre-Sassoulas added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Jul 6, 2022
@jacobtylerwalls
Copy link
Member

Thanks for the report. @overload is a known missing feature for pylint. In the past I've suggested closing tickets for @overload as duplicates of pylint-dev/astroid#1015, so I'll keep on with that now.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Duplicate 🐫 Duplicate of an already existing issue False Negative 🦋 No message is emitted but something is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

No branches or pull requests

4 participants