Document #[tracked] recommendation for AstNodeRef

Author: MichaReiser

crates/red_knot_python_semantic/src/ast_node_ref.rs

@@ -13,6 +13,28 @@ use ruff_db::parsed::ParsedModule;
 ///
 /// ## Equality
 /// Two `AstNodeRef` are considered equal if their pointer addresses are equal.
+///
+/// ## Usage in salsa tracked structs
+/// It's important that [`AstNodeRef`] fields in salsa tracked structs are tracked fields
+/// (attributed with `#[tracked`]). It prevents that the tracked struct gets a new ID
+/// everytime the AST changes, which in turn, invalidates the result of any query
+/// that takes said tracked struct as a query argument or returns the tracked struct as part of its result.
+///
+/// For example, marking the [`AstNodeRef`] as tracked on `Expression`
+/// has the effect that salsa will consider the expression as "unchanged" for as long as it:
+///
+/// * belongs to the same file
+/// * belongs to the same scope
+/// * has the same kind
+/// * was created in the same order
+///
+/// This means that changes to expressions in other scopes don't invalidate the expression's id, giving
+/// us some form of scope-stable identity for expressions. Only queries accessing the node field
+/// run on every AST change. All other queries only run when the expression's identity changes.
+///
+/// The one exception to this is if it is known that all queries tacking the tracked struct
+/// as argument or returning it as part of their result are known to access the node field.
+/// Marking the field tracked is then unnecessary.
 #[derive(Clone)]
 pub struct AstNodeRef<T> {
     /// Owned reference to the node's [`ParsedModule`].

crates/red_knot_python_semantic/src/semantic_index/definition.rs

@@ -432,6 +432,13 @@ impl DefinitionCategory {
     }
 }
 
+/// The kind of a definition.
+///
+/// ## Usage in salsa tracked structs
+///
+/// [`DefinitionKind`] fields in salsa tracked structs should be tracked (attributed with `#[tracked]`)
+/// because the kind is a thin wrapper around [`AstNodeRef`]. See the [`AstNodeRef`] documentation
+/// for an in-depth explanation of why this is necessary.
 #[derive(Clone, Debug, Hash)]
 pub enum DefinitionKind<'db> {
     Import(AstNodeRef<ast::Alias>),

crates/red_knot_python_semantic/src/semantic_index/expression.rs

@@ -39,8 +39,9 @@ pub(crate) struct Expression<'db> {
     pub(crate) file_scope: FileScopeId,
 
     /// The expression node.
-    #[return_ref]
+    #[no_eq]
     #[tracked]
+    #[return_ref]
     pub(crate) node_ref: AstNodeRef<ast::Expr>,
 
     /// Should this expression be inferred as a normal expression or a type expression?