From 3dc73395eabde6f29c9de145033a7ac6a2117331 Mon Sep 17 00:00:00 2001 From: Charlie Marsh Date: Fri, 14 Jul 2023 22:04:15 -0400 Subject: [PATCH] Move `Literal` flag detection into recurse phase (#5768) ## Summary The AST pass is broken up into three phases: pre-visit (which includes analysis), recurse (visit all members), and post-visit (clean-up). We're not supposed to edit semantic model flags in the pre-visit phase, but it looks like we were for literal detection. This didn't matter in practice, but I'm looking into some AST refactors for which this _does_ cause issues. No behavior changes expected. ## Test Plan Good test coverage on these. --- crates/ruff/src/checkers/ast/mod.rs | 17 ++++++----- .../src/analyze/typing.rs | 29 +++++++++++++------ crates/ruff_python_stdlib/src/typing.rs | 10 +++++++ 3 files changed, 40 insertions(+), 16 deletions(-) diff --git a/crates/ruff/src/checkers/ast/mod.rs b/crates/ruff/src/checkers/ast/mod.rs index 2f8cb70545..449d5096ac 100644 --- a/crates/ruff/src/checkers/ast/mod.rs +++ b/crates/ruff/src/checkers/ast/mod.rs @@ -2222,9 +2222,6 @@ where } } - if self.semantic.match_typing_expr(value, "Literal") { - self.semantic.flags |= SemanticModelFlags::LITERAL; - } if self.any_enabled(&[ Rule::SysVersionSlice3, Rule::SysVersion2, @@ -3810,14 +3807,21 @@ where ) { Some(subscript) => { match subscript { + // Ex) Literal["Class"] + typing::SubscriptKind::Literal => { + self.semantic.flags |= SemanticModelFlags::LITERAL; + self.visit_expr(value); + self.visit_type_definition(slice); + self.visit_expr_context(ctx); + } // Ex) Optional[int] - typing::SubscriptKind::AnnotatedSubscript => { + typing::SubscriptKind::Generic => { self.visit_expr(value); self.visit_type_definition(slice); self.visit_expr_context(ctx); } // Ex) Annotated[int, "Hello, world!"] - typing::SubscriptKind::PEP593AnnotatedSubscript => { + typing::SubscriptKind::PEP593Annotation => { // First argument is a type (including forward references); the // rest are arbitrary Python objects. self.visit_expr(value); @@ -3836,8 +3840,7 @@ where } } else { error!( - "Found non-Expr::Tuple argument to PEP 593 \ - Annotation." + "Found non-Expr::Tuple argument to PEP 593 Annotation." ); } } diff --git a/crates/ruff_python_semantic/src/analyze/typing.rs b/crates/ruff_python_semantic/src/analyze/typing.rs index 39ea75af86..eb74c3e449 100644 --- a/crates/ruff_python_semantic/src/analyze/typing.rs +++ b/crates/ruff_python_semantic/src/analyze/typing.rs @@ -7,9 +7,9 @@ use ruff_python_ast::call_path::{from_qualified_name, from_unqualified_name, Cal use ruff_python_ast::helpers::is_const_false; use ruff_python_stdlib::typing::{ as_pep_585_generic, has_pep_585_generic, is_immutable_generic_type, - is_immutable_non_generic_type, is_immutable_return_type, is_mutable_return_type, - is_pep_593_generic_member, is_pep_593_generic_type, is_standard_library_generic, - is_standard_library_generic_member, + is_immutable_non_generic_type, is_immutable_return_type, is_literal_member, + is_mutable_return_type, is_pep_593_generic_member, is_pep_593_generic_type, + is_standard_library_generic, is_standard_library_generic_member, is_standard_library_literal, }; use crate::model::SemanticModel; @@ -27,8 +27,12 @@ pub enum Callable { #[derive(Copy, Clone)] pub enum SubscriptKind { - AnnotatedSubscript, - PEP593AnnotatedSubscript, + /// A subscript of the form `typing.Literal["foo", "bar"]`, i.e., a literal. + Literal, + /// A subscript of the form `typing.List[int]`, i.e., a generic. + Generic, + /// A subscript of the form `typing.Annotated[int, "foo"]`, i.e., a PEP 593 annotation. + PEP593Annotation, } pub fn match_annotated_subscript<'a>( @@ -38,28 +42,35 @@ pub fn match_annotated_subscript<'a>( extend_generics: &[String], ) -> Option { semantic.resolve_call_path(expr).and_then(|call_path| { + if is_standard_library_literal(call_path.as_slice()) { + return Some(SubscriptKind::Literal); + } + if is_standard_library_generic(call_path.as_slice()) || extend_generics .iter() .map(|target| from_qualified_name(target)) .any(|target| call_path == target) { - return Some(SubscriptKind::AnnotatedSubscript); + return Some(SubscriptKind::Generic); } if is_pep_593_generic_type(call_path.as_slice()) { - return Some(SubscriptKind::PEP593AnnotatedSubscript); + return Some(SubscriptKind::PEP593Annotation); } for module in typing_modules { let module_call_path: CallPath = from_unqualified_name(module); if call_path.starts_with(&module_call_path) { if let Some(member) = call_path.last() { + if is_literal_member(member) { + return Some(SubscriptKind::Literal); + } if is_standard_library_generic_member(member) { - return Some(SubscriptKind::AnnotatedSubscript); + return Some(SubscriptKind::Generic); } if is_pep_593_generic_member(member) { - return Some(SubscriptKind::PEP593AnnotatedSubscript); + return Some(SubscriptKind::PEP593Annotation); } } } diff --git a/crates/ruff_python_stdlib/src/typing.rs b/crates/ruff_python_stdlib/src/typing.rs index 796f7c3a07..c36455025d 100644 --- a/crates/ruff_python_stdlib/src/typing.rs +++ b/crates/ruff_python_stdlib/src/typing.rs @@ -182,6 +182,11 @@ pub fn is_pep_593_generic_type(call_path: &[&str]) -> bool { matches!(call_path, ["typing" | "typing_extensions", "Annotated"]) } +/// Returns `true` if a call path is `Literal`. +pub fn is_standard_library_literal(call_path: &[&str]) -> bool { + matches!(call_path, ["typing" | "typing_extensions", "Literal"]) +} + /// Returns `true` if a name matches that of a generic from the Python standard library (e.g. /// `list` or `Set`). /// @@ -267,6 +272,11 @@ pub fn is_pep_593_generic_member(member: &str) -> bool { matches!(member, "Annotated") } +/// Returns `true` if a name matches that of the `Literal` generic. +pub fn is_literal_member(member: &str) -> bool { + matches!(member, "Literal") +} + /// Returns `true` if a call path represents that of an immutable, non-generic type from the Python /// standard library (e.g. `int` or `str`). pub fn is_immutable_non_generic_type(call_path: &[&str]) -> bool {