Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use generator for UP007 autofix #7137

Merged
merged 1 commit into from
Sep 5, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP007.py
Original file line number Diff line number Diff line change
Expand Up @@ -95,3 +95,11 @@ def f(x: Optional[str, int : float]) -> None:

def f(x: Optional[int, float]) -> None:
...


# Regression test for: https://github.com/astral-sh/ruff/issues/7131
class ServiceRefOrValue:
service_specification: Optional[
list[ServiceSpecificationRef]
| list[ServiceSpecification]
] = None
53 changes: 31 additions & 22 deletions crates/ruff/src/rules/pyupgrade/rules/use_pep604_annotation.rs
Original file line number Diff line number Diff line change
@@ -1,12 +1,8 @@
use itertools::Either::{Left, Right};
use itertools::Itertools;
use ruff_python_ast::{self as ast, Expr};

use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
use ruff_python_ast::{self as ast, Constant, Expr, ExprContext, Operator};
use ruff_python_semantic::analyze::typing::Pep604Operator;
use ruff_source_file::Locator;
use ruff_text_size::Ranged;
use ruff_text_size::{Ranged, TextRange};

use crate::checkers::ast::Checker;
use crate::registry::AsRule;
Expand Down Expand Up @@ -83,7 +79,7 @@ pub(crate) fn use_pep604_annotation(
}
_ => {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
optional(slice, checker.locator()),
checker.generator().expr(&optional(slice)),
expr.range(),
)));
}
Expand All @@ -100,7 +96,7 @@ pub(crate) fn use_pep604_annotation(
}
Expr::Tuple(ast::ExprTuple { elts, .. }) => {
diagnostic.set_fix(Fix::suggested(Edit::range_replacement(
union(elts, checker.locator()),
checker.generator().expr(&union(elts)),
expr.range(),
)));
}
Expand All @@ -119,23 +115,36 @@ pub(crate) fn use_pep604_annotation(
}

/// Format the expression as a PEP 604-style optional.
fn optional(expr: &Expr, locator: &Locator) -> String {
format!("{} | None", locator.slice(expr))
fn optional(expr: &Expr) -> Expr {
ast::ExprBinOp {
left: Box::new(expr.clone()),
op: Operator::BitOr,
right: Box::new(Expr::Constant(ast::ExprConstant {
value: Constant::None,
kind: None,
range: TextRange::default(),
})),
range: TextRange::default(),
}
.into()
}

/// Format the expressions as a PEP 604-style union.
fn union(elts: &[Expr], locator: &Locator) -> String {
let mut elts = elts
.iter()
.flat_map(|expr| match expr {
Expr::Tuple(ast::ExprTuple { elts, .. }) => Left(elts.iter()),
_ => Right(std::iter::once(expr)),
})
.peekable();
if elts.peek().is_none() {
"()".to_string()
} else {
elts.map(|expr| locator.slice(expr)).join(" | ")
fn union(elts: &[Expr]) -> Expr {
match elts {
[] => Expr::Tuple(ast::ExprTuple {
elts: vec![],
ctx: ExprContext::Load,
range: TextRange::default(),
}),
[Expr::Tuple(ast::ExprTuple { elts, .. })] => union(elts),
[elt] => elt.clone(),
[elt, rest @ ..] => Expr::BinOp(ast::ExprBinOp {
left: Box::new(union(&[elt.clone()])),
op: Operator::BitOr,
right: Box::new(union(rest)),
range: TextRange::default(),
}),
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ UP007.py:14:10: UP007 [*] Use `X | Y` for type annotations
12 12 |
13 13 |
14 |-def f(x: Union[str, int, Union[float, bytes]]) -> None:
14 |+def f(x: str | int | Union[float, bytes]) -> None:
14 |+def f(x: str | (int | Union[float, bytes])) -> None:
15 15 | ...
16 16 |
17 17 |
Expand Down Expand Up @@ -176,7 +176,7 @@ UP007.py:38:11: UP007 [*] Use `X | Y` for type annotations
36 36 |
37 37 |
38 |-def f(x: "Union[str, int, Union[float, bytes]]") -> None:
38 |+def f(x: "str | int | Union[float, bytes]") -> None:
38 |+def f(x: "str | (int | Union[float, bytes])") -> None:
39 39 | ...
40 40 |
41 41 |
Expand Down Expand Up @@ -369,4 +369,27 @@ UP007.py:96:10: UP007 Use `X | Y` for type annotations
|
= help: Convert to `X | Y`

UP007.py:102:28: UP007 [*] Use `X | Y` for type annotations
|
100 | # Regression test for: https://github.com/astral-sh/ruff/issues/7131
101 | class ServiceRefOrValue:
102 | service_specification: Optional[
| ____________________________^
103 | | list[ServiceSpecificationRef]
104 | | | list[ServiceSpecification]
105 | | ] = None
| |_____^ UP007
|
= help: Convert to `X | Y`

ℹ Suggested fix
99 99 |
100 100 | # Regression test for: https://github.com/astral-sh/ruff/issues/7131
101 101 | class ServiceRefOrValue:
102 |- service_specification: Optional[
103 |- list[ServiceSpecificationRef]
104 |- | list[ServiceSpecification]
105 |- ] = None
102 |+ service_specification: list[ServiceSpecificationRef] | list[ServiceSpecification] | None = None


Loading