diff --git a/crates/oak_db/src/imports.rs b/crates/oak_db/src/imports.rs index 535540d02..7335c2dc1 100644 --- a/crates/oak_db/src/imports.rs +++ b/crates/oak_db/src/imports.rs @@ -2,6 +2,8 @@ use aether_path::FilePath; use camino::Utf8Component; use camino::Utf8Path; use camino::Utf8PathBuf; +use oak_semantic::effects_registry; +use oak_semantic::Effects; use oak_semantic::ImportsResolver; use oak_semantic::SourceResolution; use url::Url; @@ -83,6 +85,22 @@ impl<'db> ImportsResolver for SalsaImportsResolver<'db> { packages, }) } + + fn resolve_effects( + &mut self, + name: &str, + _attached: &[String], + _lazy: bool, + ) -> Option { + // Base is the always-attached layer at the bottom of the search path, + // resolved through the same registry lookup as any package. + // + // TODO!: walk the rest of the search path too (flow-order attaches, + // package siblings via `who_defines`, NAMESPACE imports, re-export chase). + effects_registry::lookup("base", name) + .copied() + .map(Effects::nse) + } } /// Anchor directory for relative `source("path")` arguments. diff --git a/crates/oak_db/src/tests/file.rs b/crates/oak_db/src/tests/file.rs index d0b55182f..f3294d481 100644 --- a/crates/oak_db/src/tests/file.rs +++ b/crates/oak_db/src/tests/file.rs @@ -140,6 +140,31 @@ fn test_semantic_index_matches_oak_semantic() { assert_eq!(via_salsa, &direct); } +#[test] +fn test_semantic_index_recognizes_bare_base_nse() { + // Base NSE resolves through the real `SalsaImportsResolver` (base-only + // `resolve_effects`): a bare `local()` still pushes a nested NSE scope, so + // `x` lands there rather than at file scope. + use oak_semantic::semantic_index::NseScope; + use oak_semantic::semantic_index::NseTiming; + use oak_semantic::semantic_index::ScopeId; + use oak_semantic::semantic_index::ScopeKind; + + let mut db = TestDb::new(); + let file = new_file(&mut db, "a.R", "local({\n x <- 1\n})\n"); + + let index = file.semantic_index(&db); + let file_scope = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert!(index.symbols(file_scope).get("x").is_none()); + assert!(index.symbols(local_scope).get("x").is_some()); +} + #[test] fn test_semantic_index_backdates_on_equivalent_content_set() { let mut db = TestDb::new(); diff --git a/crates/oak_db/src/tests/resolver.rs b/crates/oak_db/src/tests/resolver.rs index 8d7b41688..b0ba0ca8b 100644 --- a/crates/oak_db/src/tests/resolver.rs +++ b/crates/oak_db/src/tests/resolver.rs @@ -198,9 +198,8 @@ fn test_closure_capture_with_source_before_function() { let bindings = fn_map.bindings_at_use(use_id); assert!(bindings.may_be_unbound()); - let symbol = index.uses(fn_scope)[use_id].symbol(); let (enclosing_scope, enclosing_bindings) = index - .enclosing_bindings(fn_scope, symbol) + .enclosing_bindings(fn_scope, use_id) .expect("`helper` should have an enclosing snapshot at the file scope"); assert_eq!(enclosing_scope, file_scope); assert!(!enclosing_bindings.definitions().is_empty()); @@ -321,8 +320,7 @@ fn test_closure_capture_with_source_after_function() { let fn_scope = ScopeId::from(1); let use_id = oak_semantic::UseId::from(0); - let symbol = index.uses(fn_scope)[use_id].symbol(); - assert!(index.enclosing_bindings(fn_scope, symbol).is_some()); + assert!(index.enclosing_bindings(fn_scope, use_id).is_some()); } #[test] diff --git a/crates/oak_semantic/src/builder.rs b/crates/oak_semantic/src/builder.rs index 77752eb61..4a297802a 100644 --- a/crates/oak_semantic/src/builder.rs +++ b/crates/oak_semantic/src/builder.rs @@ -1,3 +1,30 @@ +//! Builds the [`SemanticIndex`] for one R file. +//! +//! The builder splits work by "scan unit": the file or a lazy body (a function, +//! a lazy NSE body like `reactive()`). A unit is coarser than a scope. An eager +//! scope nested inside it, like `local({ ... })`, is part of the same scan unit, +//! while a lazy body starts a new one. +//! +//! Each scan unit is built in two passes: a scan, then a walk. The walk is the +//! pass that writes the arenas (scopes, symbols, definitions, uses, use-def +//! maps). It can only write them correctly if it already knows two things about +//! the scope it's in, and neither is knowable at its own cursor: +//! +//! - Which calls are NSE, so it can push the scope for `local({ ... })` inline +//! as it reaches the call. That turns on whether the callee is shadowed at +//! that point in the flow. +//! +//! - The complete set of names the scope binds, so it can resolve a nested +//! scope's free variable to an ancestor binding. A lazy body (a function, a +//! `reactive()`) can reference a definition the ancestor's own walk hasn't +//! reached yet. That ancestor lookup is what the walk records as an enclosing +//! snapshot. +//! +//! So there are two flow states, on purpose. The scan's flow state tracks only +//! eager bindings and is allowed to stay coarse (across `if` branches it +//! over-approximates to "bound on some path"). The walk builds the precise +//! structures, such as the use-def map. + use std::sync::Arc; use aether_syntax::AnyRArgumentName; @@ -6,6 +33,7 @@ use aether_syntax::AnyRParameterName; use aether_syntax::AnyRValue; use aether_syntax::RArgumentList; use aether_syntax::RBinaryExpression; +use aether_syntax::RCall; use aether_syntax::RExpressionList; use aether_syntax::RFunctionDefinition; use aether_syntax::RNamespaceExpression; @@ -27,9 +55,11 @@ use oak_core::syntax_ext::RStringValueExt; use oak_index_vec::Idx; use oak_index_vec::IndexVec; use rustc_hash::FxHashMap; -use smallvec::SmallVec; +use rustc_hash::FxHashSet; +use crate::effects::ArgumentsAnnotation; use crate::resolver::ImportsResolver; +use crate::resolver::SourceResolution; use crate::semantic_index::Definition; use crate::semantic_index::DefinitionId; use crate::semantic_index::DefinitionKind; @@ -37,25 +67,40 @@ use crate::semantic_index::EnclosingSnapshotId; use crate::semantic_index::EnclosingSnapshotKey; use crate::semantic_index::NamespaceAccess; use crate::semantic_index::NamespaceAccessKind; +use crate::semantic_index::NseScope; +use crate::semantic_index::NseTiming; use crate::semantic_index::Scope; use crate::semantic_index::ScopeId; use crate::semantic_index::ScopeKind; use crate::semantic_index::SemanticCall; use crate::semantic_index::SemanticCallKind; +use crate::semantic_index::SemanticDiagnostic; use crate::semantic_index::SemanticIndex; use crate::semantic_index::SymbolFlags; +use crate::semantic_index::SymbolId; use crate::semantic_index::SymbolTableBuilder; use crate::semantic_index::Use; use crate::semantic_index::UseId; use crate::use_def_map::UseDefMapBuilder; +mod builder_nse; + /// Build a [`SemanticIndex`] from a parsed R file with cross-file /// information supplied by `resolver`. See [`ImportsResolver`] for the /// available impls. +/// +/// See the module docs for the scan/walk split. The scan +/// ([`scan_expression`]) runs first over each scope, then the walk +/// ([`collect_expression`]) reuses its decisions and pushes NSE scopes inline. +/// +/// [`scan_expression`]: SemanticIndexBuilder::scan_expression +/// [`collect_expression`]: SemanticIndexBuilder::collect_expression pub fn build_index(root: &RRoot, resolver: impl ImportsResolver) -> SemanticIndex { let range = root.syntax().text_trimmed_range(); + let mut builder = SemanticIndexBuilder::new(range, resolver); - builder.pre_scan_scope(root.syntax()); + builder.begin_scan(); + builder.scan_expression_list(&root.expressions()); builder.collect_expression_list(&root.expressions()); builder.finish() } @@ -64,17 +109,36 @@ pub fn build_index(root: &RRoot, resolver: impl ImportsResolver) -> SemanticInde // parallel arrays are pushed in lockstep so they stay indexed by the same // `ScopeId`. struct SemanticIndexBuilder { + resolver: R, scopes: IndexVec, symbol_tables: IndexVec, definitions: IndexVec>, uses: IndexVec>, use_def_maps: IndexVec, current_scope: ScopeId, - pre_scans: IndexVec, + bound_names: IndexVec, enclosing_snapshots: FxHashMap, + // Snapshots shared across every use of a free variable in lazy contexts, + // keyed by (nested scope, nested symbol). + lazy_snapshots: FxHashMap<(ScopeId, SymbolId), (ScopeId, EnclosingSnapshotId)>, semantic_calls: Vec, namespace_accesses: Vec, - resolver: R, + // Per-call facts resolved by the scanner in flow order, keyed by the call's + // range. See `CallResolution`. + call_resolutions: FxHashMap, + // Diagnostics collected during the build and logged on `finish()`. A minimal + // channel for now, no user-facing surface. + diagnostics: Vec, + // The scan's flow-precise binding state for the scope being scanned, reset + // at each scope's `begin_scan()`. See [`FlowState`]. + flow_state: FlowState, + // Names inherited from enclosing scopes at this scope's entry point, keyed + // by the scope's range. Captured from `flow_state`, and read by + // `begin_scan()` to seed the scope's own scan. + enclosing_flow: FxHashMap, + // Bound names of Eager + Nested bodies like `local()` are discovered inline + // by the scanner. See `EagerNestedDescent`. + eager_descent: EagerNestedDescent, } impl SemanticIndexBuilder { @@ -84,7 +148,7 @@ impl SemanticIndexBuilder { let mut definitions = IndexVec::new(); let mut uses = IndexVec::new(); let mut use_def_maps = IndexVec::new(); - let mut pre_scans = IndexVec::new(); + let mut bound_names = IndexVec::new(); // The descendants range starts empty (`n+1..n+1`). `pop_scope` later // fills in `descendants.end` with the current arena length. Everything @@ -103,7 +167,7 @@ impl SemanticIndexBuilder { definitions.push(IndexVec::new()); uses.push(IndexVec::new()); use_def_maps.push(UseDefMapBuilder::new()); - pre_scans.push(PreScanScope::new()); + bound_names.push(BoundNames::new()); Self { scopes, @@ -112,10 +176,16 @@ impl SemanticIndexBuilder { uses, use_def_maps, current_scope: file_scope, - pre_scans, + bound_names, enclosing_snapshots: FxHashMap::default(), + lazy_snapshots: FxHashMap::default(), semantic_calls: Vec::new(), namespace_accesses: Vec::new(), + call_resolutions: FxHashMap::default(), + flow_state: FlowState::default(), + enclosing_flow: FxHashMap::default(), + eager_descent: EagerNestedDescent::default(), + diagnostics: Vec::new(), resolver, } } @@ -140,7 +210,7 @@ impl SemanticIndexBuilder { self.definitions.push(IndexVec::new()); self.uses.push(IndexVec::new()); self.use_def_maps.push(UseDefMapBuilder::new()); - self.pre_scans.push(PreScanScope::new()); + self.bound_names.push(BoundNames::new()); id } @@ -162,6 +232,17 @@ impl SemanticIndexBuilder { kind: DefinitionKind, range: TextRange, ) { + // `Nse(Current, Lazy)` scopes don't own any definitions. We add the + // definitions to the real enclosing owner scope. Note that `Current + + // Eager` never reaches here because it doesn't push a scope. + if matches!( + self.scopes[self.current_scope].kind, + ScopeKind::Nse(NseScope::Current, NseTiming::Lazy) + ) { + self.add_definition_to_owner(name, flags, kind, range); + return; + } + let symbol_id = self.symbol_tables[self.current_scope].intern(name, flags); let def_id = self.definitions[self.current_scope].push(Definition { symbol: symbol_id, @@ -172,6 +253,59 @@ impl SemanticIndexBuilder { self.use_def_maps[self.current_scope].record_definition(symbol_id, def_id); } + /// Route a definition from a `Current + Lazy` scope to the scope that + /// owns it. That's the nearest ancestor scope which holds its own + /// definitions. A chain of `Current + Lazy` scopes (e.g. `on_load()` nested + /// in `on_load()`) is skipped: each one routes to its own owner, so they + /// all land in the same outer scope. + pub(super) fn add_definition_to_owner( + &mut self, + name: &str, + flags: SymbolFlags, + kind: DefinitionKind, + range: TextRange, + ) { + let Some(target_scope) = self.definition_owner() else { + stdext::debug_panic!("Current + Lazy scope has no parent"); + return; + }; + + let symbol_id = self.symbol_tables[target_scope].intern(name, flags); + let def_id = self.definitions[target_scope].push(Definition { + symbol: symbol_id, + kind, + range, + }); + + self.use_def_maps[target_scope].ensure_symbol(symbol_id); + + // Deferred: the body executes at an unknown later time, so the + // definition shouldn't shadow what's already live. This is the same + // mechanism as `<<-`. + // + // Known imprecision: the deferred def is visible to ALL uses in + // the parent scope (with `may_be_unbound: true`), including + // file-level uses that run before the lazy body executes. Ideally + // these defs would only be reachable from lazy scopes (functions), + // not from eager/file-level code. + self.use_def_maps[target_scope].record_deferred_definition(symbol_id, def_id); + } + + /// The scope that owns definitions of a `Current + Lazy` NSE scope. The + /// climb is iterative to handle e.g. `on_load(on_load(...))`. Every other + /// scope kind (`File`, `Function`, `Nse(Nested, _)`) owns its definitions + /// and stops the climb. + fn definition_owner(&self) -> Option { + let mut scope = self.scopes[self.current_scope].parent?; + while matches!( + self.scopes[scope].kind, + ScopeKind::Nse(NseScope::Current, NseTiming::Lazy) + ) { + scope = self.scopes[scope].parent?; + } + Some(scope) + } + // Super-assignment is lexically in the current scope but binds in an // ancestor. We record the definition in the current scope and append // it to the target scope's use-def map (without shadowing prior @@ -256,55 +390,83 @@ impl SemanticIndexBuilder { // Associate free variables with the enclosing snapshot where the // variable is defined if self.use_def_maps[self.current_scope].is_may_be_unbound(symbol_id) { - let use_key = EnclosingSnapshotKey { - nested_scope: self.current_scope, - nested_symbol: symbol_id, - }; - self.register_enclosing_snapshot(name, use_key); + self.register_enclosing_snapshot(name, symbol_id, use_id); } } - fn register_enclosing_snapshot(&mut self, name: &str, use_key: EnclosingSnapshotKey) { + fn register_enclosing_snapshot(&mut self, name: &str, nested_symbol: SymbolId, use_id: UseId) { // We're looking for a parent definition for this scope's free variable // so start from parent let Some(mut current_scope) = self.scopes[self.current_scope].parent else { return; }; + // Eager vs lazy snapshot for this free variable. Eager snapshots are + // precise, lazy ones over-approximate. For instance in: + // + // ``` + // x <- 1 + // local({ x }) + // x <- 2 + // ``` + // + // The eager body in `local()` captures `x <- 1` but not `x <- 2`. If + // the body was inside a lazy context like `function()` instead, the use + // of `x` could run at any time and we'd fall back to the accumulated + // union `{1, 2}`, which is an over-approximation. + // + // A precise enclosing snapshot requires eagerness throughout, which we + // track with `all_eager`. + let mut all_eager = !self.scopes[self.current_scope].kind.is_lazy(); + loop { - let found_by_flag = self.symbol_tables[current_scope] - .id(name) - .is_some_and(|sym_id| { - self.symbol_tables[current_scope] - .symbol(sym_id) - .flags() - .contains(SymbolFlags::IS_BOUND) - }); - - let found_by_prescan = self.pre_scans[current_scope].has_name(name); - - if found_by_flag || found_by_prescan { + if self.scope_binds_anywhere(current_scope, name) { // Intern with empty flags: we just need a stable `SymbolId` for - // the lookup key. If found via `found_by_flag`, the symbol - // already exists with `IS_BOUND`. If found via pre-scan only, - // the later `add_definition` call during the full walk will set - // `IS_BOUND`. + // the lookup key. If the symbol was found via its `IS_BOUND` + // flag, it already exists. If found via pre-scan only, the later + // `add_definition()` call during the full walk will set `IS_BOUND`. let enclosing_symbol_id = self.symbol_tables[current_scope].intern(name, SymbolFlags::empty()); + self.use_def_maps[current_scope].ensure_symbol(enclosing_symbol_id); - if self.enclosing_snapshots.contains_key(&use_key) { - return; - } + let entry = if all_eager { + // Eager: a fresh point-in-time snapshot per use, no dedup and + // no watcher. Two uses at different points in the body can + // capture different enclosing states (e.g. either side of a + // `<<-`), so they can't share. + let snapshot_id = self.use_def_maps[current_scope] + .register_eager_snapshot(enclosing_symbol_id); + (current_scope, snapshot_id) + } else { + // Lazy: every use of this symbol resolves to the same + // growing snapshot, so dedup on (nested scope, nested symbol) + // and reuse it across uses. + let dedup_key = (self.current_scope, nested_symbol); + + if let Some(&entry) = self.lazy_snapshots.get(&dedup_key) { + entry + } else { + let snapshot_id = self.use_def_maps[current_scope] + .register_lazy_snapshot(enclosing_symbol_id); + let entry = (current_scope, snapshot_id); + self.lazy_snapshots.insert(dedup_key, entry); + entry + } + }; - self.use_def_maps[current_scope].ensure_symbol(enclosing_symbol_id); - let snapshot_id = self.use_def_maps[current_scope] - .register_enclosing_snapshot(enclosing_symbol_id); - self.enclosing_snapshots - .insert(use_key, (current_scope, snapshot_id)); + let use_key = EnclosingSnapshotKey { + nested_scope: self.current_scope, + nested_use: use_id, + }; + self.enclosing_snapshots.insert(use_key, entry); return; } + if self.scopes[current_scope].kind.is_lazy() { + all_eager = false; + } + let Some(parent) = self.scopes[current_scope].parent else { return; }; @@ -312,6 +474,356 @@ impl SemanticIndexBuilder { } } + /// Whether `scope` binds `name` anywhere, regardless of flow position: an + /// already-recorded `IS_BOUND` definition or a pre-scanned assignment. The + /// pre-scan covers definitions the walk hasn't reached yet in this scope. + fn scope_binds_anywhere(&self, scope: ScopeId, name: &str) -> bool { + self.walked_binding(scope, name).is_some() || self.bound_names[scope].binds(name) + } + + /// The site where `scope` binds `name`, matching what + /// [`scope_binds_anywhere`](Self::scope_binds_anywhere) counts as a binding + /// (so it returns `Some` on exactly the same names). Prefers the + /// scan-collected site in `bound_names`, falling back to the range of an + /// already-walked `IS_BOUND` definition (e.g. a parameter, which the scan + /// seeds straight into `flow_state` without a `bound_names` entry). Used to + /// point the lazy-shadow diagnostic at the overwrite. + fn scope_binding_range(&self, scope: ScopeId, name: &str) -> Option { + if let Some(range) = self.bound_names[scope].binding_range(name) { + return Some(range); + } + + // `IS_BOUND` always has a matching `Definition` row (see the invariant + // in `resolve_symbol()`), so the find never misses when the flag is set. + let sym_id = self.walked_binding(scope, name)?; + self.definitions[scope] + .iter() + .find(|(_id, def)| def.symbol == sym_id) + .map(|(_id, def)| def.range) + } + + /// The symbol `name` interns to in `scope`, if the walk has already recorded + /// an `IS_BOUND` definition for it. + fn walked_binding(&self, scope: ScopeId, name: &str) -> Option { + let sym_id = self.symbol_tables[scope].id(name)?; + self.symbol_tables[scope] + .symbol(sym_id) + .flags() + .contains(SymbolFlags::IS_BOUND) + .then_some(sym_id) + } + + /// Record the names a child scope (function body, NSE argument) about to be + /// created at `range` inherits from its ancestors, to seed the child's scan + /// in `begin_scan`. Called during the scan, where `flow_state` is the + /// parent's flow-precise state at the child's definition point (already + /// carrying the parent's own inherited ancestors, so the child inherits + /// transitively). + pub(super) fn record_enclosing_flow(&mut self, range: TextRange) { + self.enclosing_flow + .insert(range, self.flow_state.snapshot()); + } + + // --- Scan pass --- + + /// Reset the flow-precise binding state for a fresh scope's scan. + /// + /// Seeds it with two things: + /// + /// - The names inherited from enclosing scopes, captured when this scope was + /// entered (`enclosing_flow`). The parent's own scan was seeded the same + /// way, so this is transitively complete: it holds every eager binding + /// visible from an ancestor at this scope's definition point. + /// - The scope's own already-bound names. For a function scope that's the + /// parameters, recorded just before the scan runs. For file and NSE scopes + /// nothing local is bound yet. + /// + /// Parameter defaults are a special case: they are scanned before the params + /// are recorded, so `collect_function` seeds the full formal set by hand + /// (all formals bind at once in R, so a default sees every parameter name). + pub(super) fn begin_scan(&mut self) { + let range = self.scopes[self.current_scope].range; + + match self.enclosing_flow.get(&range).cloned() { + Some(entry) => self.flow_state.restore(entry), + None => self.flow_state.clear(), + } + + for (_id, symbol) in self.symbol_tables[self.current_scope].iter() { + if symbol.flags().contains(SymbolFlags::IS_BOUND) { + self.flow_state.bind(symbol.name().to_string()); + } + } + } + + pub(super) fn scan_expression_list(&mut self, list: &RExpressionList) { + for expr in list.iter() { + self.scan_expression(&expr); + } + } + + /// Scan for NSE calls and collect the scope's bound names, in flow order. + /// + /// Runs before the walk of a scope. It decides NSE-ness at each call the + /// same way the walk's [`is_locally_bound`](Self::is_locally_bound) would, + /// records the decision in `call_resolutions` for the walk to reuse, and adds + /// non-skipped definition names to `bound_names`. The bound names must be + /// complete before the walk descends into any child scope, because a lazy + /// child body can reference an ancestor def the ancestor's walk hasn't + /// reached yet. + /// + /// A scan unit is the file or a lazy body (function, `Nested + Lazy`, + /// `Current + Lazy`). Each unit is scanned once. Within a unit the scan + /// descends through every eager boundary it meets, in flow order: + /// + /// - A `Current + Eager` body pushes no scope, so it stays part of this + /// scope's direct level and is scanned through transparently. + /// - A `Nested + Eager` body is descended into with a save/restore of + /// `flow_state`, and the names it binds are left pending for the walk to + /// install without re-scanning. + /// - Function and lazy bodies (`Nested + Lazy`, `Current + Lazy`) are their + /// own scan units, scanned separately when the walk enters them, because + /// NSE resolution there needs the child's own flow context. + /// + /// Branch analysis is precise. In `if (c) local <- f else local({ y <- 1 + /// })` the else branch sees an NSE call because `local` is unbound on the + /// else path, which prevents `y` from leaking into the scope. + pub(super) fn scan_expression(&mut self, expr: &AnyRExpression) { + match expr { + AnyRExpression::RFunctionDefinition(func) => { + // A function body is a child scope, scanned when it's entered. + // Record the names it inherits now so that when we later resolve + // an NSE callee inside the body, we can check whether one of them + // shadows it (see `enclosing_flow`). + self.record_enclosing_flow(func.syntax().text_trimmed_range()); + }, + + AnyRExpression::RBracedExpressions(braced) => { + self.scan_expression_list(&braced.expressions()); + }, + + AnyRExpression::RBinaryExpression(bin) => { + if is_assignment(bin) { + let right = is_right_assignment(bin); + + // Value side first, mirroring `collect_assignment`: it may + // hold NSE calls or nested defs that flow before the binding. + let value = if right { bin.left() } else { bin.right() }; + if let Ok(value) = value { + self.scan_expression(&value); + } + + let target = if right { bin.right() } else { bin.left() }; + if let Ok(target) = target { + match assignment_name(&target) { + // `<<-` binds in an ancestor, not here, so it doesn't + // shadow a callee in this scope (matching the walk). + Some((name, range)) if !is_super_assignment(bin) => { + self.record_binding(name, range); + }, + Some(_) => {}, + // Complex target (`x$foo <- v`): no binding, but the + // target may hold NSE calls. + None => self.scan_expression(&target), + } + } + } else { + if let Ok(lhs) = bin.left() { + self.scan_expression(&lhs); + } + if let Ok(rhs) = bin.right() { + self.scan_expression(&rhs); + } + } + }, + + AnyRExpression::RCall(call) => { + if let Ok(func) = call.function() { + self.scan_expression(&func); + } + self.scan_call(call); + self.scan_semantic_call(call); + }, + + AnyRExpression::RForStatement(stmt) => { + // The for-variable is always bound (R sets it to NULL for empty + // sequences), so it binds before the body regardless of flow. + if let Ok(variable) = stmt.variable() { + self.record_binding( + variable.name_text(), + variable.syntax().text_trimmed_range(), + ); + } + if let Ok(sequence) = stmt.sequence() { + self.scan_expression(&sequence); + } + // A loop body only adds bindings (a name bound inside still + // "reaches" on the ran path), so no restore is needed, unlike + // the two-branch `if`/`else` below. + if let Ok(body) = stmt.body() { + self.scan_expression(&body); + } + }, + + AnyRExpression::RIfStatement(stmt) => { + if let Ok(condition) = stmt.condition() { + self.scan_expression(&condition); + } + + let pre_if = self.flow_state.snapshot(); + + if let Ok(consequence) = stmt.consequence() { + self.scan_expression(&consequence); + } + + let post_if = self.flow_state.snapshot(); + self.flow_state.restore(pre_if); + + if let Some(else_clause) = stmt.else_clause() { + if let Ok(alternative) = else_clause.alternative() { + self.scan_expression(&alternative); + } + } + + // Both branches' bindings are live afterwards. + self.flow_state.merge(post_if); + }, + + // `while`/`repeat` loops, subsets, extractions, parentheses, unary + // ops, and literals: recurse into child expressions. Loops need no + // flow restore (see the `for` arm). Identifiers and dots are leaves + // with no bindings or calls, so they fall through to a no-op walk. + _ => { + self.scan_descendants(expr.syntax()); + }, + } + } + + /// Walk descendant nodes of `expr`, scanning the outermost + /// `AnyRExpression` children. The scan analog of + /// `collect_descendants`. + fn scan_descendants(&mut self, node: &RSyntaxNode) { + let mut preorder = node.preorder(); + preorder.next(); + + while let Some(event) = preorder.next() { + let WalkEvent::Enter(node) = event else { + continue; + }; + if let Some(expr) = node.cast::() { + self.scan_expression(&expr); + preorder.skip_subtree(); + } + } + } + + fn scan_parameter_defaults(&mut self, params: &RParameters) { + // Seed `flow_state` with every parameter names so a callee inside a + // default value sees the full formal set + for param in params.items().iter() { + let Ok(param) = param else { continue }; + let Ok(name) = param.name() else { continue }; + let text = match &name { + AnyRParameterName::RIdentifier(ident) => ident.name_text(), + AnyRParameterName::RDots(_) => String::from("..."), + AnyRParameterName::RDotDotI(ddi) => ddi.syntax().text_trimmed().to_string(), + }; + self.flow_state.bind(text); + } + + for param in params.items().iter() { + let Ok(param) = param else { continue }; + let Some(default) = param.default() else { + continue; + }; + if let Ok(value) = default.value() { + self.scan_expression(&value); + } + } + } + + /// Scan-time analog of [`collect_semantic_call`]. + /// + /// Only `source()` needs handling here. Its injected bindings shadow NSE + /// callees, and the walk injects them too late for a later call in the same + /// scope to see. `library()`/`require()` attaches don't affect the scan + /// decisions yet, so they stay with the walk. + /// + /// [`collect_semantic_call`]: Self::collect_semantic_call + fn scan_semantic_call(&mut self, call: &aether_syntax::RCall) { + let Ok(AnyRExpression::RIdentifier(ident)) = call.function() else { + return; + }; + if ident.name_text() == "source" { + self.scan_source_call(call); + } + } + + /// Resolve a `source()` call once, cache it, and bind the sourced names. + /// + /// The binding is eager: `source()` runs at its position, so the sourced + /// names ARE bound afterwards and can shadow a later NSE callee (e.g. a + /// sourced `local` masking base `local`). The resolution is cached by call + /// range so the walk reuses it instead of consulting the resolver again. + fn scan_source_call(&mut self, call: &aether_syntax::RCall) { + let Some(path) = self.parse_source_path(call) else { + return; + }; + let Some(resolution) = self.resolver.resolve_source(&path) else { + return; + }; + + // Sourced names originate in another file, so they have no binding site + // here. Anchor the overwrite range at the `source()` call instead. + let range = call.syntax().text_trimmed_range(); + for name in &resolution.names { + self.record_binding(name.clone(), range); + } + + self.call_resolutions.entry(range).or_default().source = Some(resolution); + } + + /// Record a binding in the scan's flow state. + /// + /// The flow-precise `flow_state` always learns the name, so a + /// later callee in this scope sees it shadowed. The bound names only get it + /// when the current scope owns it. A `Current + Lazy` scope routes its defs + /// to the owner, so the name is added to the owner's bound names instead, the + /// same routing `add_definition_to_owner` does during the walk. + fn record_binding(&mut self, name: String, range: TextRange) { + self.record_owner_name(name.clone(), range); + self.flow_state.bind(name); + } + + /// Route a binding NAME into its owner scope's bound names, matching + /// `add_definition`'s routing. When a descent is open the name goes to the + /// descent top, which is always an eager `Nested` body scanned inline and so + /// owns its bindings. Otherwise a `Current + Lazy` scope routes to + /// `definition_owner()` and every other scope owns its bindings. + /// + /// Split from `record_binding` so `scan_lazy_owner_bindings` can add + /// a deferred body's names to the owner's bound names without also marking them + /// bound in `flow_state` (see that helper for why). + fn record_owner_name(&mut self, name: String, range: TextRange) { + if let Some(bound) = self.eager_descent.open.last_mut() { + bound.add(name, range); + return; + } + + if let Some(target) = match self.scopes[self.current_scope].kind { + ScopeKind::Nse(NseScope::Current, NseTiming::Lazy) => self.definition_owner(), + _ => Some(self.current_scope), + } { + self.bound_names[target].add(name, range); + } + } + + fn nse_effect(&self, call: &RCall) -> Option { + self.call_resolutions + .get(&call.syntax().text_trimmed_range()) + .and_then(|resolution| resolution.nse) + } + // --- Recursive descent --- fn collect_expression_list(&mut self, list: &RExpressionList) { @@ -366,16 +878,18 @@ impl SemanticIndexBuilder { // clauses contain `RIdentifier` nodes that should not be recorded // as uses. AnyRExpression::RCall(call) => { + // Record the callee as a use (a no-op for `pkg::fn`) before + // handling NSE. if let Ok(func) = call.function() { self.collect_expression(&func); } - if let Ok(args) = call.arguments() { + + if let Some(annotation) = self.nse_effect(call) { + self.collect_nse_call(call, annotation) + } else if let Ok(args) = call.arguments() { self.collect_arguments(&args.items()); } - // TODO(nse): When eager NSE scopes land (e.g. `local()`) we should - // also consider nested scopes as long as they're not lazy (e.g. - // function definitions or NSE calls that don't evaluate - // immediately. + self.collect_semantic_call(call); }, AnyRExpression::RSubset(subset) => { @@ -550,61 +1064,28 @@ impl SemanticIndexBuilder { let scope = self.push_scope(ScopeKind::Function, fun.syntax().text_trimmed_range()); if let Ok(params) = fun.parameters() { + // Scan the default values before collecting them. R binds all + // formals into the frame at once, so a default sees every parameter + // name regardless of position: `function(local, b = local(...))` is + // not NSE. So we seed the whole formal set into `flow_state` + // up front rather than flow-ordered, then scan each default. + self.begin_scan(); + self.scan_parameter_defaults(¶ms); + + // `collect_parameters` adds the parameter definitions and walks + // each default in source order, finding the NSE decisions the scan + // above recorded. self.collect_parameters(¶ms); } if let Ok(body) = fun.body() { - self.pre_scan_scope(body.syntax()); + self.begin_scan(); + self.scan_expression(&body); self.collect_expression(&body); } self.pop_scope(scope); } - /// Pre-scan a scope to collect all definition names (skipping nested - /// function bodies). Runs before the full walk so that enclosing - /// snapshot registration can find where free variables are bound, - /// even when the walk in the parent scope hasn't reached the - /// definition yet. Must stay in sync with the full walk's definition - /// handling: any construct that calls `add_definition` should have a - /// corresponding entry here. - fn pre_scan_scope(&mut self, node: &RSyntaxNode) { - let mut preorder = node.preorder(); - while let Some(event) = preorder.next() { - let WalkEvent::Enter(node) = event else { - continue; - }; - let Some(expr) = AnyRExpression::cast(node) else { - continue; - }; - match &expr { - // NSE scopes (e.g. `local({...})`) will also need to - // be skipped here once recognized, since their - // definitions belong to a child scope. - AnyRExpression::RFunctionDefinition(_) => { - preorder.skip_subtree(); - }, - AnyRExpression::RBinaryExpression(bin) - if is_assignment(bin) && !is_super_assignment(bin) => - { - let right = is_right_assignment(bin); - let target = if right { bin.right() } else { bin.left() }; - if let Ok(target) = target { - if let Some((name, range)) = assignment_name(&target) { - self.pre_scans[self.current_scope].add(name, range); - } - } - }, - AnyRExpression::RForStatement(stmt) => { - if let Ok(variable) = stmt.variable() { - self.pre_scans[self.current_scope] - .add(variable.name_text(), variable.syntax().text_trimmed_range()); - } - }, - _ => {}, - } - } - } - fn collect_parameters(&mut self, params: &RParameters) { for param in params.items().iter() { let Ok(param) = param else { continue }; @@ -800,52 +1281,21 @@ impl SemanticIndexBuilder { // regardless to keep the sourcing mechanism simple. A future diagnostic // should suggest `local = TRUE` in nested contexts. fn collect_source_call(&mut self, call: &aether_syntax::RCall) { - let Ok(args) = call.arguments() else { + let Some(path) = self.parse_source_path(call) else { return; }; - let mut path: Option = None; - let mut bail = false; + let range = call.syntax().text_trimmed_range(); + let call_offset = range.start(); - for item in args.items().iter() { - let Ok(arg) = item else { continue }; - - if let Some(name_clause) = arg.name_clause() { - let Ok(AnyRArgumentName::RIdentifier(name_ident)) = name_clause.name() else { - continue; - }; - if name_ident.name_text() == "local" { - if let Some(value) = arg.value() { - match value { - // TRUE/FALSE are fine, we resolve uniformly. For - // the FALSE in nested context case, we'll emit a - // diagnostic. - AnyRExpression::RTrueExpression(_) | - AnyRExpression::RFalseExpression(_) => {}, - // With anything else (environment, non-statically - // resolvable expression) is not we need to bail. - _ => bail = true, - } - } - } - } else if path.is_none() { - // First positional argument: the file path - if let Some(AnyRExpression::AnyRValue(AnyRValue::RStringValue(s))) = arg.value() { - path = s.string_text(); - } - } - } - - if bail { - return; - } - - let Some(path) = path else { - return; - }; - - let call_offset = call.syntax().text_trimmed_range().start(); - let resolution = self.resolver.resolve_source(&path); + // Read the resolution the scan already computed. The scan is the + // single point that consults `resolve_source`, so the walk never + // re-resolves. A cache miss means the scan bailed or the resolver + // returned `None`, both of which record the call with `resolved: None`. + let resolution = self + .call_resolutions + .get(&range) + .and_then(|resolution| resolution.source.clone()); // Record every `source()` call site, independent of whether the // resolution was successful. `resolved` pins the canonical URL when @@ -899,9 +1349,67 @@ impl SemanticIndexBuilder { } } + /// Parse the file path out of a `source("path")` call. + /// + /// Shared by the scan and the walk so they agree on which calls are + /// statically analyzable. Returns `None` when there's no positional path, + /// or when `local =` is set to something other than TRUE/FALSE (an + /// environment or a non-literal expression we can't follow). + fn parse_source_path(&self, call: &aether_syntax::RCall) -> Option { + let args = call.arguments().ok()?; + + let mut path: Option = None; + + for item in args.items().iter() { + let Ok(arg) = item else { continue }; + + if let Some(name_clause) = arg.name_clause() { + let Ok(AnyRArgumentName::RIdentifier(name_ident)) = name_clause.name() else { + continue; + }; + if name_ident.name_text() == "local" { + if let Some(value) = arg.value() { + match value { + // TRUE/FALSE are fine, we resolve uniformly. For + // the FALSE in nested context case, we'll emit a + // diagnostic. + AnyRExpression::RTrueExpression(_) | + AnyRExpression::RFalseExpression(_) => {}, + // Anything else (environment, non-statically + // resolvable expression) means we bail. + _ => return None, + } + } + } + } else if path.is_none() { + // First positional argument: the file path + if let Some(AnyRExpression::AnyRValue(AnyRValue::RStringValue(s))) = arg.value() { + path = s.string_text(); + } + } + } + + path + } + fn finish(mut self) -> SemanticIndex { self.scopes[ScopeId::from(0)].descendants.end = self.scopes.next_id(); + // TODO(diagnostics): Diagnostics are not surfaced yet, so log them for now + for diagnostic in &self.diagnostics { + match diagnostic { + SemanticDiagnostic::LazyShadowAmbiguity { + name, + call_range, + overwrite_range, + } => log::warn!( + "NSE lazy-shadow ambiguity: callee `{name}` at {call_range:?} is recognized \ + as NSE, but a lazy-crossed ancestor binds it at {overwrite_range:?} with \ + undetermined timing" + ), + } + } + let symbol_tables = self .symbol_tables .into_iter() @@ -928,57 +1436,131 @@ impl SemanticIndexBuilder { self.enclosing_snapshots, self.semantic_calls, self.namespace_accesses, + self.diagnostics, file_final_bindings, ) } } -/// All definitions in a scope, collected before the full walk. Skips nested -/// function bodies (those belong to child scopes). Two consumers: +/// What the scan resolved a single call to, for the walk to reuse. A call can +/// carry both facts at once. +/// +/// - `nse`: the NSE effect the call resolved to, filled in flow order. `None` +/// means "not NSE". +/// - `source`: the resolution of a `source()` call. The scan fills it once +/// (consulting `resolve_source`), the walk reads it back, so the resolver is +/// queried exactly once per `source()` call site. +#[derive(Default)] +struct CallResolution { + nse: Option, + source: Option, +} + +/// The scan's flow-precise binding state: which names are bound at the current +/// point of the current scan unit, in flow order. +/// +/// It's the scan's own flow state, a coarse variant of the walk's use-def map, +/// which isn't built yet. It answers one question, "is this name bound here?", +/// so the scan can tell whether a callee is shadowed at each call and decide +/// whether a call is NSE. It tracks only eager bindings, and it is allowed to +/// stay coarse: `merge()` unions the two sides of an `if`, so that a single +/// branch marks a name as bound. +#[derive(Clone, Default)] +struct FlowState { + bound: FxHashSet, +} + +impl FlowState { + /// Save the current state, to rewind to or to seed a child scan unit from. + fn snapshot(&self) -> FlowState { + self.clone() + } + + /// Rewind to `snapshot`, dropping any bindings recorded since it was taken. + fn restore(&mut self, snapshot: FlowState) { + *self = snapshot; + } + + /// Union `snapshot` in, so a name reads as bound here if it was bound on + /// either path. This is the `if`/`else` join. + fn merge(&mut self, snapshot: FlowState) { + self.bound.extend(snapshot.bound); + } + + /// Record `name` as bound from here on. + fn bind(&mut self, name: String) { + self.bound.insert(name); + } + + /// Whether `name` is bound at the current point. + fn is_bound(&self, name: &str) -> bool { + self.bound.contains(name) + } + + /// Drop all bindings, to start a fresh scan unit (see `begin_scan()`). + fn clear(&mut self) { + self.bound.clear(); + } +} + +/// Tracks eager `Nested` NSE bodies scanned inline during the scan. +/// +/// An eager `Nested` body like `local()` runs immediately at its call site, so +/// we scan it inline instead of deferring it to the walk. `open` is the stack +/// of bodies being scanned right now, with the innermost on top. +/// `record_owner_name()` routes a binding to the top so names land on the body +/// that owns them. When a descent finishes, its names move to `pending`, keyed +/// by the body's range. +/// +/// `pending` is keyed by range rather than written straight into +/// `bound_names[scope]` because the body's arena scope doesn't exist yet: the +/// walk allocates scopes in preorder, and allocating one mid-scan would break +/// the `Scope::descendants` invariant. The range is the body's pre-arena +/// identity until the walk pushes its scope. /// -/// - Enclosing snapshots: `has_name()` checks whether a symbol will be -/// defined in an ancestor scope (even when the ancestor's walk hasn't reached -/// that definition yet), so that `register_enclosing_snapshot()` can find the -/// right ancestor for free variables. -/// - NSE resolution: With NSE, each function call potentially pushes a scope -/// (which can be lazy or eager). We need to resolve the called function's -/// semantic during the walk. Inside lazy scopes (e.g. function bodies), -/// `by_name` provides the complete set of parent definitions so that the -/// function can be resolved against all the parent scope's definitions (if NSE -/// semantics don't match across definitions, we pick one and lint). Intra-scope -/// resolution is linear and uses the current `symbol_states` directly instead. -struct PreScanScope { - _defs: Vec, - by_name: FxHashMap>, +/// Once the walk pushes that scope, it installs the pending names into it +/// instead of re-scanning. It does this before collecting the body, because a +/// lazy child inside (a function or lazy NSE body) runs later than the walk +/// reaches it, so it can reference a binding defined further down this scope. +/// Resolving that name checks whether an enclosing scope binds it +/// (`scope_binds_anywhere()`), and the walk hasn't recorded that binding yet, so +/// the scan-populated bound set has to be complete up front. That's the reason +/// the scan collects bound names ahead of the walk at all. +#[derive(Default)] +struct EagerNestedDescent { + open: Vec, + pending: FxHashMap, } -/// A single definition site found during the pre-scan. Fields are not -/// read yet but will be used for NSE lookup. -struct PreScanDef { - _name: String, - _range: TextRange, +/// All definitions in a scope, collected by the scan pass before the +/// walk. Skips child-scope bodies (nested functions and `Nested` NSE bodies). +/// +/// Keeps each name's earliest binding site in scan order, which is source +/// order within the scope. A name bound several times reads as bound +/// throughout, and this earliest site is what the lazy-shadow diagnostic points +/// at, once `is_lazily_shadowed` has picked the nearest binding ancestor. +struct BoundNames { + by_name: FxHashMap, } -impl PreScanScope { +impl BoundNames { fn new() -> Self { Self { - _defs: Vec::new(), by_name: FxHashMap::default(), } } fn add(&mut self, name: String, range: TextRange) { - let idx = self._defs.len(); - self.by_name.entry(name.clone()).or_default().push(idx); - self._defs.push(PreScanDef { - _name: name, - _range: range, - }); + self.by_name.entry(name).or_insert(range); } - fn has_name(&self, name: &str) -> bool { + fn binds(&self, name: &str) -> bool { self.by_name.contains_key(name) } + + fn binding_range(&self, name: &str) -> Option { + self.by_name.get(name).copied() + } } fn is_assignment(bin: &RBinaryExpression) -> bool { diff --git a/crates/oak_semantic/src/builder/builder_nse.rs b/crates/oak_semantic/src/builder/builder_nse.rs new file mode 100644 index 000000000..daa6ad85b --- /dev/null +++ b/crates/oak_semantic/src/builder/builder_nse.rs @@ -0,0 +1,528 @@ +use aether_syntax::AnyRArgumentName; +use aether_syntax::AnyRExpression; +use aether_syntax::RArgumentList; +use aether_syntax::RCall; +use biome_rowan::AstNode; +use biome_rowan::AstNodeList; +use biome_rowan::AstSeparatedList; +use biome_rowan::TextRange; +use oak_core::syntax_ext::AnyRSelectorExt; +use oak_core::syntax_ext::RIdentifierExt; +use oak_core::syntax_ext::RStringValueExt; + +use super::assignment_name; +use super::is_assignment; +use super::is_right_assignment; +use super::is_super_assignment; +use super::BoundNames; +use super::SemanticIndexBuilder; +use crate::effects::Argument; +use crate::effects::ArgumentsAnnotation; +use crate::effects::Effects; +use crate::effects_registry; +use crate::resolver::ImportsResolver; +use crate::semantic_index::NseScope; +use crate::semantic_index::NseTiming; +use crate::semantic_index::ScopeKind; +use crate::semantic_index::SemanticDiagnostic; + +impl SemanticIndexBuilder { + /// Scan a call for effects (e.g. NSE scopes) and record its decision for + /// the walk to reuse. + /// + /// If the callee resolves to an NSE annotation, the annotation is recorded + /// in `call_resolutions` under the call's range (as the entry's `nse`). + /// Arguments evaluated in nested calls are scanned accordingly. Otherwise + /// all arguments are scanned in the current scope. + /// + /// `Current + Eager` and `Nested + Eager` arguments are scanned here: + /// `Current + Eager` transparently, `Nested + Eager` by descending into the + /// body and holding the names it binds as pending. `Nested + Lazy` and + /// `Current + Lazy` bodies are their own scan units and deferred to the walk + /// because resolution of effects in these lazy scopes needs the child's own + /// flow context. + pub(super) fn scan_call(&mut self, call: &RCall) { + let Some(annotation) = self.resolve_nse(call) else { + if let Ok(args) = call.arguments() { + for item in args.items().iter() { + let Ok(arg) = item else { continue }; + if let Some(value) = arg.value() { + self.scan_expression(&value); + } + } + } + return; + }; + + self.call_resolutions + .entry(call.syntax().text_trimmed_range()) + .or_default() + .nse = Some(annotation); + + let Ok(args) = call.arguments() else { + return; + }; + let items = args.items(); + let nse_args = self.match_nse_arguments(&items, annotation); + + for (i, item) in items.iter().enumerate() { + let Ok(arg) = item else { continue }; + let Some(value) = arg.value() else { continue }; + + match nse_args[i] { + None => self.scan_expression(&value), + Some(nse_arg) => match (nse_arg.scope, nse_arg.timing) { + // Calls like `evalq()` + (NseScope::Current, NseTiming::Eager) => self.scan_expression(&value), + + // Calls like `on_load()`. Its body runs later, so its defs + // land in the enclosing scope. We don't resolve the body's + // calls here. The walk does that once it enters the child + // scope. But we do grab the names it defines now, so the + // owner's bound names are complete before the walk reaches a sibling. + (NseScope::Current, NseTiming::Lazy) => { + self.record_enclosing_flow(value.syntax().text_trimmed_range()); + self.scan_lazy_owner_bindings(&value); + }, + + // Calls like `local()`. Its body runs eagerly at the call + // site, so its environment IS the current `flow_state`. + // Descend now, holding the names bound in this scope as + // pending so the walk has access to them. No `flow_state` + // reset: the child sees exactly what `begin_scan()` would + // have seeded. + // No `record_enclosing_flow()`: eager `Nested` bodies are + // never scanned at walk time, so nothing would read it. + (NseScope::Nested, NseTiming::Eager) => { + let old = self.flow_state.snapshot(); + + let range = value.syntax().text_trimmed_range(); + self.eager_descent.open.push(BoundNames::new()); + self.scan_expression(&value); + if let Some(bound) = self.eager_descent.open.pop() { + self.eager_descent.pending.insert(range, bound); + } + + self.flow_state.restore(old); + }, + + // Calls like `reactive()`. Its body runs at an unknown + // later time, so it's a child scope scanned when the walk + // enters it. Record the names it inherits for its callee + // resolution, same as a function body. + (NseScope::Nested, NseTiming::Lazy) => { + self.record_enclosing_flow(value.syntax().text_trimmed_range()); + }, + }, + } + } + } + + /// Copy the names a `Current + Lazy` body defines into the owner's + /// bound names, without marking them bound in the scan's flow state. + /// + /// This feeds enclosing snapshots only. A free variable elsewhere can + /// resolve to a name an `on_load({ ... })` defines in the owner, and + /// `register_enclosing_snapshot()` reads `bound_names` to find that ancestor. + /// The scan doesn't descend into these bodies otherwise, so their names + /// would only reach the owner when the walk later gets to the call, too late + /// for a sibling scanned before then. Collecting them now keeps the owner's + /// bound names complete before the walk touches any sibling. + /// + /// NSE shadow resolution does not read `bound_names`, so an incomplete + /// collection here can't flip an NSE decision. `is_locally_bound` reads the + /// captured eager bindings, which exclude deferred names by construction. + /// + /// We cover the realistic shapes, direct assignments and control flow, e.g. + /// `on_load({ x <- 1 })`. We stop at nested calls and function bodies + /// however, so we only add names the walk will also route, never a phantom. + /// `register_enclosing_snapshot` reads `binds()` as "a real definition + /// exists", so a phantom would send it chasing a binding that isn't there. + /// The price is a binding buried in a nested transparent call, e.g. + /// `on_load({ evalq(helper <- ...) })`, which we miss here, so a free + /// variable can't resolve to it. TODO(nse): We could potentially walk + /// transparent (Current) nested calls to collect those too. + /// + /// The names go to `bound_names` only, never to `flow_state`. The body + /// runs at some later time, so at an eager position after the call these + /// names aren't bound yet, and an eager callee there must still treat them + /// as unbound. + fn scan_lazy_owner_bindings(&mut self, expr: &AnyRExpression) { + match expr { + AnyRExpression::RBracedExpressions(braced) => { + for expr in braced.expressions().iter() { + self.scan_lazy_owner_bindings(&expr); + } + }, + + AnyRExpression::RBinaryExpression(bin) => { + // `<<-` binds in an ancestor, not the owner, so it's not routed + // here (matching `add_definition`). + if !is_assignment(bin) || is_super_assignment(bin) { + return; + } + let target = if is_right_assignment(bin) { + bin.right() + } else { + bin.left() + }; + if let Ok(target) = target { + if let Some((name, range)) = assignment_name(&target) { + self.record_owner_name(name, range); + } + } + }, + + AnyRExpression::RIfStatement(stmt) => { + if let Ok(consequence) = stmt.consequence() { + self.scan_lazy_owner_bindings(&consequence); + } + if let Some(else_clause) = stmt.else_clause() { + if let Ok(alternative) = else_clause.alternative() { + self.scan_lazy_owner_bindings(&alternative); + } + } + }, + + AnyRExpression::RForStatement(stmt) => { + if let Ok(variable) = stmt.variable() { + self.record_owner_name( + variable.name_text(), + variable.syntax().text_trimmed_range(), + ); + } + if let Ok(body) = stmt.body() { + self.scan_lazy_owner_bindings(&body); + } + }, + + AnyRExpression::RWhileStatement(stmt) => { + if let Ok(body) = stmt.body() { + self.scan_lazy_owner_bindings(&body); + } + }, + + AnyRExpression::RRepeatStatement(stmt) => { + if let Ok(body) = stmt.body() { + self.scan_lazy_owner_bindings(&body); + } + }, + + // Stop everywhere else: function bodies are child scopes, and a + // call's arguments aren't part of this scope's direct level. + _ => {}, + } + } + + /// Resolve a call's callee to an NSE annotation. + /// + /// Two cases resolve here: + /// - A bare identifier. If bound locally it goes through the local + /// [`resolve_local_effects`](Self::resolve_local_effects). Otherwise the + /// cross-file `ImportsResolver::resolve_effects()` resolves it across the + /// search path. + /// - A `pkg::fn` namespace expression, resolved through + /// `ImportsResolver::resolve_qualified_effects()`. `::` names the package, + /// so there's no search-path disambiguation; the resolver answers from + /// per-package knowledge (the static registry, plus cross-file knowledge + /// like the re-export chase once that lands). + /// + /// The bound check reads the scan pass's flow-precise binding state + /// for the current scope, so this must run during the scan, not the walk. + fn resolve_nse(&mut self, call: &RCall) -> Option { + let func = call.function().ok()?; + + match &func { + AnyRExpression::RIdentifier(ident) => { + let name = ident.name_text(); + + // First check for a local definition (which in the future may + // contain NSE annotations that we resolve here) + // + // Looked up from `flow_state` which already carries every + // eager binding visible here: the scope's own flow-precise + // bindings so far, plus the enclosing eager environment seeded + // at `begin_scan()`. Forward and deferred (lazy-routed) + // bindings are excluded. A forward one isn't in `flow_state` + // yet, and a deferred one (`on_load`, `<<-`) never enters it. + if self.flow_state.is_bound(&name) { + return self + .resolve_local_effects(&name) + .and_then(|effects| effects.nse); + } + + // Bail early if it is known that no package annotates this name + // with effects. This speeds up the common case of no known annotations. + if !effects_registry::annotates(&name) { + return None; + } + + // Now check imports since the symbol is locally unbound. The + // arena's `current_scope` is the scan unit's scope (the descent + // pushes no arena scopes), so its laziness is the "am I in a lazy + // context" test the resolver needs. + let lazy = self.scopes[self.current_scope].kind.is_lazy(); + let nse = self + .resolver + .resolve_effects(&name, &[], lazy) + .and_then(|effects| effects.nse)?; + + // The callee is unbound by any eager binding, so it is NSE. + // If a lazy-crossed ancestor binds it whole-scope, that binding's + // timing relative to this deferred body is undetermined, so the + // decision is a guess. Flag it. + if let Some(overwrite_range) = self.is_lazily_shadowed(&name) { + self.record_lazy_shadow_ambiguity( + name, + call.syntax().text_trimmed_range(), + overwrite_range, + ); + } + Some(nse) + }, + + AnyRExpression::RNamespaceExpression(ns_expr) => { + let left = ns_expr.left().ok()?; + let right = ns_expr.right().ok()?; + let pkg = left.identifier_text()?; + let func_name = right.identifier_text()?; + + if !effects_registry::annotates(&func_name) { + return None; + } + + self.resolver + .resolve_qualified_effects(&pkg, &func_name) + .and_then(|effects| effects.nse) + }, + + _ => None, + } + } + + /// Local resolver for declared effects, mirroring the imports resolver's + /// `resolve_effects()` method on the cross-file side. + /// TODO(nse, annotations): always `None` until `declare()` parsing lands. + fn resolve_local_effects(&self, _name: &str) -> Option { + None + } + + /// Detect ambiguities caused by laziness. + /// + /// We've decided `name` is NSE because it was locally unbound at the + /// current flow cursor, and eager-flow resolution found an NSE effect. If + /// we're in a lazy context, that decision could be wrong: an enclosing + /// scope may bind `name` with a timing we can't pin down, either a later + /// assignment, or one from another deferred body that could run before or + /// after us We detect this ambiguity here so it can be linted. + /// + /// Returns the site of the shadowing binding. + fn is_lazily_shadowed(&self, name: &str) -> Option { + let mut scope = self.current_scope; + let mut crossed_lazy = self.scopes[scope].kind.is_lazy(); + + while let Some(parent) = self.scopes[scope].parent { + if crossed_lazy { + if let Some(range) = self.scope_binding_range(parent, name) { + return Some(range); + } + } + + if self.scopes[parent].kind.is_lazy() { + crossed_lazy = true; + } + scope = parent; + } + + None + } + + fn record_lazy_shadow_ambiguity( + &mut self, + name: String, + call_range: TextRange, + overwrite_range: TextRange, + ) { + self.diagnostics + .push(SemanticDiagnostic::LazyShadowAmbiguity { + name, + call_range, + overwrite_range, + }); + } + + /// Process a call the scan pass decided is NSE. Match its arguments + /// against the annotation, then handle each scoped argument, pushing NSE + /// scopes inline. + pub(super) fn collect_nse_call(&mut self, call: &RCall, annotation: ArgumentsAnnotation) { + let Ok(args) = call.arguments() else { + return; + }; + let items = args.items(); + let nse_args = self.match_nse_arguments(&items, annotation); + + for (i, item) in items.iter().enumerate() { + let Ok(arg) = item else { continue }; + let Some(value) = arg.value() else { continue }; + + match nse_args[i] { + None => self.collect_expression(&value), + Some(nse_arg) => self.collect_nse_argument(nse_arg, &value), + } + } + } + + /// Match a call's arguments against an NSE annotation. Returns, per argument + /// in call order, the scoped argument it matched (if any). Named arguments + /// match first, then unmatched positions fill by call-site position. + /// + /// FIXME: This is a stopgap helper. In the future, `Effects` will be + /// returned from the resolvers with the function signature, and we'll + /// implement a proper argument matching routine. + fn match_nse_arguments( + &self, + items: &RArgumentList, + annotation: ArgumentsAnnotation, + ) -> Vec> { + let arg_count = items.iter().count(); + let mut nse_args: Vec> = vec![None; arg_count]; + let mut consumed = vec![false; annotation.arguments.len()]; + + // Named pass + for (i, item) in items.iter().enumerate() { + let Ok(arg) = item else { continue }; + if let Some(nse_idx) = match_named_arg(&arg, &annotation, &consumed) { + consumed[nse_idx] = true; + nse_args[i] = Some(&annotation.arguments[nse_idx]); + } + } + + // Positional pass. Only unnamed args reach the match, and none of them + // were set by the named pass, so no need to re-check `nse_args[i]`. + let mut position = 0usize; + for (i, item) in items.iter().enumerate() { + let Ok(arg) = item else { + position += 1; + continue; + }; + if arg.name_clause().is_some() { + position += 1; + continue; + } + if let Some(scoped_idx) = match_positional_arg(&annotation, position, &consumed) { + consumed[scoped_idx] = true; + nse_args[i] = Some(&annotation.arguments[scoped_idx]); + } + position += 1; + } + + nse_args + } + + /// Walk a single NSE argument body, pushing a scope when appropriate. + /// + /// `Current + Eager` stays in the current scope. `Nested + Eager` was + /// already scanned by the descent, so we install its pending names and only + /// walk. The remaining lazy bodies are their own scan units that we scan + /// here on entry. + fn collect_nse_argument(&mut self, nse_arg: &Argument, value: &AnyRExpression) { + match (nse_arg.scope, nse_arg.timing) { + // Calls like `evalq()` + (NseScope::Current, NseTiming::Eager) => { + self.collect_expression(value); + }, + + // Calls like `local()` + (NseScope::Nested, NseTiming::Eager) => { + let range = value.syntax().text_trimmed_range(); + let kind = ScopeKind::Nse(NseScope::Nested, NseTiming::Eager); + let scope = self.push_scope(kind, range); + + // Install the pending names the descent recorded for this body, + // before collecting so lazy children inside can see them via + // `scope_binds_anywhere()`. + match self.eager_descent.pending.remove(&range) { + Some(bound) => self.bound_names[scope] = bound, + None => { + // An eager NSE scope is reachable only through the scan + // unit that descended into it, so the pending set must + // exist. If not this is a builder bug. In release + // builds we still scan the body here so the walk can + // proceed. This fallback runs with an empty eager + // environment and its shadow decisions are more + // degraded than a real lazy unit's. + stdext::debug_panic!( + "Missing pending bound names for eager NSE body at {range:?}" + ); + self.begin_scan(); + self.scan_expression(value); + }, + } + + self.collect_expression(value); + self.pop_scope(scope); + }, + + (nse_scope, nse_timing) => { + let kind = ScopeKind::Nse(nse_scope, nse_timing); + let scope = self.push_scope(kind, value.syntax().text_trimmed_range()); + + // Scan the child body before walking it. A `Current + Lazy` + // scope routes its defs to the owner and holds no bound names of its + // own, which `record_binding` handles; the scan still runs to + // record the body's NSE decisions in the child's flow context. + self.begin_scan(); + self.scan_expression(value); + self.collect_expression(value); + self.pop_scope(scope); + }, + } + } +} + +/// Match a named argument against the annotation's arguments. Returns the +/// index into `annotation.arguments` if matched. +/// +/// Should we do partial argument matching? Or rely on partial matching being linted? +fn match_named_arg( + arg: &aether_syntax::RArgument, + annotation: &ArgumentsAnnotation, + consumed: &[bool], +) -> Option { + let clause = arg.name_clause()?; + let name = clause.name().ok()?; + let name_text = match &name { + AnyRArgumentName::RIdentifier(ident) => ident.name_text(), + AnyRArgumentName::RStringValue(s) => s.string_text()?, + _ => return None, + }; + annotation + .arguments + .iter() + .enumerate() + .find(|(i, nse_arg)| !consumed[*i] && nse_arg.name == name_text.as_str()) + .map(|(i, _)| i) +} + +/// Match an unnamed argument at `position` against the annotation's arguments. +/// Returns the index into `annotation.arguments` if matched. +/// +/// FIXME: This matches positionally on call-site position only: an unnamed +/// argument at position N matches an annotation argument declared at position +/// N. It doesn't replicate R's full matching, where named arguments are pulled +/// out first and the rest fill the remaining formals in order. So `test_that({ +/// ... }, desc = "d")`, with the block at position 0 but the `code` formal at +/// position 1, won't match. Good enough without the callee's formal list; +/// revisit if it misses real cases. +fn match_positional_arg( + annotation: &ArgumentsAnnotation, + position: usize, + consumed: &[bool], +) -> Option { + annotation + .arguments + .iter() + .enumerate() + .find(|(i, scoped)| !consumed[*i] && scoped.position == position) + .map(|(i, _)| i) +} diff --git a/crates/oak_semantic/src/effects.rs b/crates/oak_semantic/src/effects.rs new file mode 100644 index 000000000..9042a0bd4 --- /dev/null +++ b/crates/oak_semantic/src/effects.rs @@ -0,0 +1,33 @@ +use crate::semantic_index::NseScope; +use crate::semantic_index::NseTiming; + +/// Effects of a resolved function. +/// +/// Currently only records NSE effects. In the future this will include other +/// effects such as `attach` (for e.g. `library()`) and `assign` (for the +/// eponymous function). +#[derive(Debug, Clone, Copy, Default)] +pub struct Effects { + pub nse: Option, +} + +impl Effects { + pub fn nse(nse: ArgumentsAnnotation) -> Self { + Self { nse: Some(nse) } + } +} + +/// Annotation describing how an NSE function's arguments create scopes. +#[derive(Debug, Clone, Copy)] +pub struct ArgumentsAnnotation { + pub arguments: &'static [Argument], +} + +/// A single argument that creates an NSE scope. +#[derive(Debug)] +pub struct Argument { + pub name: &'static str, + pub position: usize, + pub scope: NseScope, + pub timing: NseTiming, +} diff --git a/crates/oak_semantic/src/effects_registry.rs b/crates/oak_semantic/src/effects_registry.rs new file mode 100644 index 000000000..0beb0cb63 --- /dev/null +++ b/crates/oak_semantic/src/effects_registry.rs @@ -0,0 +1,71 @@ +use crate::effects::Argument; +use crate::effects::ArgumentsAnnotation; +use crate::semantic_index::NseScope::Current; +use crate::semantic_index::NseScope::Nested; +use crate::semantic_index::NseTiming::Eager; +use crate::semantic_index::NseTiming::Lazy; + +struct Entry { + package: &'static str, + function: &'static str, + annotation: ArgumentsAnnotation, +} + +/// Look up the NSE annotation for a `(package, function)` pair. +pub fn lookup(package: &str, function: &str) -> Option<&'static ArgumentsAnnotation> { + REGISTRY + .iter() + .find(|e| e.package == package && e.function == function) + .map(|e| &e.annotation) +} + +/// Whether any registry entry annotates `name`. This is the bare-callee front +/// gate: an unannotated name can't resolve to an effect no matter which provider +/// wins, so recognition skips resolution entirely. +/// +/// TODO: Should be a workspace-wide Salsa-cached query (similar to: does this +/// function dispatches). +pub fn annotates(name: &str) -> bool { + REGISTRY.iter().any(|e| e.function == name) +} + +/// One registry entry. Each `(name, position, scope, laziness)` tuple is a +/// scoped argument; list more than one for a function that scopes several. +macro_rules! entry { + ($pkg:literal, $func:literal, $(($name:literal, $pos:literal, $scope:expr, $timing:expr)),+ $(,)?) => { + Entry { + package: $pkg, + function: $func, + annotation: ArgumentsAnnotation { + arguments: &[$(Argument { + name: $name, + position: $pos, + scope: $scope, + timing: $timing, + }),+], + }, + } + }; +} + +static REGISTRY: &[Entry] = &[ + // base + entry!("base", "evalq", ("expr", 0, Current, Eager)), + entry!("base", "local", ("expr", 0, Nested, Eager)), + entry!("base", "with", ("expr", 1, Nested, Eager)), + entry!("base", "with.default", ("expr", 1, Nested, Eager)), + entry!("base", "within", ("expr", 1, Nested, Eager)), + entry!("base", "within.data.frame", ("expr", 1, Nested, Eager)), + // rlang + entry!("rlang", "on_load", ("expr", 0, Current, Lazy)), + // shiny + entry!("shiny", "observe", ("x", 0, Nested, Lazy)), + entry!("shiny", "reactive", ("x", 0, Nested, Lazy)), + entry!("shiny", "renderPlot", ("expr", 0, Nested, Lazy)), + entry!("shiny", "renderPrint", ("expr", 0, Nested, Lazy)), + entry!("shiny", "renderTable", ("expr", 0, Nested, Lazy)), + entry!("shiny", "renderText", ("expr", 0, Nested, Lazy)), + entry!("shiny", "renderUI", ("expr", 0, Nested, Lazy)), + // testthat + entry!("testthat", "test_that", ("code", 1, Nested, Eager)), +]; diff --git a/crates/oak_semantic/src/lib.rs b/crates/oak_semantic/src/lib.rs index 999456950..233eb97eb 100644 --- a/crates/oak_semantic/src/lib.rs +++ b/crates/oak_semantic/src/lib.rs @@ -1,9 +1,12 @@ pub mod builder; +pub mod effects; +pub mod effects_registry; pub mod resolver; pub mod semantic_index; pub mod use_def_map; pub use builder::build_index; +pub use effects::Effects; pub use resolver::ImportsResolver; pub use resolver::NoopImportsResolver; pub use resolver::SourceResolution; diff --git a/crates/oak_semantic/src/resolver.rs b/crates/oak_semantic/src/resolver.rs index bd54ccb3d..65b442c0b 100644 --- a/crates/oak_semantic/src/resolver.rs +++ b/crates/oak_semantic/src/resolver.rs @@ -1,5 +1,8 @@ use url::Url; +use crate::effects::Effects; +use crate::effects_registry; + /// The result of resolving a `source()` call. Returned by /// [`ImportsResolver::resolve_source`]. #[derive(Clone)] @@ -28,14 +31,15 @@ pub struct SourceResolution { /// for isolated indexing (CLI tools, unit tests). /// - `oak_db::SalsaImportsResolver`: salsa-backed lookup against the source graph. /// -/// The trait grows along two axes as new analyses land: +/// The trait has three queries: /// /// - [`resolve_source`](ImportsResolver::resolve_source) is the bulk /// query, "enumerate every name this `source("path")` brings in," used /// to inject `DefinitionKind::Import` entries at each source() offset. -/// - A future `resolve_name(scope, name)` is the point query, "find -/// the import that resolves this specific name in this scope," used by -/// NSE call-site analysis. +/// - [`resolve_effects`](ImportsResolver::resolve_effects) resolves a bare +/// callee against imports, e.g. the search path, and returns known effects. +/// - [`resolve_qualified_effects`](ImportsResolver::resolve_qualified_effects) +/// resolves the effects of a `pkg::fn` (or `:::) callee against a named package. pub trait ImportsResolver { /// Resolve a `source("path")` call to the target file's exported names /// and transitive `library()` attachments. The path is the literal @@ -43,6 +47,25 @@ pub trait ImportsResolver { /// anchoring it (workspace root, calling file's directory, ...). /// Returns `None` when the target can't be located. fn resolve_source(&mut self, path: &str) -> Option; + + /// Resolve a bare callee `name` to its effects. The builder state is passed + /// in because the resolver can't query our own semantic index without + /// creating a cycle: + /// + /// - `attached`: packages attached at this point, in flow order. + /// - `lazy`: whether the callee sits in a lazy context like a function. + fn resolve_effects(&mut self, name: &str, attached: &[String], lazy: bool) -> Option { + let _ = (name, attached, lazy); + None + } + + /// Resolve a namespace-qualified callee `pkg::fn` (or equivalently with + /// `:::`) to its effects. + fn resolve_qualified_effects(&mut self, package: &str, name: &str) -> Option { + effects_registry::lookup(package, name) + .copied() + .map(Effects::nse) + } } /// Resolver that returns nothing. The builder skips all cross-file diff --git a/crates/oak_semantic/src/semantic_index.rs b/crates/oak_semantic/src/semantic_index.rs index 12ca4d7d4..5d4a71cc6 100644 --- a/crates/oak_semantic/src/semantic_index.rs +++ b/crates/oak_semantic/src/semantic_index.rs @@ -85,6 +85,10 @@ pub struct SemanticIndex { // `package:::symbol` namespace_accesses: Vec, + // Diagnostics surfaced during indexing, for downstream consumers to turn + // into user-facing diagnostics. + diagnostics: Vec, + // The file scope's exit flow state: for each top-level symbol, the // definitions still in effect once the file has run top to bottom. This is // the file's exports (see `exports()`). Only the file scope's exit state is @@ -102,6 +106,7 @@ impl SemanticIndex { enclosing_snapshots: FxHashMap, semantic_calls: Vec, namespace_accesses: Vec, + diagnostics: Vec, final_bindings: IndexVec, ) -> Self { Self { @@ -113,6 +118,7 @@ impl SemanticIndex { enclosing_snapshots, semantic_calls, namespace_accesses, + diagnostics, final_bindings, } } @@ -190,6 +196,12 @@ impl SemanticIndex { &self.namespace_accesses } + /// Diagnostics surfaced during indexing, for downstream consumers to turn + /// into user-facing diagnostics. + pub fn diagnostics(&self) -> &[SemanticDiagnostic] { + &self.diagnostics + } + /// Find the innermost scope containing `offset`. pub fn scope_at(&self, offset: biome_rowan::TextSize) -> (ScopeId, &Scope) { // Start at the file scope @@ -323,8 +335,7 @@ impl SemanticIndex { let local = bindings.definitions().iter().map(move |&d| (scope_id, d)); let enclosing = if bindings.may_be_unbound() { - let symbol_id = self.uses(scope_id)[use_id].symbol(); - self.enclosing_bindings(scope_id, symbol_id) + self.enclosing_bindings(scope_id, use_id) } else { None }; @@ -337,10 +348,10 @@ impl SemanticIndex { /// Resolve a free variable's bindings from the enclosing scope. /// - /// When a use in `scope` may be unbound (`may_be_unbound: true`), some - /// control-flow paths fall through to an enclosing scope. This looks up - /// the enclosing snapshot that was registered during the build and - /// returns the ancestor scope and its bindings. This covers both purely + /// When the use `use_id` in `scope` may be unbound (`may_be_unbound: true`), + /// some control-flow paths fall through to an enclosing scope. This looks up + /// the enclosing snapshot that was registered for that use during the build + /// and returns the ancestor scope and its bindings. This covers both purely /// free variables (no local definitions) and conditionally defined /// variables (local definitions exist but don't cover all paths). /// @@ -350,11 +361,11 @@ impl SemanticIndex { pub fn enclosing_bindings( &self, scope: ScopeId, - symbol: SymbolId, + use_id: UseId, ) -> Option<(ScopeId, &Bindings)> { let key = EnclosingSnapshotKey { nested_scope: scope, - nested_symbol: symbol, + nested_use: use_id, }; let &(enclosing_scope, snapshot_id) = self.enclosing_snapshots.get(&key)?; let bindings = self.use_def_maps[enclosing_scope].enclosing_snapshot(snapshot_id); @@ -363,18 +374,17 @@ impl SemanticIndex { } /// Key for looking up an enclosing snapshot. Keyed by the nested scope and the -/// symbol's `SymbolId` in the nested scope's symbol table (not the enclosing -/// scope's), so consumers can do an O(1) lookup directly from a `UseId` without -/// re-walking the ancestor chain. +/// `UseId` of the free variable in that scope, so consumers do an O(1) lookup +/// straight from a use without re-walking the ancestor chain. /// -/// When we implement NSE, we will add a `laziness: ScopeLaziness` field to -/// distinguish lazy snapshots (functions, accumulated union via watchers) from -/// eager snapshots (NSE scopes like `local()`, point-in-time capture at the -/// call site). Currently all nested scopes are lazy, so the field is omitted. +/// Keyed per use, not per symbol, because eager snapshots (e.g. `local()`) are +/// point-in-time: two uses of the same free variable at different points in an +/// eager body can see different enclosing states, so each gets its own +/// snapshot. Lazy uses of one symbol still share a single snapshot. #[derive(Debug, Clone, PartialEq, Eq, Hash)] pub struct EnclosingSnapshotKey { pub nested_scope: ScopeId, - pub nested_symbol: SymbolId, + pub nested_use: UseId, } // --- Scope --- @@ -384,9 +394,9 @@ pub struct EnclosingSnapshotKey { // by the file: walking `parent` from any scope eventually reaches the // `File` scope which itself has `parent: None`. // -// Currently only `function()` creates a new scope. In the future, constructs -// like `local()`, `with()`, `within()` may also create scopes (determined -// by function declarations resolved via salsa queries). +// `function()` creates `Function` scopes. NSE constructs like `local()`, +// `with()`, `test_that()` create `Nse` scopes, recognized by resolving the +// call target against for their effects annotations during the walk. #[derive(Debug, PartialEq, Eq)] pub struct Scope { pub(crate) parent: Option, @@ -405,6 +415,45 @@ pub enum ScopeKind { // cross-file resolution (package namespace, session, etc.) takes over. File, Function, + Nse(NseScope, NseTiming), +} + +/// Where definitions in an NSE scope land. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NseScope { + /// Definitions go to the current (parent) environment. + /// e.g. `rlang::on_load()` + Current, + /// Definitions go to a nested environment. + /// e.g. `local()`, `test_that()`, `with()` + Nested, +} + +/// Whether an NSE scope evaluates eagerly (at the call site) or lazily +/// (at an unknown later time). +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum NseTiming { + /// Expression that runs at the call site. Free variables resolve against + /// the linear state right there. E.g. `local()`, `evalq()`, `test_that()`. + Eager, + /// Expression that runs at an unknown later time, so free variables resolve + /// against the accumulated union of enclosing definitions. E.g. + /// `shiny::reactive()`, `rlang::on_load()`. + Lazy, +} + +impl ScopeKind { + /// Whether free variables in this scope resolve against the union of all + /// enclosing definitions (lazy) or against a point-in-time snapshot at the + /// call site (eager). `Function` bodies run at an unknown later time, so + /// they're always lazy. + pub fn is_lazy(self) -> bool { + match self { + ScopeKind::File => false, + ScopeKind::Function => true, + ScopeKind::Nse(_, timing) => timing == NseTiming::Lazy, + } + } } impl Scope { @@ -764,6 +813,22 @@ pub enum NamespaceAccessKind { Internal, } +/// A diagnostic surfaced while building the semantic index, for downstream +/// consumers to turn into user-facing diagnostics. +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum SemanticDiagnostic { + /// An NSE call recognized in a lazy context whose callee is also bound by a + /// lazy-crossed ancestor with undetermined timing, so the NSE decision is a + /// guess. `call_range` points at the NSE call we recognized, `overwrite_range` + /// at the ancestor binding that could invalidate it (a later assignment in + /// parent code, or one from another lazy context). + LazyShadowAmbiguity { + name: String, + call_range: TextRange, + overwrite_range: TextRange, + }, +} + // --- Iterators --- pub struct ChildScopeIdsIter<'a> { diff --git a/crates/oak_semantic/src/use_def_map.rs b/crates/oak_semantic/src/use_def_map.rs index c66a6eb78..259eaa415 100644 --- a/crates/oak_semantic/src/use_def_map.rs +++ b/crates/oak_semantic/src/use_def_map.rs @@ -186,10 +186,13 @@ use crate::semantic_index::UseId; // The consumer combines both: the local bindings and the enclosing // snapshot give the full picture of what `x` could be. // -// For eager NSE scopes (e.g. `local()`), the snapshot will be even more -// precise: since the body executes at the call site, the snapshot is a -// point-in-time capture with no watcher, reflecting exactly the linear state. -// No union over-approximation needed. +// For eager NSE scopes (e.g. `local()`), the snapshot is more precise: since +// the body executes at the call site, it's a point-in-time capture reflecting +// exactly the linear state, with no union over definitions that come later in +// the enclosing scope. Each use in the body captures its own snapshot, so a use +// after a `<<-` in the body sees the mutated binding while an earlier use does +// not. Eager snapshots take no watcher: they never fold in a later definition +// (see `register_eager_snapshot()`). /// The immutable use-def map for a single scope. For each use site, stores the /// set of definitions that can reach it through control flow. @@ -304,7 +307,10 @@ pub(crate) struct UseDefMapBuilder { // Currently used for `<<-` extra definitions in ancestor scopes. deferred_defs: Vec<(SymbolId, DefinitionId)>, enclosing_snapshots: IndexVec, - snapshot_watchers: FxHashMap>, + // Lazy snapshots (e.g. `reactive()`), notified of every definition of a + // symbol so they fold in the whole union. Eager snapshots (e.g. `local()`) + // are point-in-time and subscribe to nothing. + lazy_watchers: FxHashMap>, } impl UseDefMapBuilder { @@ -314,7 +320,7 @@ impl UseDefMapBuilder { bindings_by_use: IndexVec::new(), deferred_defs: Vec::new(), enclosing_snapshots: IndexVec::new(), - snapshot_watchers: FxHashMap::default(), + lazy_watchers: FxHashMap::default(), } } @@ -333,7 +339,14 @@ impl UseDefMapBuilder { /// live definitions for that symbol. pub(crate) fn record_definition(&mut self, symbol_id: SymbolId, def_id: DefinitionId) { self.symbol_states[symbol_id].record_definition(def_id); - self.update_enclosing_snapshots(symbol_id, def_id); + // A plain def reaches lazy snapshots only. Eager snapshots are + // point-in-time and ignore defs that land after the call. + Self::notify_watchers( + &mut self.enclosing_snapshots, + &self.lazy_watchers, + symbol_id, + def_id, + ); } /// After visiting a loop body, retroactively patch uses so that @@ -396,7 +409,15 @@ impl UseDefMapBuilder { pub(crate) fn record_deferred_definition(&mut self, symbol_id: SymbolId, def_id: DefinitionId) { self.symbol_states[symbol_id].add_definition(def_id); self.deferred_defs.push((symbol_id, def_id)); - self.update_enclosing_snapshots(symbol_id, def_id); + // A deferred def reaches lazy snapshots like any other def. Eager + // snapshots take no watcher, so a `<<-` inside an eager body reaches + // them only through the point-in-time clone of a use that follows it. + Self::notify_watchers( + &mut self.enclosing_snapshots, + &self.lazy_watchers, + symbol_id, + def_id, + ); } /// Record a use of `symbol_id`. Clones the current live bindings for that @@ -458,23 +479,58 @@ impl UseDefMapBuilder { /// registered so that each subsequent definition of this symbol we /// encounter is conservatively merged in, because we can't know statically /// when the nested scope will be called. - pub(crate) fn register_enclosing_snapshot( - &mut self, - symbol_id: SymbolId, - ) -> EnclosingSnapshotId { + pub(crate) fn register_lazy_snapshot(&mut self, symbol_id: SymbolId) -> EnclosingSnapshotId { let bindings = self.symbol_states[symbol_id].clone(); let id = self.enclosing_snapshots.push(bindings); - self.snapshot_watchers - .entry(symbol_id) - .or_default() - .push(id); + self.lazy_watchers.entry(symbol_id).or_default().push(id); id } - fn update_enclosing_snapshots(&mut self, symbol_id: SymbolId, def_id: DefinitionId) { - if let Some(watchers) = self.snapshot_watchers.get(&symbol_id) { - for &snapshot_id in watchers { - self.enclosing_snapshots[snapshot_id].add_definition(def_id); + /// Register a point-in-time enclosing snapshot for `symbol_id`. Used for + /// eager NSE scopes like `local()`: the body runs at the call site, so the + /// snapshot reflects exactly the linear state, with no union over the + /// definitions that come later in the enclosing scope. + /// + /// Unlike [`register_lazy_snapshot`](Self::register_lazy_snapshot), this + /// takes no watcher. The clone is the whole answer. Each use in the eager + /// body registers its own snapshot, so it captures the enclosing state at + /// that exact point in the flow. A `<<-` inside the body is already live in + /// `symbol_states` by the time a later use clones, so that use sees it while + /// an earlier one does not: + /// + /// ```r + /// x <- 1 + /// local({ + /// x # {1}: cloned before the `<<-` + /// x <<- 2 # deferred def, now live in the enclosing state + /// x # {1, 2}: cloned after the `<<-` + /// }) + /// ``` + /// + /// And since nothing fires later, a definition recorded after the body has + /// run never folds in, which is correct: the eager body is already done. + /// + /// ```r + /// x <- 1 + /// local({ x }) # {1} + /// f <- function() { x <<- 2 } # f's `<<-` can't reach the finished body + /// rlang::on_load({ x <- 3 }) # routed def can't reach it either + /// x <- 4 # plain `<-` after the call, out of the snapshot + /// ``` + pub(crate) fn register_eager_snapshot(&mut self, symbol_id: SymbolId) -> EnclosingSnapshotId { + let bindings = self.symbol_states[symbol_id].clone(); + self.enclosing_snapshots.push(bindings) + } + + fn notify_watchers( + enclosing_snapshots: &mut IndexVec, + watchers: &FxHashMap>, + symbol_id: SymbolId, + def_id: DefinitionId, + ) { + if let Some(ids) = watchers.get(&symbol_id) { + for &snapshot_id in ids { + enclosing_snapshots[snapshot_id].add_definition(def_id); } } } diff --git a/crates/oak_semantic/tests/integration/builder_nse.rs b/crates/oak_semantic/tests/integration/builder_nse.rs new file mode 100644 index 000000000..2a16af948 --- /dev/null +++ b/crates/oak_semantic/tests/integration/builder_nse.rs @@ -0,0 +1,1479 @@ +use aether_parser::parse; +use aether_parser::RParserOptions; +use oak_semantic::build_index; +use oak_semantic::semantic_index::DefinitionId; +use oak_semantic::semantic_index::NseScope; +use oak_semantic::semantic_index::NseTiming; +use oak_semantic::semantic_index::ScopeId; +use oak_semantic::semantic_index::ScopeKind; +use oak_semantic::semantic_index::SemanticDiagnostic; +use oak_semantic::semantic_index::SemanticIndex; +use oak_semantic::semantic_index::SymbolFlags; +use oak_semantic::semantic_index::UseId; +use oak_semantic::NoopImportsResolver; + +use crate::resolvers::TestImportsResolver; + +fn index(source: &str) -> SemanticIndex { + build_with(source, TestImportsResolver::with_base()) +} + +fn build_with(source: &str, resolver: impl oak_semantic::ImportsResolver) -> SemanticIndex { + let parsed = parse(source, RParserOptions::default()); + + if parsed.has_error() { + panic!("source has syntax errors: {source}"); + } + + build_index(&parsed.tree(), resolver) +} + +// --- NSE scopes --- + +#[test] +fn test_nse_local_creates_nested_eager_scope() { + let index = index( + "\ +local({ + x <- 1 +}) +", + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + // `local` is used at file scope + assert_eq!(index.symbols(file).len(), 1); + assert_eq!( + index.symbols(file).get("local").unwrap().flags(), + SymbolFlags::IS_USED + ); + + // `x` is defined inside the NSE scope, not at file level + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(local_scope).parent(), Some(file)); + assert_eq!(index.symbols(local_scope).len(), 1); + assert_eq!( + index.symbols(local_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_local_definition_not_in_parent() { + // Definitions inside `local()` should NOT leak to the file scope. + let index = index( + "\ +local({ + x <- 1 +}) +x +", + ); + let file = ScopeId::from(0); + + // `x` at file scope is only IS_USED (from the bare `x` on the last line), + // not IS_BOUND (from the assignment inside local). + let x = index.symbols(file).get("x").unwrap(); + assert_eq!(x.flags(), SymbolFlags::IS_USED); +} + +#[test] +fn test_nse_evalq_no_scope_push() { + // `evalq` is Current + Eager: no scope push, walk body in place. + let index = index( + "\ +evalq({ + x <- 1 +}) +", + ); + let file = ScopeId::from(0); + + // Only the file scope exists (plus no child scopes) + assert_eq!( + index.symbols(file).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + // evalq is used, x is bound + assert_eq!(index.symbols(file).len(), 2); +} + +#[test] +fn test_nse_namespace_qualified_call() { + // `testthat::test_that` should be recognized via namespace resolution. + let index = index( + r#" +testthat::test_that("description", { + x <- 1 +}) +"#, + ); + let file = ScopeId::from(0); + let test_scope = ScopeId::from(1); + + // File scope has no symbols (namespace expressions don't record uses) + assert_eq!(index.symbols(file).len(), 0); + + // Test scope contains `x` + assert_eq!( + index.scope(test_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!( + index.symbols(test_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_shadowed_name_no_scope() { + // If `local` is locally defined (shadowed), it's not recognized as NSE. + let index = index( + "\ +local <- identity +local({ + x <- 1 +}) +", + ); + let file = ScopeId::from(0); + + // `local` is defined at file scope, shadowing the base function. + // No NSE scope should be created. `x` is defined at file scope. + assert_eq!( + index.symbols(file).get("local").unwrap().flags(), + SymbolFlags::IS_BOUND.union(SymbolFlags::IS_USED) + ); + assert_eq!( + index.symbols(file).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_ancestor_shadowed_name_no_scope() { + // A `local` binding in an ENCLOSING scope shadows the base function too, + // even when the call site sits in a nested scope where `local` is free. + let index = index( + "\ +local <- function(x) x +f <- function() { + local({ + y <- 1 + }) +} +", + ); + let file = ScopeId::from(0); + let identity_fn = ScopeId::from(1); + let f_scope = ScopeId::from(2); + + // Only three scopes: no NSE scope is pushed for the shadowed `local()`. + assert_eq!(index.scope_ids().count(), 3); + assert_eq!(index.scope(file).kind(), ScopeKind::File); + assert_eq!(index.scope(identity_fn).kind(), ScopeKind::Function); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + + // `local` is bound at file scope. + assert!(index + .symbols(file) + .get("local") + .unwrap() + .flags() + .contains(SymbolFlags::IS_BOUND)); + + // `y` is defined flat in `f`, not moved into an NSE child scope. + assert_eq!( + index.symbols(f_scope).get("y").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_forward_def_visible_to_nested_function() { + // A function defined inside an eager NSE body that references a name bound + // LATER in that same body must still resolve to the NSE scope. This relies + // on the NSE scope's own pre-scan seeing the forward definition, which the + // pre-scan must collect despite the body range being a Nested NSE range. + let index = index( + "\ +local({ + f <- function() x + x <- 1 +}) +", + ); + let local_scope = ScopeId::from(1); + let f_scope = ScopeId::from(2); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + + // `x` inside `f` resolves to the `local` scope (not the file scope), and + // its lazy snapshot picks up `x <- 1` (DefinitionId 1 in the local scope: + // `f` is DefinitionId 0, `x` is DefinitionId 1). + let (enclosing_scope, bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); + assert_eq!(enclosing_scope, local_scope); + assert_eq!(bindings.definitions(), &[DefinitionId::from(1)]); +} + +#[test] +fn test_nse_moves_definitions_into_nested_scope() { + // Definitions inside an NSE body land in the NSE child scope, not the + // parent, even with sibling definitions on either side at file level. + let index = index( + "\ +x <- 0 +local({ + y <- 1 +}) +z <- 2 +", + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + // File scope: x, z are bound; local is used; y is NOT in file scope + assert!(index.symbols(file).get("x").is_some()); + assert!(index.symbols(file).get("z").is_some()); + assert!(index.symbols(file).get("local").is_some()); + assert!(index.symbols(file).get("y").is_none()); + + // local scope: y is bound + assert_eq!( + index.symbols(local_scope).get("y").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_test_that_second_arg() { + // `test_that` has the scoped arg at position 1 (the `code` parameter). + // The first argument (description) should be processed normally. + let index = index( + r#" +testthat::test_that("description", { + x <- 1 + y +}) +"#, + ); + let test_scope = ScopeId::from(1); + + // Inside the test scope + assert_eq!( + index.symbols(test_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + assert_eq!( + index.symbols(test_scope).get("y").unwrap().flags(), + SymbolFlags::IS_USED + ); +} + +#[test] +fn test_nse_named_argument_matching() { + // Named argument matching: `code = {...}` should be recognized. + let index = index( + r#" +testthat::test_that(code = { + x <- 1 +}, desc = "foo") +"#, + ); + let test_scope = ScopeId::from(1); + + assert_eq!( + index.scope(test_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!( + index.symbols(test_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_nested_function_inside_local() { + // A function defined inside `local()` creates a nested Function scope. + let index = index( + "\ +local({ + f <- function(x) x +}) +", + ); + let local_scope = ScopeId::from(1); + let fun_scope = ScopeId::from(2); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(fun_scope).kind(), ScopeKind::Function); + assert_eq!(index.scope(fun_scope).parent(), Some(local_scope)); +} + +#[test] +fn test_nse_prescan_skips_nested_bodies() { + // The file scope's bound names must NOT include definitions from inside + // `local()` bodies. This means a function defined AFTER the local() call + // should not see `x` from inside local via the bound names. + let index = index( + "\ +local({ + x <- 1 +}) +f <- function() x +", + ); + let file = ScopeId::from(0); + let fun_scope = ScopeId::from(2); + + // `x` should NOT be in the file scope + assert!(index.symbols(file).get("x").is_none()); + + // In `f`, `x` is free and unbound -- no enclosing snapshot should find it + // in the file scope. + assert_eq!(index.enclosing_bindings(fun_scope, UseId::from(0)), None); +} + +#[test] +fn test_nse_eager_snapshot_precise() { + // Eager NSE scope at file level should see a point-in-time snapshot: + // only definitions that precede the call site, not later ones. + let index = index( + "\ +x <- 1 +local({ + x +}) +x <- 2 +", + ); + let local_scope = ScopeId::from(1); + + // `x` inside local is free. Its enclosing snapshot should be eager + // (point-in-time). At the call site, only `x <- 1` (DefinitionId 0) is + // live. `x <- 2` (DefinitionId 2) comes after and should NOT be included. + let (enclosing_scope, bindings) = index + .enclosing_bindings(local_scope, UseId::from(0)) + .unwrap(); + assert_eq!(enclosing_scope, ScopeId::from(0)); + assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); + assert!(!bindings.may_be_unbound()); +} + +#[test] +fn test_nse_lazy_snapshot_accumulates() { + // Lazy NSE scope (e.g. inside a function) should accumulate definitions + // via watchers, just like function scopes do. + let index = index( + "\ +x <- 1 +f <- function() { + x +} +x <- 2 +", + ); + let fun_scope = ScopeId::from(1); + + // Function is lazy: snapshot includes both x <- 1 and x <- 2. + let (_, bindings) = index.enclosing_bindings(fun_scope, UseId::from(0)).unwrap(); + assert_eq!(bindings.definitions(), &[ + DefinitionId::from(0), + DefinitionId::from(2) + ]); +} + +#[test] +fn test_nse_current_lazy_routes_defs_to_parent() { + // `rlang::on_load` is Current + Lazy: a scope is pushed (for lazy + // snapshot resolution) but definitions route to the parent. + let index = index( + "\ +rlang::on_load({ + x <- 1 +}) +", + ); + let file = ScopeId::from(0); + let nse_scope = ScopeId::from(1); + + // `x` is routed to the file scope + assert_eq!( + index.symbols(file).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + + // The NSE scope exists with `Current + Lazy` kind + assert_eq!( + index.scope(nse_scope).kind(), + ScopeKind::Nse(NseScope::Current, NseTiming::Lazy) + ); + assert_eq!(index.scope(nse_scope).parent(), Some(file)); + + // `x` is not in the child scope's symbol table (routed to parent) + assert!(index.symbols(nse_scope).get("x").is_none()); +} + +#[test] +fn test_nse_current_lazy_deferred_definition() { + // `on_load` definitions are deferred (like `<<-`): they add to the set + // of live definitions without shadowing what's already there. + let index = index( + "\ +x <- 1 +rlang::on_load({ + x <- 2 +}) +f <- function() x +", + ); + let fun_scope = ScopeId::from(2); + + // `f` is lazy, so its snapshot for `x` should include BOTH defs: + // `x <- 1` (file-level) and `x <- 2` (from on_load, deferred). + // If on_load's definition shadowed, we'd only see `x <- 2`. + let (enclosing_scope, bindings) = index.enclosing_bindings(fun_scope, UseId::from(0)).unwrap(); + assert_eq!(enclosing_scope, ScopeId::from(0)); + assert_eq!(bindings.definitions(), &[ + DefinitionId::from(0), + DefinitionId::from(1) + ]); +} + +#[test] +fn test_nse_unmasked_call_via_nested_scope() { + // Redefining `local` inside a `local()` body doesn't shadow a later + // `local()` call: the rebind lands in the first body's NSE scope, so it + // never enters the file's bound names. The scan walks the first `local()` + // inline, so the second call sees `local` still unbound in the same pass. + let index = index( + "\ +local({ + local <- identity +}) +local({ + x <- 1 +}) +", + ); + let file = ScopeId::from(0); + let first_local = ScopeId::from(1); + let second_local = ScopeId::from(2); + + // Both calls create Nested + Eager scopes + assert_eq!( + index.scope(first_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!( + index.scope(second_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + + // `local <- identity` is in the first scope, not at file level + assert!(index + .symbols(file) + .get("local") + .unwrap() + .flags() + .contains(SymbolFlags::IS_USED)); + assert!(!index + .symbols(file) + .get("local") + .unwrap() + .flags() + .contains(SymbolFlags::IS_BOUND)); + + // `x <- 1` is in the second scope, not at file level + assert!(index.symbols(file).get("x").is_none()); + assert_eq!( + index.symbols(second_local).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_ancestor_unmask_across_function() { + // The `local <- identity` rebind lives inside the outer `local()` body, so + // it never enters the file's bound names. The scan of `f`'s body therefore + // sees base `local` unbound and marks the inner `local()` NSE, cutting its + // body out of `f`'s bound names in the same pass. So `x <- 1` lands in the + // inner NSE scope and `g`'s free `x` stays unresolved (the sibling + // `local()` binds `x` in its own env, invisible to `g`). The old re-walk + // needed a second iteration to reach this; the scan gets it in one. + let index = index( + "\ +local({ + local <- identity +}) +f <- function() { + g <- function() x + local({ + x <- 1 + }) +} +", + ); + let file = ScopeId::from(0); + let outer_local = ScopeId::from(1); + let f_scope = ScopeId::from(2); + let g_scope = ScopeId::from(3); + let inner_local = ScopeId::from(4); + + assert_eq!(index.scope_ids().count(), 5); + assert_eq!( + index.scope(outer_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(outer_local).parent(), Some(file)); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + assert_eq!(index.scope(g_scope).kind(), ScopeKind::Function); + assert_eq!(index.scope(g_scope).parent(), Some(f_scope)); + assert_eq!( + index.scope(inner_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(inner_local).parent(), Some(f_scope)); + + // `x <- 1` lands in the inner local scope, not in `f`. + assert!(index.symbols(f_scope).get("x").is_none()); + assert_eq!( + index.symbols(inner_local).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + + // `g`'s free `x` resolves to nothing: the sibling `local()` binds `x` in + // its own scope, not in `f`. Flow-insensitive bound names would wrongly + // point it at a stray `x` in `f`. + assert_eq!(index.enclosing_bindings(g_scope, UseId::from(0)), None); +} + +#[test] +fn test_nse_sibling_branch_flow_precise() { + // Flow-precise scan across `if`/`else`. `local` is bound only on the + // consequence path, so on the else path base `local` is still unbound and + // `local({...})` is NSE. Flow-insensitive bound names would see `local` + // bound (from the consequence) and miss the NSE call, leaking `y` into the + // file scope. + let index = index( + "\ +if (c) local <- identity else local({ + y <- 1 +}) +", + ); + let file = ScopeId::from(0); + let nse_scope = ScopeId::from(1); + + // Only the file scope and the else branch's NSE scope exist. + assert_eq!(index.scope_ids().count(), 2); + assert_eq!( + index.scope(nse_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(nse_scope).parent(), Some(file)); + + // `y` lands in the NSE scope, not the file scope. + assert!(index.symbols(file).get("y").is_none()); + assert_eq!( + index.symbols(nse_scope).get("y").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + + // `local` is bound at file scope (from the consequence branch). + assert!(index + .symbols(file) + .get("local") + .unwrap() + .flags() + .contains(SymbolFlags::IS_BOUND)); +} + +#[test] +fn test_nse_eager_lazy_split_on_later_binding() { + // A later file-level `local <- identity` does NOT shadow `local()` inside the + // function `f`. `f`'s body is lazy, so its run time relative to the binding + // is unknown, and `is_locally_bound` reads only `f`'s eager predecessors (the + // predecessor snapshot, empty here). So the lazy `local()` is optimistically + // NSE and `x` moves into its own scope. The genuine ambiguity (does `f` run + // before or after the binding?) is the overturn lint's job, not a shadow. + // + // The eager `local()` at file scope is NSE too, but for a determined reason: + // it runs before the binding, so its flow-precise state has `local` unbound. + let index = index( + "\ +f <- function() { + local({ + x <- 1 + }) +} +local({ + y <- 1 +}) +local <- identity +", + ); + let file = ScopeId::from(0); + let f_scope = ScopeId::from(1); + let f_local = ScopeId::from(2); + let eager_local = ScopeId::from(3); + + // Four scopes: file, `f`, the NSE `local()` in `f`, and the eager `local()`. + assert_eq!(index.scope_ids().count(), 4); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + + // Lazy `local()` in `f` is NSE (later binding is not a predecessor), so `x` + // moves into its own scope. + assert_eq!( + index.scope(f_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(f_local).parent(), Some(f_scope)); + assert!(index.symbols(f_scope).get("x").is_none()); + assert_eq!( + index.symbols(f_local).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + + // Eager file-level `local()` runs before the binding, so it is NSE and `y` + // lands in its own scope. + assert_eq!( + index.scope(eager_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(eager_local).parent(), Some(file)); + assert!(index.symbols(f_scope).get("y").is_none()); + assert_eq!( + index.symbols(eager_local).get("y").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_local_inside_function() { + // `local()` inside a function: the function boundary is lazy, so the + // eager snapshot precision of `local` is bounded by the function's + // laziness. Free variables in `local` resolve through the function's + // lazy snapshot. + let index = index( + "\ +x <- 1 +f <- function() { + local({ + x + }) +} +x <- 2 +", + ); + let fun_scope = ScopeId::from(1); + let local_scope = ScopeId::from(2); + + assert_eq!(index.scope(fun_scope).kind(), ScopeKind::Function); + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(local_scope).parent(), Some(fun_scope)); + + // `x` is free in the local scope, resolves through to the function scope, + // then to the file scope. The function scope is lazy, so both defs are + // visible despite `local` being eager. + let (enclosing_scope, bindings) = index + .enclosing_bindings(local_scope, UseId::from(0)) + .unwrap(); + assert_eq!(enclosing_scope, ScopeId::from(0)); + assert_eq!(bindings.definitions(), &[ + DefinitionId::from(0), + DefinitionId::from(2) + ]); +} + +#[test] +fn test_nse_nested_local_scopes() { + // Nested `local()` inside `local()`: both create child scopes. + let index = index( + "\ +local({ + x <- 1 + local({ + y <- 2 + }) +}) +", + ); + let file = ScopeId::from(0); + let outer_local = ScopeId::from(1); + let inner_local = ScopeId::from(2); + + assert_eq!( + index.scope(outer_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(outer_local).parent(), Some(file)); + assert_eq!( + index.scope(inner_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(inner_local).parent(), Some(outer_local)); + + // Definitions land in their respective scopes + assert!(index.symbols(file).get("x").is_none()); + assert!(index.symbols(file).get("y").is_none()); + assert_eq!( + index.symbols(outer_local).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + assert!(index.symbols(outer_local).get("y").is_none()); + assert_eq!( + index.symbols(inner_local).get("y").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_super_assignment_inside_local() { + // `<<-` inside `local()` should target the grandparent (file scope), + // not the local scope itself. + let index = index( + "\ +x <- 1 +local({ + x <<- 2 +}) +", + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + // `x` is bound at file scope (from `x <- 1` and the `<<-`) + assert!(index + .symbols(file) + .get("x") + .unwrap() + .flags() + .contains(SymbolFlags::IS_BOUND)); + + // `x` in the local scope is IS_SUPER_BOUND (the `<<-` site) + assert!(index + .symbols(local_scope) + .get("x") + .unwrap() + .flags() + .contains(SymbolFlags::IS_SUPER_BOUND)); +} + +#[test] +fn test_nse_eager_super_assignment_visible_to_later_use() { + // A `<<-` inside an eager NSE body mutates the enclosing binding mid-run. + // Each use captures its own point-in-time snapshot, so the use before the + // `<<-` sees only `x <- 1` while the use after it also sees the `<<-`. + let index = index( + "\ +x <- 1 +local({ + x + x <<- 2 + x +}) +", + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + // Use 0 is before the `<<-`: only `x <- 1` (DefinitionId 0). + let (before_scope, before) = index + .enclosing_bindings(local_scope, UseId::from(0)) + .unwrap(); + assert_eq!(before_scope, file); + assert_eq!(before.definitions(), &[DefinitionId::from(0)]); + + // Use 1 is after the `<<-`: `x <- 1` (0) and the `<<-` target (1). + let (after_scope, after) = index + .enclosing_bindings(local_scope, UseId::from(1)) + .unwrap(); + assert_eq!(after_scope, file); + assert_eq!(after.definitions(), &[ + DefinitionId::from(0), + DefinitionId::from(1) + ]); +} + +#[test] +fn test_nse_eager_snapshot_excludes_unrelated_super_assignment() { + // The eager snapshot is point-in-time with no watcher, so a `<<-` in a + // function defined after the `local()` call can't fold into it. `f`'s body + // runs at an unknown later time and can't reach the already-run eager body. + let index = index( + "\ +x <- 1 +local({ + x +}) +f <- function() { + x <<- 2 +} +", + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + // Only `x <- 1` (DefinitionId 0). The `<<-` target recorded later while f's + // body is walked is excluded, since the eager snapshot took no watcher. + let (enclosing_scope, bindings) = index + .enclosing_bindings(local_scope, UseId::from(0)) + .unwrap(); + assert_eq!(enclosing_scope, file); + assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); +} + +#[test] +fn test_nse_eager_snapshot_excludes_unrelated_routed_definition() { + // `on_load` is `Current + Lazy`, so its `x <- 2` routes to the file scope + // as a deferred def recorded after `local()`. The eager snapshot takes no + // watcher, so the routed def is excluded, correct since it can't reach the + // already-run body. + let index = index( + "\ +x <- 1 +local({ + x +}) +rlang::on_load({ + x <- 2 +}) +", + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + // Only `x <- 1` (DefinitionId 0). The routed `x <- 2` is excluded. + let (enclosing_scope, bindings) = index + .enclosing_bindings(local_scope, UseId::from(0)) + .unwrap(); + assert_eq!(enclosing_scope, file); + assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); +} + +// --- Resolver-driven recognition --- + +#[test] +fn test_nse_noop_resolver_bare_local_stays_flat() { + // Under Noop, `resolve_effects` returns `None`, so a bare `local` isn't + // recognized as NSE: no scope is pushed and `x` stays at file scope. + let index = build_with( + "\ +local({ + x <- 1 +}) +", + NoopImportsResolver, + ); + let file = ScopeId::from(0); + + assert_eq!(index.scope_ids().count(), 1); + assert_eq!( + index.symbols(file).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_noop_resolver_namespaced_local_pushes_scope() { + // `pkg::fn` resolves through the resolver's default `resolve_qualified_effects`, + // which reads the static registry. `::` names the package, so there's no + // shadowing and no cross-file context needed, hence `base::local` is + // recognized as NSE even under Noop. + let index = build_with( + "\ +base::local({ + x <- 1 +}) +", + NoopImportsResolver, + ); + let local_scope = ScopeId::from(1); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!( + index.symbols(local_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_front_gate_skips_resolver_for_unannotated_name() { + // A bare callee whose name no package annotates never reaches the resolver: + // the `is_annotated_name` gate short-circuits before consultation. + let resolver = TestImportsResolver::with_base(); + let consultations = resolver.consultations(); + + build_with("frobnicate({ x <- 1 })", resolver); + + assert_eq!(consultations.get(), 0); +} + +#[test] +fn test_nse_front_gate_consults_resolver_for_annotated_name() { + // An annotated bare callee does reach the resolver (contrast with the gate + // test above). + let resolver = TestImportsResolver::with_base(); + let consultations = resolver.consultations(); + + build_with("local({ x <- 1 })", resolver); + + assert!(consultations.get() > 0); +} + +// --- source() bindings visible to the scan --- + +#[test] +fn test_nse_sourced_name_shadows_base_callee() { + // A `source()`-injected `local` shadows base `local`, so the later + // `local({...})` is NOT NSE. The scan binds the sourced names eagerly + // (source() runs at its position), so the later callee sees the shadow in + // the same pass, even though the walk injects the Import def later. + let index = build_with( + "\ +source(\"utils.R\") +local({ + x <- 1 +}) +", + TestImportsResolver::with_base().with_source("utils.R", &["local"]), + ); + let file = ScopeId::from(0); + + // No NSE scope: the sourced `local` shadows base, so `x` stays flat. + assert_eq!(index.scope_ids().count(), 1); + assert_eq!( + index.symbols(file).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_sourced_file_without_name_leaves_callee_nse() { + // Same shape, but the sourced file does not define `local`, so base + // `local` is unshadowed and `local({...})` IS NSE. + let index = build_with( + "\ +source(\"utils.R\") +local({ + x <- 1 +}) +", + TestImportsResolver::with_base().with_source("utils.R", &["other"]), + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(local_scope).parent(), Some(file)); + assert!(index.symbols(file).get("x").is_none()); + assert_eq!( + index.symbols(local_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +// --- Current + Lazy owner bindings visible before the walk --- + +#[test] +fn test_nse_on_load_binding_order_independent() { + // A `local` bound inside a `Current + Lazy` `on_load` body is deferred + // (lazy-provenance), so it is not a precise predecessor for the lazy `local()` + // in a sibling function `f`. Both bodies run in an order the engine can't + // know, so whether the shadow holds when `f` runs is undetermined. The + // predecessor snapshot excludes the deferred `local`, so `f`'s `local()` is + // optimistically NSE in both orderings (the overturn lint, pending, flags the + // ambiguity). `x` moves into its own scope regardless of order. + let first = index( + "\ +f <- function() local({ x <- 1 }) +rlang::on_load({ local <- identity }) +", + ); + // Walk order: file, f, f's `local()` scope, on_load. + let f_first = ScopeId::from(1); + let f_local_first = ScopeId::from(2); + assert_eq!(first.scope_ids().count(), 4); + assert_eq!(first.scope(f_first).kind(), ScopeKind::Function); + assert_eq!( + first.scope(f_local_first).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(first.scope(f_local_first).parent(), Some(f_first)); + assert!(first.symbols(f_first).get("x").is_none()); + assert_eq!( + first.symbols(f_local_first).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + + let second = index( + "\ +rlang::on_load({ local <- identity }) +f <- function() local({ x <- 1 }) +", + ); + // Walk order: file, on_load, f, f's `local()` scope. + let f_second = ScopeId::from(2); + let f_local_second = ScopeId::from(3); + assert_eq!(second.scope_ids().count(), 4); + assert_eq!(second.scope(f_second).kind(), ScopeKind::Function); + assert_eq!( + second.scope(f_local_second).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(second.scope(f_local_second).parent(), Some(f_second)); + assert!(second.symbols(f_second).get("x").is_none()); + assert_eq!( + second.symbols(f_local_second).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_on_load_nested_binding_order_independent() { + // Same as the direct-binding case above, but the shadowing binding is buried + // in a nested transparent call (`evalq(...)`). It makes no difference to the + // NSE decision: the predecessor snapshot reads only `f`'s eager predecessors, + // and `on_load`'s deferred `local` is not one of them however it is written. + // So `f`'s `local()` is optimistically NSE in both orderings and `x` moves + // into its own scope. (Under the old whole-scope read this case was + // order-dependent, because it hinged on whether the walk had routed `local` + // to the owner's bound names before it reached `f`.) + + // `f` before the `on_load`. + let first = index( + "\ +f <- function() local({ x <- 1 }) +rlang::on_load({ evalq(local <- identity) }) +", + ); + let f = ScopeId::from(1); + let nested = ScopeId::from(2); + assert_eq!(first.scope_ids().count(), 4); + assert_eq!(first.scope(f).kind(), ScopeKind::Function); + assert_eq!( + first.scope(nested).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(first.scope(nested).parent(), Some(f)); + assert!(first.symbols(f).get("x").is_none()); + assert_eq!( + first.symbols(nested).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + + // `f` after the `on_load`: same result, `local()` is still NSE. + let second = index( + "\ +rlang::on_load({ evalq(local <- identity) }) +f <- function() local({ x <- 1 }) +", + ); + let f_second = ScopeId::from(2); + let nested_second = ScopeId::from(3); + assert_eq!(second.scope_ids().count(), 4); + assert_eq!(second.scope(f_second).kind(), ScopeKind::Function); + assert_eq!( + second.scope(nested_second).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(second.scope(nested_second).parent(), Some(f_second)); + assert!(second.symbols(f_second).get("x").is_none()); + assert_eq!( + second.symbols(nested_second).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_on_load_deferred_binding_unbound_at_eager_position() { + // The eager stance: `on_load`'s `local` reaches only the owner's bound names, + // never `bound_so_far`. A file-level (eager) `local()` after the + // `on_load` runs before the deferred body, so it treats `local` as unbound + // and IS NSE. Contrast with the lazy sibling case above. + let index = index( + "\ +rlang::on_load({ local <- identity }) +local({ x <- 1 }) +", + ); + let file = ScopeId::from(0); + let on_load_scope = ScopeId::from(1); + let local_scope = ScopeId::from(2); + + assert_eq!(index.scope_ids().count(), 3); + assert_eq!( + index.scope(on_load_scope).kind(), + ScopeKind::Nse(NseScope::Current, NseTiming::Lazy) + ); + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(local_scope).parent(), Some(file)); + assert!(index.symbols(file).get("x").is_none()); + assert_eq!( + index.symbols(local_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +// --- NSE calls in parameter defaults --- + +#[test] +fn test_nse_parameter_default_pushes_scope() { + // An NSE call in a parameter default is recognized and pushes its scope. + let index = index("f <- function(a = local({ x <- 1 })) a\n"); + let f_scope = ScopeId::from(1); + let local_scope = ScopeId::from(2); + + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(local_scope).parent(), Some(f_scope)); + + // `x` lands in the default's NSE scope, not the function scope. + assert!(index.symbols(f_scope).get("x").is_none()); + assert_eq!( + index.symbols(local_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_parameter_default_shadowed_by_param() { + // All formals bind at once, so a `local` parameter shadows base `local` in + // a later default, regardless of order: `local({...})` is NOT NSE and `x` + // stays flat in the function scope. + let index = index("f <- function(local, a = local({ x <- 1 })) a\n"); + let f_scope = ScopeId::from(1); + + assert_eq!(index.scope_ids().count(), 2); + assert_eq!( + index.symbols(f_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +// --- Lazy shadow ambiguity diagnostics --- + +#[test] +fn test_diagnostic_lazy_shadow_later_eager_binding() { + // `f`'s `local()` is optimistically NSE, but a later file-level `local` + // binding could shadow it depending on when `f` runs. Flagged. + let source = "\ +f <- function() local({ x <- 1 }) +local <- identity +"; + let index = index(source); + + let diagnostics = index.diagnostics(); + assert_eq!(diagnostics.len(), 1); + match &diagnostics[0] { + SemanticDiagnostic::LazyShadowAmbiguity { + name, + call_range, + overwrite_range, + } => { + assert_eq!(name, "local"); + + let call_start = u32::from(call_range.start()) as usize; + let call_end = u32::from(call_range.end()) as usize; + assert_eq!(&source[call_start..call_end], "local({ x <- 1 })"); + + let overwrite_start = u32::from(overwrite_range.start()) as usize; + let overwrite_end = u32::from(overwrite_range.end()) as usize; + assert_eq!(&source[overwrite_start..overwrite_end], "local"); + }, + } +} + +#[test] +fn test_diagnostic_lazy_shadow_on_load_binding() { + // A deferred `on_load` binding of `local` and a lazy sibling's `local()` + // run in an unknown order. Flagged. + let index = index( + "\ +f <- function() local({ x <- 1 }) +rlang::on_load({ local <- identity }) +", + ); + + let diagnostics = index.diagnostics(); + assert_eq!(diagnostics.len(), 1); + match &diagnostics[0] { + SemanticDiagnostic::LazyShadowAmbiguity { name, .. } => assert_eq!(name, "local"), + } +} + +#[test] +fn test_diagnostic_none_at_eager_position() { + // The file-level `local()` runs before the `on_load` hook fires, so its + // "unbound" reading is determined, not a guess. No diagnostic. + let index = index( + "\ +rlang::on_load({ local <- identity }) +local({ x <- 1 }) +", + ); + assert!(index.diagnostics().is_empty()); +} + +#[test] +fn test_diagnostic_none_when_callee_unbound_everywhere() { + // `local` is never bound anywhere, so the NSE decision is certain and + // nothing competes with it. No diagnostic. + let index = index( + "\ +x <- 1 +f <- function() local({ x }) +", + ); + assert!(index.diagnostics().is_empty()); +} + +#[test] +fn test_diagnostic_none_with_eager_predecessor() { + // `local` is bound before `f` is defined, a sure shadow, so `f`'s `local()` + // is not NSE at all. No diagnostic. + let index = index( + "\ +local <- identity +f <- function() local({ x }) +", + ); + assert!(index.diagnostics().is_empty()); +} + +// --- Eager linear scan: descent and pending names --- + +#[test] +fn test_nse_descent_consults_each_call_once() { + // The inner `local` sits inside the outer `local`'s eager body. The descent + // scans it once and the walk installs the pending names instead of + // re-scanning, so each of the two calls reaches the resolver exactly once. + let resolver = TestImportsResolver::with_base(); + let consultations = resolver.consultations(); + + build_with("local({ local({ x <- 1 }) })", resolver); + + assert_eq!(consultations.get(), 2); +} + +#[test] +fn test_nse_descent_current_lazy_owner_routes_to_descent_top() { + // A `Current + Lazy` body (`on_load`) inside an eager `local` body binds `x`. + // During the descent, `record_owner_name` must route `x` to the descent top + // (local), not to the current scope. `scan_lazy_owner_bindings` runs while + // the arena's `current_scope` is still the file, so only the descent-top + // shortcut lands `x` in local's pending names. + // + // We pin it through a FORWARD reference: `f` uses `x` before `on_load` binds + // it, so the walk resolves the use through local's `bound_names` (the pending + // set), not through an already-recorded definition. If the routing regressed, + // `x` would land in the file and the use would resolve to the file scope. + let index = index( + "\ +local({ + f <- function() x + rlang::on_load({ x <- 1 }) +}) +", + ); + let local_scope = ScopeId::from(1); + let f_scope = ScopeId::from(2); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + + let (enclosing_scope, _bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); + assert_eq!(enclosing_scope, local_scope); +} + +#[test] +fn test_nse_descent_snapshot_through_pending_scope() { + // The descent records `y` as pending for `local`'s scope; the walk installs + // it before walking `f`, so `f`'s use of `y` resolves to the enclosing + // snapshot in `local`. + let index = index( + "\ +local({ + y <- 1 + f <- function() y +}) +", + ); + let file = ScopeId::from(0); + let local_scope = ScopeId::from(1); + let f_scope = ScopeId::from(2); + + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(local_scope).parent(), Some(file)); + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + assert_eq!(index.scope(f_scope).parent(), Some(local_scope)); + + // `y` lands in local's scope. + assert_eq!( + index.symbols(local_scope).get("y").unwrap().flags(), + SymbolFlags::IS_BOUND + ); + + // `f`'s use of `y` resolves to local's snapshot. In local, `y` is + // DefinitionId 0 (`f` is DefinitionId 1). + let (enclosing_scope, bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); + assert_eq!(enclosing_scope, local_scope); + assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); +} + +#[test] +fn test_nse_descent_eager_under_lazy() { + // `local` resolves during `f`'s walk-time scan (unit = `f`), which descends + // into the body and records its names as pending. `x` lands in local's + // Nested+Eager scope, not in `f`. + let index = index( + "\ +f <- function() { + local({ + x <- 1 + }) +} +", + ); + let f_scope = ScopeId::from(1); + let local_scope = ScopeId::from(2); + + assert_eq!(index.scope(f_scope).kind(), ScopeKind::Function); + assert_eq!( + index.scope(local_scope).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(local_scope).parent(), Some(f_scope)); + + assert!(index.symbols(f_scope).get("x").is_none()); + assert_eq!( + index.symbols(local_scope).get("x").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_descent_nested_eager_in_eager() { + // `local({ local({ y <- 1 }) })`: descent stack depth 2, each body's names + // pending under its own range. `y` lands in the inner scope. + let index = index( + "\ +local({ + local({ + y <- 1 + }) +}) +", + ); + let file = ScopeId::from(0); + let outer_local = ScopeId::from(1); + let inner_local = ScopeId::from(2); + + assert_eq!(index.scope_ids().count(), 3); + assert_eq!( + index.scope(outer_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(outer_local).parent(), Some(file)); + assert_eq!( + index.scope(inner_local).kind(), + ScopeKind::Nse(NseScope::Nested, NseTiming::Eager) + ); + assert_eq!(index.scope(inner_local).parent(), Some(outer_local)); + + assert!(index.symbols(outer_local).get("y").is_none()); + assert_eq!( + index.symbols(inner_local).get("y").unwrap().flags(), + SymbolFlags::IS_BOUND + ); +} + +#[test] +fn test_nse_descent_lazy_flag_eager_vs_lazy_context() { + // An eager callee at file scope consults with `lazy = false`; the same + // callee inside a function body consults with `lazy = true`. + let resolver = TestImportsResolver::with_base(); + let log = resolver.consultation_log(); + + build_with( + "\ +local({ x <- 1 }) +f <- function() { + local({ y <- 1 }) +} +", + resolver, + ); + + let records = log.borrow(); + let local_lazy: Vec = records + .iter() + .filter(|(name, _lazy)| name == "local") + .map(|(_name, lazy)| *lazy) + .collect(); + assert_eq!(local_lazy, vec![false, true]); +} + +#[test] +fn test_nse_descent_eager_in_eager_in_function_stays_lazy() { + // An eager `local` nested inside another eager `local` inside a function + // still consults with `lazy = true`. Laziness is a property of the enclosing + // scan unit (the function), which the descent preserves by keeping + // `current_scope` on the function while it scans both eager bodies inline. If + // the inner `local` were resolved against its immediate eager scope instead, + // `is_lazy()` would read `false` and the flag would regress. + let resolver = TestImportsResolver::with_base(); + let log = resolver.consultation_log(); + + build_with( + "\ +f <- function() { + local({ + local({ x <- 1 }) + }) +} +", + resolver, + ); + + let records = log.borrow(); + let local_lazy: Vec = records + .iter() + .filter(|(name, _lazy)| name == "local") + .map(|(_name, lazy)| *lazy) + .collect(); + assert_eq!(local_lazy, vec![true, true]); +} diff --git a/crates/oak_semantic/tests/integration/main.rs b/crates/oak_semantic/tests/integration/main.rs index da7516d89..0eca846ff 100644 --- a/crates/oak_semantic/tests/integration/main.rs +++ b/crates/oak_semantic/tests/integration/main.rs @@ -1,2 +1,4 @@ mod builder; +mod builder_nse; +mod resolvers; mod use_def_map; diff --git a/crates/oak_semantic/tests/integration/resolvers.rs b/crates/oak_semantic/tests/integration/resolvers.rs new file mode 100644 index 000000000..b84d84942 --- /dev/null +++ b/crates/oak_semantic/tests/integration/resolvers.rs @@ -0,0 +1,86 @@ +use std::cell::Cell; +use std::cell::RefCell; +use std::collections::HashMap; +use std::rc::Rc; + +use oak_semantic::effects_registry; +use oak_semantic::Effects; +use oak_semantic::ImportsResolver; +use oak_semantic::SourceResolution; +use url::Url; + +/// Test resolver: an explicit search path resolved against the registry. +/// +/// Resolves a bare callee by walking `attached` (LIFO) then its own +/// always-attached packages, returning the first package's registry annotation +/// for the name. Base is a normal entry in the always-attached list, not a +/// special case. Flat: no re-export chase, that's the salsa resolver's job. +pub struct TestImportsResolver { + /// Packages always on the search path, base last. These stand in for the + /// non-flow layers (base, default search path) the salsa resolver derives. + always_attached: Vec, + /// Count of `resolve_effects` consultations, so tests can assert the front + /// gate keeps unannotated names off the resolver. + consultations: Rc>, + /// Per-consultation `(name, lazy)` records, so tests can pin the `lazy` flag + /// the builder derives from the callee's context. + consultation_log: Rc>>, + /// `source()` paths this resolver knows, mapped to the names they export. + sources: HashMap, +} + +impl TestImportsResolver { + /// Resolver with base always attached: the minimum for the bare base NSE + /// functions (`local`, `with`, `within`, `evalq`) to resolve. + pub fn with_base() -> Self { + Self { + always_attached: vec![String::from("base")], + consultations: Rc::new(Cell::new(0)), + consultation_log: Rc::new(RefCell::new(Vec::new())), + sources: HashMap::new(), + } + } + + /// Register a sourced file at `path` exporting `names`, so `resolve_source` + /// returns a resolution for it. The URL is synthesized from the path. + pub fn with_source(mut self, path: &str, names: &[&str]) -> Self { + let resolution = SourceResolution { + url: Url::parse(&format!("file:///{path}")).unwrap(), + names: names.iter().map(|name| name.to_string()).collect(), + packages: vec![], + }; + self.sources.insert(path.to_string(), resolution); + self + } + + /// A handle to the consultation counter. Clone it before moving the + /// resolver into `build_index`, then read it after the build. + pub fn consultations(&self) -> Rc> { + Rc::clone(&self.consultations) + } + + /// A handle to the per-consultation `(name, lazy)` log. Clone it before + /// moving the resolver into `build_index`, then read it after the build. + pub fn consultation_log(&self) -> Rc>> { + Rc::clone(&self.consultation_log) + } +} + +impl ImportsResolver for TestImportsResolver { + fn resolve_source(&mut self, path: &str) -> Option { + self.sources.get(path).cloned() + } + + fn resolve_effects(&mut self, name: &str, attached: &[String], lazy: bool) -> Option { + self.consultations.set(self.consultations.get() + 1); + self.consultation_log + .borrow_mut() + .push((name.to_string(), lazy)); + attached + .iter() + .rev() + .chain(self.always_attached.iter()) + .find_map(|pkg| effects_registry::lookup(pkg, name).copied()) + .map(Effects::nse) + } +} diff --git a/crates/oak_semantic/tests/integration/use_def_map.rs b/crates/oak_semantic/tests/integration/use_def_map.rs index 94af04e06..6d7fc0409 100644 --- a/crates/oak_semantic/tests/integration/use_def_map.rs +++ b/crates/oak_semantic/tests/integration/use_def_map.rs @@ -965,9 +965,7 @@ f <- function() x let fun = ScopeId::from(1); // `x` in the function is free, resolves to file scope - let (enclosing_scope, bindings) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (enclosing_scope, bindings) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(0)); assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); assert_not!(bindings.may_be_unbound()); @@ -986,9 +984,7 @@ x <- 1 // `x` is defined after `f` in the file scope. The pre-scan finds it. // The snapshot is initialized at f's definition point (x unbound) // then updated when x <- 1 is encountered. - let (enclosing_scope, bindings) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (enclosing_scope, bindings) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(0)); assert_eq!(bindings.definitions(), &[DefinitionId::from(1)]); assert!(bindings.may_be_unbound()); @@ -1007,9 +1003,7 @@ x <- 2 // Lazy snapshot: union of all defs from definition point onward. // Initialized with {x <- 1}, updated with {x <- 2}. - let (_, bindings) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (_, bindings) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(bindings.definitions(), &[ DefinitionId::from(0), DefinitionId::from(2) @@ -1031,9 +1025,7 @@ f <- function() { let fun = ScopeId::from(1); // `x` is locally bound in the function, not free - assert!(index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .is_none()); + assert!(index.enclosing_bindings(fun, UseId::from(0)).is_none()); } #[test] @@ -1047,9 +1039,7 @@ f <- function(x) x let fun = ScopeId::from(1); // `x` is a parameter, not free - assert!(index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .is_none()); + assert!(index.enclosing_bindings(fun, UseId::from(0)).is_none()); } #[test] @@ -1067,9 +1057,7 @@ f <- function() { // x is free in g. f (scope 1) has no binding for x, so the lookup // skips f entirely and resolves to the file scope (scope 0). - let (enclosing_scope, bindings) = index - .enclosing_bindings(g_scope, index.uses(g_scope)[UseId::from(0)].symbol()) - .unwrap(); + let (enclosing_scope, bindings) = index.enclosing_bindings(g_scope, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(0)); assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); assert_not!(bindings.may_be_unbound()); @@ -1090,9 +1078,7 @@ f <- function() { // x is free in g. Both the file scope (scope 0) and f (scope 1) bind x, // but f is the nearest enclosing scope with a binding, so it wins. - let (enclosing_scope, bindings) = index - .enclosing_bindings(g_scope, index.uses(g_scope)[UseId::from(0)].symbol()) - .unwrap(); + let (enclosing_scope, bindings) = index.enclosing_bindings(g_scope, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(1)); assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); assert_not!(bindings.may_be_unbound()); @@ -1110,9 +1096,7 @@ f <- function() x // x is conditionally defined. The snapshot captures the state at f's // definition point: {x <- 1, may_be_unbound: true} - let (_, bindings) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (_, bindings) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); assert!(bindings.may_be_unbound()); } @@ -1130,9 +1114,7 @@ g <- function() { x <<- 2 } // The <<- from g adds a def to the file scope. The watcher on x // should update f's snapshot to include this def. - let (_, bindings) = index - .enclosing_bindings(f_scope, index.uses(f_scope)[UseId::from(0)].symbol()) - .unwrap(); + let (_, bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); assert_eq!(bindings.definitions(), &[ DefinitionId::from(0), DefinitionId::from(2) @@ -1150,9 +1132,7 @@ f <- function() x let fun = ScopeId::from(1); // x is not defined anywhere in the file. No enclosing snapshot. - assert!(index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .is_none()); + assert!(index.enclosing_bindings(fun, UseId::from(0)).is_none()); } #[test] @@ -1178,9 +1158,7 @@ f <- function(cond) { assert!(local.may_be_unbound()); // The enclosing snapshot should also be registered, capturing x <- 1. - let (enclosing_scope, bindings) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(1)].symbol()) - .unwrap(); + let (enclosing_scope, bindings) = index.enclosing_bindings(fun, UseId::from(1)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(0)); assert_eq!(bindings.definitions(), &[DefinitionId::from(0)]); assert_not!(bindings.may_be_unbound()); @@ -1206,9 +1184,7 @@ f <- function() { assert!(local.may_be_unbound()); // Enclosing snapshot registered for the fallthrough path. - let (enclosing_scope, _) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (enclosing_scope, _) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(0)); } @@ -1232,9 +1208,7 @@ f <- function() { assert_not!(local.may_be_unbound()); // No enclosing snapshot needed. - assert!(index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .is_none()); + assert!(index.enclosing_bindings(fun, UseId::from(0)).is_none()); } #[test] @@ -1252,15 +1226,11 @@ f <- function() { let fun = ScopeId::from(1); // Two independent free variables, each gets its own snapshot - let (scope_x, bindings_x) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (scope_x, bindings_x) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(scope_x, ScopeId::from(0)); assert_eq!(bindings_x.definitions(), &[DefinitionId::from(0)]); - let (scope_y, bindings_y) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(1)].symbol()) - .unwrap(); + let (scope_y, bindings_y) = index.enclosing_bindings(fun, UseId::from(1)).unwrap(); assert_eq!(scope_y, ScopeId::from(0)); assert_eq!(bindings_y.definitions(), &[DefinitionId::from(1)]); } @@ -1279,12 +1249,8 @@ f <- function() { let fun = ScopeId::from(1); // Both uses of `x` are free and resolve to the same enclosing snapshot - let (scope1, bindings1) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); - let (scope2, bindings2) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(1)].symbol()) - .unwrap(); + let (scope1, bindings1) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); + let (scope2, bindings2) = index.enclosing_bindings(fun, UseId::from(1)).unwrap(); assert_eq!(scope1, scope2); assert_eq!(bindings1, bindings2); } @@ -1307,9 +1273,7 @@ x <- 2 // x is free in f, resolves to file scope. The lazy snapshot // captures both x <- 1 (from initialization) and x <- 2 (from // watcher update). - let (enclosing_scope, bindings) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (enclosing_scope, bindings) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(0)); assert_eq!(bindings.definitions(), &[ DefinitionId::from(0), @@ -1338,12 +1302,8 @@ f <- function() { assert!(local0.definitions().is_empty()); assert!(local0.may_be_unbound()); - let (scope0, bindings0) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); - let (scope1, bindings1) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(1)].symbol()) - .unwrap(); + let (scope0, bindings0) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); + let (scope1, bindings1) = index.enclosing_bindings(fun, UseId::from(1)).unwrap(); assert_eq!(scope0, scope1); assert_eq!(bindings0, bindings1); assert_eq!(bindings0.definitions(), &[DefinitionId::from(0)]); @@ -1368,9 +1328,7 @@ f <- function(cond) { // f (scope 1, conditional x <- 2) bind x. f is the nearest enclosing // scope with a binding, so it wins. The snapshot captures f's state at // g's definition point: {x <- 2, may_be_unbound: true}. - let (enclosing_scope, bindings) = index - .enclosing_bindings(g_scope, index.uses(g_scope)[UseId::from(0)].symbol()) - .unwrap(); + let (enclosing_scope, bindings) = index.enclosing_bindings(g_scope, UseId::from(0)).unwrap(); assert_eq!(enclosing_scope, ScopeId::from(1)); assert_eq!(bindings.definitions(), &[DefinitionId::from(1)]); assert!(bindings.may_be_unbound()); @@ -1389,9 +1347,7 @@ f <- function() x // x <- 0 was shadowed by x <- 1 before f was defined. // The snapshot should contain only x <- 1, not both. - let (_, bindings) = index - .enclosing_bindings(fun, index.uses(fun)[UseId::from(0)].symbol()) - .unwrap(); + let (_, bindings) = index.enclosing_bindings(fun, UseId::from(0)).unwrap(); assert_eq!(bindings.definitions(), &[DefinitionId::from(1)]); assert_not!(bindings.may_be_unbound()); } @@ -1410,9 +1366,7 @@ g <- function() x // f is defined after x <- 1. Its snapshot is initialized with {x <- 1}, // then the watcher adds x <- 2: snapshot {x <- 1, x <- 2}. let f_scope = ScopeId::from(1); - let (_, f_bindings) = index - .enclosing_bindings(f_scope, index.uses(f_scope)[UseId::from(0)].symbol()) - .unwrap(); + let (_, f_bindings) = index.enclosing_bindings(f_scope, UseId::from(0)).unwrap(); assert_eq!(f_bindings.definitions(), &[ DefinitionId::from(0), DefinitionId::from(2) @@ -1423,9 +1377,7 @@ g <- function() x // initialized with {x <- 2} only. No subsequent definitions, so it // stays {x <- 2}. let g_scope = ScopeId::from(2); - let (_, g_bindings) = index - .enclosing_bindings(g_scope, index.uses(g_scope)[UseId::from(0)].symbol()) - .unwrap(); + let (_, g_bindings) = index.enclosing_bindings(g_scope, UseId::from(0)).unwrap(); assert_eq!(g_bindings.definitions(), &[DefinitionId::from(2)]); assert_not!(g_bindings.may_be_unbound()); }