Skip to content

Commit

Permalink
Extend PEP 604 rewrites to support some quoted annotations (#5725)
Browse files Browse the repository at this point in the history
## Summary

Python doesn't allow `"Foo" | None` if the annotation will be evaluated
at runtime (see the comments in the PR, or the semantic model
documentation for more on what this means and when it is true), but it
_does_ allow it if the annotation is typing-only.

This, for example, is invalid, as Python will evaluate `"Foo" | None` at
runtime in order to
populate the function's `__annotations__`:

```python
def f(x: "Foo" | None): ...
```

This, however, is valid:

```python
def f():
    x: "Foo" | None
```

As is this:

```python
from __future__ import annotations

def f(x: "Foo" | None): ...
```

Closes #5706.
  • Loading branch information
charliermarsh committed Jul 13, 2023
1 parent 549173b commit 932c9a4
Show file tree
Hide file tree
Showing 3 changed files with 38 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@ pub(crate) fn use_pep604_annotation(
// Avoid fixing forward references, or types not in an annotation.
let fixable = checker.semantic().in_type_definition()
&& !checker.semantic().in_complex_string_type_definition();

match operator {
Pep604Operator::Optional => {
let mut diagnostic = Diagnostic::new(NonPEP604Annotation, expr.range());
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,4 +276,20 @@ UP007.py:60:8: UP007 [*] Use `X | Y` for type annotations
60 |+ x: str | int
61 61 | x: Union["str", "int"]

UP007.py:61:8: UP007 [*] Use `X | Y` for type annotations
|
59 | x = Union["str", "int"]
60 | x: Union[str, int]
61 | x: Union["str", "int"]
| ^^^^^^^^^^^^^^^^^^^ UP007
|
= help: Convert to `X | Y`

Suggested fix
58 58 | x = Union[str, int]
59 59 | x = Union["str", "int"]
60 60 | x: Union[str, int]
61 |- x: Union["str", "int"]
61 |+ x: "str" | "int"


28 changes: 21 additions & 7 deletions crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -127,22 +127,36 @@ pub fn to_pep604_operator(
slice: &Expr,
semantic: &SemanticModel,
) -> Option<Pep604Operator> {
/// Returns `true` if any argument in the slice is a string.
fn any_arg_is_str(slice: &Expr) -> bool {
/// Returns `true` if any argument in the slice is a quoted annotation).
fn quoted_annotation(slice: &Expr) -> bool {
match slice {
Expr::Constant(ast::ExprConstant {
value: Constant::Str(_),
..
}) => true,
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().any(any_arg_is_str),
Expr::Tuple(ast::ExprTuple { elts, .. }) => elts.iter().any(quoted_annotation),
_ => false,
}
}

// If any of the _arguments_ are forward references, we can't use PEP 604.
// Ex) `Union["str", "int"]` can't be converted to `"str" | "int"`.
if any_arg_is_str(slice) {
return None;
// If the slice is a forward reference (e.g., `Optional["Foo"]`), it can only be rewritten
// if we're in a typing-only context.
//
// This, for example, is invalid, as Python will evaluate `"Foo" | None` at runtime in order to
// populate the function's `__annotations__`:
// ```python
// def f(x: "Foo" | None): ...
// ```
//
// This, however, is valid:
// ```python
// def f():
// x: "Foo" | None
// ```
if quoted_annotation(slice) {
if semantic.execution_context().is_runtime() {
return None;
}
}

semantic
Expand Down

0 comments on commit 932c9a4

Please sign in to comment.