Skip to content

Commit

Permalink
[red-knot] support reveal_type as pseudo-builtin (#13403)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
carljm authored Sep 19, 2024
1 parent afdb659 commit a6d3d2f
Show file tree
Hide file tree
Showing 3 changed files with 53 additions and 6 deletions.
10 changes: 10 additions & 0 deletions crates/red_knot_python_semantic/src/stdlib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ enum CoreStdlibModule {
Builtins,
Types,
Typeshed,
TypingExtensions,
}

impl CoreStdlibModule {
Expand All @@ -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!"))
Expand Down Expand Up @@ -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.
Expand Down
4 changes: 3 additions & 1 deletion crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down
45 changes: 40 additions & 5 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
}
Expand Down Expand Up @@ -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
};
Expand Down Expand Up @@ -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();
Expand Down

0 comments on commit a6d3d2f

Please sign in to comment.