Skip to content

Commit

Permalink
additional tweaks
Browse files Browse the repository at this point in the history
  • Loading branch information
@diceroll123
diceroll123 committed Dec 16, 2023
1 parent 25d5ffa commit e88cd07
Show file tree
Hide file tree
Showing 4 changed files with 83 additions and 33 deletions.
6 changes: 6 additions & 0 deletions crates/ruff_linter/resources/test/fixtures/flake8_pyi/PYI055.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -56,3 +56,9 @@ def convert_union(union: UnionType) -> _T | None:
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__
...
87 changes: 55 additions & 32 deletions crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -83,17 +83,21 @@ pub(crate) fn unnecessary_type_union<'a>(checker: &mut Checker, union: &'a Expr)
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);
} else {
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);
}
}
};

Expand All @@ -116,36 +120,55 @@ 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(),
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(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(),
})),
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(),
})),
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 {
Expand Down
21 changes: 21 additions & 0 deletions ...c/rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.py.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -119,5 +119,26 @@ PYI055.py:56:15: PYI055 [*] Multiple `type` members in a union. Combine them int
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 changes: 1 addition & 1 deletion .../rules/flake8_pyi/snapshots/ruff_linter__rules__flake8_pyi__tests__PYI055_PYI055.pyi.snap
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down

0 comments on commit e88cd07

Please sign in to comment.