mirror of
https://github.com/astral-sh/ruff.git
synced 2025-08-03 10:22:24 +00:00
[flake8-pie
] Mark fix as unsafe if the following statement is a string literal (PIE790
) (#14393)
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz (push) Blocked by required conditions
CI / Fuzz the parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / benchmarks (push) Blocked by required conditions
Some checks are pending
CI / Determine changes (push) Waiting to run
CI / cargo fmt (push) Waiting to run
CI / cargo clippy (push) Blocked by required conditions
CI / cargo test (linux) (push) Blocked by required conditions
CI / cargo test (windows) (push) Blocked by required conditions
CI / cargo test (wasm) (push) Blocked by required conditions
CI / cargo build (release) (push) Blocked by required conditions
CI / cargo build (msrv) (push) Blocked by required conditions
CI / cargo fuzz (push) Blocked by required conditions
CI / Fuzz the parser (push) Blocked by required conditions
CI / test scripts (push) Blocked by required conditions
CI / ecosystem (push) Blocked by required conditions
CI / cargo shear (push) Blocked by required conditions
CI / python package (push) Waiting to run
CI / pre-commit (push) Waiting to run
CI / mkdocs (push) Waiting to run
CI / formatter instabilities and black similarity (push) Blocked by required conditions
CI / test ruff-lsp (push) Blocked by required conditions
CI / benchmarks (push) Blocked by required conditions
## Summary Resolves #12616. ## Test Plan `cargo nextest run` and `cargo insta test`. --------- Co-authored-by: Charlie Marsh <charlie.r.marsh@gmail.com>
This commit is contained in:
parent
3c9e76eb66
commit
0a27c9dabd
4 changed files with 104 additions and 24 deletions
|
@ -235,3 +235,15 @@ if typing.TYPE_CHECKING:
|
|||
def contains_meaningful_ellipsis() -> list[int]:
|
||||
"""Allow this in a TYPE_CHECKING block."""
|
||||
...
|
||||
|
||||
# https://github.com/astral-sh/ruff/issues/12616
|
||||
class PotentialDocstring1:
|
||||
pass
|
||||
"""
|
||||
Lorem ipsum dolor sit amet.
|
||||
"""
|
||||
|
||||
|
||||
class PotentialDocstring2:
|
||||
...
|
||||
'Lorem ipsum dolor sit amet.'
|
||||
|
|
|
@ -22,18 +22,18 @@ use crate::fix::codemods::CodegenStylist;
|
|||
use crate::line_width::{IndentWidth, LineLength, LineWidthBuilder};
|
||||
use crate::Locator;
|
||||
|
||||
/// Return the `Fix` to use when deleting a `Stmt`.
|
||||
/// Return the [`Edit`] to use when deleting a [`Stmt`].
|
||||
///
|
||||
/// In some cases, this is as simple as deleting the `Range` of the `Stmt`
|
||||
/// In some cases, this is as simple as deleting the [`TextRange`] of the [`Stmt`]
|
||||
/// itself. However, there are a few exceptions:
|
||||
/// - If the `Stmt` is _not_ the terminal statement in a multi-statement line,
|
||||
/// - If the [`Stmt`] is _not_ the terminal statement in a multi-statement line,
|
||||
/// we need to delete up to the start of the next statement (and avoid
|
||||
/// deleting any content that precedes the statement).
|
||||
/// - If the `Stmt` is the terminal statement in a multi-statement line, we need
|
||||
/// - If the [`Stmt`] is the terminal statement in a multi-statement line, we need
|
||||
/// to avoid deleting any content that precedes the statement.
|
||||
/// - If the `Stmt` has no trailing and leading content, then it's convenient to
|
||||
/// - If the [`Stmt`] has no trailing and leading content, then it's convenient to
|
||||
/// remove the entire start and end lines.
|
||||
/// - If the `Stmt` is the last statement in its parent body, replace it with a
|
||||
/// - If the [`Stmt`] is the last statement in its parent body, replace it with a
|
||||
/// `pass` instead.
|
||||
pub(crate) fn delete_stmt(
|
||||
stmt: &Stmt,
|
||||
|
|
|
@ -1,9 +1,9 @@
|
|||
use ruff_diagnostics::AlwaysFixableViolation;
|
||||
use ruff_diagnostics::{AlwaysFixableViolation, Applicability};
|
||||
use ruff_diagnostics::{Diagnostic, Edit, Fix};
|
||||
use ruff_macros::{derive_message_formats, violation};
|
||||
use ruff_python_ast::helpers::map_subscript;
|
||||
use ruff_python_ast::whitespace::trailing_comment_start_offset;
|
||||
use ruff_python_ast::Stmt;
|
||||
use ruff_python_ast::{Expr, ExprStringLiteral, Stmt, StmtExpr};
|
||||
use ruff_python_semantic::{ScopeKind, SemanticModel};
|
||||
use ruff_text_size::Ranged;
|
||||
|
||||
|
@ -51,6 +51,11 @@ use crate::fix;
|
|||
/// """Placeholder docstring."""
|
||||
/// ```
|
||||
///
|
||||
/// ## Fix safety
|
||||
/// This rule's fix is marked as unsafe in the rare case that the `pass` or ellipsis
|
||||
/// is followed by a string literal, since removal of the placeholder would convert the
|
||||
/// subsequent string literal into a docstring.
|
||||
///
|
||||
/// ## References
|
||||
/// - [Python documentation: The `pass` statement](https://docs.python.org/3/reference/simple_stmts.html#the-pass-statement)
|
||||
#[violation]
|
||||
|
@ -82,19 +87,19 @@ pub(crate) fn unnecessary_placeholder(checker: &mut Checker, body: &[Stmt]) {
|
|||
return;
|
||||
}
|
||||
|
||||
for stmt in body {
|
||||
for (index, stmt) in body.iter().enumerate() {
|
||||
let kind = match stmt {
|
||||
Stmt::Pass(_) => Placeholder::Pass,
|
||||
Stmt::Expr(expr) if expr.value.is_ellipsis_literal_expr() => {
|
||||
// In a type-checking block, a trailing ellipsis might be meaningful. A
|
||||
// user might be using the type-checking context to declare a stub.
|
||||
// In a type-checking block, a trailing ellipsis might be meaningful.
|
||||
// A user might be using the type-checking context to declare a stub.
|
||||
if checker.semantic().in_type_checking_block() {
|
||||
return;
|
||||
}
|
||||
|
||||
// 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.
|
||||
// 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;
|
||||
}
|
||||
|
@ -103,19 +108,46 @@ pub(crate) fn unnecessary_placeholder(checker: &mut Checker, body: &[Stmt]) {
|
|||
_ => continue,
|
||||
};
|
||||
|
||||
let mut diagnostic = Diagnostic::new(UnnecessaryPlaceholder { kind }, stmt.range());
|
||||
let edit = if let Some(index) = trailing_comment_start_offset(stmt, checker.source()) {
|
||||
Edit::range_deletion(stmt.range().add_end(index))
|
||||
} else {
|
||||
fix::edits::delete_stmt(stmt, None, checker.locator(), checker.indexer())
|
||||
};
|
||||
diagnostic.set_fix(Fix::safe_edit(edit).isolate(Checker::isolation(
|
||||
checker.semantic().current_statement_id(),
|
||||
)));
|
||||
checker.diagnostics.push(diagnostic);
|
||||
let next_stmt = body.get(index + 1);
|
||||
add_diagnostic(checker, stmt, next_stmt, kind);
|
||||
}
|
||||
}
|
||||
|
||||
/// Add a diagnostic for the given statement.
|
||||
fn add_diagnostic(
|
||||
checker: &mut Checker,
|
||||
stmt: &Stmt,
|
||||
next_stmt: Option<&Stmt>,
|
||||
placeholder_kind: Placeholder,
|
||||
) {
|
||||
let edit = if let Some(index) = trailing_comment_start_offset(stmt, checker.source()) {
|
||||
Edit::range_deletion(stmt.range().add_end(index))
|
||||
} else {
|
||||
fix::edits::delete_stmt(stmt, None, checker.locator(), checker.indexer())
|
||||
};
|
||||
let applicability = match next_stmt {
|
||||
// Mark the fix as unsafe if the following statement is a string literal,
|
||||
// as it will become the module/class/function's docstring after the fix.
|
||||
Some(Stmt::Expr(StmtExpr { value, .. })) => match value.as_ref() {
|
||||
Expr::StringLiteral(ExprStringLiteral { .. }) => Applicability::Unsafe,
|
||||
_ => Applicability::Safe,
|
||||
},
|
||||
_ => Applicability::Safe,
|
||||
};
|
||||
|
||||
let isolation_level = Checker::isolation(checker.semantic().current_statement_id());
|
||||
let fix = Fix::applicable_edit(edit, applicability).isolate(isolation_level);
|
||||
|
||||
let diagnostic = Diagnostic::new(
|
||||
UnnecessaryPlaceholder {
|
||||
kind: placeholder_kind,
|
||||
},
|
||||
stmt.range(),
|
||||
);
|
||||
|
||||
checker.diagnostics.push(diagnostic.with_fix(fix));
|
||||
}
|
||||
|
||||
#[derive(Debug, PartialEq, Eq)]
|
||||
enum Placeholder {
|
||||
Pass,
|
||||
|
|
|
@ -675,3 +675,39 @@ PIE790.py:209:9: PIE790 [*] Unnecessary `...` literal
|
|||
210 209 |
|
||||
211 210 |
|
||||
212 211 | class Repro(Protocol[int]):
|
||||
|
||||
PIE790.py:241:5: PIE790 [*] Unnecessary `pass` statement
|
||||
|
|
||||
239 | # https://github.com/astral-sh/ruff/issues/12616
|
||||
240 | class PotentialDocstring1:
|
||||
241 | pass
|
||||
| ^^^^ PIE790
|
||||
242 | """
|
||||
243 | Lorem ipsum dolor sit amet.
|
||||
|
|
||||
= help: Remove unnecessary `pass`
|
||||
|
||||
ℹ Unsafe fix
|
||||
238 238 |
|
||||
239 239 | # https://github.com/astral-sh/ruff/issues/12616
|
||||
240 240 | class PotentialDocstring1:
|
||||
241 |- pass
|
||||
242 241 | """
|
||||
243 242 | Lorem ipsum dolor sit amet.
|
||||
244 243 | """
|
||||
|
||||
PIE790.py:248:5: PIE790 [*] Unnecessary `...` literal
|
||||
|
|
||||
247 | class PotentialDocstring2:
|
||||
248 | ...
|
||||
| ^^^ PIE790
|
||||
249 | 'Lorem ipsum dolor sit amet.'
|
||||
|
|
||||
= help: Remove unnecessary `...`
|
||||
|
||||
ℹ Unsafe fix
|
||||
245 245 |
|
||||
246 246 |
|
||||
247 247 | class PotentialDocstring2:
|
||||
248 |- ...
|
||||
249 248 | 'Lorem ipsum dolor sit amet.'
|
||||
|
|
Loading…
Add table
Add a link
Reference in a new issue