mirrors/ruff
1
0
Fork 0
mirror of https://github.com/astral-sh/ruff.git synced 2025-10-01 14:21:24 +00:00
Code Issues Projects Releases Packages Wiki Activity Actions 5

Fix dropped union expressions for piped non-types in PYI055 autofix (#9161)

Browse source 
## Summary

Fix dropped union expressions for piped non-types in `PYI055` autofix

If you had `type[int] | type[str] | str`, it would have dropped the
`str`, which breaks the type!

Closes #9156 

## Test Plan

`cargo test`
This commit is contained in:
Steve C 2023-12-16 15:58:28 -05:00 • committed by GitHub
parent 0029b4fd07
commit 85b27a994f
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 198 additions and 53 deletions
Download patch file Download diff file Expand all files Collapse all files

25
crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py vendored
View file

@ -37,3 +37,28 @@ def func():
# PYI055
x: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker
def convert_union(union: UnionType) -> _T | None:
converters: tuple[
type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055
] = union.__args__
...
def convert_union(union: UnionType) -> _T | None:
converters: tuple[
Union[type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055
] = union.__args__
...
def convert_union(union: UnionType) -> _T | None:
converters: tuple[
Union[type[_T] | type[Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055
] = union.__args__
...
def convert_union(union: UnionType) -> _T | None:
converters: tuple[
Union[type[_T] | type[Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055
] = union.__args__
...

138
crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs
View file

@ -80,17 +80,24 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr)
}
let mut type_exprs = Vec::new();
let mut other_exprs = Vec::new();
let mut collect_type_exprs = |expr: &'a Expr, _| {
let Some(subscript) = expr.as_subscript_expr() else {
return;
};
if checker
.semantic()
.resolve_call_path(subscript.value.as_ref())
.is_some_and(|call_path| matches!(call_path.as_slice(), ["" | "builtins", "type"]))
{
type_exprs.push(&subscript.slice);
let subscript = expr.as_subscript_expr();
if subscript.is_none() {
other_exprs.push(expr);
} else {
let unwrapped = subscript.unwrap();
if checker
.semantic()
.resolve_call_path(unwrapped.value.as_ref())
.is_some_and(|call_path| matches!(call_path.as_slice(), ["" | "builtins", "type"]))
{
type_exprs.push(&unwrapped.slice);
} else {
other_exprs.push(expr);
}
}
};
@ -113,55 +120,82 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr)
if checker.semantic().is_builtin("type") {
let content = if let Some(subscript) = subscript {
checker
.generator()
.expr(&Expr::Subscript(ast::ExprSubscript {
value: Box::new(Expr::Name(ast::ExprName {
id: "type".into(),
ctx: ExprContext::Load,
range: TextRange::default(),
})),
slice: Box::new(Expr::Subscript(ast::ExprSubscript {
value: subscript.value.clone(),
slice: Box::new(Expr::Tuple(ast::ExprTuple {
elts: type_members
.into_iter()
.map(|type_member| {
Expr::Name(ast::ExprName {
id: type_member,
ctx: ExprContext::Load,
range: TextRange::default(),
})
})
.collect(),
ctx: ExprContext::Load,
range: TextRange::default(),
})),
ctx: ExprContext::Load,
range: TextRange::default(),
})),
let types = &Expr::Subscript(ast::ExprSubscript {
value: Box::new(Expr::Name(ast::ExprName {
id: "type".into(),
ctx: ExprContext::Load,
range: TextRange::default(),
}))
} else {
checker
.generator()
.expr(&Expr::Subscript(ast::ExprSubscript {
value: Box::new(Expr::Name(ast::ExprName {
id: "type".into(),
ctx: ExprContext::Load,
range: TextRange::default(),
})),
slice: Box::new(concatenate_bin_ors(
type_exprs
.clone()
})),
slice: Box::new(Expr::Subscript(ast::ExprSubscript {
value: subscript.value.clone(),
slice: Box::new(Expr::Tuple(ast::ExprTuple {
elts: type_members
.into_iter()
.map(std::convert::AsRef::as_ref)
.map(|type_member| {
Expr::Name(ast::ExprName {
id: type_member,
ctx: ExprContext::Load,
range: TextRange::default(),
})
})
.collect(),
)),
ctx: ExprContext::Load,
range: TextRange::default(),
})),
ctx: ExprContext::Load,
range: TextRange::default(),
}))
})),
ctx: ExprContext::Load,
range: TextRange::default(),
});
if other_exprs.is_empty() {
checker.generator().expr(types)
} else {
let mut exprs = Vec::new();
exprs.push(types);
exprs.extend(other_exprs);
let union = Expr::Subscript(ast::ExprSubscript {
value: subscript.value.clone(),
slice: Box::new(Expr::Tuple(ast::ExprTuple {
elts: exprs.into_iter().cloned().collect(),
ctx: ExprContext::Load,
range: TextRange::default(),
})),
ctx: ExprContext::Load,
range: TextRange::default(),
});
checker.generator().expr(&union)
}
} else {
let types = &Expr::Subscript(ast::ExprSubscript {
value: Box::new(Expr::Name(ast::ExprName {
id: "type".into(),
ctx: ExprContext::Load,
range: TextRange::default(),
})),
slice: Box::new(concatenate_bin_ors(
type_exprs
.clone()
.into_iter()
.map(std::convert::AsRef::as_ref)
.collect(),
)),
ctx: ExprContext::Load,
range: TextRange::default(),
});
if other_exprs.is_empty() {
checker.generator().expr(types)
} else {
let mut exprs = Vec::new();
exprs.push(types);
exprs.extend(other_exprs);
checker.generator().expr(&concatenate_bin_ors(exprs))
}
};
diagnostic.set_fix(Fix::safe_edit(Edit::range_replacement(

86
crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap
View file

@ -54,5 +54,91 @@ PYI055.py:39:8: PYI055 [*] Multiple `type` members in a union. Combine them into
38 38 | # PYI055
39 |- x: Union[type[requests_mock.Mocker], type[httpretty], type[str]] = requests_mock.Mocker
39 |+ x: type[Union[requests_mock.Mocker, httpretty, str]] = requests_mock.Mocker
40 40 |
41 41 |
42 42 | def convert_union(union: UnionType) -> _T | None:
PYI055.py:44:9: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`.
|
42 | def convert_union(union: UnionType) -> _T | None:
43 | converters: tuple[
44 | type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055
45 | ] = union.__args__
46 | ...
|
= help: Combine multiple `type` members
ℹ Safe fix
41 41 |
42 42 | def convert_union(union: UnionType) -> _T | None:
43 43 | converters: tuple[
44 |- type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055
44 |+ type[_T | Converter[_T]] | Converter[_T] | Callable[[str], _T], ... # PYI055
45 45 | ] = union.__args__
46 46 | ...
47 47 |
PYI055.py:50:15: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`.
|
48 | def convert_union(union: UnionType) -> _T | None:
49 | converters: tuple[
50 | Union[type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055
51 | ] = union.__args__
52 | ...
|
= help: Combine multiple `type` members
ℹ Safe fix
47 47 |
48 48 | def convert_union(union: UnionType) -> _T | None:
49 49 | converters: tuple[
50 |- Union[type[_T] | type[Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055
50 |+ Union[type[_T | Converter[_T]] | Converter[_T] | Callable[[str], _T]], ... # PYI055
51 51 | ] = union.__args__
52 52 | ...
53 53 |
PYI055.py:56:15: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`.
|
54 | def convert_union(union: UnionType) -> _T | None:
55 | converters: tuple[
56 | Union[type[_T] | type[Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055
57 | ] = union.__args__
58 | ...
|
= help: Combine multiple `type` members
ℹ Safe fix
53 53 |
54 54 | def convert_union(union: UnionType) -> _T | None:
55 55 | converters: tuple[
56 |- Union[type[_T] | type[Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055
56 |+ Union[type[_T | Converter[_T]]] | Converter[_T] | Callable[[str], _T], ... # PYI055
57 57 | ] = union.__args__
58 58 | ...
59 59 |
PYI055.py:62:15: PYI055 [*] Multiple `type` members in a union. Combine them into one, e.g., `type[_T | Converter[_T]]`.
|
60 | def convert_union(union: UnionType) -> _T | None:
61 | converters: tuple[
62 | Union[type[_T] | type[Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ PYI055
63 | ] = union.__args__
64 | ...
|
= help: Combine multiple `type` members
ℹ Safe fix
59 59 |
60 60 | def convert_union(union: UnionType) -> _T | None:
61 61 | converters: tuple[
62 |- Union[type[_T] | type[Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055
62 |+ Union[type[_T | Converter[_T]] | str] | Converter[_T] | Callable[[str], _T], ... # PYI055
63 63 | ] = union.__args__
64 64 | ...

2
crates/ruff_linter/src/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap
View file

@ -120,7 +120,7 @@ PYI055.pyi:10:15: PYI055 [*] Multiple `type` members in a union. Combine them in
8 8 | z: Union[type[float, int], type[complex]]
9 9 |
10 |-def func(arg: type[int] | str | type[float]) -> None: ...
10 |+def func(arg: type[int | float]) -> None: ...
10 |+def func(arg: type[int | float] | str) -> None: ...
11 11 |
12 12 | # OK
13 13 | x: type[int, str, float]