Skip to content

Commit

Permalink
[red-knot] rename module_global to global (#12385)
Browse files Browse the repository at this point in the history
Per comments in #12269, "module
global" is kind of long, and arguably redundant.

I tried just using "module" but there were too many cases where I felt
this was ambiguous. I like the way "global" works out better, though it
does require an understanding that in Python "global" generally means
"module global" not "globally global" (though in a sense module globals
are also globally global since modules are singletons).
  • Loading branch information
carljm committed Jul 18, 2024
1 parent 519eca9 commit 181e7b3
Show file tree
Hide file tree
Showing 7 changed files with 82 additions and 93 deletions.
2 changes: 1 addition & 1 deletion crates/red_knot/src/lint.rs
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ fn lint_bad_override(context: &SemanticLintContext, class: &ast::StmtClassDef) {
return;
};

let override_ty = semantic.module_global_symbol_ty(&typing, "override");
let override_ty = semantic.global_symbol_ty(&typing, "override");

let Type::Class(class_ty) = class.ty(semantic) else {
return;
Expand Down
4 changes: 2 additions & 2 deletions crates/red_knot_python_semantic/src/db.rs
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ use ruff_db::{Db as SourceDb, Upcast};
use crate::semantic_index::definition::Definition;
use crate::semantic_index::expression::Expression;
use crate::semantic_index::symbol::ScopeId;
use crate::semantic_index::{module_global_scope, semantic_index, symbol_table, use_def_map};
use crate::semantic_index::{global_scope, semantic_index, symbol_table, use_def_map};
use crate::types::{
infer_definition_types, infer_expression_types, infer_scope_types, ClassType, FunctionType,
IntersectionType, UnionType,
Expand All @@ -23,7 +23,7 @@ pub struct Jar(
IntersectionType<'_>,
symbol_table,
use_def_map,
module_global_scope,
global_scope,
semantic_index,
infer_definition_types,
infer_expression_types,
Expand Down
114 changes: 54 additions & 60 deletions crates/red_knot_python_semantic/src/semantic_index.rs
Original file line number Diff line number Diff line change
Expand Up @@ -68,10 +68,10 @@ pub(crate) fn use_def_map<'db>(db: &'db dyn Db, scope: ScopeId<'db>) -> Arc<UseD

/// Returns the module global scope of `file`.
#[salsa::tracked]
pub(crate) fn module_global_scope(db: &dyn Db, file: File) -> ScopeId<'_> {
let _span = tracing::trace_span!("module_global_scope", ?file).entered();
pub(crate) fn global_scope(db: &dyn Db, file: File) -> ScopeId<'_> {
let _span = tracing::trace_span!("global_scope", ?file).entered();

FileScopeId::module_global().to_scope_id(db, file)
FileScopeId::global().to_scope_id(db, file)
}

/// The symbol tables and use-def maps for all scopes in a file.
Expand Down Expand Up @@ -309,7 +309,7 @@ mod tests {
use crate::semantic_index::ast_ids::HasScopedUseId;
use crate::semantic_index::definition::DefinitionKind;
use crate::semantic_index::symbol::{FileScopeId, Scope, ScopeKind, SymbolTable};
use crate::semantic_index::{module_global_scope, semantic_index, symbol_table, use_def_map};
use crate::semantic_index::{global_scope, semantic_index, symbol_table, use_def_map};
use crate::Db;

struct TestCase {
Expand All @@ -336,38 +336,38 @@ mod tests {
#[test]
fn empty() {
let TestCase { db, file } = test_case("");
let module_global_table = symbol_table(&db, module_global_scope(&db, file));
let global_table = symbol_table(&db, global_scope(&db, file));

let module_global_names = names(&module_global_table);
let global_names = names(&global_table);

assert_eq!(module_global_names, Vec::<&str>::new());
assert_eq!(global_names, Vec::<&str>::new());
}

#[test]
fn simple() {
let TestCase { db, file } = test_case("x");
let module_global_table = symbol_table(&db, module_global_scope(&db, file));
let global_table = symbol_table(&db, global_scope(&db, file));

assert_eq!(names(&module_global_table), vec!["x"]);
assert_eq!(names(&global_table), vec!["x"]);
}

#[test]
fn annotation_only() {
let TestCase { db, file } = test_case("x: int");
let module_global_table = symbol_table(&db, module_global_scope(&db, file));
let global_table = symbol_table(&db, global_scope(&db, file));

assert_eq!(names(&module_global_table), vec!["int", "x"]);
assert_eq!(names(&global_table), vec!["int", "x"]);
// TODO record definition
}

#[test]
fn import() {
let TestCase { db, file } = test_case("import foo");
let scope = module_global_scope(&db, file);
let module_global_table = symbol_table(&db, scope);
let scope = global_scope(&db, file);
let global_table = symbol_table(&db, scope);

assert_eq!(names(&module_global_table), vec!["foo"]);
let foo = module_global_table.symbol_id_by_name("foo").unwrap();
assert_eq!(names(&global_table), vec!["foo"]);
let foo = global_table.symbol_id_by_name("foo").unwrap();

let use_def = use_def_map(&db, scope);
let [definition] = use_def.public_definitions(foo) else {
Expand All @@ -379,36 +379,36 @@ mod tests {
#[test]
fn import_sub() {
let TestCase { db, file } = test_case("import foo.bar");
let module_global_table = symbol_table(&db, module_global_scope(&db, file));
let global_table = symbol_table(&db, global_scope(&db, file));

assert_eq!(names(&module_global_table), vec!["foo"]);
assert_eq!(names(&global_table), vec!["foo"]);
}

#[test]
fn import_as() {
let TestCase { db, file } = test_case("import foo.bar as baz");
let module_global_table = symbol_table(&db, module_global_scope(&db, file));
let global_table = symbol_table(&db, global_scope(&db, file));

assert_eq!(names(&module_global_table), vec!["baz"]);
assert_eq!(names(&global_table), vec!["baz"]);
}

#[test]
fn import_from() {
let TestCase { db, file } = test_case("from bar import foo");
let scope = module_global_scope(&db, file);
let module_global_table = symbol_table(&db, scope);
let scope = global_scope(&db, file);
let global_table = symbol_table(&db, scope);

assert_eq!(names(&module_global_table), vec!["foo"]);
assert_eq!(names(&global_table), vec!["foo"]);
assert!(
module_global_table
global_table
.symbol_by_name("foo")
.is_some_and(|symbol| { symbol.is_defined() && !symbol.is_used() }),
"symbols that are defined get the defined flag"
);

let use_def = use_def_map(&db, scope);
let [definition] = use_def.public_definitions(
module_global_table
global_table
.symbol_id_by_name("foo")
.expect("symbol to exist"),
) else {
Expand All @@ -423,22 +423,20 @@ mod tests {
#[test]
fn assign() {
let TestCase { db, file } = test_case("x = foo");
let scope = module_global_scope(&db, file);
let module_global_table = symbol_table(&db, scope);
let scope = global_scope(&db, file);
let global_table = symbol_table(&db, scope);

assert_eq!(names(&module_global_table), vec!["foo", "x"]);
assert_eq!(names(&global_table), vec!["foo", "x"]);
assert!(
module_global_table
global_table
.symbol_by_name("foo")
.is_some_and(|symbol| { !symbol.is_defined() && symbol.is_used() }),
"a symbol used but not defined in a scope should have only the used flag"
);
let use_def = use_def_map(&db, scope);
let [definition] = use_def.public_definitions(
module_global_table
.symbol_id_by_name("x")
.expect("symbol exists"),
) else {
let [definition] =
use_def.public_definitions(global_table.symbol_id_by_name("x").expect("symbol exists"))
else {
panic!("expected one definition");
};
assert!(matches!(
Expand All @@ -456,14 +454,14 @@ class C:
y = 2
",
);
let module_global_table = symbol_table(&db, module_global_scope(&db, file));
let global_table = symbol_table(&db, global_scope(&db, file));

assert_eq!(names(&module_global_table), vec!["C", "y"]);
assert_eq!(names(&global_table), vec!["C", "y"]);

let index = semantic_index(&db, file);

let [(class_scope_id, class_scope)] = index
.child_scopes(FileScopeId::module_global())
.child_scopes(FileScopeId::global())
.collect::<Vec<_>>()[..]
else {
panic!("expected one child scope")
Expand Down Expand Up @@ -496,12 +494,12 @@ y = 2
",
);
let index = semantic_index(&db, file);
let module_global_table = index.symbol_table(FileScopeId::module_global());
let global_table = index.symbol_table(FileScopeId::global());

assert_eq!(names(&module_global_table), vec!["func", "y"]);
assert_eq!(names(&global_table), vec!["func", "y"]);

let [(function_scope_id, function_scope)] = index
.child_scopes(FileScopeId::module_global())
.child_scopes(FileScopeId::global())
.collect::<Vec<_>>()[..]
else {
panic!("expected one child scope")
Expand Down Expand Up @@ -537,11 +535,11 @@ def func():
",
);
let index = semantic_index(&db, file);
let module_global_table = index.symbol_table(FileScopeId::module_global());
let global_table = index.symbol_table(FileScopeId::global());

assert_eq!(names(&module_global_table), vec!["func"]);
assert_eq!(names(&global_table), vec!["func"]);
let [(func_scope1_id, func_scope_1), (func_scope2_id, func_scope_2)] = index
.child_scopes(FileScopeId::module_global())
.child_scopes(FileScopeId::global())
.collect::<Vec<_>>()[..]
else {
panic!("expected two child scopes");
Expand All @@ -558,9 +556,9 @@ def func():
assert_eq!(names(&func1_table), vec!["x"]);
assert_eq!(names(&func2_table), vec!["y"]);

let use_def = index.use_def_map(FileScopeId::module_global());
let use_def = index.use_def_map(FileScopeId::global());
let [definition] = use_def.public_definitions(
module_global_table
global_table
.symbol_id_by_name("func")
.expect("symbol exists"),
) else {
Expand All @@ -579,12 +577,12 @@ def func[T]():
);

let index = semantic_index(&db, file);
let module_global_table = index.symbol_table(FileScopeId::module_global());
let global_table = index.symbol_table(FileScopeId::global());

assert_eq!(names(&module_global_table), vec!["func"]);
assert_eq!(names(&global_table), vec!["func"]);

let [(ann_scope_id, ann_scope)] = index
.child_scopes(FileScopeId::module_global())
.child_scopes(FileScopeId::global())
.collect::<Vec<_>>()[..]
else {
panic!("expected one child scope");
Expand Down Expand Up @@ -616,12 +614,12 @@ class C[T]:
);

let index = semantic_index(&db, file);
let module_global_table = index.symbol_table(FileScopeId::module_global());
let global_table = index.symbol_table(FileScopeId::global());

assert_eq!(names(&module_global_table), vec!["C"]);
assert_eq!(names(&global_table), vec!["C"]);

let [(ann_scope_id, ann_scope)] = index
.child_scopes(FileScopeId::module_global())
.child_scopes(FileScopeId::global())
.collect::<Vec<_>>()[..]
else {
panic!("expected one child scope");
Expand Down Expand Up @@ -653,7 +651,7 @@ class C[T]:
fn reachability_trivial() {
let TestCase { db, file } = test_case("x = 1; x");
let parsed = parsed_module(&db, file);
let scope = module_global_scope(&db, file);
let scope = global_scope(&db, file);
let ast = parsed.syntax();
let ast::Stmt::Expr(ast::StmtExpr {
value: x_use_expr, ..
Expand Down Expand Up @@ -694,7 +692,7 @@ class C[T]:
let x = &x_stmt.targets[0];

assert_eq!(index.expression_scope(x).kind(), ScopeKind::Module);
assert_eq!(index.expression_scope_id(x), FileScopeId::module_global());
assert_eq!(index.expression_scope_id(x), FileScopeId::global());

let def = ast.body[1].as_function_def_stmt().unwrap();
let y_stmt = def.body[0].as_assign_stmt().unwrap();
Expand Down Expand Up @@ -731,28 +729,24 @@ def x():

let index = semantic_index(&db, file);

let descendents = index.descendent_scopes(FileScopeId::module_global());
let descendents = index.descendent_scopes(FileScopeId::global());
assert_eq!(
scope_names(descendents, &db, file),
vec!["Test", "foo", "bar", "baz", "x"]
);

let children = index.child_scopes(FileScopeId::module_global());
let children = index.child_scopes(FileScopeId::global());
assert_eq!(scope_names(children, &db, file), vec!["Test", "x"]);

let test_class = index
.child_scopes(FileScopeId::module_global())
.next()
.unwrap()
.0;
let test_class = index.child_scopes(FileScopeId::global()).next().unwrap().0;
let test_child_scopes = index.child_scopes(test_class);
assert_eq!(
scope_names(test_child_scopes, &db, file),
vec!["foo", "baz"]
);

let bar_scope = index
.descendent_scopes(FileScopeId::module_global())
.descendent_scopes(FileScopeId::global())
.nth(2)
.unwrap()
.0;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ pub struct FileScopeId;

impl FileScopeId {
/// Returns the scope id of the module-global scope.
pub fn module_global() -> Self {
pub fn global() -> Self {
FileScopeId::from_u32(0)
}

Expand Down
6 changes: 3 additions & 3 deletions crates/red_knot_python_semantic/src/semantic_model.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use ruff_python_ast::{Expr, ExpressionRef, StmtClassDef};

use crate::semantic_index::ast_ids::HasScopedAstId;
use crate::semantic_index::semantic_index;
use crate::types::{definition_ty, infer_scope_types, module_global_symbol_ty_by_name, Type};
use crate::types::{definition_ty, global_symbol_ty_by_name, infer_scope_types, Type};
use crate::Db;

pub struct SemanticModel<'db> {
Expand All @@ -28,8 +28,8 @@ impl<'db> SemanticModel<'db> {
resolve_module(self.db.upcast(), module_name)
}

pub fn module_global_symbol_ty(&self, module: &Module, symbol_name: &str) -> Type<'db> {
module_global_symbol_ty_by_name(self.db, module.file(), symbol_name)
pub fn global_symbol_ty(&self, module: &Module, symbol_name: &str) -> Type<'db> {
global_symbol_ty_by_name(self.db, module.file(), symbol_name)
}
}

Expand Down
12 changes: 4 additions & 8 deletions crates/red_knot_python_semantic/src/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use ruff_python_ast::name::Name;

use crate::semantic_index::definition::Definition;
use crate::semantic_index::symbol::{ScopeId, ScopedSymbolId};
use crate::semantic_index::{module_global_scope, symbol_table, use_def_map};
use crate::semantic_index::{global_scope, symbol_table, use_def_map};
use crate::{Db, FxOrderSet};

mod display;
Expand Down Expand Up @@ -43,12 +43,8 @@ pub(crate) fn symbol_ty_by_name<'db>(
}

/// Shorthand for `symbol_ty` that looks up a module-global symbol in a file.
pub(crate) fn module_global_symbol_ty_by_name<'db>(
db: &'db dyn Db,
file: File,
name: &str,
) -> Type<'db> {
symbol_ty_by_name(db, module_global_scope(db, file), name)
pub(crate) fn global_symbol_ty_by_name<'db>(db: &'db dyn Db, file: File, name: &str) -> Type<'db> {
symbol_ty_by_name(db, global_scope(db, file), name)
}

/// Infer the type of a [`Definition`].
Expand Down Expand Up @@ -145,7 +141,7 @@ impl<'db> Type<'db> {
Type::Unbound => Type::Unbound,
Type::None => todo!("attribute lookup on None type"),
Type::Function(_) => todo!("attribute lookup on Function type"),
Type::Module(file) => module_global_symbol_ty_by_name(db, *file, name),
Type::Module(file) => global_symbol_ty_by_name(db, *file, name),
Type::Class(class) => class.class_member(db, name),
Type::Instance(_) => {
// TODO MRO? get_own_instance_member, get_instance_member
Expand Down
Loading

0 comments on commit 181e7b3

Please sign in to comment.