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

False positive on 'abstract-method' when extending ABC classes #3098

Closed
jnsnow opened this issue Sep 8, 2019 · 2 comments · Fixed by #3446
Closed

False positive on 'abstract-method' when extending ABC classes #3098

jnsnow opened this issue Sep 8, 2019 · 2 comments · Fixed by #3446

Comments

@jnsnow
Copy link

jnsnow commented Sep 8, 2019

Steps to reproduce

Consider this snippet:

import abc
from collections.abc import Mapping
import logging

class SomeDictMixin(Mapping, abc.ABC):
    def logged_get(self, key, default=None):
        tmp = self.get(key, default)
        logging.debug("Got %s for key %s", tmp, key)
        return tmp

The stated purpose of this class would be to create an abstract extension of Mapping that provides additional functionality as a "mixin". By deriving from Mapping we declare our requirement that self.get will be present. By inheriting abc.ABC we stipulate that this class itself cannot be instantiated until all of its abstracted methods are defined.

Current behavior

Pylint does not appear to understand the semantic consequences of inheriting from abc.ABC:

example.py:5:0: W0223: Method '__getitem__' is abstract in class 'Mapping' but is not overridden (abstract-method)
example.py:5:0: W0223: Method '__iter__' is abstract in class 'Iterable' but is not overridden (abstract-method)
example.py:5:0: W0223: Method '__len__' is abstract in class 'Sized' but is not overridden (abstract-method)

Namely, that we are explicitly stating that this class is itself abstract.

Expected behavior

Pylint will understand that we are implementing an abstract class and not produce these errors.

pylint --version output

pylint 2.3.1
astroid 2.2.5
Python 3.7.4 (default, Jul  9 2019, 16:32:37) 
[GCC 9.1.1 20190503 (Red Hat 9.1.1-1)]
@PCManticore
Copy link
Contributor

Thanks for opening an issue!

Right now pylint assumes that any subclass of an abstract class needs to implement the corresponding abstract methods, as such pylint does not properly support intermediary abstract classes, such as this one. It's a long standing issue with how pylint assumes that an abstract class should operate, but in this particular case, we might be able to retrieve a hint of its abstractness from the mixin naming.

@jnsnow
Copy link
Author

jnsnow commented Sep 9, 2019

It may as well be a core python issue, too. Even if you don't inherit ABC, you can't instantiate the mixin, so there's not really a clean way to semantically declare your intent.

Inheriting ABC does quiet Pycharm's warnings, though, hence that trick. Pycharm seems to use that to understand that the class is intended to be abstract.

That does add useless depth to the MRO, though.

(Sorry for filing so many issues rapid-fire! Thank you for your triage work.)

slavfox added a commit to slavfox/pylint that referenced this issue Mar 18, 2020
Related to pylint-dev#179 and pylint-dev#3098. Tweaks `utils.class_is_abstract` to not
depend purely on the presence of abstract methods, and instead also
return True for classes that either explicitly inherit from `abc.ABC`,
or explicitly define `abc.ABCMeta` as a metaclass.

This means that classes like:

    class Foo(AbstractParent, ABC):
        ...

    class Bar(AbstractParent, metaclass=ABCMeta):
        ...

no longer trigger W0223 (Method is abstract in class ... but is not
overriden).
PCManticore pushed a commit that referenced this issue Mar 24, 2020
…lass=ABCMeta` as abstract (#3446)

Related to #179 and #3098. Tweaks `utils.class_is_abstract` to not
depend purely on the presence of abstract methods, and instead also
return True for classes that either explicitly inherit from `abc.ABC`,
or explicitly define `abc.ABCMeta` as a metaclass.

This means that classes like:

    class Foo(AbstractParent, ABC):
        ...

    class Bar(AbstractParent, metaclass=ABCMeta):
        ...

no longer trigger W0223.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants