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

Improper "arguments-differ" for implementation of overloaded methods when using an abstract base class (ABC) #5706

Closed
flisboac opened this issue Jan 21, 2022 · 3 comments
Labels
C: arguments-differ Issues related to 'arguments-differ' and 'signature-differs' checks Duplicate 🐫 Duplicate of an already existing issue False Positive 🦟 A message is emitted but nothing is wrong with the code

Comments

@flisboac
Copy link

Bug description

Title says it all.

Consider the following snippet of code. Save it as test_pylint.py somewhere:

"""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"""

DefaultService implements the abstract base class Service. As it is an implementation, I expect to only need to define the method implementation, and not repeat overloads in the class.

I also expect the implementation signature to be complete, in both the ABC and implementation class, as it covers all possible parameters, accepting no more than what's necessary (except for the *, which is inevitable for me because I don't have access to a Python 3.8+ interpreter everywhere and hence no access to the Positional-only parameters separator /).

Configuration

# Using defaults, in an empty folder. No pylint.ini, no [tool.pylint], etc., 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)

Expected behavior

Mypy accepts the definition in DefaultService.run_job, as I'd expect, but pylint raises a warning for some reason:

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)

This should not happen. The definition's signature is complete.

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

Windows (no WSL, etc; but on Git bash)

Additional dependencies

No response

@flisboac flisboac added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Jan 21, 2022
@flisboac
Copy link
Author

If I may take this moment to make a further question. Please see the comments on the following code snippet. Consider my previous snippet (in the issue description) as a base:

# I also have some other questions...
# Consider the following function. `service` will be passed by callers.

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")

@flisboac flisboac changed the title Improper "arguments-differ" for implementation of overloaded methods in an abstract base class (ABC) Improper "arguments-differ" for implementation of overloaded methods when using an abstract base class (ABC) Jan 21, 2022
@Pierre-Sassoulas Pierre-Sassoulas added False Positive 🦟 A message is emitted but nothing is wrong with the code and removed Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling labels Jan 22, 2022
@DanielNoord DanielNoord added the C: arguments-differ Issues related to 'arguments-differ' and 'signature-differs' checks label Jan 23, 2022
@DanielNoord
Copy link
Collaborator

The second comment is a completely different issue. Would be nice to support I think, but we should create a separate issue for it. The first issue is a duplicate of #5264.

@flisboac Could you create a new issue for your second comment so we can discuss it on its own. I will then close this as a duplicate.

@flisboac
Copy link
Author

Created issue #5712, as requested. Thank you!

Gonna close this issue, then.

@Pierre-Sassoulas Pierre-Sassoulas added the Duplicate 🐫 Duplicate of an already existing issue label Jan 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
C: arguments-differ Issues related to 'arguments-differ' and 'signature-differs' checks Duplicate 🐫 Duplicate of an already existing issue False Positive 🦟 A message is emitted but nothing is wrong with the code
Projects
None yet
Development

No branches or pull requests

3 participants