From f8e5accf40909cfdf1b649a68096a7fd0ada4dfc Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 8 Jul 2023 02:06:08 +0530 Subject: [PATCH 01/13] Move `TypingTarget` related logic out of rule file --- crates/ruff/src/rules/ruff/mod.rs | 1 + .../src/rules/ruff/rules/implicit_optional.rs | 272 +----------------- crates/ruff/src/rules/ruff/typing.rs | 264 +++++++++++++++++ 3 files changed, 269 insertions(+), 268 deletions(-) create mode 100644 crates/ruff/src/rules/ruff/typing.rs diff --git a/crates/ruff/src/rules/ruff/mod.rs b/crates/ruff/src/rules/ruff/mod.rs index 0c9aded910c523..79062bc839351f 100644 --- a/crates/ruff/src/rules/ruff/mod.rs +++ b/crates/ruff/src/rules/ruff/mod.rs @@ -1,6 +1,7 @@ //! Ruff-specific rules. pub(crate) mod rules; +pub(crate) mod typing; #[cfg(test)] mod tests { diff --git a/crates/ruff/src/rules/ruff/rules/implicit_optional.rs b/crates/ruff/src/rules/ruff/rules/implicit_optional.rs index 64fb85c073b647..d9f03e26997a3e 100644 --- a/crates/ruff/src/rules/ruff/rules/implicit_optional.rs +++ b/crates/ruff/src/rules/ruff/rules/implicit_optional.rs @@ -6,18 +6,16 @@ use rustpython_parser::ast::{self, ArgWithDefault, Arguments, Constant, Expr, Op use ruff_diagnostics::{AutofixKind, Diagnostic, Edit, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; -use ruff_python_ast::call_path::CallPath; use ruff_python_ast::helpers::is_const_none; -use ruff_python_ast::source_code::Locator; use ruff_python_ast::typing::parse_type_annotation; -use ruff_python_semantic::SemanticModel; -use ruff_python_stdlib::sys::is_known_standard_library; use crate::checkers::ast::Checker; use crate::importer::ImportRequest; use crate::registry::AsRule; use crate::settings::types::PythonVersion; +use super::super::typing::type_hint_explicitly_allows_none; + /// ## What it does /// Checks for the use of implicit `Optional` in type annotations when the /// default parameter value is `None`. @@ -121,231 +119,6 @@ impl From for ConversionType { } } -/// Custom iterator to collect all the `|` separated expressions in a PEP 604 -/// union type. -struct PEP604UnionIterator<'a> { - stack: Vec<&'a Expr>, -} - -impl<'a> PEP604UnionIterator<'a> { - fn new(expr: &'a Expr) -> Self { - Self { stack: vec![expr] } - } -} - -impl<'a> Iterator for PEP604UnionIterator<'a> { - type Item = &'a Expr; - - fn next(&mut self) -> Option { - while let Some(expr) = self.stack.pop() { - match expr { - Expr::BinOp(ast::ExprBinOp { - left, - op: Operator::BitOr, - right, - .. - }) => { - self.stack.push(left); - self.stack.push(right); - } - _ => return Some(expr), - } - } - None - } -} - -/// Returns `true` if the given call path is a known type. -/// -/// A known type is either a builtin type, any object from the standard library, -/// or a type from the `typing_extensions` module. -fn is_known_type(call_path: &CallPath, target_version: PythonVersion) -> bool { - match call_path.as_slice() { - ["" | "typing_extensions", ..] => true, - [module, ..] => is_known_standard_library(target_version.minor(), module), - _ => false, - } -} - -#[derive(Debug)] -enum TypingTarget<'a> { - None, - Any, - Object, - Optional, - ForwardReference(Expr), - Union(Vec<&'a Expr>), - Literal(Vec<&'a Expr>), - Annotated(&'a Expr), -} - -impl<'a> TypingTarget<'a> { - fn try_from_expr( - expr: &'a Expr, - semantic: &SemanticModel, - locator: &Locator, - target_version: PythonVersion, - ) -> Option { - match expr { - Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { - if semantic.match_typing_expr(value, "Optional") { - return Some(TypingTarget::Optional); - } - let Expr::Tuple(ast::ExprTuple { elts: elements, .. }) = slice.as_ref() else { - return None; - }; - if semantic.match_typing_expr(value, "Literal") { - Some(TypingTarget::Literal(elements.iter().collect())) - } else if semantic.match_typing_expr(value, "Union") { - Some(TypingTarget::Union(elements.iter().collect())) - } else if semantic.match_typing_expr(value, "Annotated") { - elements.first().map(TypingTarget::Annotated) - } else { - semantic.resolve_call_path(value).map_or( - // If we can't resolve the call path, it must be defined - // in the same file, so we assume it's `Any` as it could - // be a type alias. - Some(TypingTarget::Any), - |call_path| { - if is_known_type(&call_path, target_version) { - None - } else { - // If it's not a known type, we assume it's `Any`. - Some(TypingTarget::Any) - } - }, - ) - } - } - Expr::BinOp(..) => Some(TypingTarget::Union( - PEP604UnionIterator::new(expr).collect(), - )), - Expr::Constant(ast::ExprConstant { - value: Constant::None, - .. - }) => Some(TypingTarget::None), - Expr::Constant(ast::ExprConstant { - value: Constant::Str(string), - range, - .. - }) => parse_type_annotation(string, *range, locator) - // In case of a parse error, we return `Any` to avoid false positives. - .map_or(Some(TypingTarget::Any), |(expr, _)| { - Some(TypingTarget::ForwardReference(expr)) - }), - _ => semantic.resolve_call_path(expr).map_or( - // If we can't resolve the call path, it must be defined in the - // same file, so we assume it's `Any` as it could be a type alias. - Some(TypingTarget::Any), - |call_path| { - if semantic.match_typing_call_path(&call_path, "Any") { - Some(TypingTarget::Any) - } else if matches!(call_path.as_slice(), ["" | "builtins", "object"]) { - Some(TypingTarget::Object) - } else if !is_known_type(&call_path, target_version) { - // If it's not a known type, we assume it's `Any`. - Some(TypingTarget::Any) - } else { - None - } - }, - ), - } - } - - /// Check if the [`TypingTarget`] explicitly allows `None`. - fn contains_none( - &self, - semantic: &SemanticModel, - locator: &Locator, - target_version: PythonVersion, - ) -> bool { - match self { - TypingTarget::None - | TypingTarget::Optional - | TypingTarget::Any - | TypingTarget::Object => true, - TypingTarget::Literal(elements) => elements.iter().any(|element| { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, target_version) - else { - return false; - }; - // Literal can only contain `None`, a literal value, other `Literal` - // or an enum value. - match new_target { - TypingTarget::None => true, - TypingTarget::Literal(_) => { - new_target.contains_none(semantic, locator, target_version) - } - _ => false, - } - }), - TypingTarget::Union(elements) => elements.iter().any(|element| { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, target_version) - else { - return false; - }; - new_target.contains_none(semantic, locator, target_version) - }), - TypingTarget::Annotated(element) => { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, target_version) - else { - return false; - }; - new_target.contains_none(semantic, locator, target_version) - } - TypingTarget::ForwardReference(expr) => { - let Some(new_target) = - TypingTarget::try_from_expr(expr, semantic, locator, target_version) - else { - return false; - }; - new_target.contains_none(semantic, locator, target_version) - } - } - } -} - -/// Check if the given annotation [`Expr`] explicitly allows `None`. -/// -/// This function will return `None` if the annotation explicitly allows `None` -/// otherwise it will return the annotation itself. If it's a `Annotated` type, -/// then the inner type will be checked. -/// -/// This function assumes that the annotation is a valid typing annotation expression. -fn type_hint_explicitly_allows_none<'a>( - annotation: &'a Expr, - semantic: &SemanticModel, - locator: &Locator, - target_version: PythonVersion, -) -> Option<&'a Expr> { - let Some(target) = TypingTarget::try_from_expr(annotation, semantic, locator, target_version) - else { - return Some(annotation); - }; - match target { - // Short circuit on top level `None`, `Any` or `Optional` - TypingTarget::None | TypingTarget::Optional | TypingTarget::Any => None, - // Top-level `Annotated` node should check for the inner type and - // return the inner type if it doesn't allow `None`. If `Annotated` - // is found nested inside another type, then the outer type should - // be returned. - TypingTarget::Annotated(expr) => { - type_hint_explicitly_allows_none(expr, semantic, locator, target_version) - } - _ => { - if target.contains_none(semantic, locator, target_version) { - None - } else { - Some(annotation) - } - } - } -} - /// Generate a [`Fix`] for the given [`Expr`] as per the [`ConversionType`]. fn generate_fix(checker: &Checker, conversion_type: ConversionType, expr: &Expr) -> Result { match conversion_type { @@ -423,7 +196,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) { &annotation, checker.semantic(), checker.locator, - checker.settings.target_version, + checker.settings.target_version.minor(), ) else { continue; }; @@ -444,7 +217,7 @@ pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) { annotation, checker.semantic(), checker.locator, - checker.settings.target_version, + checker.settings.target_version.minor(), ) else { continue; }; @@ -459,40 +232,3 @@ pub(crate) fn implicit_optional(checker: &mut Checker, arguments: &Arguments) { } } } - -#[cfg(test)] -mod tests { - use ruff_python_ast::call_path::CallPath; - - use crate::settings::types::PythonVersion; - - use super::is_known_type; - - #[test] - fn test_is_known_type() { - assert!(is_known_type( - &CallPath::from_slice(&["", "int"]), - PythonVersion::Py311 - )); - assert!(is_known_type( - &CallPath::from_slice(&["builtins", "int"]), - PythonVersion::Py311 - )); - assert!(is_known_type( - &CallPath::from_slice(&["typing", "Optional"]), - PythonVersion::Py311 - )); - assert!(is_known_type( - &CallPath::from_slice(&["typing_extensions", "Literal"]), - PythonVersion::Py311 - )); - assert!(is_known_type( - &CallPath::from_slice(&["zoneinfo", "ZoneInfo"]), - PythonVersion::Py311 - )); - assert!(!is_known_type( - &CallPath::from_slice(&["zoneinfo", "ZoneInfo"]), - PythonVersion::Py38 - )); - } -} diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs new file mode 100644 index 00000000000000..de98d0a91b36a5 --- /dev/null +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -0,0 +1,264 @@ +use rustpython_parser::ast::{self, Constant, Expr, Operator}; + +use ruff_python_ast::call_path::CallPath; +use ruff_python_ast::source_code::Locator; +use ruff_python_ast::typing::parse_type_annotation; +use ruff_python_semantic::SemanticModel; +use ruff_python_stdlib::sys::is_known_standard_library; + +/// Custom iterator to collect all the `|` separated expressions in a PEP 604 +/// union type. +struct PEP604UnionIterator<'a> { + stack: Vec<&'a Expr>, +} + +impl<'a> PEP604UnionIterator<'a> { + fn new(expr: &'a Expr) -> Self { + Self { stack: vec![expr] } + } +} + +impl<'a> Iterator for PEP604UnionIterator<'a> { + type Item = &'a Expr; + + fn next(&mut self) -> Option { + while let Some(expr) = self.stack.pop() { + match expr { + Expr::BinOp(ast::ExprBinOp { + left, + op: Operator::BitOr, + right, + .. + }) => { + self.stack.push(left); + self.stack.push(right); + } + _ => return Some(expr), + } + } + None + } +} + +/// Returns `true` if the given call path is a known type. +/// +/// A known type is either a builtin type, any object from the standard library, +/// or a type from the `typing_extensions` module. +fn is_known_type(call_path: &CallPath, minor_version: u32) -> bool { + match call_path.as_slice() { + ["" | "typing_extensions", ..] => true, + [module, ..] => is_known_standard_library(minor_version, module), + _ => false, + } +} + +#[derive(Debug)] +enum TypingTarget<'a> { + None, + Any, + Object, + ForwardReference(Expr), + Union(Vec<&'a Expr>), + Literal(Vec<&'a Expr>), + Optional(&'a Expr), + Annotated(&'a Expr), +} + +impl<'a> TypingTarget<'a> { + fn try_from_expr( + expr: &'a Expr, + semantic: &SemanticModel, + locator: &Locator, + minor_version: u32, + ) -> Option { + match expr { + Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { + if semantic.match_typing_expr(value, "Optional") { + return Some(TypingTarget::Optional(slice.as_ref())); + } + let Expr::Tuple(ast::ExprTuple { elts: elements, .. }) = slice.as_ref() else { + return None; + }; + if semantic.match_typing_expr(value, "Literal") { + Some(TypingTarget::Literal(elements.iter().collect())) + } else if semantic.match_typing_expr(value, "Union") { + Some(TypingTarget::Union(elements.iter().collect())) + } else if semantic.match_typing_expr(value, "Annotated") { + elements.first().map(TypingTarget::Annotated) + } else { + semantic.resolve_call_path(value).map_or( + // If we can't resolve the call path, it must be defined + // in the same file, so we assume it's `Any` as it could + // be a type alias. + Some(TypingTarget::Any), + |call_path| { + if is_known_type(&call_path, minor_version) { + None + } else { + // If it's not a known type, we assume it's `Any`. + Some(TypingTarget::Any) + } + }, + ) + } + } + Expr::BinOp(..) => Some(TypingTarget::Union( + PEP604UnionIterator::new(expr).collect(), + )), + Expr::Constant(ast::ExprConstant { + value: Constant::None, + .. + }) => Some(TypingTarget::None), + Expr::Constant(ast::ExprConstant { + value: Constant::Str(string), + range, + .. + }) => parse_type_annotation(string, *range, locator) + // In case of a parse error, we return `Any` to avoid false positives. + .map_or(Some(TypingTarget::Any), |(expr, _)| { + Some(TypingTarget::ForwardReference(expr)) + }), + _ => semantic.resolve_call_path(expr).map_or( + // If we can't resolve the call path, it must be defined in the + // same file, so we assume it's `Any` as it could be a type alias. + Some(TypingTarget::Any), + |call_path| { + if semantic.match_typing_call_path(&call_path, "Any") { + Some(TypingTarget::Any) + } else if matches!(call_path.as_slice(), ["" | "builtins", "object"]) { + Some(TypingTarget::Object) + } else if !is_known_type(&call_path, minor_version) { + // If it's not a known type, we assume it's `Any`. + Some(TypingTarget::Any) + } else { + None + } + }, + ), + } + } + + /// Check if the [`TypingTarget`] explicitly allows `None`. + fn contains_none( + &self, + semantic: &SemanticModel, + locator: &Locator, + minor_version: u32, + ) -> bool { + match self { + TypingTarget::None + | TypingTarget::Optional(_) + | TypingTarget::Any + | TypingTarget::Object => true, + TypingTarget::Literal(elements) => elements.iter().any(|element| { + let Some(new_target) = + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + else { + return false; + }; + // Literal can only contain `None`, a literal value, other `Literal` + // or an enum value. + match new_target { + TypingTarget::None => true, + TypingTarget::Literal(_) => { + new_target.contains_none(semantic, locator, minor_version) + } + _ => false, + } + }), + TypingTarget::Union(elements) => elements.iter().any(|element| { + let Some(new_target) = + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + else { + return false; + }; + new_target.contains_none(semantic, locator, minor_version) + }), + TypingTarget::Annotated(element) => { + let Some(new_target) = + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + else { + return false; + }; + new_target.contains_none(semantic, locator, minor_version) + } + TypingTarget::ForwardReference(expr) => { + let Some(new_target) = + TypingTarget::try_from_expr(expr, semantic, locator, minor_version) + else { + return false; + }; + new_target.contains_none(semantic, locator, minor_version) + } + } + } +} + +/// Check if the given annotation [`Expr`] explicitly allows `None`. +/// +/// This function will return `None` if the annotation explicitly allows `None` +/// otherwise it will return the annotation itself. If it's a `Annotated` type, +/// then the inner type will be checked. +/// +/// This function assumes that the annotation is a valid typing annotation expression. +pub(crate) fn type_hint_explicitly_allows_none<'a>( + annotation: &'a Expr, + semantic: &SemanticModel, + locator: &Locator, + minor_version: u32, +) -> Option<&'a Expr> { + let Some(target) = TypingTarget::try_from_expr(annotation, semantic, locator, minor_version) + else { + return Some(annotation); + }; + match target { + // Short circuit on top level `None`, `Any` or `Optional` + TypingTarget::None | TypingTarget::Optional(_) | TypingTarget::Any => None, + // Top-level `Annotated` node should check for the inner type and + // return the inner type if it doesn't allow `None`. If `Annotated` + // is found nested inside another type, then the outer type should + // be returned. + TypingTarget::Annotated(expr) => { + type_hint_explicitly_allows_none(expr, semantic, locator, minor_version) + } + _ => { + if target.contains_none(semantic, locator, minor_version) { + None + } else { + Some(annotation) + } + } + } +} + +#[cfg(test)] +mod tests { + use ruff_python_ast::call_path::CallPath; + + use super::is_known_type; + + #[test] + fn test_is_known_type() { + assert!(is_known_type(&CallPath::from_slice(&["", "int"]), 11)); + assert!(is_known_type( + &CallPath::from_slice(&["builtins", "int"]), + 11 + )); + assert!(is_known_type( + &CallPath::from_slice(&["typing", "Optional"]), + 11 + )); + assert!(is_known_type( + &CallPath::from_slice(&["typing_extensions", "Literal"]), + 11 + )); + assert!(is_known_type( + &CallPath::from_slice(&["zoneinfo", "ZoneInfo"]), + 11 + )); + assert!(!is_known_type( + &CallPath::from_slice(&["zoneinfo", "ZoneInfo"]), + 8 + )); + } +} From 1553d2ac9ee0c64c235bd67b900b5daf0d023d99 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 8 Jul 2023 02:06:39 +0530 Subject: [PATCH 02/13] Check if `TypingTarget` can be resolved to `Any` --- .../flake8_annotations/rules/definition.rs | 78 +++++++++++-------- crates/ruff/src/rules/ruff/typing.rs | 63 +++++++++++++++ 2 files changed, 109 insertions(+), 32 deletions(-) diff --git a/crates/ruff/src/rules/flake8_annotations/rules/definition.rs b/crates/ruff/src/rules/flake8_annotations/rules/definition.rs index 47232b9e04c63a..56094110ff83d2 100644 --- a/crates/ruff/src/rules/flake8_annotations/rules/definition.rs +++ b/crates/ruff/src/rules/flake8_annotations/rules/definition.rs @@ -1,4 +1,4 @@ -use rustpython_parser::ast::{ArgWithDefault, Expr, Ranged, Stmt}; +use rustpython_parser::ast::{self, ArgWithDefault, Constant, Expr, Ranged, Stmt}; use ruff_diagnostics::{AlwaysAutofixableViolation, Diagnostic, Fix, Violation}; use ruff_macros::{derive_message_formats, violation}; @@ -6,12 +6,14 @@ use ruff_python_ast::cast; use ruff_python_ast::helpers::ReturnStatementVisitor; use ruff_python_ast::identifier::Identifier; use ruff_python_ast::statement_visitor::StatementVisitor; +use ruff_python_ast::typing::parse_type_annotation; use ruff_python_semantic::analyze::visibility; -use ruff_python_semantic::{Definition, Member, MemberKind, SemanticModel}; +use ruff_python_semantic::{Definition, Member, MemberKind}; use ruff_python_stdlib::typing::simple_magic_return_type; use crate::checkers::ast::Checker; use crate::registry::{AsRule, Rule}; +use crate::rules::ruff::typing::type_hint_resolves_to_any; use super::super::fixes; use super::super::helpers::match_function_def; @@ -432,20 +434,46 @@ fn is_none_returning(body: &[Stmt]) -> bool { /// ANN401 fn check_dynamically_typed( + checker: &Checker, annotation: &Expr, func: F, diagnostics: &mut Vec, - is_overridden: bool, - semantic: &SemanticModel, ) where F: FnOnce() -> String, { - if !is_overridden && semantic.match_typing_expr(annotation, "Any") { - diagnostics.push(Diagnostic::new( - AnyType { name: func() }, - annotation.range(), - )); - }; + if let Expr::Constant(ast::ExprConstant { + range, + value: Constant::Str(string), + .. + }) = annotation + { + // Quoted annotations + if let Ok((annotation, _)) = parse_type_annotation(string, *range, checker.locator) { + if type_hint_resolves_to_any( + &annotation, + checker.semantic(), + checker.locator, + checker.settings.target_version.minor(), + ) { + diagnostics.push(Diagnostic::new( + AnyType { name: func() }, + annotation.range(), + )); + } + } + } else { + if type_hint_resolves_to_any( + annotation, + checker.semantic(), + checker.locator, + checker.settings.target_version.minor(), + ) { + diagnostics.push(Diagnostic::new( + AnyType { name: func() }, + annotation.range(), + )); + } + } } /// Generate flake8-annotation checks for a given `Definition`. @@ -500,13 +528,12 @@ pub(crate) fn definition( // ANN401 for dynamically typed arguments if let Some(annotation) = &def.annotation { has_any_typed_arg = true; - if checker.enabled(Rule::AnyType) { + if checker.enabled(Rule::AnyType) && !is_overridden { check_dynamically_typed( + checker, annotation, || def.arg.to_string(), &mut diagnostics, - is_overridden, - checker.semantic(), ); } } else { @@ -530,15 +557,9 @@ pub(crate) fn definition( if let Some(expr) = &arg.annotation { has_any_typed_arg = true; if !checker.settings.flake8_annotations.allow_star_arg_any { - if checker.enabled(Rule::AnyType) { + if checker.enabled(Rule::AnyType) && !is_overridden { let name = &arg.arg; - check_dynamically_typed( - expr, - || format!("*{name}"), - &mut diagnostics, - is_overridden, - checker.semantic(), - ); + check_dynamically_typed(checker, expr, || format!("*{name}"), &mut diagnostics); } } } else { @@ -562,14 +583,13 @@ pub(crate) fn definition( if let Some(expr) = &arg.annotation { has_any_typed_arg = true; if !checker.settings.flake8_annotations.allow_star_arg_any { - if checker.enabled(Rule::AnyType) { + if checker.enabled(Rule::AnyType) && !is_overridden { let name = &arg.arg; check_dynamically_typed( + checker, expr, || format!("**{name}"), &mut diagnostics, - is_overridden, - checker.semantic(), ); } } @@ -629,14 +649,8 @@ pub(crate) fn definition( // ANN201, ANN202, ANN401 if let Some(expr) = &returns { has_typed_return = true; - if checker.enabled(Rule::AnyType) { - check_dynamically_typed( - expr, - || name.to_string(), - &mut diagnostics, - is_overridden, - checker.semantic(), - ); + if checker.enabled(Rule::AnyType) && !is_overridden { + check_dynamically_typed(checker, expr, || name.to_string(), &mut diagnostics); } } else if !( // Allow omission of return annotation if the function only returns `None` diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs index de98d0a91b36a5..6e35b8e91c3738 100644 --- a/crates/ruff/src/rules/ruff/typing.rs +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -192,6 +192,44 @@ impl<'a> TypingTarget<'a> { } } } + + /// Check if the [`TypingTarget`] explicitly allows `Any`. + fn contains_any( + &self, + semantic: &SemanticModel, + locator: &Locator, + minor_version: u32, + ) -> bool { + match self { + TypingTarget::Any => true, + // `Literal` cannot contain `Any` as it's a dynamic value. + TypingTarget::Literal(_) | TypingTarget::None | TypingTarget::Object => false, + TypingTarget::Union(elements) => elements.iter().any(|element| { + let Some(new_target) = + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + else { + return false; + }; + new_target.contains_any(semantic, locator, minor_version) + }), + TypingTarget::Annotated(element) | TypingTarget::Optional(element) => { + let Some(new_target) = + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + else { + return false; + }; + new_target.contains_any(semantic, locator, minor_version) + } + TypingTarget::ForwardReference(expr) => { + let Some(new_target) = + TypingTarget::try_from_expr(expr, semantic, locator, minor_version) + else { + return false; + }; + new_target.contains_any(semantic, locator, minor_version) + } + } + } } /// Check if the given annotation [`Expr`] explicitly allows `None`. @@ -231,6 +269,31 @@ pub(crate) fn type_hint_explicitly_allows_none<'a>( } } +/// Check if the given annotation [`Expr`] resolves to `Any`. +/// +/// This function assumes that the annotation is a valid typing annotation expression. +pub(crate) fn type_hint_resolves_to_any( + annotation: &Expr, + semantic: &SemanticModel, + locator: &Locator, + minor_version: u32, +) -> bool { + let Some(target) = TypingTarget::try_from_expr(annotation, semantic, locator, minor_version) + else { + return false; + }; + match target { + // Short circuit on top level `Any` + TypingTarget::Any => true, + // Top-level `Annotated` node should check if the inner type resolves + // to `Any`. + TypingTarget::Annotated(expr) => { + type_hint_resolves_to_any(expr, semantic, locator, minor_version) + } + _ => target.contains_any(semantic, locator, minor_version), + } +} + #[cfg(test)] mod tests { use ruff_python_ast::call_path::CallPath; From 53f4a2a8b233e976f9b245eade4193ea5c73dc42 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 8 Jul 2023 11:03:23 +0530 Subject: [PATCH 03/13] Add `Unknown` type to keep `Any` separate --- crates/ruff/src/rules/ruff/typing.rs | 30 ++++++++++++++++++---------- 1 file changed, 20 insertions(+), 10 deletions(-) diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs index 6e35b8e91c3738..4ae4c4dfb501b8 100644 --- a/crates/ruff/src/rules/ruff/typing.rs +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -62,6 +62,14 @@ enum TypingTarget<'a> { Literal(Vec<&'a Expr>), Optional(&'a Expr), Annotated(&'a Expr), + + /// Special type used to represent an unknown type (and not a typing target). + /// This could be a type alias. It's also used when parsing a forward + /// reference fails. + /// + /// Note that this is not the same as returning `None` in `try_from_expr` as + /// `None` means that it's a known type but we don't know which one. + Unknown, } impl<'a> TypingTarget<'a> { @@ -88,15 +96,13 @@ impl<'a> TypingTarget<'a> { } else { semantic.resolve_call_path(value).map_or( // If we can't resolve the call path, it must be defined - // in the same file, so we assume it's `Any` as it could - // be a type alias. - Some(TypingTarget::Any), + // in the same file and could be a type alias. + Some(TypingTarget::Unknown), |call_path| { if is_known_type(&call_path, minor_version) { None } else { - // If it's not a known type, we assume it's `Any`. - Some(TypingTarget::Any) + Some(TypingTarget::Unknown) } }, ) @@ -115,13 +121,13 @@ impl<'a> TypingTarget<'a> { .. }) => parse_type_annotation(string, *range, locator) // In case of a parse error, we return `Any` to avoid false positives. - .map_or(Some(TypingTarget::Any), |(expr, _)| { + .map_or(Some(TypingTarget::Unknown), |(expr, _)| { Some(TypingTarget::ForwardReference(expr)) }), _ => semantic.resolve_call_path(expr).map_or( // If we can't resolve the call path, it must be defined in the // same file, so we assume it's `Any` as it could be a type alias. - Some(TypingTarget::Any), + Some(TypingTarget::Unknown), |call_path| { if semantic.match_typing_call_path(&call_path, "Any") { Some(TypingTarget::Any) @@ -129,7 +135,7 @@ impl<'a> TypingTarget<'a> { Some(TypingTarget::Object) } else if !is_known_type(&call_path, minor_version) { // If it's not a known type, we assume it's `Any`. - Some(TypingTarget::Any) + Some(TypingTarget::Unknown) } else { None } @@ -149,7 +155,8 @@ impl<'a> TypingTarget<'a> { TypingTarget::None | TypingTarget::Optional(_) | TypingTarget::Any - | TypingTarget::Object => true, + | TypingTarget::Object + | TypingTarget::Unknown => true, TypingTarget::Literal(elements) => elements.iter().any(|element| { let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator, minor_version) @@ -203,7 +210,10 @@ impl<'a> TypingTarget<'a> { match self { TypingTarget::Any => true, // `Literal` cannot contain `Any` as it's a dynamic value. - TypingTarget::Literal(_) | TypingTarget::None | TypingTarget::Object => false, + TypingTarget::Literal(_) + | TypingTarget::None + | TypingTarget::Object + | TypingTarget::Unknown => false, TypingTarget::Union(elements) => elements.iter().any(|element| { let Some(new_target) = TypingTarget::try_from_expr(element, semantic, locator, minor_version) From 713d5007300adf45e0c88ea01e8204efe2121ef2 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Sat, 8 Jul 2023 11:04:24 +0530 Subject: [PATCH 04/13] Tests --- .../flake8_annotations/annotation_presence.py | 26 +++++++-- .../flake8_annotations/rules/definition.rs | 4 +- ...__flake8_annotations__tests__defaults.snap | 56 +++++++++++++++++++ 3 files changed, 78 insertions(+), 8 deletions(-) diff --git a/crates/ruff/resources/test/fixtures/flake8_annotations/annotation_presence.py b/crates/ruff/resources/test/fixtures/flake8_annotations/annotation_presence.py index d37f178bbb938a..7778c85072b9a8 100644 --- a/crates/ruff/resources/test/fixtures/flake8_annotations/annotation_presence.py +++ b/crates/ruff/resources/test/fixtures/flake8_annotations/annotation_presence.py @@ -1,4 +1,4 @@ -from typing import Any, Type +from typing import Annotated, Any, Optional, Type, Union from typing_extensions import override # Error @@ -95,27 +95,27 @@ def foo(self: "Foo", a: int, *params: Any, **options: str) -> int: def foo(self: "Foo", a: int, *params: str, **options: Any) -> int: pass - # ANN401 + # OK @override def foo(self: "Foo", a: Any, *params: str, **options: str) -> int: pass - # ANN401 + # OK @override def foo(self: "Foo", a: int, *params: str, **options: str) -> Any: pass - # ANN401 + # OK @override def foo(self: "Foo", a: int, *params: Any, **options: Any) -> int: pass - # ANN401 + # OK @override def foo(self: "Foo", a: int, *params: Any, **options: str) -> int: pass - # ANN401 + # OK @override def foo(self: "Foo", a: int, *params: str, **options: Any) -> int: pass @@ -137,3 +137,17 @@ def foo(self, /, a: int, b: int) -> int: # OK def f(*args: *tuple[int]) -> None: ... +def f(a: object) -> None: ... +def f(a: str | bytes) -> None: ... +def f(a: Union[str, bytes]) -> None: ... +def f(a: Optional[str]) -> None: ... +def f(a: Annotated[str, ...]) -> None: ... +def f(a: "Union[str, bytes]") -> None: ... + +# ANN401 +def f(a: Any | int) -> None: ... +def f(a: int | Any) -> None: ... +def f(a: Union[str, bytes, Any]) -> None: ... +def f(a: Optional[Any]) -> None: ... +def f(a: Annotated[Any, ...]) -> None: ... +def f(a: "Union[str, bytes, Any]") -> None: ... diff --git a/crates/ruff/src/rules/flake8_annotations/rules/definition.rs b/crates/ruff/src/rules/flake8_annotations/rules/definition.rs index 56094110ff83d2..f672afd9b7956f 100644 --- a/crates/ruff/src/rules/flake8_annotations/rules/definition.rs +++ b/crates/ruff/src/rules/flake8_annotations/rules/definition.rs @@ -448,9 +448,9 @@ fn check_dynamically_typed( }) = annotation { // Quoted annotations - if let Ok((annotation, _)) = parse_type_annotation(string, *range, checker.locator) { + if let Ok((parsed_annotation, _)) = parse_type_annotation(string, *range, checker.locator) { if type_hint_resolves_to_any( - &annotation, + &parsed_annotation, checker.semantic(), checker.locator, checker.settings.target_version.minor(), diff --git a/crates/ruff/src/rules/flake8_annotations/snapshots/ruff__rules__flake8_annotations__tests__defaults.snap b/crates/ruff/src/rules/flake8_annotations/snapshots/ruff__rules__flake8_annotations__tests__defaults.snap index 0eba6a0469bf57..7cd87414d40d77 100644 --- a/crates/ruff/src/rules/flake8_annotations/snapshots/ruff__rules__flake8_annotations__tests__defaults.snap +++ b/crates/ruff/src/rules/flake8_annotations/snapshots/ruff__rules__flake8_annotations__tests__defaults.snap @@ -186,4 +186,60 @@ annotation_presence.py:134:13: ANN101 Missing type annotation for `self` in meth 135 | pass | +annotation_presence.py:148:10: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `a` + | +147 | # ANN401 +148 | def f(a: Any | int) -> None: ... + | ^^^^^^^^^ ANN401 +149 | def f(a: int | Any) -> None: ... +150 | def f(a: Union[str, bytes, Any]) -> None: ... + | + +annotation_presence.py:149:10: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `a` + | +147 | # ANN401 +148 | def f(a: Any | int) -> None: ... +149 | def f(a: int | Any) -> None: ... + | ^^^^^^^^^ ANN401 +150 | def f(a: Union[str, bytes, Any]) -> None: ... +151 | def f(a: Optional[Any]) -> None: ... + | + +annotation_presence.py:150:10: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `a` + | +148 | def f(a: Any | int) -> None: ... +149 | def f(a: int | Any) -> None: ... +150 | def f(a: Union[str, bytes, Any]) -> None: ... + | ^^^^^^^^^^^^^^^^^^^^^^ ANN401 +151 | def f(a: Optional[Any]) -> None: ... +152 | def f(a: Annotated[Any, ...]) -> None: ... + | + +annotation_presence.py:151:10: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `a` + | +149 | def f(a: int | Any) -> None: ... +150 | def f(a: Union[str, bytes, Any]) -> None: ... +151 | def f(a: Optional[Any]) -> None: ... + | ^^^^^^^^^^^^^ ANN401 +152 | def f(a: Annotated[Any, ...]) -> None: ... +153 | def f(a: "Union[str, bytes, Any]") -> None: ... + | + +annotation_presence.py:152:10: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `a` + | +150 | def f(a: Union[str, bytes, Any]) -> None: ... +151 | def f(a: Optional[Any]) -> None: ... +152 | def f(a: Annotated[Any, ...]) -> None: ... + | ^^^^^^^^^^^^^^^^^^^ ANN401 +153 | def f(a: "Union[str, bytes, Any]") -> None: ... + | + +annotation_presence.py:153:10: ANN401 Dynamically typed expressions (typing.Any) are disallowed in `a` + | +151 | def f(a: Optional[Any]) -> None: ... +152 | def f(a: Annotated[Any, ...]) -> None: ... +153 | def f(a: "Union[str, bytes, Any]") -> None: ... + | ^^^^^^^^^^^^^^^^^^^^^^^^ ANN401 + | + From 182def173ff02b143aa0368f6c72ceb78fabe7d8 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 12 Jul 2023 22:41:22 +0530 Subject: [PATCH 05/13] Explicit `Known` type, avoid flagging in case of failures --- crates/ruff/src/rules/ruff/typing.rs | 172 ++++++++++++++------------- 1 file changed, 89 insertions(+), 83 deletions(-) diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs index 4ae4c4dfb501b8..777f6181779e34 100644 --- a/crates/ruff/src/rules/ruff/typing.rs +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -52,24 +52,56 @@ fn is_known_type(call_path: &CallPath, minor_version: u32) -> bool { } } +/// Returns a vector of all the expressions in a slice. +/// +/// ## Panics +/// +/// Panics if the slice is not a tuple or a constant. This should never happen +/// as this function is only used in typing context where the slice cannot be +/// anything else. +fn resolve_slice_value(slice: &Expr) -> Vec<&Expr> { + match slice { + Expr::Tuple(ast::ExprTuple { elts: elements, .. }) => elements.iter().collect(), + Expr::Constant(_) => vec![slice], + _ => unreachable!("type slice must be a tuple or a constant"), + } +} + #[derive(Debug)] enum TypingTarget<'a> { + /// Literal `None` type. None, + + /// A `typing.Any` type. Any, + + /// Literal `object` type. Object, + + /// Forward reference to a type e.g., `"List[str]"`. ForwardReference(Expr), + + /// A `typing.Union` type or `|` separated types e.g., `Union[int, str]` + /// or `int | str`. Union(Vec<&'a Expr>), + + /// A `typing.Literal` type e.g., `Literal[1, 2, 3]`. Literal(Vec<&'a Expr>), + + /// A `typing.Optional` type e.g., `Optional[int]`. Optional(&'a Expr), + + /// A `typing.Annotated` type e.g., `Annotated[int, ...]`. Annotated(&'a Expr), - /// Special type used to represent an unknown type (and not a typing target). - /// This could be a type alias. It's also used when parsing a forward - /// reference fails. - /// - /// Note that this is not the same as returning `None` in `try_from_expr` as - /// `None` means that it's a known type but we don't know which one. + /// Special type used to represent an unknown type (and not a typing target) + /// which could be a type alias. Unknown, + + /// Special type used to represent a known type (and not a typing target). + /// A known type is either a builtin type, any object from the standard + /// library, or a type from the `typing_extensions` module. + Known, } impl<'a> TypingTarget<'a> { @@ -82,17 +114,15 @@ impl<'a> TypingTarget<'a> { match expr { Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { if semantic.match_typing_expr(value, "Optional") { - return Some(TypingTarget::Optional(slice.as_ref())); - } - let Expr::Tuple(ast::ExprTuple { elts: elements, .. }) = slice.as_ref() else { - return None; - }; - if semantic.match_typing_expr(value, "Literal") { - Some(TypingTarget::Literal(elements.iter().collect())) + Some(TypingTarget::Optional(slice.as_ref())) + } else if semantic.match_typing_expr(value, "Literal") { + Some(TypingTarget::Literal(resolve_slice_value(slice))) } else if semantic.match_typing_expr(value, "Union") { - Some(TypingTarget::Union(elements.iter().collect())) + Some(TypingTarget::Union(resolve_slice_value(slice))) } else if semantic.match_typing_expr(value, "Annotated") { - elements.first().map(TypingTarget::Annotated) + resolve_slice_value(slice.as_ref()) + .first() + .map(|&expr| TypingTarget::Annotated(expr)) } else { semantic.resolve_call_path(value).map_or( // If we can't resolve the call path, it must be defined @@ -100,7 +130,7 @@ impl<'a> TypingTarget<'a> { Some(TypingTarget::Unknown), |call_path| { if is_known_type(&call_path, minor_version) { - None + Some(TypingTarget::Known) } else { Some(TypingTarget::Unknown) } @@ -120,10 +150,7 @@ impl<'a> TypingTarget<'a> { range, .. }) => parse_type_annotation(string, *range, locator) - // In case of a parse error, we return `Any` to avoid false positives. - .map_or(Some(TypingTarget::Unknown), |(expr, _)| { - Some(TypingTarget::ForwardReference(expr)) - }), + .map_or(None, |(expr, _)| Some(TypingTarget::ForwardReference(expr))), _ => semantic.resolve_call_path(expr).map_or( // If we can't resolve the call path, it must be defined in the // same file, so we assume it's `Any` as it could be a type alias. @@ -137,7 +164,7 @@ impl<'a> TypingTarget<'a> { // If it's not a known type, we assume it's `Any`. Some(TypingTarget::Unknown) } else { - None + Some(TypingTarget::Known) } }, ), @@ -157,45 +184,35 @@ impl<'a> TypingTarget<'a> { | TypingTarget::Any | TypingTarget::Object | TypingTarget::Unknown => true, + TypingTarget::Known => false, TypingTarget::Literal(elements) => elements.iter().any(|element| { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, minor_version) - else { - return false; - }; // Literal can only contain `None`, a literal value, other `Literal` // or an enum value. - match new_target { - TypingTarget::None => true, - TypingTarget::Literal(_) => { + match TypingTarget::try_from_expr(element, semantic, locator, minor_version) { + None | Some(TypingTarget::None) => true, + Some(new_target @ TypingTarget::Literal(_)) => { new_target.contains_none(semantic, locator, minor_version) } _ => false, } }), TypingTarget::Union(elements) => elements.iter().any(|element| { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, minor_version) - else { - return false; - }; - new_target.contains_none(semantic, locator, minor_version) + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_none(semantic, locator, minor_version) + }) }), TypingTarget::Annotated(element) => { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, minor_version) - else { - return false; - }; - new_target.contains_none(semantic, locator, minor_version) + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_none(semantic, locator, minor_version) + }) } TypingTarget::ForwardReference(expr) => { - let Some(new_target) = - TypingTarget::try_from_expr(expr, semantic, locator, minor_version) - else { - return false; - }; - new_target.contains_none(semantic, locator, minor_version) + TypingTarget::try_from_expr(expr, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_none(semantic, locator, minor_version) + }) } } } @@ -213,30 +230,25 @@ impl<'a> TypingTarget<'a> { TypingTarget::Literal(_) | TypingTarget::None | TypingTarget::Object + | TypingTarget::Known | TypingTarget::Unknown => false, TypingTarget::Union(elements) => elements.iter().any(|element| { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, minor_version) - else { - return false; - }; - new_target.contains_any(semantic, locator, minor_version) + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_any(semantic, locator, minor_version) + }) }), TypingTarget::Annotated(element) | TypingTarget::Optional(element) => { - let Some(new_target) = - TypingTarget::try_from_expr(element, semantic, locator, minor_version) - else { - return false; - }; - new_target.contains_any(semantic, locator, minor_version) + TypingTarget::try_from_expr(element, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_any(semantic, locator, minor_version) + }) } TypingTarget::ForwardReference(expr) => { - let Some(new_target) = - TypingTarget::try_from_expr(expr, semantic, locator, minor_version) - else { - return false; - }; - new_target.contains_any(semantic, locator, minor_version) + TypingTarget::try_from_expr(expr, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_any(semantic, locator, minor_version) + }) } } } @@ -255,21 +267,18 @@ pub(crate) fn type_hint_explicitly_allows_none<'a>( locator: &Locator, minor_version: u32, ) -> Option<&'a Expr> { - let Some(target) = TypingTarget::try_from_expr(annotation, semantic, locator, minor_version) - else { - return Some(annotation); - }; - match target { - // Short circuit on top level `None`, `Any` or `Optional` - TypingTarget::None | TypingTarget::Optional(_) | TypingTarget::Any => None, + match TypingTarget::try_from_expr(annotation, semantic, locator, minor_version) { + None | + // Short circuit on top level `None`, `Any` or `Optional` + Some(TypingTarget::None | TypingTarget::Optional(_) | TypingTarget::Any) => None, // Top-level `Annotated` node should check for the inner type and // return the inner type if it doesn't allow `None`. If `Annotated` // is found nested inside another type, then the outer type should // be returned. - TypingTarget::Annotated(expr) => { + Some(TypingTarget::Annotated(expr)) => { type_hint_explicitly_allows_none(expr, semantic, locator, minor_version) } - _ => { + Some(target) => { if target.contains_none(semantic, locator, minor_version) { None } else { @@ -288,19 +297,16 @@ pub(crate) fn type_hint_resolves_to_any( locator: &Locator, minor_version: u32, ) -> bool { - let Some(target) = TypingTarget::try_from_expr(annotation, semantic, locator, minor_version) - else { - return false; - }; - match target { - // Short circuit on top level `Any` - TypingTarget::Any => true, + match TypingTarget::try_from_expr(annotation, semantic, locator, minor_version) { + None | + // Short circuit on top level `Any` + Some(TypingTarget::Any) => true, // Top-level `Annotated` node should check if the inner type resolves // to `Any`. - TypingTarget::Annotated(expr) => { + Some(TypingTarget::Annotated(expr)) => { type_hint_resolves_to_any(expr, semantic, locator, minor_version) } - _ => target.contains_any(semantic, locator, minor_version), + Some(target) => target.contains_any(semantic, locator, minor_version), } } From b082d103091ad6db80110558587b3255b10b1198 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 12 Jul 2023 23:02:45 +0530 Subject: [PATCH 06/13] Wrap the only expr in the vector instead of panic --- crates/ruff/src/rules/ruff/typing.rs | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs index 777f6181779e34..f96ea22d8ef221 100644 --- a/crates/ruff/src/rules/ruff/typing.rs +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -52,18 +52,12 @@ fn is_known_type(call_path: &CallPath, minor_version: u32) -> bool { } } -/// Returns a vector of all the expressions in a slice. -/// -/// ## Panics -/// -/// Panics if the slice is not a tuple or a constant. This should never happen -/// as this function is only used in typing context where the slice cannot be -/// anything else. +/// Returns a vector of all the expressions in a slice. If the slice is not a +/// tuple, it will be wrapped in a vector. fn resolve_slice_value(slice: &Expr) -> Vec<&Expr> { match slice { Expr::Tuple(ast::ExprTuple { elts: elements, .. }) => elements.iter().collect(), - Expr::Constant(_) => vec![slice], - _ => unreachable!("type slice must be a tuple or a constant"), + _ => vec![slice], } } From efbfc9c5af610ad784b11405a59843d2a9a20936 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 12 Jul 2023 23:40:25 +0530 Subject: [PATCH 07/13] Remove bugfix code (results in false negatives) --- ..._ruff__tests__PY39_RUF013_RUF013_0.py.snap | 36 ------------------- ...ules__ruff__tests__RUF013_RUF013_0.py.snap | 36 ------------------- crates/ruff/src/rules/ruff/typing.rs | 25 +++++-------- 3 files changed, 9 insertions(+), 88 deletions(-) diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap index 9ee735a80b1669..7d4b9a9f004034 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap @@ -37,42 +37,6 @@ RUF013_0.py:25:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 27 27 | 28 28 | -RUF013_0.py:29:12: RUF013 [*] PEP 484 prohibits implicit `Optional` - | -29 | def f(arg: typing.List[str] = None): # RUF013 - | ^^^^^^^^^^^^^^^^ RUF013 -30 | pass - | - = help: Convert to `Optional[T]` - -ℹ Suggested fix -26 26 | pass -27 27 | -28 28 | -29 |-def f(arg: typing.List[str] = None): # RUF013 - 29 |+def f(arg: Optional[typing.List[str]] = None): # RUF013 -30 30 | pass -31 31 | -32 32 | - -RUF013_0.py:33:12: RUF013 [*] PEP 484 prohibits implicit `Optional` - | -33 | def f(arg: Tuple[str] = None): # RUF013 - | ^^^^^^^^^^ RUF013 -34 | pass - | - = help: Convert to `Optional[T]` - -ℹ Suggested fix -30 30 | pass -31 31 | -32 32 | -33 |-def f(arg: Tuple[str] = None): # RUF013 - 33 |+def f(arg: Optional[Tuple[str]] = None): # RUF013 -34 34 | pass -35 35 | -36 36 | - RUF013_0.py:67:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | 67 | def f(arg: Union = None): # RUF013 diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap index 2f21fcd56b6a4a..341aecce5e0d2e 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap @@ -37,42 +37,6 @@ RUF013_0.py:25:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 27 27 | 28 28 | -RUF013_0.py:29:12: RUF013 [*] PEP 484 prohibits implicit `Optional` - | -29 | def f(arg: typing.List[str] = None): # RUF013 - | ^^^^^^^^^^^^^^^^ RUF013 -30 | pass - | - = help: Convert to `T | None` - -ℹ Suggested fix -26 26 | pass -27 27 | -28 28 | -29 |-def f(arg: typing.List[str] = None): # RUF013 - 29 |+def f(arg: typing.List[str] | None = None): # RUF013 -30 30 | pass -31 31 | -32 32 | - -RUF013_0.py:33:12: RUF013 [*] PEP 484 prohibits implicit `Optional` - | -33 | def f(arg: Tuple[str] = None): # RUF013 - | ^^^^^^^^^^ RUF013 -34 | pass - | - = help: Convert to `T | None` - -ℹ Suggested fix -30 30 | pass -31 31 | -32 32 | -33 |-def f(arg: Tuple[str] = None): # RUF013 - 33 |+def f(arg: Tuple[str] | None = None): # RUF013 -34 34 | pass -35 35 | -36 36 | - RUF013_0.py:67:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | 67 | def f(arg: Union = None): # RUF013 diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs index f96ea22d8ef221..127c213fef3919 100644 --- a/crates/ruff/src/rules/ruff/typing.rs +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -52,15 +52,6 @@ fn is_known_type(call_path: &CallPath, minor_version: u32) -> bool { } } -/// Returns a vector of all the expressions in a slice. If the slice is not a -/// tuple, it will be wrapped in a vector. -fn resolve_slice_value(slice: &Expr) -> Vec<&Expr> { - match slice { - Expr::Tuple(ast::ExprTuple { elts: elements, .. }) => elements.iter().collect(), - _ => vec![slice], - } -} - #[derive(Debug)] enum TypingTarget<'a> { /// Literal `None` type. @@ -108,15 +99,17 @@ impl<'a> TypingTarget<'a> { match expr { Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { if semantic.match_typing_expr(value, "Optional") { - Some(TypingTarget::Optional(slice.as_ref())) - } else if semantic.match_typing_expr(value, "Literal") { - Some(TypingTarget::Literal(resolve_slice_value(slice))) + return Some(TypingTarget::Optional(slice.as_ref())); + } + let Expr::Tuple(ast::ExprTuple { elts: elements, .. }) = slice.as_ref() else { + return None; + }; + if semantic.match_typing_expr(value, "Literal") { + Some(TypingTarget::Literal(elements.iter().collect())) } else if semantic.match_typing_expr(value, "Union") { - Some(TypingTarget::Union(resolve_slice_value(slice))) + Some(TypingTarget::Union(elements.iter().collect())) } else if semantic.match_typing_expr(value, "Annotated") { - resolve_slice_value(slice.as_ref()) - .first() - .map(|&expr| TypingTarget::Annotated(expr)) + elements.first().map(TypingTarget::Annotated) } else { semantic.resolve_call_path(value).map_or( // If we can't resolve the call path, it must be defined From 01866f94c35ad88093a6d6f5e77d17b4fe08f70c Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 13 Jul 2023 12:11:16 +0530 Subject: [PATCH 08/13] Temporarily disable `ANN401` in scripts --- scripts/pyproject.toml | 1 + 1 file changed, 1 insertion(+) diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index bdb5a08d5ee4fd..1a3a318a1a36aa 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -19,6 +19,7 @@ ignore = [ "T", # flake8-print "FBT", # flake8-boolean-trap "PERF", # perflint + "ANN401", ] [tool.ruff.isort] From aa5d0d1639b25863e5079f4fd6048020e1c0949c Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 12 Jul 2023 23:06:35 +0530 Subject: [PATCH 09/13] Update implicit optional tests for coverage --- .../resources/test/fixtures/ruff/RUF013_0.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/crates/ruff/resources/test/fixtures/ruff/RUF013_0.py b/crates/ruff/resources/test/fixtures/ruff/RUF013_0.py index 21c59089798958..e4807c9f5ac611 100644 --- a/crates/ruff/resources/test/fixtures/ruff/RUF013_0.py +++ b/crates/ruff/resources/test/fixtures/ruff/RUF013_0.py @@ -48,6 +48,10 @@ def f(arg: typing.Optional[int] = None): # Union +def f(arg: Union[None] = None): + pass + + def f(arg: Union[None, int] = None): pass @@ -68,6 +72,10 @@ def f(arg: Union = None): # RUF013 pass +def f(arg: Union[int] = None): # RUF013 + pass + + def f(arg: Union[int, str] = None): # RUF013 pass @@ -106,10 +114,18 @@ def f(arg: None = None): pass +def f(arg: Literal[None] = None): + pass + + def f(arg: Literal[1, 2, None, 3] = None): pass +def f(arg: Literal[1] = None): # RUF013 + pass + + def f(arg: Literal[1, "foo"] = None): # RUF013 pass From f79d1b143950ebf4ad1f2149023ab9aea2ba4b36 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 12 Jul 2023 23:44:46 +0530 Subject: [PATCH 10/13] Account for one element subscript inside typing --- ...ules__ruff__tests__RUF013_RUF013_0.py.snap | 438 ++++++++++-------- crates/ruff/src/rules/ruff/typing.rs | 25 +- 2 files changed, 271 insertions(+), 192 deletions(-) diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap index 341aecce5e0d2e..3ff56b6bc20d90 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__RUF013_RUF013_0.py.snap @@ -37,28 +37,46 @@ RUF013_0.py:25:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 27 27 | 28 28 | -RUF013_0.py:67:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:29:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -67 | def f(arg: Union = None): # RUF013 - | ^^^^^ RUF013 -68 | pass +29 | def f(arg: typing.List[str] = None): # RUF013 + | ^^^^^^^^^^^^^^^^ RUF013 +30 | pass | = help: Convert to `T | None` ℹ Suggested fix -64 64 | pass -65 65 | -66 66 | -67 |-def f(arg: Union = None): # RUF013 - 67 |+def f(arg: Union | None = None): # RUF013 -68 68 | pass -69 69 | -70 70 | +26 26 | pass +27 27 | +28 28 | +29 |-def f(arg: typing.List[str] = None): # RUF013 + 29 |+def f(arg: typing.List[str] | None = None): # RUF013 +30 30 | pass +31 31 | +32 32 | + +RUF013_0.py:33:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +33 | def f(arg: Tuple[str] = None): # RUF013 + | ^^^^^^^^^^ RUF013 +34 | pass + | + = help: Convert to `T | None` + +ℹ Suggested fix +30 30 | pass +31 31 | +32 32 | +33 |-def f(arg: Tuple[str] = None): # RUF013 + 33 |+def f(arg: Tuple[str] | None = None): # RUF013 +34 34 | pass +35 35 | +36 36 | RUF013_0.py:71:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -71 | def f(arg: Union[int, str] = None): # RUF013 - | ^^^^^^^^^^^^^^^ RUF013 +71 | def f(arg: Union = None): # RUF013 + | ^^^^^ RUF013 72 | pass | = help: Convert to `T | None` @@ -67,16 +85,16 @@ RUF013_0.py:71:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 68 68 | pass 69 69 | 70 70 | -71 |-def f(arg: Union[int, str] = None): # RUF013 - 71 |+def f(arg: Union[int, str] | None = None): # RUF013 +71 |-def f(arg: Union = None): # RUF013 + 71 |+def f(arg: Union | None = None): # RUF013 72 72 | pass 73 73 | 74 74 | RUF013_0.py:75:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -75 | def f(arg: typing.Union[int, str] = None): # RUF013 - | ^^^^^^^^^^^^^^^^^^^^^^ RUF013 +75 | def f(arg: Union[int] = None): # RUF013 + | ^^^^^^^^^^ RUF013 76 | pass | = help: Convert to `T | None` @@ -85,260 +103,314 @@ RUF013_0.py:75:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 72 72 | pass 73 73 | 74 74 | -75 |-def f(arg: typing.Union[int, str] = None): # RUF013 - 75 |+def f(arg: typing.Union[int, str] | None = None): # RUF013 +75 |-def f(arg: Union[int] = None): # RUF013 + 75 |+def f(arg: Union[int] | None = None): # RUF013 76 76 | pass 77 77 | 78 78 | -RUF013_0.py:94:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:79:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -94 | def f(arg: int | float = None): # RUF013 - | ^^^^^^^^^^^ RUF013 -95 | pass +79 | def f(arg: Union[int, str] = None): # RUF013 + | ^^^^^^^^^^^^^^^ RUF013 +80 | pass | = help: Convert to `T | None` ℹ Suggested fix -91 91 | pass -92 92 | -93 93 | -94 |-def f(arg: int | float = None): # RUF013 - 94 |+def f(arg: int | float | None = None): # RUF013 -95 95 | pass -96 96 | -97 97 | - -RUF013_0.py:98:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +76 76 | pass +77 77 | +78 78 | +79 |-def f(arg: Union[int, str] = None): # RUF013 + 79 |+def f(arg: Union[int, str] | None = None): # RUF013 +80 80 | pass +81 81 | +82 82 | + +RUF013_0.py:83:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -98 | def f(arg: int | float | str | bytes = None): # RUF013 - | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 -99 | pass +83 | def f(arg: typing.Union[int, str] = None): # RUF013 + | ^^^^^^^^^^^^^^^^^^^^^^ RUF013 +84 | pass | = help: Convert to `T | None` ℹ Suggested fix -95 95 | pass -96 96 | -97 97 | -98 |-def f(arg: int | float | str | bytes = None): # RUF013 - 98 |+def f(arg: int | float | str | bytes | None = None): # RUF013 -99 99 | pass +80 80 | pass +81 81 | +82 82 | +83 |-def f(arg: typing.Union[int, str] = None): # RUF013 + 83 |+def f(arg: typing.Union[int, str] | None = None): # RUF013 +84 84 | pass +85 85 | +86 86 | + +RUF013_0.py:102:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +102 | def f(arg: int | float = None): # RUF013 + | ^^^^^^^^^^^ RUF013 +103 | pass + | + = help: Convert to `T | None` + +ℹ Suggested fix +99 99 | pass 100 100 | 101 101 | +102 |-def f(arg: int | float = None): # RUF013 + 102 |+def f(arg: int | float | None = None): # RUF013 +103 103 | pass +104 104 | +105 105 | + +RUF013_0.py:106:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +106 | def f(arg: int | float | str | bytes = None): # RUF013 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 +107 | pass + | + = help: Convert to `T | None` -RUF013_0.py:113:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +ℹ Suggested fix +103 103 | pass +104 104 | +105 105 | +106 |-def f(arg: int | float | str | bytes = None): # RUF013 + 106 |+def f(arg: int | float | str | bytes | None = None): # RUF013 +107 107 | pass +108 108 | +109 109 | + +RUF013_0.py:125:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +125 | def f(arg: Literal[1] = None): # RUF013 + | ^^^^^^^^^^ RUF013 +126 | pass + | + = help: Convert to `T | None` + +ℹ Suggested fix +122 122 | pass +123 123 | +124 124 | +125 |-def f(arg: Literal[1] = None): # RUF013 + 125 |+def f(arg: Literal[1] | None = None): # RUF013 +126 126 | pass +127 127 | +128 128 | + +RUF013_0.py:129:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -113 | def f(arg: Literal[1, "foo"] = None): # RUF013 +129 | def f(arg: Literal[1, "foo"] = None): # RUF013 | ^^^^^^^^^^^^^^^^^ RUF013 -114 | pass +130 | pass | = help: Convert to `T | None` ℹ Suggested fix -110 110 | pass -111 111 | -112 112 | -113 |-def f(arg: Literal[1, "foo"] = None): # RUF013 - 113 |+def f(arg: Literal[1, "foo"] | None = None): # RUF013 -114 114 | pass -115 115 | -116 116 | - -RUF013_0.py:117:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +126 126 | pass +127 127 | +128 128 | +129 |-def f(arg: Literal[1, "foo"] = None): # RUF013 + 129 |+def f(arg: Literal[1, "foo"] | None = None): # RUF013 +130 130 | pass +131 131 | +132 132 | + +RUF013_0.py:133:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -117 | def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 +133 | def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 -118 | pass +134 | pass | = help: Convert to `T | None` ℹ Suggested fix -114 114 | pass -115 115 | -116 116 | -117 |-def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 - 117 |+def f(arg: typing.Literal[1, "foo", True] | None = None): # RUF013 -118 118 | pass -119 119 | -120 120 | - -RUF013_0.py:136:22: RUF013 [*] PEP 484 prohibits implicit `Optional` +130 130 | pass +131 131 | +132 132 | +133 |-def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 + 133 |+def f(arg: typing.Literal[1, "foo", True] | None = None): # RUF013 +134 134 | pass +135 135 | +136 136 | + +RUF013_0.py:152:22: RUF013 [*] PEP 484 prohibits implicit `Optional` | -136 | def f(arg: Annotated[int, ...] = None): # RUF013 +152 | def f(arg: Annotated[int, ...] = None): # RUF013 | ^^^ RUF013 -137 | pass +153 | pass | = help: Convert to `T | None` ℹ Suggested fix -133 133 | pass -134 134 | -135 135 | -136 |-def f(arg: Annotated[int, ...] = None): # RUF013 - 136 |+def f(arg: Annotated[int | None, ...] = None): # RUF013 -137 137 | pass -138 138 | -139 139 | +149 149 | pass +150 150 | +151 151 | +152 |-def f(arg: Annotated[int, ...] = None): # RUF013 + 152 |+def f(arg: Annotated[int | None, ...] = None): # RUF013 +153 153 | pass +154 154 | +155 155 | -RUF013_0.py:140:32: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:156:32: RUF013 [*] PEP 484 prohibits implicit `Optional` | -140 | def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 +156 | def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 | ^^^^^^^^^ RUF013 -141 | pass +157 | pass | = help: Convert to `T | None` ℹ Suggested fix -137 137 | pass -138 138 | -139 139 | -140 |-def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 - 140 |+def f(arg: Annotated[Annotated[int | str | None, ...], ...] = None): # RUF013 -141 141 | pass -142 142 | -143 143 | - -RUF013_0.py:156:11: RUF013 [*] PEP 484 prohibits implicit `Optional` +153 153 | pass +154 154 | +155 155 | +156 |-def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 + 156 |+def f(arg: Annotated[Annotated[int | str | None, ...], ...] = None): # RUF013 +157 157 | pass +158 158 | +159 159 | + +RUF013_0.py:172:11: RUF013 [*] PEP 484 prohibits implicit `Optional` | -155 | def f( -156 | arg1: int = None, # RUF013 +171 | def f( +172 | arg1: int = None, # RUF013 | ^^^ RUF013 -157 | arg2: Union[int, float] = None, # RUF013 -158 | arg3: Literal[1, 2, 3] = None, # RUF013 +173 | arg2: Union[int, float] = None, # RUF013 +174 | arg3: Literal[1, 2, 3] = None, # RUF013 | = help: Convert to `T | None` ℹ Suggested fix -153 153 | -154 154 | -155 155 | def f( -156 |- arg1: int = None, # RUF013 - 156 |+ arg1: int | None = None, # RUF013 -157 157 | arg2: Union[int, float] = None, # RUF013 -158 158 | arg3: Literal[1, 2, 3] = None, # RUF013 -159 159 | ): - -RUF013_0.py:157:11: RUF013 [*] PEP 484 prohibits implicit `Optional` +169 169 | +170 170 | +171 171 | def f( +172 |- arg1: int = None, # RUF013 + 172 |+ arg1: int | None = None, # RUF013 +173 173 | arg2: Union[int, float] = None, # RUF013 +174 174 | arg3: Literal[1, 2, 3] = None, # RUF013 +175 175 | ): + +RUF013_0.py:173:11: RUF013 [*] PEP 484 prohibits implicit `Optional` | -155 | def f( -156 | arg1: int = None, # RUF013 -157 | arg2: Union[int, float] = None, # RUF013 +171 | def f( +172 | arg1: int = None, # RUF013 +173 | arg2: Union[int, float] = None, # RUF013 | ^^^^^^^^^^^^^^^^^ RUF013 -158 | arg3: Literal[1, 2, 3] = None, # RUF013 -159 | ): +174 | arg3: Literal[1, 2, 3] = None, # RUF013 +175 | ): | = help: Convert to `T | None` ℹ Suggested fix -154 154 | -155 155 | def f( -156 156 | arg1: int = None, # RUF013 -157 |- arg2: Union[int, float] = None, # RUF013 - 157 |+ arg2: Union[int, float] | None = None, # RUF013 -158 158 | arg3: Literal[1, 2, 3] = None, # RUF013 -159 159 | ): -160 160 | pass - -RUF013_0.py:158:11: RUF013 [*] PEP 484 prohibits implicit `Optional` +170 170 | +171 171 | def f( +172 172 | arg1: int = None, # RUF013 +173 |- arg2: Union[int, float] = None, # RUF013 + 173 |+ arg2: Union[int, float] | None = None, # RUF013 +174 174 | arg3: Literal[1, 2, 3] = None, # RUF013 +175 175 | ): +176 176 | pass + +RUF013_0.py:174:11: RUF013 [*] PEP 484 prohibits implicit `Optional` | -156 | arg1: int = None, # RUF013 -157 | arg2: Union[int, float] = None, # RUF013 -158 | arg3: Literal[1, 2, 3] = None, # RUF013 +172 | arg1: int = None, # RUF013 +173 | arg2: Union[int, float] = None, # RUF013 +174 | arg3: Literal[1, 2, 3] = None, # RUF013 | ^^^^^^^^^^^^^^^^ RUF013 -159 | ): -160 | pass +175 | ): +176 | pass | = help: Convert to `T | None` ℹ Suggested fix -155 155 | def f( -156 156 | arg1: int = None, # RUF013 -157 157 | arg2: Union[int, float] = None, # RUF013 -158 |- arg3: Literal[1, 2, 3] = None, # RUF013 - 158 |+ arg3: Literal[1, 2, 3] | None = None, # RUF013 -159 159 | ): -160 160 | pass -161 161 | - -RUF013_0.py:186:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +171 171 | def f( +172 172 | arg1: int = None, # RUF013 +173 173 | arg2: Union[int, float] = None, # RUF013 +174 |- arg3: Literal[1, 2, 3] = None, # RUF013 + 174 |+ arg3: Literal[1, 2, 3] | None = None, # RUF013 +175 175 | ): +176 176 | pass +177 177 | + +RUF013_0.py:202:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -186 | def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 +202 | def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 -187 | pass +203 | pass | = help: Convert to `T | None` ℹ Suggested fix -183 183 | pass -184 184 | -185 185 | -186 |-def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 - 186 |+def f(arg: Union[Annotated[int, ...], Union[str, bytes]] | None = None): # RUF013 -187 187 | pass -188 188 | -189 189 | - -RUF013_0.py:193:13: RUF013 [*] PEP 484 prohibits implicit `Optional` +199 199 | pass +200 200 | +201 201 | +202 |-def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 + 202 |+def f(arg: Union[Annotated[int, ...], Union[str, bytes]] | None = None): # RUF013 +203 203 | pass +204 204 | +205 205 | + +RUF013_0.py:209:13: RUF013 [*] PEP 484 prohibits implicit `Optional` | -193 | def f(arg: "int" = None): # RUF013 +209 | def f(arg: "int" = None): # RUF013 | ^^^ RUF013 -194 | pass +210 | pass | = help: Convert to `T | None` ℹ Suggested fix -190 190 | # Quoted -191 191 | -192 192 | -193 |-def f(arg: "int" = None): # RUF013 - 193 |+def f(arg: "int | None" = None): # RUF013 -194 194 | pass -195 195 | -196 196 | - -RUF013_0.py:197:13: RUF013 [*] PEP 484 prohibits implicit `Optional` +206 206 | # Quoted +207 207 | +208 208 | +209 |-def f(arg: "int" = None): # RUF013 + 209 |+def f(arg: "int | None" = None): # RUF013 +210 210 | pass +211 211 | +212 212 | + +RUF013_0.py:213:13: RUF013 [*] PEP 484 prohibits implicit `Optional` | -197 | def f(arg: "str" = None): # RUF013 +213 | def f(arg: "str" = None): # RUF013 | ^^^ RUF013 -198 | pass +214 | pass | = help: Convert to `T | None` ℹ Suggested fix -194 194 | pass -195 195 | -196 196 | -197 |-def f(arg: "str" = None): # RUF013 - 197 |+def f(arg: "str | None" = None): # RUF013 -198 198 | pass -199 199 | -200 200 | +210 210 | pass +211 211 | +212 212 | +213 |-def f(arg: "str" = None): # RUF013 + 213 |+def f(arg: "str | None" = None): # RUF013 +214 214 | pass +215 215 | +216 216 | -RUF013_0.py:201:12: RUF013 PEP 484 prohibits implicit `Optional` +RUF013_0.py:217:12: RUF013 PEP 484 prohibits implicit `Optional` | -201 | def f(arg: "st" "r" = None): # RUF013 +217 | def f(arg: "st" "r" = None): # RUF013 | ^^^^^^^^ RUF013 -202 | pass +218 | pass | = help: Convert to `T | None` -RUF013_0.py:209:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:225:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -209 | def f(arg: Union["int", "str"] = None): # RUF013 +225 | def f(arg: Union["int", "str"] = None): # RUF013 | ^^^^^^^^^^^^^^^^^^^ RUF013 -210 | pass +226 | pass | = help: Convert to `T | None` ℹ Suggested fix -206 206 | pass -207 207 | -208 208 | -209 |-def f(arg: Union["int", "str"] = None): # RUF013 - 209 |+def f(arg: Union["int", "str"] | None = None): # RUF013 -210 210 | pass -211 211 | -212 212 | +222 222 | pass +223 223 | +224 224 | +225 |-def f(arg: Union["int", "str"] = None): # RUF013 + 225 |+def f(arg: Union["int", "str"] | None = None): # RUF013 +226 226 | pass +227 227 | +228 228 | diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs index 127c213fef3919..f96ea22d8ef221 100644 --- a/crates/ruff/src/rules/ruff/typing.rs +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -52,6 +52,15 @@ fn is_known_type(call_path: &CallPath, minor_version: u32) -> bool { } } +/// Returns a vector of all the expressions in a slice. If the slice is not a +/// tuple, it will be wrapped in a vector. +fn resolve_slice_value(slice: &Expr) -> Vec<&Expr> { + match slice { + Expr::Tuple(ast::ExprTuple { elts: elements, .. }) => elements.iter().collect(), + _ => vec![slice], + } +} + #[derive(Debug)] enum TypingTarget<'a> { /// Literal `None` type. @@ -99,17 +108,15 @@ impl<'a> TypingTarget<'a> { match expr { Expr::Subscript(ast::ExprSubscript { value, slice, .. }) => { if semantic.match_typing_expr(value, "Optional") { - return Some(TypingTarget::Optional(slice.as_ref())); - } - let Expr::Tuple(ast::ExprTuple { elts: elements, .. }) = slice.as_ref() else { - return None; - }; - if semantic.match_typing_expr(value, "Literal") { - Some(TypingTarget::Literal(elements.iter().collect())) + Some(TypingTarget::Optional(slice.as_ref())) + } else if semantic.match_typing_expr(value, "Literal") { + Some(TypingTarget::Literal(resolve_slice_value(slice))) } else if semantic.match_typing_expr(value, "Union") { - Some(TypingTarget::Union(elements.iter().collect())) + Some(TypingTarget::Union(resolve_slice_value(slice))) } else if semantic.match_typing_expr(value, "Annotated") { - elements.first().map(TypingTarget::Annotated) + resolve_slice_value(slice.as_ref()) + .first() + .map(|&expr| TypingTarget::Annotated(expr)) } else { semantic.resolve_call_path(value).map_or( // If we can't resolve the call path, it must be defined From 9e8416ea58dcb5dfaeae77abb3f534b876682bdb Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 13 Jul 2023 00:46:52 +0530 Subject: [PATCH 11/13] Increase max iteration in tests --- ..._ruff__tests__PY39_RUF013_RUF013_0.py.snap | 438 ++++++++++-------- crates/ruff/src/test.rs | 2 +- 2 files changed, 256 insertions(+), 184 deletions(-) diff --git a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap index 7d4b9a9f004034..1e25245a2a42aa 100644 --- a/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap +++ b/crates/ruff/src/rules/ruff/snapshots/ruff__rules__ruff__tests__PY39_RUF013_RUF013_0.py.snap @@ -37,28 +37,46 @@ RUF013_0.py:25:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 27 27 | 28 28 | -RUF013_0.py:67:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:29:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -67 | def f(arg: Union = None): # RUF013 - | ^^^^^ RUF013 -68 | pass +29 | def f(arg: typing.List[str] = None): # RUF013 + | ^^^^^^^^^^^^^^^^ RUF013 +30 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -64 64 | pass -65 65 | -66 66 | -67 |-def f(arg: Union = None): # RUF013 - 67 |+def f(arg: Optional[Union] = None): # RUF013 -68 68 | pass -69 69 | -70 70 | +26 26 | pass +27 27 | +28 28 | +29 |-def f(arg: typing.List[str] = None): # RUF013 + 29 |+def f(arg: Optional[typing.List[str]] = None): # RUF013 +30 30 | pass +31 31 | +32 32 | + +RUF013_0.py:33:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +33 | def f(arg: Tuple[str] = None): # RUF013 + | ^^^^^^^^^^ RUF013 +34 | pass + | + = help: Convert to `Optional[T]` + +ℹ Suggested fix +30 30 | pass +31 31 | +32 32 | +33 |-def f(arg: Tuple[str] = None): # RUF013 + 33 |+def f(arg: Optional[Tuple[str]] = None): # RUF013 +34 34 | pass +35 35 | +36 36 | RUF013_0.py:71:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -71 | def f(arg: Union[int, str] = None): # RUF013 - | ^^^^^^^^^^^^^^^ RUF013 +71 | def f(arg: Union = None): # RUF013 + | ^^^^^ RUF013 72 | pass | = help: Convert to `Optional[T]` @@ -67,16 +85,16 @@ RUF013_0.py:71:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 68 68 | pass 69 69 | 70 70 | -71 |-def f(arg: Union[int, str] = None): # RUF013 - 71 |+def f(arg: Optional[Union[int, str]] = None): # RUF013 +71 |-def f(arg: Union = None): # RUF013 + 71 |+def f(arg: Optional[Union] = None): # RUF013 72 72 | pass 73 73 | 74 74 | RUF013_0.py:75:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -75 | def f(arg: typing.Union[int, str] = None): # RUF013 - | ^^^^^^^^^^^^^^^^^^^^^^ RUF013 +75 | def f(arg: Union[int] = None): # RUF013 + | ^^^^^^^^^^ RUF013 76 | pass | = help: Convert to `Optional[T]` @@ -85,260 +103,314 @@ RUF013_0.py:75:12: RUF013 [*] PEP 484 prohibits implicit `Optional` 72 72 | pass 73 73 | 74 74 | -75 |-def f(arg: typing.Union[int, str] = None): # RUF013 - 75 |+def f(arg: Optional[typing.Union[int, str]] = None): # RUF013 +75 |-def f(arg: Union[int] = None): # RUF013 + 75 |+def f(arg: Optional[Union[int]] = None): # RUF013 76 76 | pass 77 77 | 78 78 | -RUF013_0.py:94:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:79:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -94 | def f(arg: int | float = None): # RUF013 - | ^^^^^^^^^^^ RUF013 -95 | pass +79 | def f(arg: Union[int, str] = None): # RUF013 + | ^^^^^^^^^^^^^^^ RUF013 +80 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -91 91 | pass -92 92 | -93 93 | -94 |-def f(arg: int | float = None): # RUF013 - 94 |+def f(arg: Optional[int | float] = None): # RUF013 -95 95 | pass -96 96 | -97 97 | - -RUF013_0.py:98:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +76 76 | pass +77 77 | +78 78 | +79 |-def f(arg: Union[int, str] = None): # RUF013 + 79 |+def f(arg: Optional[Union[int, str]] = None): # RUF013 +80 80 | pass +81 81 | +82 82 | + +RUF013_0.py:83:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -98 | def f(arg: int | float | str | bytes = None): # RUF013 - | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 -99 | pass +83 | def f(arg: typing.Union[int, str] = None): # RUF013 + | ^^^^^^^^^^^^^^^^^^^^^^ RUF013 +84 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -95 95 | pass -96 96 | -97 97 | -98 |-def f(arg: int | float | str | bytes = None): # RUF013 - 98 |+def f(arg: Optional[int | float | str | bytes] = None): # RUF013 -99 99 | pass +80 80 | pass +81 81 | +82 82 | +83 |-def f(arg: typing.Union[int, str] = None): # RUF013 + 83 |+def f(arg: Optional[typing.Union[int, str]] = None): # RUF013 +84 84 | pass +85 85 | +86 86 | + +RUF013_0.py:102:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +102 | def f(arg: int | float = None): # RUF013 + | ^^^^^^^^^^^ RUF013 +103 | pass + | + = help: Convert to `Optional[T]` + +ℹ Suggested fix +99 99 | pass 100 100 | 101 101 | +102 |-def f(arg: int | float = None): # RUF013 + 102 |+def f(arg: Optional[int | float] = None): # RUF013 +103 103 | pass +104 104 | +105 105 | + +RUF013_0.py:106:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +106 | def f(arg: int | float | str | bytes = None): # RUF013 + | ^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 +107 | pass + | + = help: Convert to `Optional[T]` -RUF013_0.py:113:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +ℹ Suggested fix +103 103 | pass +104 104 | +105 105 | +106 |-def f(arg: int | float | str | bytes = None): # RUF013 + 106 |+def f(arg: Optional[int | float | str | bytes] = None): # RUF013 +107 107 | pass +108 108 | +109 109 | + +RUF013_0.py:125:12: RUF013 [*] PEP 484 prohibits implicit `Optional` + | +125 | def f(arg: Literal[1] = None): # RUF013 + | ^^^^^^^^^^ RUF013 +126 | pass + | + = help: Convert to `Optional[T]` + +ℹ Suggested fix +122 122 | pass +123 123 | +124 124 | +125 |-def f(arg: Literal[1] = None): # RUF013 + 125 |+def f(arg: Optional[Literal[1]] = None): # RUF013 +126 126 | pass +127 127 | +128 128 | + +RUF013_0.py:129:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -113 | def f(arg: Literal[1, "foo"] = None): # RUF013 +129 | def f(arg: Literal[1, "foo"] = None): # RUF013 | ^^^^^^^^^^^^^^^^^ RUF013 -114 | pass +130 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -110 110 | pass -111 111 | -112 112 | -113 |-def f(arg: Literal[1, "foo"] = None): # RUF013 - 113 |+def f(arg: Optional[Literal[1, "foo"]] = None): # RUF013 -114 114 | pass -115 115 | -116 116 | - -RUF013_0.py:117:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +126 126 | pass +127 127 | +128 128 | +129 |-def f(arg: Literal[1, "foo"] = None): # RUF013 + 129 |+def f(arg: Optional[Literal[1, "foo"]] = None): # RUF013 +130 130 | pass +131 131 | +132 132 | + +RUF013_0.py:133:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -117 | def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 +133 | def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 -118 | pass +134 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -114 114 | pass -115 115 | -116 116 | -117 |-def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 - 117 |+def f(arg: Optional[typing.Literal[1, "foo", True]] = None): # RUF013 -118 118 | pass -119 119 | -120 120 | - -RUF013_0.py:136:22: RUF013 [*] PEP 484 prohibits implicit `Optional` +130 130 | pass +131 131 | +132 132 | +133 |-def f(arg: typing.Literal[1, "foo", True] = None): # RUF013 + 133 |+def f(arg: Optional[typing.Literal[1, "foo", True]] = None): # RUF013 +134 134 | pass +135 135 | +136 136 | + +RUF013_0.py:152:22: RUF013 [*] PEP 484 prohibits implicit `Optional` | -136 | def f(arg: Annotated[int, ...] = None): # RUF013 +152 | def f(arg: Annotated[int, ...] = None): # RUF013 | ^^^ RUF013 -137 | pass +153 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -133 133 | pass -134 134 | -135 135 | -136 |-def f(arg: Annotated[int, ...] = None): # RUF013 - 136 |+def f(arg: Annotated[Optional[int], ...] = None): # RUF013 -137 137 | pass -138 138 | -139 139 | +149 149 | pass +150 150 | +151 151 | +152 |-def f(arg: Annotated[int, ...] = None): # RUF013 + 152 |+def f(arg: Annotated[Optional[int], ...] = None): # RUF013 +153 153 | pass +154 154 | +155 155 | -RUF013_0.py:140:32: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:156:32: RUF013 [*] PEP 484 prohibits implicit `Optional` | -140 | def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 +156 | def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 | ^^^^^^^^^ RUF013 -141 | pass +157 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -137 137 | pass -138 138 | -139 139 | -140 |-def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 - 140 |+def f(arg: Annotated[Annotated[Optional[int | str], ...], ...] = None): # RUF013 -141 141 | pass -142 142 | -143 143 | - -RUF013_0.py:156:11: RUF013 [*] PEP 484 prohibits implicit `Optional` +153 153 | pass +154 154 | +155 155 | +156 |-def f(arg: Annotated[Annotated[int | str, ...], ...] = None): # RUF013 + 156 |+def f(arg: Annotated[Annotated[Optional[int | str], ...], ...] = None): # RUF013 +157 157 | pass +158 158 | +159 159 | + +RUF013_0.py:172:11: RUF013 [*] PEP 484 prohibits implicit `Optional` | -155 | def f( -156 | arg1: int = None, # RUF013 +171 | def f( +172 | arg1: int = None, # RUF013 | ^^^ RUF013 -157 | arg2: Union[int, float] = None, # RUF013 -158 | arg3: Literal[1, 2, 3] = None, # RUF013 +173 | arg2: Union[int, float] = None, # RUF013 +174 | arg3: Literal[1, 2, 3] = None, # RUF013 | = help: Convert to `Optional[T]` ℹ Suggested fix -153 153 | -154 154 | -155 155 | def f( -156 |- arg1: int = None, # RUF013 - 156 |+ arg1: Optional[int] = None, # RUF013 -157 157 | arg2: Union[int, float] = None, # RUF013 -158 158 | arg3: Literal[1, 2, 3] = None, # RUF013 -159 159 | ): - -RUF013_0.py:157:11: RUF013 [*] PEP 484 prohibits implicit `Optional` +169 169 | +170 170 | +171 171 | def f( +172 |- arg1: int = None, # RUF013 + 172 |+ arg1: Optional[int] = None, # RUF013 +173 173 | arg2: Union[int, float] = None, # RUF013 +174 174 | arg3: Literal[1, 2, 3] = None, # RUF013 +175 175 | ): + +RUF013_0.py:173:11: RUF013 [*] PEP 484 prohibits implicit `Optional` | -155 | def f( -156 | arg1: int = None, # RUF013 -157 | arg2: Union[int, float] = None, # RUF013 +171 | def f( +172 | arg1: int = None, # RUF013 +173 | arg2: Union[int, float] = None, # RUF013 | ^^^^^^^^^^^^^^^^^ RUF013 -158 | arg3: Literal[1, 2, 3] = None, # RUF013 -159 | ): +174 | arg3: Literal[1, 2, 3] = None, # RUF013 +175 | ): | = help: Convert to `Optional[T]` ℹ Suggested fix -154 154 | -155 155 | def f( -156 156 | arg1: int = None, # RUF013 -157 |- arg2: Union[int, float] = None, # RUF013 - 157 |+ arg2: Optional[Union[int, float]] = None, # RUF013 -158 158 | arg3: Literal[1, 2, 3] = None, # RUF013 -159 159 | ): -160 160 | pass - -RUF013_0.py:158:11: RUF013 [*] PEP 484 prohibits implicit `Optional` +170 170 | +171 171 | def f( +172 172 | arg1: int = None, # RUF013 +173 |- arg2: Union[int, float] = None, # RUF013 + 173 |+ arg2: Optional[Union[int, float]] = None, # RUF013 +174 174 | arg3: Literal[1, 2, 3] = None, # RUF013 +175 175 | ): +176 176 | pass + +RUF013_0.py:174:11: RUF013 [*] PEP 484 prohibits implicit `Optional` | -156 | arg1: int = None, # RUF013 -157 | arg2: Union[int, float] = None, # RUF013 -158 | arg3: Literal[1, 2, 3] = None, # RUF013 +172 | arg1: int = None, # RUF013 +173 | arg2: Union[int, float] = None, # RUF013 +174 | arg3: Literal[1, 2, 3] = None, # RUF013 | ^^^^^^^^^^^^^^^^ RUF013 -159 | ): -160 | pass +175 | ): +176 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -155 155 | def f( -156 156 | arg1: int = None, # RUF013 -157 157 | arg2: Union[int, float] = None, # RUF013 -158 |- arg3: Literal[1, 2, 3] = None, # RUF013 - 158 |+ arg3: Optional[Literal[1, 2, 3]] = None, # RUF013 -159 159 | ): -160 160 | pass -161 161 | - -RUF013_0.py:186:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +171 171 | def f( +172 172 | arg1: int = None, # RUF013 +173 173 | arg2: Union[int, float] = None, # RUF013 +174 |- arg3: Literal[1, 2, 3] = None, # RUF013 + 174 |+ arg3: Optional[Literal[1, 2, 3]] = None, # RUF013 +175 175 | ): +176 176 | pass +177 177 | + +RUF013_0.py:202:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -186 | def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 +202 | def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ RUF013 -187 | pass +203 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -183 183 | pass -184 184 | -185 185 | -186 |-def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 - 186 |+def f(arg: Optional[Union[Annotated[int, ...], Union[str, bytes]]] = None): # RUF013 -187 187 | pass -188 188 | -189 189 | - -RUF013_0.py:193:13: RUF013 [*] PEP 484 prohibits implicit `Optional` +199 199 | pass +200 200 | +201 201 | +202 |-def f(arg: Union[Annotated[int, ...], Union[str, bytes]] = None): # RUF013 + 202 |+def f(arg: Optional[Union[Annotated[int, ...], Union[str, bytes]]] = None): # RUF013 +203 203 | pass +204 204 | +205 205 | + +RUF013_0.py:209:13: RUF013 [*] PEP 484 prohibits implicit `Optional` | -193 | def f(arg: "int" = None): # RUF013 +209 | def f(arg: "int" = None): # RUF013 | ^^^ RUF013 -194 | pass +210 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -190 190 | # Quoted -191 191 | -192 192 | -193 |-def f(arg: "int" = None): # RUF013 - 193 |+def f(arg: "Optional[int]" = None): # RUF013 -194 194 | pass -195 195 | -196 196 | - -RUF013_0.py:197:13: RUF013 [*] PEP 484 prohibits implicit `Optional` +206 206 | # Quoted +207 207 | +208 208 | +209 |-def f(arg: "int" = None): # RUF013 + 209 |+def f(arg: "Optional[int]" = None): # RUF013 +210 210 | pass +211 211 | +212 212 | + +RUF013_0.py:213:13: RUF013 [*] PEP 484 prohibits implicit `Optional` | -197 | def f(arg: "str" = None): # RUF013 +213 | def f(arg: "str" = None): # RUF013 | ^^^ RUF013 -198 | pass +214 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -194 194 | pass -195 195 | -196 196 | -197 |-def f(arg: "str" = None): # RUF013 - 197 |+def f(arg: "Optional[str]" = None): # RUF013 -198 198 | pass -199 199 | -200 200 | +210 210 | pass +211 211 | +212 212 | +213 |-def f(arg: "str" = None): # RUF013 + 213 |+def f(arg: "Optional[str]" = None): # RUF013 +214 214 | pass +215 215 | +216 216 | -RUF013_0.py:201:12: RUF013 PEP 484 prohibits implicit `Optional` +RUF013_0.py:217:12: RUF013 PEP 484 prohibits implicit `Optional` | -201 | def f(arg: "st" "r" = None): # RUF013 +217 | def f(arg: "st" "r" = None): # RUF013 | ^^^^^^^^ RUF013 -202 | pass +218 | pass | = help: Convert to `Optional[T]` -RUF013_0.py:209:12: RUF013 [*] PEP 484 prohibits implicit `Optional` +RUF013_0.py:225:12: RUF013 [*] PEP 484 prohibits implicit `Optional` | -209 | def f(arg: Union["int", "str"] = None): # RUF013 +225 | def f(arg: Union["int", "str"] = None): # RUF013 | ^^^^^^^^^^^^^^^^^^^ RUF013 -210 | pass +226 | pass | = help: Convert to `Optional[T]` ℹ Suggested fix -206 206 | pass -207 207 | -208 208 | -209 |-def f(arg: Union["int", "str"] = None): # RUF013 - 209 |+def f(arg: Optional[Union["int", "str"]] = None): # RUF013 -210 210 | pass -211 211 | -212 212 | +222 222 | pass +223 223 | +224 224 | +225 |-def f(arg: Union["int", "str"] = None): # RUF013 + 225 |+def f(arg: Optional[Union["int", "str"]] = None): # RUF013 +226 226 | pass +227 227 | +228 228 | diff --git a/crates/ruff/src/test.rs b/crates/ruff/src/test.rs index 1c2eb7aefb46aa..82814908db0d9a 100644 --- a/crates/ruff/src/test.rs +++ b/crates/ruff/src/test.rs @@ -83,7 +83,7 @@ pub fn test_snippet(contents: &str, settings: &Settings) -> Vec { } thread_local! { - static MAX_ITERATIONS: std::cell::Cell = std::cell::Cell::new(20); + static MAX_ITERATIONS: std::cell::Cell = std::cell::Cell::new(30); } pub fn set_max_iterations(max: usize) { From a01c378f764cc9a72b3a60ad0029610f0941e772 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 13 Jul 2023 11:01:18 +0530 Subject: [PATCH 12/13] Avoid allocation while resolving slice value Co-authored-by: Charlie Marsh --- crates/ruff/src/rules/ruff/typing.rs | 55 +++++++++++++++++----------- 1 file changed, 34 insertions(+), 21 deletions(-) diff --git a/crates/ruff/src/rules/ruff/typing.rs b/crates/ruff/src/rules/ruff/typing.rs index f96ea22d8ef221..35baa3f9ce8812 100644 --- a/crates/ruff/src/rules/ruff/typing.rs +++ b/crates/ruff/src/rules/ruff/typing.rs @@ -1,3 +1,4 @@ +use itertools::Either::{Left, Right}; use rustpython_parser::ast::{self, Constant, Expr, Operator}; use ruff_python_ast::call_path::CallPath; @@ -52,12 +53,12 @@ fn is_known_type(call_path: &CallPath, minor_version: u32) -> bool { } } -/// Returns a vector of all the expressions in a slice. If the slice is not a -/// tuple, it will be wrapped in a vector. -fn resolve_slice_value(slice: &Expr) -> Vec<&Expr> { +/// Returns an iterator over the expressions in a slice. If the slice is not a +/// tuple, the iterator will only yield the slice. +fn resolve_slice_value(slice: &Expr) -> impl Iterator { match slice { - Expr::Tuple(ast::ExprTuple { elts: elements, .. }) => elements.iter().collect(), - _ => vec![slice], + Expr::Tuple(ast::ExprTuple { elts: elements, .. }) => Left(elements.iter()), + _ => Right(std::iter::once(slice)), } } @@ -75,12 +76,14 @@ enum TypingTarget<'a> { /// Forward reference to a type e.g., `"List[str]"`. ForwardReference(Expr), - /// A `typing.Union` type or `|` separated types e.g., `Union[int, str]` - /// or `int | str`. - Union(Vec<&'a Expr>), + /// A `typing.Union` type e.g., `Union[int, str]`. + Union(&'a Expr), + + /// A PEP 604 union type e.g., `int | str`. + PEP604Union(&'a Expr), /// A `typing.Literal` type e.g., `Literal[1, 2, 3]`. - Literal(Vec<&'a Expr>), + Literal(&'a Expr), /// A `typing.Optional` type e.g., `Optional[int]`. Optional(&'a Expr), @@ -110,13 +113,13 @@ impl<'a> TypingTarget<'a> { if semantic.match_typing_expr(value, "Optional") { Some(TypingTarget::Optional(slice.as_ref())) } else if semantic.match_typing_expr(value, "Literal") { - Some(TypingTarget::Literal(resolve_slice_value(slice))) + Some(TypingTarget::Literal(slice)) } else if semantic.match_typing_expr(value, "Union") { - Some(TypingTarget::Union(resolve_slice_value(slice))) + Some(TypingTarget::Union(slice)) } else if semantic.match_typing_expr(value, "Annotated") { resolve_slice_value(slice.as_ref()) - .first() - .map(|&expr| TypingTarget::Annotated(expr)) + .next() + .map(TypingTarget::Annotated) } else { semantic.resolve_call_path(value).map_or( // If we can't resolve the call path, it must be defined @@ -132,9 +135,7 @@ impl<'a> TypingTarget<'a> { ) } } - Expr::BinOp(..) => Some(TypingTarget::Union( - PEP604UnionIterator::new(expr).collect(), - )), + Expr::BinOp(..) => Some(TypingTarget::PEP604Union(expr)), Expr::Constant(ast::ExprConstant { value: Constant::None, .. @@ -179,7 +180,7 @@ impl<'a> TypingTarget<'a> { | TypingTarget::Object | TypingTarget::Unknown => true, TypingTarget::Known => false, - TypingTarget::Literal(elements) => elements.iter().any(|element| { + TypingTarget::Literal(slice) => resolve_slice_value(slice).any(|element| { // Literal can only contain `None`, a literal value, other `Literal` // or an enum value. match TypingTarget::try_from_expr(element, semantic, locator, minor_version) { @@ -190,17 +191,23 @@ impl<'a> TypingTarget<'a> { _ => false, } }), - TypingTarget::Union(elements) => elements.iter().any(|element| { + TypingTarget::Union(slice) => resolve_slice_value(slice).any(|element| { TypingTarget::try_from_expr(element, semantic, locator, minor_version) .map_or(true, |new_target| { new_target.contains_none(semantic, locator, minor_version) }) }), - TypingTarget::Annotated(element) => { + TypingTarget::PEP604Union(expr) => PEP604UnionIterator::new(expr).any(|element| { TypingTarget::try_from_expr(element, semantic, locator, minor_version) .map_or(true, |new_target| { new_target.contains_none(semantic, locator, minor_version) }) + }), + TypingTarget::Annotated(expr) => { + TypingTarget::try_from_expr(expr, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_none(semantic, locator, minor_version) + }) } TypingTarget::ForwardReference(expr) => { TypingTarget::try_from_expr(expr, semantic, locator, minor_version) @@ -226,17 +233,23 @@ impl<'a> TypingTarget<'a> { | TypingTarget::Object | TypingTarget::Known | TypingTarget::Unknown => false, - TypingTarget::Union(elements) => elements.iter().any(|element| { + TypingTarget::Union(slice) => resolve_slice_value(slice).any(|element| { TypingTarget::try_from_expr(element, semantic, locator, minor_version) .map_or(true, |new_target| { new_target.contains_any(semantic, locator, minor_version) }) }), - TypingTarget::Annotated(element) | TypingTarget::Optional(element) => { + TypingTarget::PEP604Union(expr) => PEP604UnionIterator::new(expr).any(|element| { TypingTarget::try_from_expr(element, semantic, locator, minor_version) .map_or(true, |new_target| { new_target.contains_any(semantic, locator, minor_version) }) + }), + TypingTarget::Annotated(expr) | TypingTarget::Optional(expr) => { + TypingTarget::try_from_expr(expr, semantic, locator, minor_version) + .map_or(true, |new_target| { + new_target.contains_any(semantic, locator, minor_version) + }) } TypingTarget::ForwardReference(expr) => { TypingTarget::try_from_expr(expr, semantic, locator, minor_version) From bd27d7ad099bef8d0d57fda1895735c5e4a0affe Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 13 Jul 2023 12:11:59 +0530 Subject: [PATCH 13/13] Revert "Temporarily disable `ANN401` in scripts" This reverts commit 01866f94c35ad88093a6d6f5e77d17b4fe08f70c. --- scripts/pyproject.toml | 1 - 1 file changed, 1 deletion(-) diff --git a/scripts/pyproject.toml b/scripts/pyproject.toml index 1a3a318a1a36aa..bdb5a08d5ee4fd 100644 --- a/scripts/pyproject.toml +++ b/scripts/pyproject.toml @@ -19,7 +19,6 @@ ignore = [ "T", # flake8-print "FBT", # flake8-boolean-trap "PERF", # perflint - "ANN401", ] [tool.ruff.isort]