Skip to content

Commit

Permalink
Retain extra ellipses in protocols and abstract methods (#8769)
Browse files Browse the repository at this point in the history
## Summary

It turns out that some type checkers rely on the presence of ellipses in
`Protocol` interfaces and abstract methods, in order to differentiate
between default implementations and stubs. This PR modifies the preview
behavior of `PIE790` to avoid flagging "unnecessary" ellipses in such
cases.

Closes #8756.

## Test Plan

`cargo test`
  • Loading branch information
charliermarsh committed Nov 19, 2023
1 parent 00a015c commit 95e2f63
Show file tree
Hide file tree
Showing 4 changed files with 82 additions and 0 deletions.
30 changes: 30 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pie/PIE790.py
Original file line number Diff line number Diff line change
Expand Up @@ -177,3 +177,33 @@ def foo():
for i in range(10):
...
pass

from typing import Protocol


class Repro(Protocol):
def func(self) -> str:
"""Docstring"""
...

def impl(self) -> str:
"""Docstring"""
return self.func()


import abc


class Repro:
@abc.abstractmethod
def func(self) -> str:
"""Docstring"""
...

def impl(self) -> str:
"""Docstring"""
return self.func()

def stub(self) -> str:
"""Docstring"""
...
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ use ruff_diagnostics::{Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::whitespace::trailing_comment_start_offset;
use ruff_python_ast::Stmt;
use ruff_python_semantic::{ScopeKind, SemanticModel};
use ruff_text_size::Ranged;

use crate::checkers::ast::Checker;
Expand Down Expand Up @@ -93,6 +94,12 @@ pub(crate) fn unnecessary_placeholder(checker: &mut Checker, body: &[Stmt]) {
if expr.value.is_ellipsis_literal_expr()
&& checker.settings.preview.is_enabled() =>
{
// Ellipses are significant in protocol methods and abstract methods. Specifically,
// Pyright uses the presence of an ellipsis to indicate that a method is a stub,
// rather than a default implementation.
if in_protocol_or_abstract_method(checker.semantic()) {
return;
}
Placeholder::Ellipsis
}
_ => continue,
Expand Down Expand Up @@ -125,3 +132,21 @@ impl std::fmt::Display for Placeholder {
}
}
}

/// Return `true` if the [`SemanticModel`] is in a `typing.Protocol` subclass or an abstract
/// method.
fn in_protocol_or_abstract_method(semantic: &SemanticModel) -> bool {
semantic.current_scopes().any(|scope| match scope.kind {
ScopeKind::Class(class_def) => class_def
.bases()
.iter()
.any(|base| semantic.match_typing_expr(base, "Protocol")),
ScopeKind::Function(function_def) => {
ruff_python_semantic::analyze::visibility::is_abstract(
&function_def.decorator_list,
semantic,
)
}
_ => false,
})
}
Original file line number Diff line number Diff line change
Expand Up @@ -473,6 +473,8 @@ PIE790.py:179:5: PIE790 [*] Unnecessary `pass` statement
178 | ...
179 | pass
| ^^^^ PIE790
180 |
181 | from typing import Protocol
|
= help: Remove unnecessary `pass`

Expand All @@ -481,5 +483,8 @@ PIE790.py:179:5: PIE790 [*] Unnecessary `pass` statement
177 177 | for i in range(10):
178 178 | ...
179 |- pass
180 179 |
181 180 | from typing import Protocol
182 181 |


Original file line number Diff line number Diff line change
Expand Up @@ -634,13 +634,17 @@ PIE790.py:178:5: PIE790 [*] Unnecessary `...` literal
177 177 | for i in range(10):
178 |- ...
179 178 | pass
180 179 |
181 180 | from typing import Protocol

PIE790.py:179:5: PIE790 [*] Unnecessary `pass` statement
|
177 | for i in range(10):
178 | ...
179 | pass
| ^^^^ PIE790
180 |
181 | from typing import Protocol
|
= help: Remove unnecessary `pass`

Expand All @@ -649,5 +653,23 @@ PIE790.py:179:5: PIE790 [*] Unnecessary `pass` statement
177 177 | for i in range(10):
178 178 | ...
179 |- pass
180 179 |
181 180 | from typing import Protocol
182 181 |

PIE790.py:209:9: PIE790 [*] Unnecessary `...` literal
|
207 | def stub(self) -> str:
208 | """Docstring"""
209 | ...
| ^^^ PIE790
|
= help: Remove unnecessary `...`

Safe fix
206 206 |
207 207 | def stub(self) -> str:
208 208 | """Docstring"""
209 |- ...


0 comments on commit 95e2f63

Please sign in to comment.