Skip to content

Commit a6d3d2f

Browse files
authored
[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.
1 parent afdb659 commit a6d3d2f

File tree

3 files changed

+53
-6
lines changed

3 files changed

+53
-6
lines changed

crates/red_knot_python_semantic/src/stdlib.rs

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@ enum CoreStdlibModule {
1111
Builtins,
1212
Types,
1313
Typeshed,
14+
TypingExtensions,
1415
}
1516

1617
impl CoreStdlibModule {
@@ -19,6 +20,7 @@ impl CoreStdlibModule {
1920
Self::Builtins => "builtins",
2021
Self::Types => "types",
2122
Self::Typeshed => "_typeshed",
23+
Self::TypingExtensions => "typing_extensions",
2224
};
2325
ModuleName::new_static(module_name)
2426
.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
6264
core_module_symbol_ty(db, CoreStdlibModule::Typeshed, symbol)
6365
}
6466

67+
/// Lookup the type of `symbol` in the `typing_extensions` module namespace.
68+
///
69+
/// Returns `Unbound` if the `typing_extensions` module isn't available for some reason.
70+
#[inline]
71+
pub(crate) fn typing_extensions_symbol_ty<'db>(db: &'db dyn Db, symbol: &str) -> Type<'db> {
72+
core_module_symbol_ty(db, CoreStdlibModule::TypingExtensions, symbol)
73+
}
74+
6575
/// Get the scope of a core stdlib module.
6676
///
6777
/// Can return `None` if a custom typeshed is used that is missing the core module in question.

crates/red_knot_python_semantic/src/types.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,9 @@ use crate::semantic_index::{
1010
global_scope, semantic_index, symbol_table, use_def_map, BindingWithConstraints,
1111
BindingWithConstraintsIterator, DeclarationsIterator,
1212
};
13-
use crate::stdlib::{builtins_symbol_ty, types_symbol_ty, typeshed_symbol_ty};
13+
use crate::stdlib::{
14+
builtins_symbol_ty, types_symbol_ty, typeshed_symbol_ty, typing_extensions_symbol_ty,
15+
};
1416
use crate::types::narrow::narrowing_constraint;
1517
use crate::{Db, FxOrderSet};
1618

crates/red_knot_python_semantic/src/types/infer.rs

Lines changed: 40 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -51,8 +51,8 @@ use crate::stdlib::builtins_module_scope;
5151
use crate::types::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics};
5252
use crate::types::{
5353
bindings_ty, builtins_symbol_ty, declarations_ty, global_symbol_ty, symbol_ty,
54-
BytesLiteralType, ClassType, FunctionType, StringLiteralType, TupleType, Type,
55-
TypeArrayDisplay, UnionType,
54+
typing_extensions_symbol_ty, BytesLiteralType, ClassType, FunctionType, StringLiteralType,
55+
TupleType, Type, TypeArrayDisplay, UnionType,
5656
};
5757
use crate::Db;
5858

@@ -2081,7 +2081,8 @@ impl<'db> TypeInferenceBuilder<'db> {
20812081
}
20822082

20832083
/// Look up a name reference that isn't bound in the local scope.
2084-
fn lookup_name(&self, name: &ast::name::Name) -> Type<'db> {
2084+
fn lookup_name(&mut self, name_node: &ast::ExprName) -> Type<'db> {
2085+
let ast::ExprName { id: name, .. } = name_node;
20852086
let file_scope_id = self.scope.file_scope_id(self.db);
20862087
let is_bound = self
20872088
.index
@@ -2126,7 +2127,17 @@ impl<'db> TypeInferenceBuilder<'db> {
21262127
};
21272128
// Fallback to builtins (without infinite recursion if we're already in builtins.)
21282129
if ty.may_be_unbound(self.db) && Some(self.scope) != builtins_module_scope(self.db) {
2129-
ty.replace_unbound_with(self.db, builtins_symbol_ty(self.db, name))
2130+
let mut builtin_ty = builtins_symbol_ty(self.db, name);
2131+
if builtin_ty.is_unbound() && name == "reveal_type" {
2132+
self.add_diagnostic(
2133+
name_node.into(),
2134+
"undefined-reveal",
2135+
format_args!(
2136+
"'reveal_type' used without importing it; this is allowed for debugging convenience but will fail at runtime."),
2137+
);
2138+
builtin_ty = typing_extensions_symbol_ty(self.db, name);
2139+
}
2140+
ty.replace_unbound_with(self.db, builtin_ty)
21302141
} else {
21312142
ty
21322143
}
@@ -2162,7 +2173,7 @@ impl<'db> TypeInferenceBuilder<'db> {
21622173
};
21632174

21642175
let unbound_ty = if may_be_unbound {
2165-
Some(self.lookup_name(id))
2176+
Some(self.lookup_name(name))
21662177
} else {
21672178
None
21682179
};
@@ -2804,6 +2815,30 @@ mod tests {
28042815
Ok(())
28052816
}
28062817

2818+
#[test]
2819+
fn reveal_type_builtin() -> anyhow::Result<()> {
2820+
let mut db = setup_db();
2821+
2822+
db.write_dedented(
2823+
"/src/a.py",
2824+
"
2825+
x = 1
2826+
reveal_type(x)
2827+
",
2828+
)?;
2829+
2830+
assert_file_diagnostics(
2831+
&db,
2832+
"/src/a.py",
2833+
&[
2834+
"'reveal_type' used without importing it; this is allowed for debugging convenience but will fail at runtime.",
2835+
"Revealed type is 'Literal[1]'.",
2836+
],
2837+
);
2838+
2839+
Ok(())
2840+
}
2841+
28072842
#[test]
28082843
fn follow_import_to_class() -> anyhow::Result<()> {
28092844
let mut db = setup_db();

0 commit comments

Comments
 (0)