False positive for unnecessary-ellipsis on Protocol methods

[llucax]

Bug description

pyright uses ... to determine the body of this method is not implemented here. Without using ..., pyright rightfully (IMHO, as is the only way it has to know the user is intentionally omitting the body) complains about a missing return to match the declared return type of the function.

Example:

"""Module docstring."""

from typing import Protocol


# pylint: disable=too-few-public-methods
class P(Protocol):
    """Class docstring"""

    def f(self) -> int:
        """Method docstring"""
        ...

When removing the ... pylint passes by pyright says:

[test] $ pyright t.py 
/tmp/test/t.py
  /tmp/test/t.py:10:20 - error: Function with declared return type "int" must return value on all code paths
    "None" is incompatible with "int" (reportGeneralTypeIssues)
1 error, 0 warnings, 0 informations 

Configuration

None.

Steps to reproduce:

$ python3.11 -m venv test
$ cd test/
$ . bin/activate
[test] $ pip install pylint pyright
Collecting pylint
  Using cached pylint-3.0.3-py3-none-any.whl.metadata (12 kB)
Collecting pyright
  Using cached pyright-1.1.341-py3-none-any.whl.metadata (6.2 kB)
Collecting platformdirs>=2.2.0 (from pylint)
  Using cached platformdirs-4.1.0-py3-none-any.whl.metadata (11 kB)
Collecting astroid<=3.1.0-dev0,>=3.0.1 (from pylint)
  Using cached astroid-3.0.2-py3-none-any.whl.metadata (4.5 kB)
Collecting isort!=5.13.0,<6,>=4.2.5 (from pylint)
  Using cached isort-5.13.2-py3-none-any.whl.metadata (12 kB)
Collecting mccabe<0.8,>=0.6 (from pylint)
  Using cached mccabe-0.7.0-py2.py3-none-any.whl (7.3 kB)
Collecting tomlkit>=0.10.1 (from pylint)
  Using cached tomlkit-0.12.3-py3-none-any.whl.metadata (2.7 kB)
Collecting dill>=0.3.6 (from pylint)
  Using cached dill-0.3.7-py3-none-any.whl.metadata (9.9 kB)
Collecting nodeenv>=1.6.0 (from pyright)
  Using cached nodeenv-1.8.0-py2.py3-none-any.whl.metadata (21 kB)
Requirement already satisfied: setuptools in ./lib/python3.11/site-packages (from nodeenv>=1.6.0->pyright) (68.1.2)
Using cached pylint-3.0.3-py3-none-any.whl (510 kB)
Using cached pyright-1.1.341-py3-none-any.whl (18 kB)
Using cached astroid-3.0.2-py3-none-any.whl (275 kB)
Using cached dill-0.3.7-py3-none-any.whl (115 kB)
Using cached isort-5.13.2-py3-none-any.whl (92 kB)
Using cached nodeenv-1.8.0-py2.py3-none-any.whl (22 kB)
Using cached platformdirs-4.1.0-py3-none-any.whl (17 kB)
Using cached tomlkit-0.12.3-py3-none-any.whl (37 kB)
Installing collected packages: tomlkit, platformdirs, nodeenv, mccabe, isort, dill, astroid, pyright, pylint
Successfully installed astroid-3.0.2 dill-0.3.7 isort-5.13.2 mccabe-0.7.0 nodeenv-1.8.0 platformdirs-4.1.0 pylint-3.0.3 pyright-1.1.341 tomlkit-0.12.3

Command used

`pylint t.py`

Pylint output

************* Module t
t.py:12:8: W2301: Unnecessary ellipsis constant (unnecessary-ellipsis)

------------------------------------------------------------------
Your code has been rated at 7.50/10 (previous run: 5.00/10, +2.50)

Expected behavior

No failures.

Pylint version

pylint 3.0.3
astroid 3.0.2
Python 3.11.7 (main, Dec  8 2023, 14:22:46) [GCC 13.2.0]

OS / Environment

Debian testing

