Skip to content
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

Don't strip an ellipsis (...) from a function's body - this has special meaning to type-checkers #10358

Closed
rsokl opened this issue Mar 12, 2024 · 3 comments · Fixed by #10413
Closed
Labels
rule Implementing or modifying a lint rule

Comments

@rsokl
Copy link

rsokl commented Mar 12, 2024

Before ruff:

if TYPE_CHECKING:
    def compress(bytes: Sequence[int], target: int) -> list[int]:
        """Docs"
        ...   # <- this tells type-checkers to treat this function like a stub and ignore its body

After ruff:

if TYPE_CHECKING:
    def compress(bytes: Sequence[int], target: int) -> list[int]:  # type: ignore
        """Docs"""
        # Without ... the type-checker assumes the return type is None
        # thus the type-checker marks the return-type annotation as an error.

The ellipses signals to the type-checker (e.g. pyright) that function is acting as a stub and the body should be ignored. In the latter case, the type-checker thinks that the function returns None and thus marks the return-type as an error.

I.e. without the ellipses, you have to either: provide a fake function body or # type: ignore the function's signature, which would mask actual type-errors in the signature.

@charliermarsh charliermarsh added the rule Implementing or modifying a lint rule label Mar 12, 2024
@zanieb
Copy link
Member

zanieb commented Mar 12, 2024

Hm we talked about this previously re protocols

Should we make another exception for if TYPE_CHECKING blocks?

@rsokl
Copy link
Author

rsokl commented Mar 12, 2024

Should we make another exception for if TYPE_CHECKING blocks?

That would be good.

Treating pass and ... as being the same seems weird to me. They are definitely not the same thing.

@zanieb
Copy link
Member

zanieb commented Mar 13, 2024

This should be relatively easy if someone is interested in it.

charliermarsh pushed a commit that referenced this issue Mar 15, 2024
## Summary

Trailing ellipses in objects defined in `typing.TYPE_CHECKING` might be
meaningful (it might be declaring a stub). Thus, we should skip the
`unnecessary-placeholder` (`PIE970`) rule in such contexts.

Closes #10358.

## Test Plan

`cargo nextest run`
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants