DAR202: No return statement in abstract method

[dabljues]

When I have a method like this:

@abstractmethod
def foo(self, bar: str) -> int:
    """foo.

    Args:
        bar (str): bar.

    Returns:
        int: spam.

    """

darglint gives me a DAR202, because I don't have a return statement in my function definition. Event if I have it in derived classes. This is a bug, which should be fixed, since we don't define abstract methods, we just declare them.

[terrencepreilly]

In fact, you can give an implementation to an abstract method in Python. The note at the end of the documentation for abstractmethod says

Unlike Java abstract methods, these abstract methods may have an implementation. This implementation can be called via the super() mechanism from the class that overrides it. This could be useful as an end-point for a super-call in a framework that uses cooperative multiple-inheritance.

I still agree with you that this could be handled better by darglint. The problem is in recognizing that an abstract method doesn't have an implementation. I think checking to see if its implementation is only pass or ... or raise NotImplementedException("...") or return NotImplemented is probably sufficient. What do you think?

[dabljues]

If you go with pass, then you end up with flake complaining about redundant pass. The other one would be good I guess, but then you have to annotate the exception in a docstring :(

[CCardosoDev]

Are there plans to work on this issue? I would like to add that this also happens with Yields, if one wants to document Yeilds in abstract methods darglint raises DAR302 which should not be the case by the same reasons already mentioned in this issue.

[terrencepreilly]

I put this aside for the moment, because there isn't a clear way to move forward on it.

We could check whether the function definition consists only of a NotImplementedException or something, and then exclude that from being required in the docstring. But that may be somewhat surprising behavior to some.

Do you have an opinion on how you would prefer it to be handled?

[CCardosoDev]

I like your suggestion here:

I still agree with you that this could be handled better by darglint. The problem is in recognizing that an abstract method doesn't have an implementation. I think checking to see if its implementation is only pass or ... or raise NotImplementedException("...") or return NotImplemented is probably sufficient. What do you think?

Basically verify if it only has pass or ... or raise NotImplementedException("...") or return NotImplemented or nothing (besides the docstring).

[CCardosoDev]

I like your suggestion here:

I still agree with you that this could be handled better by darglint. The problem is in recognizing that an abstract method doesn't have an implementation. I think checking to see if its implementation is only pass or ... or raise NotImplementedException("...") or return NotImplemented is probably sufficient. What do you think?

Basically verify if it only has pass or ... or raise NotImplementedException("...") or return NotImplemented or nothing (besides the docstring).

What do you think?

[terrencepreilly]

That sounds fine, so long as it's not just "pass" or "nothing" (as linters will complain about those.) I don't have much time recently, so it may take me a while to get around to this. I'm always happy for pull requests, though!

[maltevesper]

@terrencepreilly I started working on this issue and have an implementation of a visitor that detects the Ellipsis and pass case, I hope to finish the raise NotImplentedException and return NotImplemented and "Nothing" case by the end of the weekend (and make a pull request then).

Leaving this here, to avoid double work.

[djmattyg007]

I'm running into a similar issue with methods in a Protocol class. These methods absolutely won't have an implementation because they are akin to an interface in a language like Java or PHP. As I have a docstring, I don't even need to add an ellipsis or pass to the method.

[AdamGleave]

With latest darglint release containing https://github.com/terrencepreilly/darglint/pull/160, I find that this issue is fixed with @abstractmethod, but not @abc.abstractmethod.

Works, test1.py:

from abc import ABC, abstractmethod

class Foo(ABC):
  @abstractmethod
  def bar():
      """Meaning of life.
      
      Returns:
           An integer.
       """

Fails, test2.py:

import abc

class Foo(abc.ABC):
  @abc.abstractmethod
  def bar():
      """Meaning of life.
      
      Returns:
           An integer.
       """

darglint output:

$ darglint test1.py 
$ darglint test2.py 
test2.py:bar:8: DAR202: + return

A little debugging showed that the check _has_decorator https://github.com/terrencepreilly/darglint/blob/develop/darglint/analysis/abstract_callable_visitor.py is returning False with abc.abstractmethod, which seems to be because it is an ast.Atribute rather than ast.Name in https://github.com/terrencepreilly/darglint/blob/develop/darglint/analysis/abstract_callable_visitor.py#L75 but I do not know what would cause this discrepancy.

[thejcannon]

@terrencepreilly can't this be closed due to #160 being merged? I'll create a new issue for abc.abstractmethod

[Midnighter]

Randomly stumbled upon this issue and just wanted to note that return NotImplemented has a different purpose. It is not to mark abstract methods but to mark a certain comparison method being implemented for a class.

See here https://docs.python.org/3/reference/datamodel.html#the-standard-type-hierarchy and https://docs.python.org/3/reference/datamodel.html#object.ge