Bug in PIE790 in Ruff v0.1.6 (... is meaningful in protocol methods and should not be removed)

[kkom]

Hey, I've got a regression in Ruff 0.1.6 to report.

PIE790 wants the ... removed, but it's necessary in this case – because foo is a protocol method:

from typing import Protocol

class Repro(Protocol):
    def foo(self) -> str:
        """docblock"""
        ...

At least according to Pyright 1.1.336, which shows this error if I allow Ruff to remove the ...:

error: Function with declared return type "str" must return value on all code paths
    "None" is incompatible with "str" (reportGeneralTypeIssues)

[zanieb]

Thanks for the report!

Would you mind reporting this on mypy? This seems like a bug on their end unless there's something in the specification that says ... is required for protocol methods.

ref https://peps.python.org/pep-0544/

[kkom]

Ah, I didn't think of it this way! Yeah, it's possible Pyright is in the (heh) wrong here. I'll report!

[erictraut]

I'm the author of pyright, and I can say that this isn't a bug in pyright. Ruff's formatter should not remove the ellipsis in this case because it has significant meaning.

A protocol class is allowed to provide a default implementation for a method, in which case all subclasses of that protocol inherit that implementation. If the implementation of a method does not include a ... as its implementation, pyright assumes it's a default implementation and type checks it accordingly.

I'll note that black doesn't remove the ellipsis in this case, so I consider this a bug in ruff's formatter.

[zanieb]

Thanks for the context Eric!

This isn't part of Ruff's formatter — it will never change the semantics of the code.

This is a fix for rule removing unnecessary pass statements which was recently expanded to include ellipses (...). If a function has a docstring, there is no need for a pass or ... for it to be syntactically valid. Should Pyright be treating a function with no expressions in its body as the default implementation? Is that part of the protocol specification or is that just what you've implemented?

Regardless, I think we should add a special exemption to this lint rule in Ruff.

[zanieb]

It looks like mypy (1.7.0) does not raise an error for this

from typing import Protocol


class Repro(Protocol):
    def foo(self) -> str:
        """docblock"""

[erictraut]

Is that part of the protocol specification or is that just what you've implemented?

The protocol spec doesn't specify this, nor do any other parts of the typing spec. Pyright's treatment of ... as a placeholder is consistent with how an ellipsis is used elsewhere in the language and typing spec.

Mypy's treatment of ... in function bodies is inconsistent. It treats ... as a placeholder in protocols but not in regular classes. Mypy's behavior has also changed over time. Pyright's behavior has been consistent and principled. I'm not inclined to change it because this would be disruptive to pyright users who rely on the current behavior.

This is a fix for rule removing unnecessary pass statements which was recently expanded to include ellipses (...).

Ah, I see. Thanks for the clarification. Yes, I agree that it would be good to add an exemption for this case. The ... is not extraneous in this situation.

[charliermarsh]

Thanks @erictraut! Does this apply anywhere outside of Protocol methods? Should be an easy fix.

[erictraut]

It also applies to abstract methods (those decorated with @abc.abstractmethod) within an abstract class. Protocols and abstract classes are closely related.

Should be an easy fix.

Great to hear!

[charliermarsh]

Okay great, thanks Eric!

[charliermarsh]

I can take this one since it's my regression :)

[rsokl]

Does this apply anywhere outside of Protocol methods?

This also applies for stubs defined inside if TYPE_CHECKING clauses. See #10358