False positive B024

[kmike]

Hey! A new release (with #273 included) raises an error

Injectable is an abstract base class, but it has no abstract methods. Remember to use @ abstractmethod, @abstractclassmethod and/or @abstractproperty decorators.

I think that's fine to use ABC without abstractmethods/etc., because that's not the only thing ABCs do. They also allow to register other classes as being subclasses of the base class - see example at https://docs.python.org/3/library/abc.html#abc.ABCMeta.register.

This is how ABC is used in our project (see https://github.com/scrapinghub/web-poet/blob/944dcd75f6c56988d42c56c56adb1b3dcafa2dbd/web_poet/pages.py#L11); a new flake8-bugbear release flags it as an error.

[alk-gpajot]

Also, ABC could just be there to declare a class variable that should be set for concrete classes in a less verbose way than using annotations and properties:

class Abstract(ABC):
    attr: ClassVar[str]

This is checked by mypy and will also raise an AttributeError if trying to access attr on a concrete class that doesn't set it. See https://peps.python.org/pep-0526/

[LefterisJP]

This is how ABC is used in our project (see https://github.com/scrapinghub/web-poet/blob/944dcd75f6c56988d42c56c56adb1b3dcafa2dbd/web_poet/pages.py#L11); a new flake8-bugbear release flags it as an error.

This is how it's used in our project too: https://github.com/rotki/rotki/blob/2c2bc209ba1d51663a86f4b710dfa6bbc24e7535/rotkehlchen/data_import/importers/binance.py#L43-L97

I don't think this is a good check to have and would opt to remove it.

[mike0sv]

Also got false positive for this case:

class A(ABC):
   @abstactmethod
   def f1(self): pass

   @abstactmethod
   def f2(self): pass

class B(A, ABC):
   def f2(self): pass

[kmike]

Hey! What's the reason the issue is closed? Is it done on purpose (essentially, "wontfix"), or is it a side-effect of some commit message?

[cooperlees]

The comments above indicate a fix was landed (via #281). If you believe the latest version still does hi-lights this issue incorrectly still we can reopen this issue.

A repro, example code (if it differs in any way) or suggestions on how to make this better always welcome.

[kmike]

I think that the whole B024 should at least be optional. It's not an issue with the implementation, but rather with the idea: that's not a mistake to use an abstract class without abstract methods. There are examples in official Python docs, as well as in real-world projects; both are linked in the ticket description.

[jakkdl]

This was closed because it now shouldn't warn for classes that inherits from multiple classes (as is the case in https://github.com/scrapinghub/web-poet/blob/944dcd75f6c56988d42c56c56adb1b3dcafa2dbd/web_poet/pages.py#L11 ) and is also silenced by having any attr's (as #278 (comment) ). So your project, at least in that case, should not be raising B024.

https://docs.python.org/3/library/abc.html#abc.ABCMeta.register will still raise a warning, and I suppose that classes that are empty (only contains docstrings, pass and/or ...) could also be filtered out - especially as the suggestion doesn't make sense in that case.

But the error message is explicitly just a suggestion

B024: Abstract base class with no abstract method. You might have forgotten to add @AbstractMethod.

as it isn't a definite error, though maybe one could word it even slighter.

[jakkdl]

Oh actually, the error message differs from the one written in the readme, and sounds more certain. Will fix that

    "B024 {} is an abstract base class, but it has no abstract methods. Remember to"
    " use the @abstractmethod decorator, potentially in conjunction with"
    " @classmethod, @property and/or @staticmethod."