Skip to content

W0223 does not get raised correctly #9979

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

Open
alibeyram opened this issue Sep 27, 2024 · 4 comments
Open

W0223 does not get raised correctly #9979

alibeyram opened this issue Sep 27, 2024 · 4 comments
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation

Comments

@alibeyram
Copy link

alibeyram commented Sep 27, 2024

Bug description

The not implemented warning does not get raised

here is a simple code that shows it

from abc import ABC, abstractmethod


class Base(ABC):
    @abstractmethod
    def do_something(self):
        pass


class Derived(Base):
    def do_something_else(self):
        pass

One way to test it is to recreate abc.ABC and use that for inheriting

from abc import ABCMeta, abstractmethod


class MYABC(metaclass=ABCMeta):
   __slots__ = ()


class Base(MYABC):
   @abstractmethod
   def do_something(self):
       pass


class Derived(Base):  #### the W0223:abstract-method raised here
   def do_something_else(self):
       pass


### Configuration

_No response_

### Command used

```shell
pylint test.py

Pylint output

Your code has been rated at 10.00/10

Expected behavior

W0223: Method 'do_something' is abstract in class 'Base' but is not overridden in child class 'Derived' (abstract-method)

Pylint version

pylint 3.3.1
astroid 3.3.4
Python 3.12.5 | packaged by conda-forge | (main, Aug  8 2024, 18:32:50) [Clang 16.0.6 ]

OS / Environment

No response

Additional dependencies

No response

@alibeyram alibeyram added the Needs triage 📥 Just created, needs acknowledgment, triage, and proper labelling label Sep 27, 2024
@mbyrnepr2 mbyrnepr2 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 Sep 28, 2024
@jacobtylerwalls jacobtylerwalls added the Needs PR This issue is accepted, sufficiently specified and now needs an implementation label Sep 29, 2024
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue Sep 30, 2024
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue Sep 30, 2024
mbyrnepr2 added a commit to mbyrnepr2/pylint that referenced this issue Sep 30, 2024
@alibeyram
Copy link
Author

@mbyrnepr2 are you gonna merge the PR?

@mbyrnepr2
Copy link
Member

@alibeyram I need to assess the output of the CI primer results to make sure the behaviour is expected. It seems the change triggers the message quite a bit more with the PR.

@mbyrnepr2
Copy link
Member

mbyrnepr2 commented Oct 25, 2024

After looking into this further, and with the helpful output of the home-assistant primer in the CI, I think the original example is not a false negative since we don't know if the Derived subclass will be instantiated or not. If we emit abstract-method for Derived we get potentially a lot of false positives as can be seen from the home-assistant output.

Here is a manufactured example which is something like the home-assistant structure where we have subclasses of the original abc.ABC class, each implementing some of abstractmethods:

# Python is happy to instantiate Class3 since it implements all 3 abstractmethods.
import abc


class Base(abc.ABC):
    @abc.abstractmethod
    def method1(self):
        print("abstract")

    @abc.abstractmethod
    def method2(self):
        print("abstract")

    @abc.abstractmethod
    def method3(self):
        print("abstract")


class Class1(Base):
    def method1(self):
        print("method1 implemented")


class Class2(Class1):
    def method2(self):
        print("method2 implemented")


class Class3(Class2):
    """Concrete class with all abstract methods implemented"""
    def method3(self):
        print("method3 implemented")


myclass = Class3()

I'll change the label to false positive for the second example that was provided:

from abc import ABCMeta, abstractmethod


class MYABC(metaclass=ABCMeta):
   __slots__ = ()


class Base(MYABC):
   @abstractmethod
   def do_something(self):
       pass


class Derived(Base):  #### the W0223:abstract-method raised here
   def do_something_else(self):
       pass

@mbyrnepr2 mbyrnepr2 added False Positive 🦟 A message is emitted but nothing is wrong with the code and removed False Negative 🦋 No message is emitted but something is wrong with the code labels Oct 25, 2024
@mbyrnepr2
Copy link
Member

mbyrnepr2 commented Oct 25, 2024

Also looks related to #7950

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
False Positive 🦟 A message is emitted but nothing is wrong with the code Needs PR This issue is accepted, sufficiently specified and now needs an implementation
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants