Cyclic0007/ruff
1
0
Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Add a fix for never-union (#9218)

Browse Source 
## Summary

Enables us to rewrite `Never | int` as `int`.
This commit is contained in:
Charlie Marsh
2023-12-21 16:01:09 -05:00
committed by GitHub
parent a9ceef5b5d
commit b0ae1199e8
4 changed files with 217 additions and 54 deletions
Download Patch File Download Diff File Expand all files Collapse all files

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

@@ -5,3 +5,4 @@ Union[NoReturn, int]
Never | int
NoReturn | int
Union[Union[Never, int], Union[NoReturn, int]]
Union[NoReturn, int, float]

14
crates/ruff_linter/src/checkers/ast/analyze/expression.rs
View File

@@ -98,12 +98,13 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::UnnecessaryTypeUnion) {
flake8_pyi::rules::unnecessary_type_union(checker, expr);
}
if checker.enabled(Rule::NeverUnion) {
ruff::rules::never_union(checker, expr);
}
}
}
if checker.enabled(Rule::NeverUnion) {
ruff::rules::never_union(checker, expr);
}
if checker.any_enabled(&[
Rule::SysVersionSlice3,
Rule::SysVersion2,
@@ -1158,6 +1159,10 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
}
}
if checker.enabled(Rule::NeverUnion) {
ruff::rules::never_union(checker, expr);
}
// Avoid duplicate checks if the parent is a union, since these rules already
// traverse nested unions.
if !checker.semantic.in_nested_union() {
@@ -1178,9 +1183,6 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) {
if checker.enabled(Rule::RuntimeStringUnion) {
flake8_type_checking::rules::runtime_string_union(checker, expr);
}
if checker.enabled(Rule::NeverUnion) {
ruff::rules::never_union(checker, expr);
}
}
}
Expr::UnaryOp(

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

@@ -1,8 +1,7 @@
use ruff_diagnostics::{Diagnostic, Violation};
use ruff_diagnostics::{AlwaysFixableViolation, Diagnostic, Edit, Fix};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::Expr;
use ruff_python_semantic::analyze::typing::traverse_union;
use ruff_text_size::Ranged;
use ruff_python_ast::{self as ast, Expr, Operator};
use ruff_text_size::{Ranged, TextRange};
use crate::checkers::ast::Checker;
@@ -40,7 +39,7 @@ pub struct NeverUnion {
union_like: UnionLike,
}
impl Violation for NeverUnion {
impl AlwaysFixableViolation for NeverUnion {
#[derive_message_formats]
fn message(&self) -> String {
let Self {
@@ -56,52 +55,121 @@ impl Violation for NeverUnion {
}
}
}
fn fix_title(&self) -> String {
let Self { never_like, .. } = self;
format!("Remove `{never_like}`")
}
}
/// RUF020
pub(crate) fn never_union<'a>(checker: &mut Checker, expr: &'a Expr) {
let mut expressions: Vec<(NeverLike, UnionLike, &'a Expr)> = Vec::new();
let mut rest: Vec<&'a Expr> = Vec::new();
pub(crate) fn never_union(checker: &mut Checker, expr: &Expr) {
match expr {
// Ex) `typing.NoReturn | int`
Expr::BinOp(ast::ExprBinOp {
op: Operator::BitOr,
left,
right,
range: _,
}) => {
// Analyze the left-hand side of the `|` operator.
if let Some(never_like) = NeverLike::from_expr(left, checker.semantic()) {
let mut diagnostic = Diagnostic::new(
NeverUnion {
never_like,
union_like: UnionLike::BinOp,
},
left.range(),
);
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
checker.locator().slice(right.as_ref()).to_string(),
expr.range(),
)));
checker.diagnostics.push(diagnostic);
}
// Find all `typing.Never` and `typing.NoReturn` expressions.
let semantic = checker.semantic();
let mut collect_never = |expr: &'a Expr, parent: Option<&'a Expr>| {
if let Some(call_path) = semantic.resolve_call_path(expr) {
let union_like = if parent.is_some_and(Expr::is_bin_op_expr) {
UnionLike::BinOp
} else {
UnionLike::TypingUnion
};
let never_like = if semantic.match_typing_call_path(&call_path, "NoReturn") {
Some(NeverLike::NoReturn)
} else if semantic.match_typing_call_path(&call_path, "Never") {
Some(NeverLike::Never)
} else {
None
};
if let Some(never_like) = never_like {
expressions.push((never_like, union_like, expr));
return;
// Analyze the right-hand side of the `|` operator.
if let Some(never_like) = NeverLike::from_expr(right, checker.semantic()) {
let mut diagnostic = Diagnostic::new(
NeverUnion {
never_like,
union_like: UnionLike::BinOp,
},
right.range(),
);
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(
checker.locator().slice(left.as_ref()).to_string(),
expr.range(),
)));
checker.diagnostics.push(diagnostic);
}
}
rest.push(expr);
};
// Ex) `typing.Union[typing.NoReturn, int]`
Expr::Subscript(ast::ExprSubscript {
value,
slice,
ctx: _,
range: _,
}) if checker.semantic().match_typing_expr(value, "Union") => {
let Expr::Tuple(ast::ExprTuple {
elts,
ctx: _,
range: _,
}) = slice.as_ref()
else {
return;
};
traverse_union(&mut collect_never, checker.semantic(), expr, None);
// Analyze each element of the `Union`.
for elt in elts {
if let Some(never_like) = NeverLike::from_expr(elt, checker.semantic()) {
// Collect the other elements of the `Union`.
let rest = elts
.iter()
.filter(|other| *other != elt)
.cloned()
.collect::<Vec<_>>();
// Create a diagnostic for each `typing.Never` and `typing.NoReturn` expression.
for (never_like, union_like, child) in expressions {
let diagnostic = Diagnostic::new(
NeverUnion {
never_like,
union_like,
},
child.range(),
);
checker.diagnostics.push(diagnostic);
// Ignore, e.g., `typing.Union[typing.NoReturn]`.
if rest.is_empty() {
return;
}
let mut diagnostic = Diagnostic::new(
NeverUnion {
never_like,
union_like: UnionLike::TypingUnion,
},
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,
ctx: ast::ExprContext::Load,
range: TextRange::default(),
})),
ctx: ast::ExprContext::Load,
range: TextRange::default(),
}))
},
expr.range(),
)));
checker.diagnostics.push(diagnostic);
}
}
}
_ => {}
}
}
@@ -121,6 +189,19 @@ enum NeverLike {
Never,
}
impl NeverLike {
fn from_expr(expr: &Expr, semantic: &ruff_python_semantic::SemanticModel) -> Option<Self> {
let call_path = semantic.resolve_call_path(expr)?;
if semantic.match_typing_call_path(&call_path, "NoReturn") {
Some(NeverLike::NoReturn)
} else if semantic.match_typing_call_path(&call_path, "Never") {
Some(NeverLike::Never)
} else {
None
}
}
}
impl std::fmt::Display for NeverLike {
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
match self {

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

@@ -1,7 +1,7 @@
---
source: crates/ruff_linter/src/rules/ruff/mod.rs
---
RUF020.py:3:7: RUF020 `Union[Never, T]` is equivalent to `T`
RUF020.py:3:7: RUF020 [*] `Union[Never, T]` is equivalent to `T`
|
1 | from typing import Never, NoReturn, Union
2 |
@@ -10,8 +10,18 @@ RUF020.py:3:7: RUF020 `Union[Never, T]` is equivalent to `T`
4 | Union[NoReturn, int]
5 | Never | int
|
= help: Remove `Never`
RUF020.py:4:7: RUF020 `Union[NoReturn, T]` is equivalent to `T`
ℹ Safe fix
1 1 | from typing import Never, NoReturn, Union
2 2 |
3 |-Union[Never, int]
3 |+int
4 4 | Union[NoReturn, int]
5 5 | Never | int
6 6 | NoReturn | int
RUF020.py:4:7: RUF020 [*] `Union[NoReturn, T]` is equivalent to `T`
|
3 | Union[Never, int]
4 | Union[NoReturn, int]
@@ -19,8 +29,19 @@ RUF020.py:4:7: RUF020 `Union[NoReturn, T]` is equivalent to `T`
5 | Never | int
6 | NoReturn | int
|
= help: Remove `NoReturn`
RUF020.py:5:1: RUF020 `Never | T` is equivalent to `T`
ℹ Safe fix
1 1 | from typing import Never, NoReturn, Union
2 2 |
3 3 | Union[Never, int]
4 |-Union[NoReturn, int]
4 |+int
5 5 | Never | int
6 6 | NoReturn | int
7 7 | Union[Union[Never, int], Union[NoReturn, int]]
RUF020.py:5:1: RUF020 [*] `Never | T` is equivalent to `T`
|
3 | Union[Never, int]
4 | Union[NoReturn, int]
@@ -29,30 +50,88 @@ RUF020.py:5:1: RUF020 `Never | T` is equivalent to `T`
6 | NoReturn | int
7 | Union[Union[Never, int], Union[NoReturn, int]]
|
= help: Remove `Never`
RUF020.py:6:1: RUF020 `NoReturn | T` is equivalent to `T`
ℹ Safe fix
2 2 |
3 3 | Union[Never, int]
4 4 | Union[NoReturn, int]
5 |-Never | int
5 |+int
6 6 | NoReturn | int
7 7 | Union[Union[Never, int], Union[NoReturn, int]]
8 8 | Union[NoReturn, int, float]
RUF020.py:6:1: RUF020 [*] `NoReturn | T` is equivalent to `T`
|
4 | Union[NoReturn, int]
5 | Never | int
6 | NoReturn | int
| ^^^^^^^^ RUF020
7 | Union[Union[Never, int], Union[NoReturn, int]]
8 | Union[NoReturn, int, float]
|
= help: Remove `NoReturn`
RUF020.py:7:13: RUF020 `Union[Never, T]` is equivalent to `T`
ℹ Safe fix
3 3 | Union[Never, int]
4 4 | Union[NoReturn, int]
5 5 | Never | int
6 |-NoReturn | int
6 |+int
7 7 | Union[Union[Never, int], Union[NoReturn, int]]
8 8 | Union[NoReturn, int, float]
RUF020.py:7:13: RUF020 [*] `Union[Never, T]` is equivalent to `T`
|
5 | Never | int
6 | NoReturn | int
7 | Union[Union[Never, int], Union[NoReturn, int]]
| ^^^^^ RUF020
8 | Union[NoReturn, int, float]
|
= help: Remove `Never`
RUF020.py:7:32: RUF020 `Union[NoReturn, T]` is equivalent to `T`
ℹ Safe fix
4 4 | Union[NoReturn, int]
5 5 | Never | int
6 6 | NoReturn | int
7 |-Union[Union[Never, int], Union[NoReturn, int]]
7 |+Union[int, Union[NoReturn, int]]
8 8 | Union[NoReturn, int, float]
RUF020.py:7:32: RUF020 [*] `Union[NoReturn, T]` is equivalent to `T`
|
5 | Never | int
6 | NoReturn | int
7 | Union[Union[Never, int], Union[NoReturn, int]]
| ^^^^^^^^ RUF020
8 | Union[NoReturn, int, float]
|
= help: Remove `NoReturn`
ℹ Safe fix
4 4 | Union[NoReturn, int]
5 5 | Never | int
6 6 | NoReturn | int
7 |-Union[Union[Never, int], Union[NoReturn, int]]
7 |+Union[Union[Never, int], int]
8 8 | Union[NoReturn, int, float]
RUF020.py:8:7: RUF020 [*] `Union[NoReturn, T]` is equivalent to `T`
|
6 | NoReturn | int
7 | Union[Union[Never, int], Union[NoReturn, int]]
8 | Union[NoReturn, int, float]
| ^^^^^^^^ RUF020
|
= help: Remove `NoReturn`
ℹ Safe fix
5 5 | Never | int
6 6 | NoReturn | int
7 7 | Union[Union[Never, int], Union[NoReturn, int]]
8 |-Union[NoReturn, int, float]
8 |+Union[int, float]