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.
This commit is contained in:
Charlie Marsh 2023-07-14 22:04:15 -04:00 committed by GitHub
parent 7c32e98d10
commit 3dc73395ea
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
3 changed files with 40 additions and 16 deletions

View file

@ -2222,9 +2222,6 @@ where
} }
} }
if self.semantic.match_typing_expr(value, "Literal") {
self.semantic.flags |= SemanticModelFlags::LITERAL;
}
if self.any_enabled(&[ if self.any_enabled(&[
Rule::SysVersionSlice3, Rule::SysVersionSlice3,
Rule::SysVersion2, Rule::SysVersion2,
@ -3810,14 +3807,21 @@ where
) { ) {
Some(subscript) => { Some(subscript) => {
match 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] // Ex) Optional[int]
typing::SubscriptKind::AnnotatedSubscript => { typing::SubscriptKind::Generic => {
self.visit_expr(value); self.visit_expr(value);
self.visit_type_definition(slice); self.visit_type_definition(slice);
self.visit_expr_context(ctx); self.visit_expr_context(ctx);
} }
// Ex) Annotated[int, "Hello, world!"] // Ex) Annotated[int, "Hello, world!"]
typing::SubscriptKind::PEP593AnnotatedSubscript => { typing::SubscriptKind::PEP593Annotation => {
// First argument is a type (including forward references); the // First argument is a type (including forward references); the
// rest are arbitrary Python objects. // rest are arbitrary Python objects.
self.visit_expr(value); self.visit_expr(value);
@ -3836,8 +3840,7 @@ where
} }
} else { } else {
error!( error!(
"Found non-Expr::Tuple argument to PEP 593 \ "Found non-Expr::Tuple argument to PEP 593 Annotation."
Annotation."
); );
} }
} }

View file

@ -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_ast::helpers::is_const_false;
use ruff_python_stdlib::typing::{ use ruff_python_stdlib::typing::{
as_pep_585_generic, has_pep_585_generic, is_immutable_generic_type, 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_immutable_non_generic_type, is_immutable_return_type, is_literal_member,
is_pep_593_generic_member, is_pep_593_generic_type, is_standard_library_generic, is_mutable_return_type, is_pep_593_generic_member, is_pep_593_generic_type,
is_standard_library_generic_member, is_standard_library_generic, is_standard_library_generic_member, is_standard_library_literal,
}; };
use crate::model::SemanticModel; use crate::model::SemanticModel;
@ -27,8 +27,12 @@ pub enum Callable {
#[derive(Copy, Clone)] #[derive(Copy, Clone)]
pub enum SubscriptKind { pub enum SubscriptKind {
AnnotatedSubscript, /// A subscript of the form `typing.Literal["foo", "bar"]`, i.e., a literal.
PEP593AnnotatedSubscript, 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>( pub fn match_annotated_subscript<'a>(
@ -38,28 +42,35 @@ pub fn match_annotated_subscript<'a>(
extend_generics: &[String], extend_generics: &[String],
) -> Option<SubscriptKind> { ) -> Option<SubscriptKind> {
semantic.resolve_call_path(expr).and_then(|call_path| { 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()) if is_standard_library_generic(call_path.as_slice())
|| extend_generics || extend_generics
.iter() .iter()
.map(|target| from_qualified_name(target)) .map(|target| from_qualified_name(target))
.any(|target| call_path == target) .any(|target| call_path == target)
{ {
return Some(SubscriptKind::AnnotatedSubscript); return Some(SubscriptKind::Generic);
} }
if is_pep_593_generic_type(call_path.as_slice()) { if is_pep_593_generic_type(call_path.as_slice()) {
return Some(SubscriptKind::PEP593AnnotatedSubscript); return Some(SubscriptKind::PEP593Annotation);
} }
for module in typing_modules { for module in typing_modules {
let module_call_path: CallPath = from_unqualified_name(module); let module_call_path: CallPath = from_unqualified_name(module);
if call_path.starts_with(&module_call_path) { if call_path.starts_with(&module_call_path) {
if let Some(member) = call_path.last() { if let Some(member) = call_path.last() {
if is_literal_member(member) {
return Some(SubscriptKind::Literal);
}
if is_standard_library_generic_member(member) { if is_standard_library_generic_member(member) {
return Some(SubscriptKind::AnnotatedSubscript); return Some(SubscriptKind::Generic);
} }
if is_pep_593_generic_member(member) { if is_pep_593_generic_member(member) {
return Some(SubscriptKind::PEP593AnnotatedSubscript); return Some(SubscriptKind::PEP593Annotation);
} }
} }
} }

View file

@ -182,6 +182,11 @@ pub fn is_pep_593_generic_type(call_path: &[&str]) -> bool {
matches!(call_path, ["typing" | "typing_extensions", "Annotated"]) 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. /// Returns `true` if a name matches that of a generic from the Python standard library (e.g.
/// `list` or `Set`). /// `list` or `Set`).
/// ///
@ -267,6 +272,11 @@ pub fn is_pep_593_generic_member(member: &str) -> bool {
matches!(member, "Annotated") 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 /// Returns `true` if a call path represents that of an immutable, non-generic type from the Python
/// standard library (e.g. `int` or `str`). /// standard library (e.g. `int` or `str`).
pub fn is_immutable_non_generic_type(call_path: &[&str]) -> bool { pub fn is_immutable_non_generic_type(call_path: &[&str]) -> bool {