Skip to content

Commit

Permalink
Change in analyze instead
Browse files Browse the repository at this point in the history
  • Loading branch information
charliermarsh committed Jul 13, 2023
1 parent 8f9ab62 commit a976d66
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 35 deletions.
3 changes: 2 additions & 1 deletion crates/ruff/src/checkers/ast/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2152,7 +2152,8 @@ where
Rule::FutureRewritableTypeAnnotation,
Rule::NonPEP604Annotation,
]) {
if let Some(operator) = typing::to_pep604_operator(value, &self.semantic) {
if let Some(operator) = typing::to_pep604_operator(value, slice, &self.semantic)
{
if self.enabled(Rule::FutureRewritableTypeAnnotation) {
if !self.is_stub
&& self.settings.target_version < PythonVersion::Py310
Expand Down
34 changes: 1 addition & 33 deletions crates/ruff/src/rules/pyupgrade/rules/use_pep604_annotation.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use itertools::Either::{Left, Right};
use itertools::Itertools;
use rustpython_parser::ast::{self, Constant, Expr, Ranged};
use rustpython_parser::ast::{self, Expr, Ranged};

use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation};
use ruff_macros::{derive_message_formats, violation};
Expand Down Expand Up @@ -57,26 +57,6 @@ pub(crate) fn use_pep604_annotation(
slice: &Expr,
operator: Pep604Operator,
) {
// 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 checker.semantic().execution_context().is_runtime() {
return;
}
}

// Avoid fixing forward references, or types not in an annotation.
let fixable = checker.semantic().in_type_definition()
&& !checker.semantic().in_complex_string_type_definition();
Expand Down Expand Up @@ -119,18 +99,6 @@ pub(crate) fn use_pep604_annotation(
}
}

/// Returns `true` if any argument in the slice is a forward reference (i.e., 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(quoted_annotation),
_ => false,
}
}

/// Format the expression as a PEP 604-style optional.
fn optional(expr: &Expr, locator: &Locator) -> String {
format!("{} | None", locator.slice(expr.range()))
Expand Down
38 changes: 37 additions & 1 deletion crates/ruff_python_semantic/src/analyze/typing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,43 @@ pub enum Pep604Operator {
}

/// Return the PEP 604 operator variant to which the given subscript [`Expr`] corresponds, if any.
pub fn to_pep604_operator(value: &Expr, semantic: &SemanticModel) -> Option<Pep604Operator> {
pub fn to_pep604_operator(
value: &Expr,
slice: &Expr,
semantic: &SemanticModel,
) -> Option<Pep604Operator> {
/// 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(quoted_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.
//
// 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
.resolve_call_path(value)
.as_ref()
Expand Down

0 comments on commit a976d66

Please sign in to comment.