From e92d79634a76404807be7f6650333488b146a077 Mon Sep 17 00:00:00 2001 From: Carl Meyer Date: Tue, 16 Jul 2024 10:30:00 -0700 Subject: [PATCH] review comments --- Cargo.lock | 25 +--- Cargo.toml | 1 - crates/red_knot_python_semantic/Cargo.toml | 2 +- crates/red_knot_python_semantic/src/db.rs | 7 + .../src/semantic_index.rs | 7 +- .../src/semantic_index/builder.rs | 5 +- .../semantic_index/{usedef.rs => use_def.rs} | 12 +- .../src/types/infer.rs | 133 ++++++++---------- 8 files changed, 81 insertions(+), 111 deletions(-) rename crates/red_knot_python_semantic/src/semantic_index/{usedef.rs => use_def.rs} (95%) diff --git a/Cargo.lock b/Cargo.lock index e31e783462384..bcf9194b17076 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1904,10 +1904,10 @@ dependencies = [ "ruff_index", "ruff_python_ast", "ruff_python_parser", + "ruff_python_trivia", "ruff_text_size", "rustc-hash 2.0.0", "salsa", - "textwrap", "tracing", ] @@ -2846,12 +2846,6 @@ version = "1.13.2" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3c5e1a9a646d36c3599cd173a41282daf47c44583ad367b8e6837255952e5c67" -[[package]] -name = "smawk" -version = "0.3.2" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "b7c388c1b5e93756d0c740965c41e8822f866621d41acbdf6336a6a168f8840c" - [[package]] name = "spin" version = "0.9.8" @@ -3003,17 +2997,6 @@ dependencies = [ "test-case-core", ] -[[package]] -name = "textwrap" -version = "0.16.1" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "23d434d3f8967a09480fb04132ebe0a3e088c173e6d0ee7897abbdf4eab0f8b9" -dependencies = [ - "smawk", - "unicode-linebreak", - "unicode-width", -] - [[package]] name = "thiserror" version = "1.0.62" @@ -3269,12 +3252,6 @@ version = "1.0.12" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "3354b9ac3fae1ff6755cb6db53683adb661634f67557942dea4facebec0fee4b" -[[package]] -name = "unicode-linebreak" -version = "0.1.5" -source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "3b09c83c3c29d37506a3e260c08c03743a6bb66a9cd432c6934ab501a190571f" - [[package]] name = "unicode-normalization" version = "0.1.23" diff --git a/Cargo.toml b/Cargo.toml index 7b96cd4101ca5..3604eb82493c4 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -127,7 +127,6 @@ strum_macros = { version = "0.26.0" } syn = { version = "2.0.55" } tempfile = { version = "3.9.0" } test-case = { version = "3.3.1" } -textwrap = { version = "0.16.1" } thiserror = { version = "1.0.58" } tikv-jemallocator = { version = "0.6.0" } toml = { version = "0.8.11" } diff --git a/crates/red_knot_python_semantic/Cargo.toml b/crates/red_knot_python_semantic/Cargo.toml index 07a2ed32208c7..7f0e13fc9a744 100644 --- a/crates/red_knot_python_semantic/Cargo.toml +++ b/crates/red_knot_python_semantic/Cargo.toml @@ -15,6 +15,7 @@ red_knot_module_resolver = { workspace = true } ruff_db = { workspace = true } ruff_index = { workspace = true } ruff_python_ast = { workspace = true } +ruff_python_trivia = { workspace = true } ruff_text_size = { workspace = true } bitflags = { workspace = true } @@ -27,7 +28,6 @@ hashbrown = { workspace = true } [dev-dependencies] anyhow = { workspace = true } ruff_python_parser = { workspace = true } -textwrap = { workspace = true } [lints] workspace = true diff --git a/crates/red_knot_python_semantic/src/db.rs b/crates/red_knot_python_semantic/src/db.rs index 6f901986017dd..c2b0456aa9ac1 100644 --- a/crates/red_knot_python_semantic/src/db.rs +++ b/crates/red_knot_python_semantic/src/db.rs @@ -47,6 +47,7 @@ pub(crate) mod tests { use ruff_db::system::{DbWithTestSystem, System, TestSystem}; use ruff_db::vendored::VendoredFileSystem; use ruff_db::{Db as SourceDb, Jar as SourceJar, Upcast}; + use ruff_python_trivia::textwrap; use super::{Db, Jar}; @@ -88,6 +89,12 @@ pub(crate) mod tests { pub(crate) fn clear_salsa_events(&mut self) { self.take_salsa_events(); } + + /// Write auto-dedented text to a file. + pub(crate) fn write_dedented(&mut self, path: &str, content: &str) -> anyhow::Result<()> { + self.write_file(path, textwrap::dedent(content))?; + Ok(()) + } } impl DbWithTestSystem for TestDb { diff --git a/crates/red_knot_python_semantic/src/semantic_index.rs b/crates/red_knot_python_semantic/src/semantic_index.rs index 25476d6055c21..88849e552e844 100644 --- a/crates/red_knot_python_semantic/src/semantic_index.rs +++ b/crates/red_knot_python_semantic/src/semantic_index.rs @@ -15,7 +15,7 @@ use crate::semantic_index::expression::Expression; use crate::semantic_index::symbol::{ FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopedSymbolId, SymbolTable, }; -use crate::semantic_index::usedef::UseDefMap; +use crate::semantic_index::use_def::UseDefMap; use crate::Db; pub mod ast_ids; @@ -23,7 +23,7 @@ mod builder; pub mod definition; pub mod expression; pub mod symbol; -pub mod usedef; +pub mod use_def; type SymbolMap = hashbrown::HashMap; @@ -188,6 +188,9 @@ impl<'db> SemanticIndex<'db> { } /// Returns the [`Expression`] ingredient for an expression node. + /// Panics if we have no expression ingredient for that node. We can only call this method for + /// standalone-inferable expressions, which we call `add_standalone_expression` for in + /// [`SemanticIndexBuilder`]. pub(crate) fn expression( &self, expression_key: impl Into, diff --git a/crates/red_knot_python_semantic/src/semantic_index/builder.rs b/crates/red_knot_python_semantic/src/semantic_index/builder.rs index 4bc4321d1723b..c9285116c341f 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/builder.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/builder.rs @@ -21,7 +21,7 @@ use crate::semantic_index::symbol::{ FileScopeId, NodeWithScopeKey, NodeWithScopeRef, Scope, ScopeId, ScopedSymbolId, SymbolFlags, SymbolTableBuilder, }; -use crate::semantic_index::usedef::{FlowSnapshot, UseDefMapBuilder}; +use crate::semantic_index::use_def::{FlowSnapshot, UseDefMapBuilder}; use crate::semantic_index::SemanticIndex; use crate::Db; @@ -176,7 +176,8 @@ impl<'db> SemanticIndexBuilder<'db> { self.definitions_by_node .insert(definition_node.key(), definition); - self.current_use_def_map().record_def(symbol, definition); + self.current_use_def_map() + .record_definition(symbol, definition); definition } diff --git a/crates/red_knot_python_semantic/src/semantic_index/usedef.rs b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs similarity index 95% rename from crates/red_knot_python_semantic/src/semantic_index/usedef.rs rename to crates/red_knot_python_semantic/src/semantic_index/use_def.rs index 17b82690efdf4..4aa0aa0f76171 100644 --- a/crates/red_knot_python_semantic/src/semantic_index/usedef.rs +++ b/crates/red_knot_python_semantic/src/semantic_index/use_def.rs @@ -44,7 +44,7 @@ struct Definitions { impl Default for Definitions { fn default() -> Self { Self { - definitions: 0..0, + definitions: Range::default(), may_be_unbound: true, } } @@ -60,7 +60,7 @@ pub(super) struct UseDefMapBuilder<'db> { definitions_by_use: IndexVec, - // builder state: currently visible definitions for each symbol + /// builder state: currently visible definitions for each symbol definitions_by_symbol: IndexVec, } @@ -78,7 +78,11 @@ impl<'db> UseDefMapBuilder<'db> { debug_assert_eq!(symbol, new_symbol); } - pub(super) fn record_def(&mut self, symbol: ScopedSymbolId, definition: Definition<'db>) { + pub(super) fn record_definition( + &mut self, + symbol: ScopedSymbolId, + definition: Definition<'db>, + ) { let def_idx = self.all_definitions.len(); self.all_definitions.push(definition); self.definitions_by_symbol[symbol] = Definitions { @@ -110,7 +114,7 @@ impl<'db> UseDefMapBuilder<'db> { pub(super) fn merge(&mut self, state: &FlowSnapshot) { for (symbol_id, to_merge) in state.definitions_by_symbol.iter_enumerated() { - let current = self.definitions_by_symbol.get_mut(symbol_id).unwrap(); + let current = &mut self.definitions_by_symbol[symbol_id]; // if the symbol can be unbound in either predecessor, it can be unbound current.may_be_unbound |= to_merge.may_be_unbound; // merge the definition ranges diff --git a/crates/red_knot_python_semantic/src/types/infer.rs b/crates/red_knot_python_semantic/src/types/infer.rs index db430a46a7b99..a863ac98083bb 100644 --- a/crates/red_knot_python_semantic/src/types/infer.rs +++ b/crates/red_knot_python_semantic/src/types/infer.rs @@ -11,9 +11,7 @@ use crate::semantic_index::definition::{Definition, DefinitionKind}; use crate::semantic_index::expression::Expression; use crate::semantic_index::symbol::{NodeWithScopeRef, ScopeId}; use crate::semantic_index::SemanticIndex; -use crate::types::{ - definitions_ty, use_def_map, ClassType, FunctionType, Name, Type, UnionTypeBuilder, -}; +use crate::types::{definitions_ty, ClassType, FunctionType, Name, Type, UnionTypeBuilder}; use crate::Db; use ruff_db::parsed::parsed_module; @@ -617,7 +615,7 @@ impl<'db> TypeInferenceBuilder<'db> { match ctx { ExprContext::Load => { - let use_def = use_def_map(self.db, self.scope); + let use_def = self.index.use_def_map(self.scope.file_scope_id(self.db)); 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)) @@ -717,7 +715,7 @@ mod tests { use ruff_db::files::{system_path_to_file, File}; use ruff_db::parsed::parsed_module; use ruff_db::system::{DbWithTestSystem, SystemPathBuf}; - use ruff_db::testing::{assert_function_query_was_not_run, assert_function_query_was_run}; + use ruff_db::testing::assert_function_query_was_not_run; use ruff_python_ast::name::Name; use crate::db::tests::TestDb; @@ -727,7 +725,6 @@ mod tests { use_def_map, Type, }; use crate::{HasTy, SemanticModel}; - use textwrap::dedent; fn setup_db() -> TestDb { let mut db = TestDb::new(); @@ -753,13 +750,6 @@ mod tests { assert_eq!(ty.display(db).to_string(), expected); } - impl TestDb { - fn write_dedented(&mut self, path: &str, content: &str) -> anyhow::Result<()> { - self.write_file(path, dedent(content))?; - Ok(()) - } - } - #[test] fn follow_import_to_class() -> anyhow::Result<()> { let mut db = setup_db(); @@ -781,11 +771,11 @@ mod tests { db.write_dedented( "src/mod.py", " - class Base: - pass + class Base: + pass - class Sub(Base): - pass + class Sub(Base): + pass ", )?; @@ -814,8 +804,8 @@ mod tests { db.write_dedented( "src/mod.py", " - class C: - def f(self): pass + class C: + def f(self): pass ", )?; @@ -869,10 +859,10 @@ mod tests { db.write_dedented( "src/a.py", " - if flag: - x = 1 - else: - x = 2 + if flag: + x = 1 + else: + x = 2 ", )?; @@ -888,11 +878,11 @@ mod tests { db.write_dedented( "src/a.py", " - a = 2 + 1 - b = a - 4 - c = a * b - d = c / 3 - e = 5 % 3 + a = 2 + 1 + b = a - 4 + c = a * b + d = c / 3 + e = 5 % 3 ", )?; @@ -935,11 +925,11 @@ mod tests { db.write_dedented( "src/a.py", " - y = 0 - z = 0 - x = (y := 1) if flag else (z := 2) - a = y - b = z + y = 0 + z = 0 + x = (y := 1) if flag else (z := 2) + a = y + b = z ", )?; @@ -963,10 +953,9 @@ mod tests { #[test] fn multi_target_assign() -> anyhow::Result<()> { - let db = setup_db(); + let mut db = setup_db(); - db.memory_file_system() - .write_file("src/a.py", "x = y = 1")?; + db.write_file("src/a.py", "x = y = 1")?; assert_public_ty(&db, "src/a.py", "x", "Literal[1]"); assert_public_ty(&db, "src/a.py", "y", "Literal[1]"); @@ -991,11 +980,11 @@ mod tests { db.write_dedented( "src/a.py", " - y = 1 - y = 2 - if flag: - y = 3 - x = y + y = 1 + y = 2 + if flag: + y = 3 + x = y ", )?; @@ -1010,9 +999,9 @@ mod tests { db.write_dedented( "src/a.py", " - if flag: - y = 3 - x = y + if flag: + y = 3 + x = y ", )?; @@ -1027,17 +1016,17 @@ mod tests { db.write_dedented( "src/a.py", " - y = 1 - y = 2 - if flag: - y = 3 - elif flag2: - y = 4 - else: - r = y - y = 5 - s = y - x = y + y = 1 + y = 2 + if flag: + y = 3 + elif flag2: + y = 4 + else: + r = y + y = 5 + s = y + x = y ", )?; @@ -1054,13 +1043,13 @@ mod tests { db.write_dedented( "src/a.py", " - y = 1 - y = 2 - if flag: - y = 3 - elif flag2: - y = 4 - x = y + y = 1 + y = 2 + if flag: + y = 3 + elif flag2: + y = 4 + x = y ", )?; @@ -1075,16 +1064,16 @@ mod tests { db.write_dedented( "src/a.py", " - class A: pass - import b - class C(b.B): pass + class A: pass + import b + class C(b.B): pass ", )?; db.write_dedented( "src/b.py", " - from a import A - class B(A): pass + from a import A + class B(A): pass ", )?; @@ -1155,20 +1144,10 @@ mod tests { let a = system_path_to_file(&db, "/src/a.py").unwrap(); - db.clear_salsa_events(); let x_ty_2 = module_global_symbol_ty_by_name(&db, a, "x"); assert_eq!(x_ty_2.display(&db).to_string(), "Literal[20]"); - let events = db.take_salsa_events(); - - assert_function_query_was_run::( - &db, - |ty| &ty.function, - &first_public_def(&db, a, "x"), - &events, - ); - Ok(()) }