Skip to content

Commit

Permalink
Avoid flagging starred expressions in UP007 (#7505)
Browse files Browse the repository at this point in the history
## Summary

These can't be fixed, because fixing them would lead to invalid syntax.
So flagging them also feels misleading.

Closes #7452.
  • Loading branch information
charliermarsh authored Sep 19, 2023
1 parent 4123d07 commit 28b48ab
Show file tree
Hide file tree
Showing 3 changed files with 27 additions and 1 deletion.
6 changes: 6 additions & 0 deletions crates/ruff/resources/test/fixtures/pyupgrade/UP007.py
Original file line number Diff line number Diff line change
Expand Up @@ -108,3 +108,9 @@ class ServiceRefOrValue:
# Regression test for: https://github.com/astral-sh/ruff/issues/7201
class ServiceRefOrValue:
service_specification: Optional[str]is not True = None


# Regression test for: https://github.com/astral-sh/ruff/issues/7452
class Collection(Protocol[*_B0]):
def __iter__(self) -> Iterator[Union[*_B0]]:
...
Original file line number Diff line number Diff line change
Expand Up @@ -410,5 +410,8 @@ UP007.py:110:28: UP007 [*] Use `X | Y` for type annotations
109 109 | class ServiceRefOrValue:
110 |- service_specification: Optional[str]is not True = None
110 |+ service_specification: str | None is not True = None
111 111 |
112 112 |
113 113 | # Regression test for: https://github.com/astral-sh/ruff/issues/7452


19 changes: 18 additions & 1 deletion crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ pub fn to_pep604_operator(
slice: &Expr,
semantic: &SemanticModel,
) -> Option<Pep604Operator> {
/// Returns `true` if any argument in the slice is a quoted annotation).
/// Returns `true` if any argument in the slice is a quoted annotation.
fn quoted_annotation(slice: &Expr) -> bool {
match slice {
Expr::Constant(ast::ExprConstant {
Expand All @@ -155,6 +155,15 @@ pub fn to_pep604_operator(
}
}

/// Returns `true` if any argument in the slice is a starred expression.
fn starred_annotation(slice: &Expr) -> bool {
match slice {
Expr::Starred(_) => true,
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().any(starred_annotation),
_ => false,
}
}

// If the slice is a forward reference (e.g., `Optional["Foo"]`), it can only be rewritten
// if we're in a typing-only context.
//
Expand All @@ -175,6 +184,14 @@ pub fn to_pep604_operator(
}
}

// If any of the elements are starred expressions, we can't rewrite the subscript:
// ```python
// def f(x: Union[*int, str]): ...
// ```
if starred_annotation(slice) {
return None;
}

semantic
.resolve_call_path(value)
.as_ref()
Expand Down

0 comments on commit 28b48ab

Please sign in to comment.