From a6d3d2fccd57d2061f42db0f7f7690ea8a39f04c Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Thu, 19 Sep 2024 07:58:08 -0700 Subject: [PATCH] [red-knot] support reveal_type as pseudo-builtin (#13403) Support using `reveal_type` without importing it, as implied by the type spec and supported by existing type checkers. We use `typing_extensions.reveal_type` for the implicit built-in; this way it exists on all Python versions. (It imports from `typing` on newer Python versions.) Emits an "undefined name" diagnostic whenever `reveal_type` is referenced in this way (in addition to the revealed-type diagnostic when it is called). This follows the mypy example (with `--enable-error-code unimported-reveal`) and I think provides a good (and easily understandable) balance for user experience. If you are using `reveal_type` for quick temporary debugging, the additional undefined-name diagnostic doesn't hinder that use case. If we make the revealed-type diagnostic a non-failing one, the undefined-name diagnostic can still be a failing diagnostic, helping prevent accidentally leaving it in place. For any use cases where you want to leave it in place, you can always import it to avoid the undefined-name diagnostic. In the future, we can easily provide configuration options to a) turn off builtin-reveal_type altogether, and/or b) silence the undefined-name diagnostic when using it, if we have users on either side (loving or hating pseudo-builtin `reveal_type`) who are dissatisfied with this compromise. --- crates/red_knot_python_semantic/src/stdlib.rs | 10 +++++ crates/red_knot_python_semantic/src/types.rs | 4 +- .../src/types/infer.rs | 45 ++++++++++++++++--- 3 files changed, 53 insertions(+), 6 deletions(-) diff --git a/crates/red_knot_python_semantic/src/stdlib.rs b/crates/red_knot_python_semantic/src/stdlib.rs index b80cf4d71e..87337055e2 100644 --- a/crates/red_knot_python_semantic/src/stdlib.rs +++ b/crates/red_knot_python_semantic/src/stdlib.rs @@ -11,6 +11,7 @@ enum CoreStdlibModule { Builtins, Types, Typeshed, + TypingExtensions, } impl CoreStdlibModule { @@ -19,6 +20,7 @@ impl CoreStdlibModule { Self::Builtins => "builtins", Self::Types => "types", Self::Typeshed => "_typeshed", + Self::TypingExtensions => "typing_extensions", }; ModuleName::new_static(module_name) .unwrap_or_else(|| panic!("{module_name} should be a valid module name!")) @@ -62,6 +64,14 @@ pub(crate) fn typeshed_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db core_module_symbol_ty(db, CoreStdlibModule::Typeshed, symbol) } +/// Lookup the type of `symbol` in the `typing_extensions` module namespace. +/// +/// Returns `Unbound` if the `typing_extensions` module isn't available for some reason. +#[inline] +pub(crate) fn typing_extensions_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db> { + core_module_symbol_ty(db, CoreStdlibModule::TypingExtensions, symbol) +} + /// Get the scope of a core stdlib module. /// /// Can return `None` if a custom typeshed is used that is missing the core module in question. diff --git a/crates/red_knot_python_semantic/src/types.rs b/crates/red_knot_python_semantic/src/types.rs index 31355eb48f..70953483f9 100644 --- a/crates/red_knot_python_semantic/src/types.rs +++ b/crates/red_knot_python_semantic/src/types.rs @@ -10,7 +10,9 @@ use crate::semantic_index::{ global_scope, semantic_index, symbol_table, use_def_map, BindingWithConstraints, BindingWithConstraintsIterator, DeclarationsIterator, }; -use crate::stdlib::{builtins_symbol_ty, types_symbol_ty, typeshed_symbol_ty}; +use crate::stdlib::{ + builtins_symbol_ty, types_symbol_ty, typeshed_symbol_ty, typing_extensions_symbol_ty, +}; use crate::types::narrow::narrowing_constraint; use crate::{Db, FxOrderSet}; diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index fa5197f5f3..f7ad760382 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -51,8 +51,8 @@ use crate::stdlib::builtins_module_scope; use crate::types::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics}; use crate::types::{ bindings_ty, builtins_symbol_ty, declarations_ty, global_symbol_ty, symbol_ty, - BytesLiteralType, ClassType, FunctionType, StringLiteralType, TupleType, Type, - TypeArrayDisplay, UnionType, + typing_extensions_symbol_ty, BytesLiteralType, ClassType, FunctionType, StringLiteralType, + TupleType, Type, TypeArrayDisplay, UnionType, }; use crate::Db; @@ -2081,7 +2081,8 @@ impl<'db> TypeInferenceBuilder<'db> { } /// Look up a name reference that isn't bound in the local scope. - fn lookup_name(&self, name: &ast::name::Name) -> Type<'db> { + fn lookup_name(&mut self, name_node: &ast::ExprName) -> Type<'db> { + let ast::ExprName { id: name, .. } = name_node; let file_scope_id = self.scope.file_scope_id(self.db); let is_bound = self .index @@ -2126,7 +2127,17 @@ impl<'db> TypeInferenceBuilder<'db> { }; // Fallback to builtins (without infinite recursion if we're already in builtins.) if ty.may_be_unbound(self.db) && Some(self.scope) != builtins_module_scope(self.db) { - ty.replace_unbound_with(self.db, builtins_symbol_ty(self.db, name)) + let mut builtin_ty = builtins_symbol_ty(self.db, name); + if builtin_ty.is_unbound() && name == "reveal_type" { + self.add_diagnostic( + name_node.into(), + "undefined-reveal", + format_args!( + "'reveal_type' used without importing it; this is allowed for debugging convenience but will fail at runtime."), + ); + builtin_ty = typing_extensions_symbol_ty(self.db, name); + } + ty.replace_unbound_with(self.db, builtin_ty) } else { ty } @@ -2162,7 +2173,7 @@ impl<'db> TypeInferenceBuilder<'db> { }; let unbound_ty = if may_be_unbound { - Some(self.lookup_name(id)) + Some(self.lookup_name(name)) } else { None }; @@ -2804,6 +2815,30 @@ mod tests { Ok(()) } + #[test] + fn reveal_type_builtin() -> anyhow::Result<()> { + let mut db = setup_db(); + + db.write_dedented( + "/src/a.py", + " + x = 1 + reveal_type(x) + ", + )?; + + assert_file_diagnostics( + &db, + "/src/a.py", + &[ + "'reveal_type' used without importing it; this is allowed for debugging convenience but will fail at runtime.", + "Revealed type is 'Literal[1]'.", + ], + ); + + Ok(()) + } + #[test] fn follow_import_to_class() -> anyhow::Result<()> { let mut db = setup_db();