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

Retain extra ellipses in protocols and abstract methods #8769

Merged
merged 1 commit into from
Nov 19, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 |- ...


Loading