Merge remote-tracking branch 'origin/main' into dcreager/better-constraint-copies

* origin/main:
  [red-knot] Prevent cross-module query dependencies in `own_instance_member` (#16268)
  Specify the `wasm-pack` version for release workflows (#16278)
  [red-knot] Separate `definitions_by_definition` into separate fields (#16277)

Author: dcreager

.github/workflows/publish-playground.yml

@@ -35,6 +35,8 @@ jobs:
           cache: "npm"
           cache-dependency-path: playground/package-lock.json
       - uses: jetli/wasm-pack-action@v0.4.0
+        with:
+          version: v0.13.1
       - uses: jetli/wasm-bindgen-action@v0.2.0
       - name: "Run wasm-pack"
         run: wasm-pack build --target web --out-dir ../../playground/src/pkg crates/ruff_wasm

.github/workflows/publish-wasm.yml

@@ -35,6 +35,8 @@ jobs:
       - name: "Install Rust toolchain"
         run: rustup target add wasm32-unknown-unknown
       - uses: jetli/wasm-pack-action@v0.4.0
+        with:
+          version: v0.13.1
       - uses: jetli/wasm-bindgen-action@v0.2.0
       - name: "Run wasm-pack build"
         run: wasm-pack build --target ${{ matrix.target }} crates/ruff_wasm

crates/red_knot_python_semantic/src/semantic_index/builder.rs

@@ -346,12 +346,14 @@ impl<'db> SemanticIndexBuilder<'db> {
         // SAFETY: `definition_node` is guaranteed to be a child of `self.module`
         let kind = unsafe { definition_node.into_owned(self.module.clone()) };
         let category = kind.category();
+        let is_reexported = kind.is_reexported();
         let definition = Definition::new(
             self.db,
             self.file,
             self.current_scope(),
             symbol,
             kind,
+            is_reexported,
             countme::Count::default(),
         );
 

crates/red_knot_python_semantic/src/semantic_index/definition.rs

@@ -33,34 +33,23 @@ pub struct Definition<'db> {
     /// The symbol defined.
     pub(crate) symbol: ScopedSymbolId,
 
+    /// WARNING: Only access this field when doing type inference for the same
+    /// file as where `Definition` is defined to avoid cross-file query dependencies.
     #[no_eq]
     #[return_ref]
     #[tracked]
     pub(crate) kind: DefinitionKind<'db>,
 
+    /// This is a dedicated field to avoid accessing `kind` to compute this value.
+    pub(crate) is_reexported: bool,
+
     count: countme::Count<Definition<'static>>,
 }
 
 impl<'db> Definition<'db> {
     pub(crate) fn scope(self, db: &'db dyn Db) -> ScopeId<'db> {
         self.file_scope(db).to_scope_id(db, self.file(db))
     }
-
-    pub(crate) fn category(self, db: &'db dyn Db) -> DefinitionCategory {
-        self.kind(db).category()
-    }
-
-    pub(crate) fn is_declaration(self, db: &'db dyn Db) -> bool {
-        self.kind(db).category().is_declaration()
-    }
-
-    pub(crate) fn is_binding(self, db: &'db dyn Db) -> bool {
-        self.kind(db).category().is_binding()
-    }
-
-    pub(crate) fn is_reexported(self, db: &'db dyn Db) -> bool {
-        self.kind(db).is_reexported()
-    }
 }
 
 #[derive(Copy, Clone, Debug)]

crates/red_knot_python_semantic/src/semantic_index/use_def.rs

@@ -293,19 +293,22 @@ pub(crate) struct UseDefMap<'db> {
     /// [`SymbolBindings`] reaching a [`ScopedUseId`].
     bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
 
-    /// [`SymbolBindings`] or [`SymbolDeclarations`] reaching a given [`Definition`].
-    ///
     /// If the definition is a binding (only) -- `x = 1` for example -- then we need
     /// [`SymbolDeclarations`] to know whether this binding is permitted by the live declarations.
     ///
+    /// If the definition is both a declaration and a binding -- `x: int = 1` for example -- then
+    /// we don't actually need anything here, all we'll need to validate is that our own RHS is a
+    /// valid assignment to our own annotation.
+    declarations_by_binding: FxHashMap<Definition<'db>, SymbolDeclarations>,
+
     /// If the definition is a declaration (only) -- `x: int` for example -- then we need
     /// [`SymbolBindings`] to know whether this declaration is consistent with the previously
     /// inferred type.
     ///
     /// If the definition is both a declaration and a binding -- `x: int = 1` for example -- then
     /// we don't actually need anything here, all we'll need to validate is that our own RHS is a
     /// valid assignment to our own annotation.
-    definitions_by_definition: FxHashMap<Definition<'db>, SymbolDefinitions>,
+    bindings_by_declaration: FxHashMap<Definition<'db>, SymbolBindings>,
 
     /// [`SymbolState`] visible at end of scope for each symbol.
     public_symbols: IndexVec<ScopedSymbolId, SymbolState>,
@@ -343,25 +346,14 @@ impl<'db> UseDefMap<'db> {
         &self,
         declaration: Definition<'db>,
     ) -> BindingWithConstraintsIterator<'_, 'db> {
-        if let SymbolDefinitions::Bindings(bindings) = &self.definitions_by_definition[&declaration]
-        {
-            self.bindings_iterator(bindings)
-        } else {
-            unreachable!("Declaration has non-Bindings in definitions_by_definition");
-        }
+        self.bindings_iterator(&self.bindings_by_declaration[&declaration])
     }
 
-    pub(crate) fn declarations_at_binding<'map>(
-        &'map self,
+    pub(crate) fn declarations_at_binding(
+        &self,
         binding: Definition<'db>,
-    ) -> DeclarationsIterator<'map, 'db> {
-        if let SymbolDefinitions::Declarations(declarations) =
-            &self.definitions_by_definition[&binding]
-        {
-            self.declarations_iterator(declarations)
-        } else {
-            unreachable!("Binding has non-Declarations in definitions_by_definition");
-        }
+    ) -> DeclarationsIterator<'_, 'db> {
+        self.declarations_iterator(&self.declarations_by_binding[&binding])
     }
 
     pub(crate) fn public_declarations<'map>(
@@ -421,13 +413,6 @@ pub(crate) struct EagerBindingsKey {
 /// A snapshot of bindings that can be used to resolve a reference in a nested eager scope.
 type EagerBindings = IndexVec<ScopedEagerBindingsId, SymbolBindings>;
 
-/// Either live bindings or live declarations for a symbol.
-#[derive(Debug, PartialEq, Eq, salsa::Update)]
-enum SymbolDefinitions {
-    Bindings(SymbolBindings),
-    Declarations(SymbolDeclarations),
-}
-
 #[derive(Debug)]
 pub(crate) struct BindingWithConstraintsIterator<'map, 'db> {
     all_definitions: &'map IndexVec<ScopedDefinitionId, Option<Definition<'db>>>,
@@ -539,8 +524,11 @@ pub(super) struct UseDefMapBuilder<'db> {
     /// Live bindings at each so-far-recorded use.
     bindings_by_use: IndexVec<ScopedUseId, SymbolBindings>,
 
-    /// Live bindings or declarations for each so-far-recorded definition.
-    definitions_by_definition: FxHashMap<Definition<'db>, SymbolDefinitions>,
+    /// Live declarations for each so-far-recorded binding.
+    declarations_by_binding: FxHashMap<Definition<'db>, SymbolDeclarations>,
+
+    /// Live bindings for each so-far-recorded declaration.
+    bindings_by_declaration: FxHashMap<Definition<'db>, SymbolBindings>,
 
     /// Currently live bindings and declarations for each symbol.
     symbol_states: IndexVec<ScopedSymbolId, SymbolState>,
@@ -558,7 +546,8 @@ impl Default for UseDefMapBuilder<'_> {
             visibility_constraints: VisibilityConstraintsBuilder::default(),
             scope_start_visibility: ScopedVisibilityConstraintId::ALWAYS_TRUE,
             bindings_by_use: IndexVec::new(),
-            definitions_by_definition: FxHashMap::default(),
+            declarations_by_binding: FxHashMap::default(),
+            bindings_by_declaration: FxHashMap::default(),
             symbol_states: IndexVec::new(),
             eager_bindings: EagerBindings::default(),
         }
@@ -580,10 +569,8 @@ impl<'db> UseDefMapBuilder<'db> {
     pub(super) fn record_binding(&mut self, symbol: ScopedSymbolId, binding: Definition<'db>) {
         let def_id = self.all_definitions.push(Some(binding));
         let symbol_state = &mut self.symbol_states[symbol];
-        self.definitions_by_definition.insert(
-            binding,
-            SymbolDefinitions::Declarations(symbol_state.declarations().clone()),
-        );
+        self.declarations_by_binding
+            .insert(binding, symbol_state.declarations().clone());
         symbol_state.record_binding(def_id, self.scope_start_visibility);
     }
 
@@ -660,10 +647,8 @@ impl<'db> UseDefMapBuilder<'db> {
     ) {
         let def_id = self.all_definitions.push(Some(declaration));
         let symbol_state = &mut self.symbol_states[symbol];
-        self.definitions_by_definition.insert(
-            declaration,
-            SymbolDefinitions::Bindings(symbol_state.bindings().clone()),
-        );
+        self.bindings_by_declaration
+            .insert(declaration, symbol_state.bindings().clone());
         symbol_state.record_declaration(def_id);
     }
 
@@ -672,7 +657,8 @@ impl<'db> UseDefMapBuilder<'db> {
         symbol: ScopedSymbolId,
         definition: Definition<'db>,
     ) {
-        // We don't need to store anything in self.definitions_by_definition.
+        // We don't need to store anything in self.bindings_by_declaration or
+        // self.declarations_by_binding.
         let def_id = self.all_definitions.push(Some(definition));
         let symbol_state = &mut self.symbol_states[symbol];
         symbol_state.record_declaration(def_id);
@@ -771,7 +757,8 @@ impl<'db> UseDefMapBuilder<'db> {
         self.all_definitions.shrink_to_fit();
         self.symbol_states.shrink_to_fit();
         self.bindings_by_use.shrink_to_fit();
-        self.definitions_by_definition.shrink_to_fit();
+        self.declarations_by_binding.shrink_to_fit();
+        self.bindings_by_declaration.shrink_to_fit();
         self.eager_bindings.shrink_to_fit();
 
         UseDefMap {
@@ -780,7 +767,8 @@ impl<'db> UseDefMapBuilder<'db> {
             visibility_constraints: self.visibility_constraints.build(),
             bindings_by_use: self.bindings_by_use,
             public_symbols: self.symbol_states,
-            definitions_by_definition: self.definitions_by_definition,
+            declarations_by_binding: self.declarations_by_binding,
+            bindings_by_declaration: self.bindings_by_declaration,
             eager_bindings: self.eager_bindings,
         }
     }

crates/red_knot_python_semantic/src/semantic_index/use_def/symbol_state.rs

@@ -112,7 +112,7 @@ type VisibilityConstraintsIterator<'a> = std::slice::Iter<'a, ScopedVisibilityCo
 
 /// Live declarations for a single symbol at some point in control flow, with their
 /// corresponding visibility constraints.
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, Default, PartialEq, Eq, salsa::Update)]
 pub(super) struct SymbolDeclarations {
     /// [`BitSet`]: which declarations (as [`ScopedDefinitionId`]) can reach the current location?
     ///
@@ -200,7 +200,7 @@ impl SymbolDeclarations {
 
 /// Live bindings for a single symbol at some point in control flow. Each live binding comes
 /// with a set of narrowing constraints and a visibility constraint.
-#[derive(Clone, Debug, Default, PartialEq, Eq)]
+#[derive(Clone, Debug, Default, PartialEq, Eq, salsa::Update)]
 pub(super) struct SymbolBindings {
     /// [`BitSet`]: which bindings (as [`ScopedDefinitionId`]) can reach the current location?
     ///

crates/red_knot_python_semantic/src/symbol.rs

@@ -293,7 +293,7 @@ fn core_module_scope(db: &dyn Db, core_module: KnownModule) -> Option<ScopeId<'_
 /// together with boundness information in a [`Symbol`].
 ///
 /// The type will be a union if there are multiple bindings with different types.
-pub(crate) fn symbol_from_bindings<'db>(
+pub(super) fn symbol_from_bindings<'db>(
     db: &'db dyn Db,
     bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>,
 ) -> Symbol<'db> {
@@ -479,6 +479,10 @@ fn symbol_impl<'db>(
 }
 
 /// Implementation of [`symbol_from_bindings`].
+///
+/// ## Implementation Note
+/// This function gets called cross-module. It, therefore, shouldn't
+/// access any AST nodes from the file containing the declarations.
 fn symbol_from_bindings_impl<'db>(
     db: &'db dyn Db,
     bindings_with_constraints: BindingWithConstraintsIterator<'_, 'db>,
@@ -564,6 +568,10 @@ fn symbol_from_bindings_impl<'db>(
 }
 
 /// Implementation of [`symbol_from_declarations`].
+///
+/// ## Implementation Note
+/// This function gets called cross-module. It, therefore, shouldn't
+/// access any AST nodes from the file containing the declarations.
 fn symbol_from_declarations_impl<'db>(
     db: &'db dyn Db,
     declarations: DeclarationsIterator<'_, 'db>,

crates/red_knot_python_semantic/src/types.rs

@@ -16,7 +16,8 @@ pub(crate) use self::diagnostic::register_lints;
 pub use self::diagnostic::{TypeCheckDiagnostic, TypeCheckDiagnostics};
 pub(crate) use self::display::TypeArrayDisplay;
 pub(crate) use self::infer::{
-    infer_deferred_types, infer_definition_types, infer_expression_types, infer_scope_types,
+    infer_deferred_types, infer_definition_types, infer_expression_type, infer_expression_types,
+    infer_scope_types,
 };
 pub use self::narrow::KnownConstraintFunction;
 pub(crate) use self::signatures::Signature;
@@ -26,7 +27,6 @@ use crate::module_resolver::{file_to_module, resolve_module, KnownModule};
 use crate::semantic_index::ast_ids::HasScopedExpressionId;
 use crate::semantic_index::attribute_assignment::AttributeAssignment;
 use crate::semantic_index::definition::Definition;
-use crate::semantic_index::expression::Expression;
 use crate::semantic_index::symbol::ScopeId;
 use crate::semantic_index::{
     attribute_assignments, imported_modules, semantic_index, symbol_table, use_def_map,
@@ -3818,16 +3818,6 @@ impl<'db> Class<'db> {
         name: &str,
         inferred_type_from_class_body: Option<Type<'db>>,
     ) -> Symbol<'db> {
-        // We use a separate salsa query here to prevent unrelated changes in the AST of an external
-        // file from triggering re-evaluations of downstream queries.
-        // See the `dependency_implicit_instance_attribute` test for more information.
-        #[salsa::tracked]
-        fn infer_expression_type<'db>(db: &'db dyn Db, expression: Expression<'db>) -> Type<'db> {
-            let inference = infer_expression_types(db, expression);
-            let expr_scope = expression.scope(db);
-            inference.expression_type(expression.node_ref(db).scoped_expression_id(db, expr_scope))
-        }
-
         // If we do not see any declarations of an attribute, neither in the class body nor in
         // any method, we build a union of `Unknown` with the inferred types of all bindings of
         // that attribute. We include `Unknown` in that union to account for the fact that the