Skip to content

Commit

Permalink
[red-knot] support implicit global name lookups (#12374)
Browse files Browse the repository at this point in the history
Support falling back to a global name lookup if a name isn't defined in
the local scope, in the cases where that is correct according to Python
semantics.

In class scopes, a name lookup checks the local namespace first, and if
the name isn't found there, looks it up in globals.

In function scopes (and type parameter scopes, which are function-like),
if a name has any definitions in the local scope, it is a local, and
accessing it when none of those definitions have executed yet just
results in an `UnboundLocalError`, it does not fall back to a global. If
the name does not have any definitions in the local scope, then it is an
implicit global.

Public symbol type lookups never include such a fall back. For example,
if a name is not defined in a class scope, it is not available as a
member on that class, even if a name lookup within the class scope would
have fallen back to a global lookup.

This PR makes the `@override` lint rule work again.

Not yet included/supported in this PR:

* Support for free variables / closures: a free symbol in a nested
function-like scope referring to a symbol in an outer function-like
scope.
* Support for `global` and `nonlocal` statements, which force a symbol
to be treated as global or nonlocal even if it has definitions in the
local scope.
* Module-global lookups should fall back to builtins if the name isn't
found in the module scope.

I would like to expose nicer APIs for the various kinds of symbols
(explicit global, implicit global, free, etc), but this will also wait
for a later PR, when more kinds of symbols are supported.
  • Loading branch information
carljm committed Jul 18, 2024
1 parent f0d589d commit 519eca9
Show file tree
Hide file tree
Showing 3 changed files with 155 additions and 23 deletions.
12 changes: 11 additions & 1 deletion crates/red_knot_python_semantic/src/semantic_index/symbol.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,17 @@ pub struct ScopeId<'db> {
}