Additional dependencies

$ pip freeze 
astroid==3.0.2
dill==0.3.7
isort==5.13.2
mccabe==0.7.0
nodeenv==1.8.0
platformdirs==4.1.0
pylint==3.0.3
pyright==1.1.341
tomlkit==0.12.3

[vtgn]

Hi!
Same problem here.
Pylint says to delete the ellipsis, but Pylance requires it and notifies its absence as an error.

Function with declared return type "int" must return value on all code paths
  "None" is not assignable to "int" Pylance[reportReturnType](https://github.com/microsoft/pyright/blob/main/docs/configuration.md#reportReturnType)

However, mypy does not notify anything with and without the ellipsis.

For Pylint it is just a warning, so I prefer to disable the pylint warning on the line of the ellipsis.
Too bad this warning is still present and not fixed yet... :/

Regards.

[DanielNoord]

To make this actionable:
pylint should not consider ... unused if it is being used in the method of a Protocol.

See the discussion in pyright: microsoft/pyright#6487
And ruff: astral-sh/ruff#8756

The discussion also says something about them not being unused in methods with abc.abstractmethod but as a linter maintainer I don't fully understand that use case. I don't see any reason why you would want a default implementation of an abstractmethod so I don't see why there is difference between abstractmethod with or without an ellipsis.

Perhaps @erictraut can help me understand why this would be needed. And also comment on whether there is any effort on getting this spelled out in the typing spec. I'm very sympathetic to the arguments of pyright and agree almost fully but I also would like not to let pylint be based on the implementation of a single type checker (which arguably seems to be doing things right in this case) but on a spec instead.

Sorry Eric for the ping, I think you're the best person I know to reflect on this issue.

[vtgn]

To make this actionable: pylint should not consider ... unused if it is being used in the method of a Protocol.

See the discussion in pyright: microsoft/pyright#6487 And ruff: astral-sh/ruff#8756

The discussion also says something about them not being unused in methods with abc.abstractmethod but as a linter maintainer I don't fully understand that use case. I don't see any reason why you would want a default implementation of an abstractmethod so I don't see why there is difference between abstractmethod with or without an ellipsis.

Perhaps @erictraut can help me understand why this would be needed. And also comment on whether there is any effort on getting this spelled out in the typing spec. I'm very sympathetic to the arguments of pyright and agree almost fully but I also would like not to let pylint be based on the implementation of a single type checker (which arguably seems to be doing things right in this case) but on a spec instead.

Sorry Eric for the ping, I think you're the best person I know to reflect on this issue.

In a protocol or abstract class, if a method has an implementation, it can't be an abstract method, because this is a non sense. So this method can be overridden in an implementation class, but this is not mandatory, contrary to an abstract method. It is useful to provide an implementation of a method in a protocol or an abstact class for several cases:

• when you want to implement an algorithm being customized partially (you implement the algorithm in an implemented method, and you call abstract methods inside that must be implemented by the implementation classes to provide the custom behavior parts.
• when your protocol/abstract class declares a lot of abstract methods, that don't need an implementation all the time by the implementation classes. To avoid to define a lot of methods doing nothing in all the implementation classes, it is sometimes better to implement this no behavior implementation in the protocol/abstract class, thus the implementation classes are smaller and clearer by not being forced to implement all the useless methods.

[DanielNoord]

Thanks for that explanation @vtgn but that doesn't really answer my question. Specifically I'm wondering why you would ever need:

from abc import ABC, abstractmethod


class A(ABC):
    @abstractmethod
    def method(self) -> str:
        return "This is a base implementation"

Having a base implementation on something that is decorated with abstractmethod defeats the purpose of the decorator. Therefore, I don't see why we would need to have difference in behaviour for the following two methods:

@abstractmethod
def method(self) -> str:
    """Implement this method in the subclass"""
    ...

@abstractmethod
def method(self) -> str:
    """Implement this method in the subclass"""

With the Protocol example I understand that the second version actually has different runtime implications. For the example I'm giving here I don't really understand why we would need to support both patterns. Yes, they work differently on runtime but I don't think the user is intending that the second implementation returns None and I think a type checker should/could just consider this unimplemented as well?

[vtgn]

@DanielNoord
My mistake: indeed, Python allows an implementation in an abstract method :
Note: 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.
Source: https://docs.python.org/3.13/library/abc.html#abc.abstractmethod

Personally I find this totally illogical, because by definition, an abstract method has no implementation. I've just tested, and even if a method decorated by abstractmethod has a default implementation, the method is still considered as abstract in the subclass, so the subclass is also considered as abstract if it doesn't override the method.

In fact, the only definition of an abstract method is a method decorated by abstractmethod: the methods not decorated by abstractmethod and having empty, pass or ... body are not considered as abstract. You can check it at runtime by watching the __abstractmethods__ attribute value for classes and protocols, or use the function isabstract of the inspect module.
In protocol classes, methods can be implemented, but this doesn't change the behavior: these methods must be implemented by the classes for which you want their instances can be typed by the protocol class.

In addition, at runtime a method with an empty, pass or ... body is callable without any error and returns None. And a protocol class always raises an error if you try to instantiate it, but you can call class methods and static methods on it.

Impossible to find an official definition and specification for the use of the ellipsis as method's body. :/ Same for empty bodies.
The only thing I found is for the stub files: The type checker should only check function signatures in stub files; It is recommended that function bodies in stub files just be a single ellipsis (...).
Source: https://peps.python.org/pep-0484/

For the use of the pass :
pass is a null operation — when it is executed, nothing happens. It is useful as a placeholder when a statement is required syntactically, but no code needs to be executed, for example:

def f(arg): pass    # a function that does nothing (yet)

class C: pass       # a class with no methods (yet)

Source: https://docs.python.org/3.13/reference/simple_stmts.html#the-pass-statement

This lack of specification and definition forces the linters developers to choose their own, or else if they don't want to choose, they must accept and consider all these cases the same way as during code execution (i.e they return None) and so check typing as it.

The only official definition we have is the one of "pass" (see above). It's purely a syntactic feature useful only to fix syntax problems in the cases for which nothing must be executed. I think that an empty body should be considered the same, but only when there is no syntax problem, for example when a method docstring block is written. This definition of pass (and so of empty body) allows to use it for:

1. implementing non abstract methods returning None
2. implementing by default abstract methods returning None
3. defining abstract methods with no default implementation
4. defining methods in a protocol class

According to ChatGPT: The use of ... (ellipsis) in Python does not have specific official documentation in the standard Python documentation, as it is a basic language feature that is often used idiomatically.
It says too that generally, an ellipsis is used to specify that the implementation shall be done further in the code (for example in the abstract methods to indicate they must be implemented in subclasses, or in the methods of a protocol to indicate they must be implemented by the compatible classes) or later (for example for methods not ready to be implemented, so a kind of TODO).

So by taking into account all these informations:

1. For non protocol classes:
   1.1. For unimplemented abstract methods (i.e. decorated by abstractmethod):

empty body, pass, and ... must be allowed

1.2. For implemented abstract methods (i.e. decorated by abstractmethod):

empty body and pass must be allowed for a method returning NoneType, and must report a type error if the method's declared returned type is not NoneType. If the user wants to implement it later, ... must be allowed as a TODO, so any call of it must report an error.

1.3. For non abstract methods (i.e. not decorated by abstractmethod)

empty body and pass must be allowed for a method returning NoneType, and must report a type error if the method's declared returned type is not NoneType. If the user wants to implement it later, ... must be allowed as a TODO, so any call of it must report an error.

2. For protocol classes:

Semantically, there is no difference between unimplemented abstract methods, implemented abstract methods, unimplemented non abstract method, implemented non abstract method: all these methods must be defined by the compatible classes.
However, the implemented methods in a protocol class must be typed checked.

2.1. For unimplemented abstract methods (i.e. decorated by abstractmethod):

empty body, pass, and ... must be allowed

2.2. For implemented abstract methods (i.e. decorated by abstractmethod):

empty body and pass must be allowed for a method returning NoneType, and must report a type error if the method's declared returned type is not NoneType. If the user wants to implement it later, ... must be allowed as a TODO, so any call of it must report an error.

2.3. For unimplemented non abstract methods

empty body, pass, and ... must be allowed

2.4. For implemented non abstract methods:

empty body and pass must be allowed for a method returning NoneType, and must report a type error if the method's declared returned type is not NoneType. If the user wants to implement it later, ... must be allowed as a TODO, so any call of it must report an error.

As you can see, there are several ambiguities in these specifications, so choices must be done by the linters:

• Ambiguity 1: 1.1 and 1.2 -> empty, pass and ... bodies are allowed for 2 different meanings (unimplemented and implemented method).
• Ambiguity 2: 2.1 and 2.2 -> empty, pass and ... bodies are allowed for 2 different meanings (unimplemented and implemented method).
• Ambiguity 3: 2.3 and 2.4 -> empty, pass and ... bodies are allowed for 2 different meanings (unimplemented and implemented method).

These 3 ambiguities have the same origin: impossible to determine if the method is unimplemented or implemented. So the linters must take a decision for each case to choose if it means the method is unimplemented or implemented. And it would be better if they all choose the same decision, and so if an official Python specification does it. Is it possible to make a demand?

[erictraut]

@vtgn, I agree that this is an area that would benefit from standardization. The Python typing spec is probably the best place to do this.

We (members of the Python typing community) have made good progress in the past year augmenting and clarifying the typing spec, but there is more work to do. I recently gave a talk where I proposed a list of priorities for the typing spec. The topic of "unimplemented methods (in protocols and ABCs)" is on that list, but I don't think anyone has made progress in that area yet.

If you or someone else would like to spearhead the effort to document and ratify a standard in this area, here is the process you can follow. As a start, you could create a new discussion in the Python typing forum.

[DanielNoord]

I think one of the issues we would face in writing this spec is that it is incredibly hard to separate the linting concerns and the type checking.
The docs mention a reason why you would want an implementation, but I think as a linter pylint would probably still discourage that use.
If we were to write down what the actual runtime behaviour (and therefore probably the typing spec) is I think it probably still wouldn't really fit with what pylint would want to do.

[vtgn]

@vtgn, I agree that this is an area that would benefit from standardization. The Python typing spec is probably the best place to do this.

We (members of the Python typing community) have made good progress in the past year augmenting and clarifying the typing spec, but there is more work to do. I recently gave a talk where I proposed a list of priorities for the typing spec. The topic of "unimplemented methods (in protocols and ABCs)" is on that list, but I don't think anyone has made progress in that area yet.

If you or someone else would like to spearhead the effort to document and ratify a standard in this area, here is the process you can follow. As a start, you could create a new discussion in the Python typing forum.

For me, implementing an abstract method should be forbidden because it is a non sense. This fixes the ambiguities 1 and 2.
And for the protocols, implementing methods should be forbidden too, because protocol has been added to be used as interface with no declared link with the compatible classes. This fixes the ambiguity 3.

[vtgn]

I think one of the issues we would face in writing this spec is that it is incredibly hard to separate the linting concerns and the type checking. The docs mention a reason why you would want an implementation, but I think as a linter pylint would probably still discourage that use. If we were to write down what the actual runtime behaviour (and therefore probably the typing spec) is I think it probably still wouldn't really fit with what pylint would want to do.

While we wait for Python specifiers to decide, I think @DanielNoord is right, and the linters should apply what I said on my previous comment:

• abstract methods with an empty, pass or ... body must be considered as unimplemented methods, so the linters must accept these 3 possible bodies.
• same with the protocols methods, because they must all be considered as abstract even without the abstractmethod decorator: so empty, pass or ... body must be considered as unimplemented methods, and so the linters must accept these 3 possible bodies.