From cac256bd820931de5a4e6e2f2b38184b1a1309f4 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Wed, 20 Dec 2023 13:35:08 -0500 Subject: [PATCH] Add rule for NoReturn --- Cargo.lock | 8 +- .../resources/test/fixtures/ruff/RUF020.py | 7 + .../src/checkers/ast/analyze/expression.rs | 7 + crates/ruff_linter/src/codes.rs | 1 + .../src/rules/flake8_pyi/helpers.rs | 54 -------- .../ruff_linter/src/rules/flake8_pyi/mod.rs | 1 - .../rules/duplicate_union_member.rs | 9 +- .../rules/redundant_literal_union.rs | 6 +- .../rules/redundant_numeric_union.rs | 5 +- .../rules/unnecessary_literal_union.rs | 3 +- .../rules/unnecessary_type_union.rs | 3 +- crates/ruff_linter/src/rules/ruff/mod.rs | 1 + .../ruff_linter/src/rules/ruff/rules/mod.rs | 4 +- .../src/rules/ruff/rules/never_union.rs | 131 ++++++++++++++++++ ..._rules__ruff__tests__RUF020_RUF020.py.snap | 58 ++++++++ crates/ruff_python_ast/src/helpers.rs | 12 +- .../src/analyze/typing.rs | 52 +++++++ ruff.schema.json | 2 + 18 files changed, 287 insertions(+), 77 deletions(-) create mode 100644 crates/ruff_linter/resources/test/fixtures/ruff/RUF020.py delete mode 100644 crates/ruff_linter/src/rules/flake8_pyi/helpers.rs create mode 100644 crates/ruff_linter/src/rules/ruff/rules/never_union.rs create mode 100644 crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF020_RUF020.py.snap diff --git a/Cargo.lock b/Cargo.lock index 87a42ff98d0d9..0fa44b1eac549 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -809,7 +809,7 @@ checksum = "0ce7134b9999ecaf8bcd65542e436736ef32ddca1b3e06094cb6ec5755203b80" [[package]] name = "flake8-to-ruff" -version = "0.1.8" +version = "0.1.9" dependencies = [ "anyhow", "clap", @@ -2063,7 +2063,7 @@ dependencies = [ [[package]] name = "ruff_cli" -version = "0.1.8" +version = "0.1.9" dependencies = [ "annotate-snippets 0.9.2", "anyhow", @@ -2199,7 +2199,7 @@ dependencies = [ [[package]] name = "ruff_linter" -version = "0.1.8" +version = "0.1.9" dependencies = [ "aho-corasick", "annotate-snippets 0.9.2", @@ -2452,7 +2452,7 @@ dependencies = [ [[package]] name = "ruff_shrinking" -version = "0.1.8" +version = "0.1.9" dependencies = [ "anyhow", "clap", diff --git a/crates/ruff_linter/resources/test/fixtures/ruff/RUF020.py b/crates/ruff_linter/resources/test/fixtures/ruff/RUF020.py new file mode 100644 index 0000000000000..3775afa8ec471 --- /dev/null +++ b/crates/ruff_linter/resources/test/fixtures/ruff/RUF020.py @@ -0,0 +1,7 @@ +from typing import Never, NoReturn, Union + +Union[Never, int] +Union[NoReturn, int] +Never | int +NoReturn | int +Union[Union[Never, int], Union[NoReturn, int]] diff --git a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs index c8b602939d352..fbde5c58803b3 100644 --- a/crates/ruff_linter/src/checkers/ast/analyze/expression.rs +++ b/crates/ruff_linter/src/checkers/ast/analyze/expression.rs @@ -81,6 +81,7 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { Rule::DuplicateUnionMember, Rule::RedundantLiteralUnion, Rule::UnnecessaryTypeUnion, + Rule::NeverUnion, ]) { // Avoid duplicate checks if the parent is a union, since these rules already // traverse nested unions. @@ -97,6 +98,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::UnnecessaryTypeUnion) { flake8_pyi::rules::unnecessary_type_union(checker, expr); } + if checker.enabled(Rule::NeverUnion) { + ruff::rules::never_union(checker, expr); + } } } @@ -1174,6 +1178,9 @@ pub(crate) fn expression(expr: &Expr, checker: &mut Checker) { if checker.enabled(Rule::RuntimeStringUnion) { flake8_type_checking::rules::runtime_string_union(checker, expr); } + if checker.enabled(Rule::NeverUnion) { + ruff::rules::never_union(checker, expr); + } } } Expr::UnaryOp( diff --git a/crates/ruff_linter/src/codes.rs b/crates/ruff_linter/src/codes.rs index 158d60d4a3f63..550960ce275af 100644 --- a/crates/ruff_linter/src/codes.rs +++ b/crates/ruff_linter/src/codes.rs @@ -901,6 +901,7 @@ pub fn code_to_rule(linter: Linter, code: &str) -> Option<(RuleGroup, Rule)> { (Ruff, "017") => (RuleGroup::Nursery, rules::ruff::rules::QuadraticListSummation), (Ruff, "018") => (RuleGroup::Preview, rules::ruff::rules::AssignmentInAssert), (Ruff, "019") => (RuleGroup::Preview, rules::ruff::rules::UnnecessaryKeyCheck), + (Ruff, "020") => (RuleGroup::Preview, rules::ruff::rules::NeverUnion), (Ruff, "100") => (RuleGroup::Stable, rules::ruff::rules::UnusedNOQA), (Ruff, "200") => (RuleGroup::Stable, rules::ruff::rules::InvalidPyprojectToml), diff --git a/crates/ruff_linter/src/rules/flake8_pyi/helpers.rs b/crates/ruff_linter/src/rules/flake8_pyi/helpers.rs deleted file mode 100644 index 25dde842e715c..0000000000000 --- a/crates/ruff_linter/src/rules/flake8_pyi/helpers.rs +++ /dev/null @@ -1,54 +0,0 @@ -use ruff_python_ast::{self as ast, Expr, Operator}; -use ruff_python_semantic::SemanticModel; - -/// Traverse a "union" type annotation, applying `func` to each union member. -/// Supports traversal of `Union` and `|` union expressions. -/// The function is called with each expression in the union (excluding declarations of nested unions) -/// and the parent expression (if any). -pub(super) fn traverse_union<'a, F>( - func: &mut F, - semantic: &SemanticModel, - expr: &'a Expr, - parent: Option<&'a Expr>, -) where - F: FnMut(&'a Expr, Option<&'a Expr>), -{ - // Ex) x | y - if let Expr::BinOp(ast::ExprBinOp { - op: Operator::BitOr, - left, - right, - range: _, - }) = expr - { - // The union data structure usually looks like this: - // a | b | c -> (a | b) | c - // - // However, parenthesized expressions can coerce it into any structure: - // a | (b | c) - // - // So we have to traverse both branches in order (left, then right), to report members - // in the order they appear in the source code. - - // Traverse the left then right arms - traverse_union(func, semantic, left, Some(expr)); - traverse_union(func, semantic, right, Some(expr)); - return; - } - - // Ex) Union[x, y] - if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr { - if semantic.match_typing_expr(value, "Union") { - if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() { - // Traverse each element of the tuple within the union recursively to handle cases - // such as `Union[..., Union[...]] - elts.iter() - .for_each(|elt| traverse_union(func, semantic, elt, Some(expr))); - return; - } - } - } - - // Otherwise, call the function on expression - func(expr, parent); -} diff --git a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs index b549b80406a7a..2fa1fc356747c 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/mod.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/mod.rs @@ -1,5 +1,4 @@ //! Rules from [flake8-pyi](https://pypi.org/project/flake8-pyi/). -mod helpers; pub(crate) mod rules; #[cfg(test)] diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_union_member.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_union_member.rs index b12c741620f86..b742be4425dda 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_union_member.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/duplicate_union_member.rs @@ -1,15 +1,16 @@ -use ruff_python_ast::{self as ast, Expr}; -use rustc_hash::FxHashSet; use std::collections::HashSet; -use crate::checkers::ast::Checker; +use rustc_hash::FxHashSet; -use crate::rules::flake8_pyi::helpers::traverse_union; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::comparable::ComparableExpr; +use ruff_python_ast::{self as ast, Expr}; +use ruff_python_semantic::analyze::typing::traverse_union; use ruff_text_size::Ranged; +use crate::checkers::ast::Checker; + /// ## What it does /// Checks for duplicate union members. /// diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs index afcaec5c40f8e..cb387e83c8c66 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_literal_union.rs @@ -1,14 +1,16 @@ -use rustc_hash::FxHashSet; use std::fmt; +use rustc_hash::FxHashSet; + use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr, LiteralExpressionRef}; +use ruff_python_semantic::analyze::typing::traverse_union; use ruff_python_semantic::SemanticModel; use ruff_text_size::Ranged; +use crate::checkers::ast::Checker; use crate::fix::snippet::SourceCodeSnippet; -use crate::{checkers::ast::Checker, rules::flake8_pyi::helpers::traverse_union}; /// ## What it does /// Checks for the presence of redundant `Literal` types and builtin super diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs index 5a14dd3c14cab..e6fcba7a85c28 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/redundant_numeric_union.rs @@ -1,11 +1,10 @@ -use ruff_python_ast::{Expr, Parameters}; - use ruff_diagnostics::{Diagnostic, Violation}; use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::{Expr, Parameters}; +use ruff_python_semantic::analyze::typing::traverse_union; use ruff_text_size::Ranged; use crate::checkers::ast::Checker; -use crate::rules::flake8_pyi::helpers::traverse_union; /// ## What it does /// Checks for union annotations that contain redundant numeric types (e.g., diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs index 7b56371e837c3..0ead862461db0 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_literal_union.rs @@ -2,12 +2,11 @@ use ast::{ExprSubscript, Operator}; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr}; +use ruff_python_semantic::analyze::typing::traverse_union; use ruff_text_size::{Ranged, TextRange}; use crate::checkers::ast::Checker; -use crate::rules::flake8_pyi::helpers::traverse_union; - /// ## What it does /// Checks for the presence of multiple literal types in a union. /// diff --git a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs index 6d74fffd5b74e..89b95be53d6d4 100644 --- a/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs +++ b/crates/ruff_linter/src/rules/flake8_pyi/rules/unnecessary_type_union.rs @@ -2,9 +2,10 @@ use ast::{ExprContext, Operator}; use ruff_diagnostics::{Diagnostic, Edit, Fix, FixAvailability, Violation}; use ruff_macros::{derive_message_formats, violation}; use ruff_python_ast::{self as ast, Expr}; +use ruff_python_semantic::analyze::typing::traverse_union; use ruff_text_size::{Ranged, TextRange}; -use crate::{checkers::ast::Checker, rules::flake8_pyi::helpers::traverse_union}; +use crate::checkers::ast::Checker; /// ## What it does /// Checks for the presence of multiple `type`s in a union. diff --git a/crates/ruff_linter/src/rules/ruff/mod.rs b/crates/ruff_linter/src/rules/ruff/mod.rs index 3993d0e14e187..f94001b30bcf7 100644 --- a/crates/ruff_linter/src/rules/ruff/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/mod.rs @@ -44,6 +44,7 @@ mod tests { #[test_case(Rule::QuadraticListSummation, Path::new("RUF017_0.py"))] #[test_case(Rule::AssignmentInAssert, Path::new("RUF018.py"))] #[test_case(Rule::UnnecessaryKeyCheck, Path::new("RUF019.py"))] + #[test_case(Rule::NeverUnion, Path::new("RUF020.py"))] fn rules(rule_code: Rule, path: &Path) -> Result<()> { let snapshot = format!("{}_{}", rule_code.noqa_code(), path.to_string_lossy()); let diagnostics = test_path( diff --git a/crates/ruff_linter/src/rules/ruff/rules/mod.rs b/crates/ruff_linter/src/rules/ruff/rules/mod.rs index eddad40cd5419..ae50ad4221771 100644 --- a/crates/ruff_linter/src/rules/ruff/rules/mod.rs +++ b/crates/ruff_linter/src/rules/ruff/rules/mod.rs @@ -9,7 +9,9 @@ pub(crate) use invalid_index_type::*; pub(crate) use invalid_pyproject_toml::*; pub(crate) use mutable_class_default::*; pub(crate) use mutable_dataclass_default::*; +pub(crate) use never_union::*; pub(crate) use pairwise_over_zipped::*; +pub(crate) use quadratic_list_summation::*; pub(crate) use static_key_dict_comprehension::*; pub(crate) use unnecessary_iterable_allocation_for_first_element::*; pub(crate) use unnecessary_key_check::*; @@ -30,6 +32,7 @@ mod invalid_index_type; mod invalid_pyproject_toml; mod mutable_class_default; mod mutable_dataclass_default; +mod never_union; mod pairwise_over_zipped; mod static_key_dict_comprehension; mod unnecessary_iterable_allocation_for_first_element; @@ -44,6 +47,5 @@ pub(crate) enum Context { Docstring, Comment, } -pub(crate) use quadratic_list_summation::*; mod quadratic_list_summation; diff --git a/crates/ruff_linter/src/rules/ruff/rules/never_union.rs b/crates/ruff_linter/src/rules/ruff/rules/never_union.rs new file mode 100644 index 0000000000000..2f5731ddf9670 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/rules/never_union.rs @@ -0,0 +1,131 @@ +use ruff_diagnostics::{Diagnostic, Violation}; +use ruff_macros::{derive_message_formats, violation}; +use ruff_python_ast::Expr; +use ruff_python_semantic::analyze::typing::traverse_union; +use ruff_text_size::Ranged; + +use crate::checkers::ast::Checker; + +/// ## What it does +/// Checks for uses of `typing.NoReturn` and `typing.Never` in union types. +/// +/// ## Why is this bad? +/// `typing.NoReturn` and `typing.Never` are special types, used to indicate +/// that a function never returns, or that a type has no values. +/// +/// Including `typing.NoReturn` or `typing.Never` in a union type is redundant, +/// as, e.g., `typing.Never | T` is equivalent to `T`. +/// +/// ## Example +/// ```python +/// from typing import Never +/// +/// +/// def func() -> Never | int: +/// ... +/// ``` +/// +/// Use instead: +/// ```python +/// def func() -> int: +/// ... +/// ``` +/// +/// ## Options +/// - [Python documentation: `typing.Never`](https://docs.python.org/3/library/typing.html#typing.Never) +/// - [Python documentation: `typing.NoReturn`](https://docs.python.org/3/library/typing.html#typing.NoReturn) +#[violation] +pub struct NeverUnion { + never_like: NeverLike, + union_like: UnionLike, +} + +impl Violation for NeverUnion { + #[derive_message_formats] + fn message(&self) -> String { + let Self { + never_like, + union_like, + } = self; + match union_like { + UnionLike::BinOp => { + format!("`{never_like} | T` is equivalent to `T`") + } + UnionLike::TypingUnion => { + format!("`Union[{never_like}, T]` is equivalent to `T`") + } + } + } +} + +/// RUF020 +pub(crate) fn never_union<'a>(checker: &mut Checker, expr: &'a Expr) { + let mut expressions: Vec<(NeverLike, UnionLike, &'a Expr)> = Vec::new(); + let mut rest: Vec<&'a Expr> = Vec::new(); + + // Find all `typing.Never` and `typing.NoReturn` expressions. + let semantic = checker.semantic(); + let mut collect_never = |expr: &'a Expr, parent: Option<&'a Expr>| { + if let Some(call_path) = semantic.resolve_call_path(expr) { + let union_like = if parent.is_some_and(Expr::is_bin_op_expr) { + UnionLike::BinOp + } else { + UnionLike::TypingUnion + }; + + let never_like = if semantic.match_typing_call_path(&call_path, "NoReturn") { + Some(NeverLike::NoReturn) + } else if semantic.match_typing_call_path(&call_path, "Never") { + Some(NeverLike::Never) + } else { + None + }; + + if let Some(never_like) = never_like { + expressions.push((never_like, union_like, expr)); + return; + } + } + + rest.push(expr); + }; + + traverse_union(&mut collect_never, checker.semantic(), expr, None); + + // Create a diagnostic for each `typing.Never` and `typing.NoReturn` expression. + for (never_like, union_like, child) in expressions { + let diagnostic = Diagnostic::new( + NeverUnion { + never_like, + union_like, + }, + child.range(), + ); + checker.diagnostics.push(diagnostic); + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum UnionLike { + /// E.g., `typing.Union[int, str]` + TypingUnion, + /// E.g., `int | str` + BinOp, +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum NeverLike { + /// E.g., `typing.NoReturn` + NoReturn, + /// E.g., `typing.Never` + Never, +} + +impl std::fmt::Display for NeverLike { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + NeverLike::NoReturn => f.write_str("NoReturn"), + NeverLike::Never => f.write_str("Never"), + } + } +} diff --git a/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF020_RUF020.py.snap b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF020_RUF020.py.snap new file mode 100644 index 0000000000000..a742caaf5b8e1 --- /dev/null +++ b/crates/ruff_linter/src/rules/ruff/snapshots/ruff_linter__rules__ruff__tests__RUF020_RUF020.py.snap @@ -0,0 +1,58 @@ +--- +source: crates/ruff_linter/src/rules/ruff/mod.rs +--- +RUF020.py:3:7: RUF020 `Union[Never, T]` is equivalent to `T` + | +1 | from typing import Never, NoReturn, Union +2 | +3 | Union[Never, int] + | ^^^^^ RUF020 +4 | Union[NoReturn, int] +5 | Never | int + | + +RUF020.py:4:7: RUF020 `Union[NoReturn, T]` is equivalent to `T` + | +3 | Union[Never, int] +4 | Union[NoReturn, int] + | ^^^^^^^^ RUF020 +5 | Never | int +6 | NoReturn | int + | + +RUF020.py:5:1: RUF020 `Never | T` is equivalent to `T` + | +3 | Union[Never, int] +4 | Union[NoReturn, int] +5 | Never | int + | ^^^^^ RUF020 +6 | NoReturn | int +7 | Union[Union[Never, int], Union[NoReturn, int]] + | + +RUF020.py:6:1: RUF020 `NoReturn | T` is equivalent to `T` + | +4 | Union[NoReturn, int] +5 | Never | int +6 | NoReturn | int + | ^^^^^^^^ RUF020 +7 | Union[Union[Never, int], Union[NoReturn, int]] + | + +RUF020.py:7:13: RUF020 `Union[Never, T]` is equivalent to `T` + | +5 | Never | int +6 | NoReturn | int +7 | Union[Union[Never, int], Union[NoReturn, int]] + | ^^^^^ RUF020 + | + +RUF020.py:7:32: RUF020 `Union[NoReturn, T]` is equivalent to `T` + | +5 | Never | int +6 | NoReturn | int +7 | Union[Union[Never, int], Union[NoReturn, int]] + | ^^^^^^^^ RUF020 + | + + diff --git a/crates/ruff_python_ast/src/helpers.rs b/crates/ruff_python_ast/src/helpers.rs index 61e80657b7a5e..94cbd009c594b 100644 --- a/crates/ruff_python_ast/src/helpers.rs +++ b/crates/ruff_python_ast/src/helpers.rs @@ -1505,6 +1505,7 @@ pub fn pep_604_union(elts: &[Expr]) -> Expr { } } +/// Format the expression as a `typing.Optional`-style optional. pub fn typing_optional(elt: Expr, binding: String) -> Expr { Expr::Subscript(ast::ExprSubscript { value: Box::new(Expr::Name(ast::ExprName { @@ -1518,18 +1519,19 @@ pub fn typing_optional(elt: Expr, binding: String) -> Expr { }) } +/// Format the expressions as a `typing.Union`-style union. pub fn typing_union(elts: &[Expr], binding: String) -> Expr { - fn tuple(elts: &[Expr]) -> Expr { + fn tuple(elts: &[Expr], binding: String) -> Expr { match elts { [] => Expr::Tuple(ast::ExprTuple { elts: vec![], ctx: ExprContext::Load, range: TextRange::default(), }), - [Expr::Tuple(ast::ExprTuple { elts, .. })] => pep_604_union(elts), + [Expr::Tuple(ast::ExprTuple { elts, .. })] => typing_union(elts, binding), [elt] => elt.clone(), [rest @ .., elt] => Expr::BinOp(ast::ExprBinOp { - left: Box::new(tuple(rest)), + left: Box::new(tuple(rest, binding)), op: Operator::BitOr, right: Box::new(elt.clone()), range: TextRange::default(), @@ -1539,11 +1541,11 @@ pub fn typing_union(elts: &[Expr], binding: String) -> Expr { Expr::Subscript(ast::ExprSubscript { value: Box::new(Expr::Name(ast::ExprName { - id: binding, + id: binding.clone(), range: TextRange::default(), ctx: ExprContext::Load, })), - slice: Box::new(tuple(elts)), + slice: Box::new(tuple(elts, binding)), ctx: ExprContext::Load, range: TextRange::default(), }) diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 2dd7f1003e398..c66769ae0b74d 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -333,6 +333,58 @@ pub fn is_sys_version_block(stmt: &ast::StmtIf, semantic: &SemanticModel) -> boo }) } +/// Traverse a "union" type annotation, applying `func` to each union member. +/// Supports traversal of `Union` and `|` union expressions. +/// The function is called with each expression in the union (excluding declarations of nested unions) +/// and the parent expression (if any). +pub fn traverse_union<'a, F>( + func: &mut F, + semantic: &SemanticModel, + expr: &'a Expr, + parent: Option<&'a Expr>, +) where + F: FnMut(&'a Expr, Option<&'a Expr>), +{ + // Ex) x | y + if let Expr::BinOp(ast::ExprBinOp { + op: Operator::BitOr, + left, + right, + range: _, + }) = expr + { + // The union data structure usually looks like this: + // a | b | c -> (a | b) | c + // + // However, parenthesized expressions can coerce it into any structure: + // a | (b | c) + // + // So we have to traverse both branches in order (left, then right), to report members + // in the order they appear in the source code. + + // Traverse the left then right arms + traverse_union(func, semantic, left, Some(expr)); + traverse_union(func, semantic, right, Some(expr)); + return; + } + + // Ex) Union[x, y] + if let Expr::Subscript(ast::ExprSubscript { value, slice, .. }) = expr { + if semantic.match_typing_expr(value, "Union") { + if let Expr::Tuple(ast::ExprTuple { elts, .. }) = slice.as_ref() { + // Traverse each element of the tuple within the union recursively to handle cases + // such as `Union[..., Union[...]] + elts.iter() + .for_each(|elt| traverse_union(func, semantic, elt, Some(expr))); + return; + } + } + } + + // Otherwise, call the function on expression + func(expr, parent); +} + /// Abstraction for a type checker, conservatively checks for the intended type(s). trait TypeChecker { /// Check annotation expression to match the intended type(s). diff --git a/ruff.schema.json b/ruff.schema.json index 3d566f7964ff2..3761d705180f7 100644 --- a/ruff.schema.json +++ b/ruff.schema.json @@ -3429,6 +3429,8 @@ "RUF017", "RUF018", "RUF019", + "RUF02", + "RUF020", "RUF1", "RUF10", "RUF100",