impl<'db> ScopeId<'db> {
pub(crate) fn is_function_like(self, db: &'db dyn Db) -> bool {
// Type parameter scopes behave like function scopes in terms of name resolution; CPython
// symbol table also uses the term "function-like" for these scopes.
matches!(
self.node(db),
NodeWithScopeKind::ClassTypeParameters(_)
| NodeWithScopeKind::FunctionTypeParameters(_)
| NodeWithScopeKind::Function(_)
)
}

#[cfg(test)]
pub(crate) fn name(self, db: &'db dyn Db) -> &'db str {
match self.node(db) {
Expand Down Expand Up @@ -193,7 +204,6 @@ impl SymbolTable {
}

/// Returns the symbol named `name`.
#[allow(unused)]
pub(crate) fn symbol_by_name(&self, name: &str) -> Option<&Symbol> {
let id = self.symbol_id_by_name(name)?;
Some(self.symbol(id))
Expand Down
33 changes: 21 additions & 12 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ pub(crate) fn symbol_ty<'db>(
definitions_ty(
db,
use_def.public_definitions(symbol),
use_def.public_may_be_unbound(symbol),
use_def
.public_may_be_unbound(symbol)
.then_some(Type::Unbound),
)
}

Expand Down Expand Up @@ -55,24 +57,31 @@ pub(crate) fn definition_ty<'db>(db: &'db dyn Db, definition: Definition<'db>) -
inference.definition_ty(definition)
}

/// Infer the combined type of an array of [`Definition`].
/// Will return a union if there are more than definition, or at least one plus the possibility of
/// Unbound.
/// Infer the combined type of an array of [`Definition`]s, plus one optional "unbound type".
///
/// Will return a union if there is more than one definition, or at least one plus an unbound
/// type.
///
/// The "unbound type" represents the type in case control flow may not have passed through any
/// definitions in this scope. If this isn't possible, then it will be `None`. If it is possible,
/// and the result in that case should be Unbound (e.g. an unbound function local), then it will be
/// `Some(Type::Unbound)`. If it is possible and the result should be something else (e.g. an
/// implicit global lookup), then `unbound_type` will be `Some(the_global_symbol_type)`.
///
/// # Panics
/// Will panic if called with zero definitions and no `unbound_ty`. This is a logic error,
/// as any symbol with zero visible definitions clearly may be unbound, and the caller should
/// provide an `unbound_ty`.
pub(crate) fn definitions_ty<'db>(
db: &'db dyn Db,
definitions: &[Definition<'db>],
may_be_unbound: bool,
unbound_ty: Option<Type<'db>>,
) -> Type<'db> {
let unbound_iter = if may_be_unbound {
[Type::Unbound].iter()
} else {
[].iter()
};
let def_types = definitions.iter().map(|def| definition_ty(db, *def));
let mut all_types = unbound_iter.copied().chain(def_types);
let mut all_types = unbound_ty.into_iter().chain(def_types);

let Some(first) = all_types.next() else {
return Type::Unbound;
panic!("definitions_ty should never be called with zero definitions and no unbound_ty.")
};

if let Some(second) = all_types.next() {
Expand Down
133 changes: 123 additions & 10 deletions crates/red_knot_python_semantic/src/types/infer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@ use crate::semantic_index::semantic_index;
use crate::semantic_index::symbol::NodeWithScopeKind;
use crate::semantic_index::symbol::{NodeWithScopeRef, ScopeId};
use crate::semantic_index::SemanticIndex;
use crate::types::{definitions_ty, ClassType, FunctionType, Name, Type, UnionTypeBuilder};
use crate::types::{
definitions_ty, module_global_symbol_ty_by_name, ClassType, FunctionType, Name, Type,
UnionTypeBuilder,
};
use crate::Db;

/// Infer all types for a [`ScopeId`], including all definitions and expressions in that scope.
Expand Down Expand Up @@ -667,18 +670,30 @@ impl<'db> TypeInferenceBuilder<'db> {
}

fn infer_name_expression(&mut self, name: &ast::ExprName) -> Type<'db> {
let ast::ExprName {
range: _,
id: _,
ctx,
} = name;
let ast::ExprName { range: _, id, ctx } = name;

match ctx {
ExprContext::Load => {
let use_def = self.index.use_def_map(self.scope.file_scope_id(self.db));
let file_scope_id = self.scope.file_scope_id(self.db);
let use_def = self.index.use_def_map(file_scope_id);
let use_id = name.scoped_use_id(self.db, self.scope);
let definitions = use_def.use_definitions(use_id);
definitions_ty(self.db, definitions, use_def.use_may_be_unbound(use_id))
let may_be_unbound = use_def.use_may_be_unbound(use_id);

let unbound_ty = if may_be_unbound {
let symbols = self.index.symbol_table(file_scope_id);
// SAFETY: the symbol table always creates a symbol for every Name node.
let symbol = symbols.symbol_by_name(id).unwrap();
if !symbol.is_defined() || !self.scope.is_function_like(self.db) {
// implicit global
Some(module_global_symbol_ty_by_name(self.db, self.file, id))
} else {
Some(Type::Unbound)
}
} else {
None
};

definitions_ty(self.db, use_def.use_definitions(use_id), unbound_ty)
}
ExprContext::Store | ExprContext::Del => Type::None,
ExprContext::Invalid => Type::Unknown,
Expand Down Expand Up @@ -778,9 +793,11 @@ mod tests {

use crate::db::tests::TestDb;
use crate::semantic_index::definition::Definition;
use crate::semantic_index::semantic_index;
use crate::semantic_index::symbol::FileScopeId;
use crate::types::{
infer_definition_types, module_global_scope, module_global_symbol_ty_by_name, symbol_table,
use_def_map, Type,
symbol_ty_by_name, use_def_map, Type,
};
use crate::{HasTy, SemanticModel};

Expand Down Expand Up @@ -1237,6 +1254,102 @@ mod tests {
Ok(())
}

/// An unbound function local that has definitions in the scope does not fall back to globals.
#[test]
fn unbound_function_local() -> anyhow::Result<()> {
let mut db = setup_db();

db.write_dedented(
"src/a.py",
"
x = 1
def f():
y = x
x = 2
",
)?;

let file = system_path_to_file(&db, "src/a.py").expect("Expected file to exist.");
let index = semantic_index(&db, file);
let function_scope = index
.child_scopes(FileScopeId::module_global())
.next()
.unwrap()
.0
.to_scope_id(&db, file);
let y_ty = symbol_ty_by_name(&db, function_scope, "y");
let x_ty = symbol_ty_by_name(&db, function_scope, "x");

assert_eq!(y_ty.display(&db).to_string(), "Unbound");
assert_eq!(x_ty.display(&db).to_string(), "Literal[2]");

Ok(())
}

/// A name reference to a never-defined symbol in a function is implicitly a global lookup.
#[test]
fn implicit_global_in_function() -> anyhow::Result<()> {
let mut db = setup_db();

db.write_dedented(
"src/a.py",
"
x = 1
def f():
y = x
",
)?;

let file = system_path_to_file(&db, "src/a.py").expect("Expected file to exist.");
let index = semantic_index(&db, file);
let function_scope = index
.child_scopes(FileScopeId::module_global())
.next()
.unwrap()
.0
.to_scope_id(&db, file);
let y_ty = symbol_ty_by_name(&db, function_scope, "y");
let x_ty = symbol_ty_by_name(&db, function_scope, "x");

assert_eq!(x_ty.display(&db).to_string(), "Unbound");
assert_eq!(y_ty.display(&db).to_string(), "Literal[1]");

Ok(())
}

/// Class name lookups do fall back to globals, but the public type never does.
#[test]
fn unbound_class_local() -> anyhow::Result<()> {
let mut db = setup_db();

db.write_dedented(
"src/a.py",
"
x = 1
class C:
y = x
if flag:
x = 2
",
)?;

let file = system_path_to_file(&db, "src/a.py").expect("Expected file to exist.");
let index = semantic_index(&db, file);
let class_scope = index
.child_scopes(FileScopeId::module_global())
.next()
.unwrap()
.0
.to_scope_id(&db, file);
let y_ty = symbol_ty_by_name(&db, class_scope, "y");
let x_ty = symbol_ty_by_name(&db, class_scope, "x");

assert_eq!(x_ty.display(&db).to_string(), "Literal[2] | Unbound");
assert_eq!(y_ty.display(&db).to_string(), "Literal[1]");

Ok(())
}

#[test]
fn local_inference() -> anyhow::Result<()> {
let mut db = setup_db();
Expand Down

0 comments on commit 519eca9

Please sign in to comment.