Python/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff synced 2026-01-21 13:30:49 -05:00
Code Issues Packages Projects Releases Wiki Activity

[ruff] Make fix unsafe if it deletes comments (RUF020) (#22664)

Browse Source 
<!--
Thank you for contributing to Ruff/ty! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title? (Please prefix
with `[ty]` for ty pull
  requests.)
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

## Test Plan

<!-- How was it tested? -->
This commit is contained in:
chiri
2026-01-20 18:02:42 +03:00
committed by GitHub
parent 7e1f07e5c6
commit 7b16239f29
3 changed files with 77 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

7
crates/ruff_linter/resources/test/fixtures/ruff/RUF020.py vendored
View File

@@ -15,3 +15,10 @@ z: None | (Never | None)
a: int | Never | None
b: Never | Never | None
def func() -> (
Never # text
| # text
int
): ...

82
crates/ruff_linter/src/rules/ruff/rules/never_union.rs
View File

@@ -1,3 +1,4 @@
use ruff_diagnostics::Applicability;
use ruff_macros::{ViolationMetadata, derive_message_formats};
use ruff_python_ast::{self as ast, Expr, ExprBinOp, Operator};
use ruff_python_semantic::{SemanticModel, analyze::typing::traverse_union};
@@ -31,6 +32,9 @@ use crate::{Edit, Fix, FixAvailability, Violation};
/// def func() -> int: ...
/// ```
///
/// ## Fix safety
/// This rule's fix is marked as safe, unless the union type contains comments.
///
/// ## References
/// - [Python documentation: `typing.Never`](https://docs.python.org/3/library/typing.html#typing.Never)
/// - [Python documentation: `typing.NoReturn`](https://docs.python.org/3/library/typing.html#typing.NoReturn)
@@ -68,9 +72,15 @@ impl Violation for NeverUnion {
/// RUF020
pub(crate) fn never_union(checker: &Checker, expr: &Expr) {
let applicability = if checker.comment_ranges().intersects(expr.range()) {
Applicability::Unsafe
} else {
Applicability::Safe
};
match expr {
// Ex) `typing.NoReturn | int`
Expr::BinOp(ast::ExprBinOp {
Expr::BinOp(ExprBinOp {
op: Operator::BitOr,
left,
right,
@@ -92,10 +102,13 @@ pub(crate) fn never_union(checker: &Checker, expr: &Expr) {
// as `Union[None, None]` is valid Python.
// See https://github.com/astral-sh/ruff/issues/14567.
if !is_pep604_union_with_bare_none(checker.semantic()) {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
checker.locator().slice(right.as_ref()).to_string(),
expr.range(),
)));
diagnostic.set_fix(Fix::applicable_edit(
Edit::range_replacement(
checker.locator().slice(right.as_ref()).to_string(),
expr.range(),
),
applicability,
));
}
}
@@ -109,10 +122,13 @@ pub(crate) fn never_union(checker: &Checker, expr: &Expr) {
right.range(),
);
if !is_pep604_union_with_bare_none(checker.semantic()) {
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
checker.locator().slice(left.as_ref()).to_string(),
expr.range(),
)));
diagnostic.set_fix(Fix::applicable_edit(
Edit::range_replacement(
checker.locator().slice(left.as_ref()).to_string(),
expr.range(),
),
applicability,
));
}
}
}
@@ -151,30 +167,34 @@ pub(crate) fn never_union(checker: &Checker, expr: &Expr) {
},
elt.range(),
);
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
if let [only] = rest.as_slice() {
// Ex) `typing.Union[typing.NoReturn, int]` -> `int`
checker.locator().slice(only).to_string()
} else {
// Ex) `typing.Union[typing.NoReturn, int, str]` -> `typing.Union[int, str]`
checker
.generator()
.expr(&Expr::Subscript(ast::ExprSubscript {
value: value.clone(),
slice: Box::new(Expr::Tuple(ast::ExprTuple {
elts: rest,
diagnostic.set_fix(Fix::applicable_edit(
Edit::range_replacement(
if let [only] = rest.as_slice() {
// Ex) `typing.Union[typing.NoReturn, int]` -> `int`
checker.locator().slice(only).to_string()
} else {
// Ex) `typing.Union[typing.NoReturn, int, str]` -> `typing.Union[int, str]`
checker
.generator()
.expr(&Expr::Subscript(ast::ExprSubscript {
value: value.clone(),
slice: Box::new(Expr::Tuple(ast::ExprTuple {
elts: rest,
ctx: ast::ExprContext::Load,
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
parenthesized: true,
})),
ctx: ast::ExprContext::Load,
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
parenthesized: true,
})),
ctx: ast::ExprContext::Load,
range: TextRange::default(),
node_index: ruff_python_ast::AtomicNodeIndex::NONE,
}))
},
expr.range(),
)));
}))
},
expr.range(),
),
applicability,
));
}
}
}
@@ -200,7 +220,7 @@ enum NeverLike {
}
impl NeverLike {
fn from_expr(expr: &Expr, semantic: &ruff_python_semantic::SemanticModel) -> Option<Self> {
fn from_expr(expr: &Expr, semantic: &SemanticModel) -> Option<Self> {
let qualified_name = semantic.resolve_qualified_name(expr)?;
if semantic.match_typing_qualified_name(&qualified_name, "NoReturn") {
Some(NeverLike::NoReturn)

19
crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF020_RUF020.py.snap
View File

@@ -197,3 +197,22 @@ RUF020 `Never | T` is equivalent to `T`
| ^^^^^
|
help: Remove `Never`
RUF020 [*] `Never | T` is equivalent to `T`
--> RUF020.py:21:9
|
20 | def func() -> (
21 | Never # text
| ^^^^^
22 | | # text
23 | int
|
help: Remove `Never`
18 |
19 |
20 | def func() -> (
- Never # text
- | # text
21 | int
22 | ): ...
note: This is an unsafe fix and may change runtime behavior