From 694e4322eb24dffe4aef4c8e70c0e0900409f0eb Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 31 Jul 2026 14:55:00 +0200 Subject: [PATCH 1/5] Introduce `SourcePath` for directory support --- crates/oak_semantic/src/builder/scan.rs | 9 ++-- crates/oak_semantic/src/effects.rs | 42 +++++++++++++++---- crates/oak_semantic/src/effects/contrib.rs | 5 ++- .../oak_semantic/tests/integration/common.rs | 17 ++++++-- .../tests/integration/contrib/base.rs | 6 ++- 5 files changed, 62 insertions(+), 17 deletions(-) diff --git a/crates/oak_semantic/src/builder/scan.rs b/crates/oak_semantic/src/builder/scan.rs index 06500ba1f..4ae00b0b2 100644 --- a/crates/oak_semantic/src/builder/scan.rs +++ b/crates/oak_semantic/src/builder/scan.rs @@ -405,14 +405,17 @@ impl SemanticIndexBuilder { // can see them. if let Some(paths) = source { let range = call.syntax().text_trimmed_range(); - for path in paths { - let resolution = self.scan_source_call(&path, range); + for sourced in paths { + let resolution = self.scan_source_call(&sourced.path, range); self.scan .call_resolutions .entry(range) .or_default() .source - .push(SourcedFile { path, resolution }); + .push(SourcedFile { + path: sourced.path, + resolution, + }); } } diff --git a/crates/oak_semantic/src/effects.rs b/crates/oak_semantic/src/effects.rs index 1a9b3a67e..0ef237293 100644 --- a/crates/oak_semantic/src/effects.rs +++ b/crates/oak_semantic/src/effects.rs @@ -51,9 +51,9 @@ pub struct Effects { pub arguments: Option, /// Attach a package pub attach: Option, - /// Source one or more files. A vector so a collation-style callee can name + /// Source one or more paths. A vector so a collation-style callee can name /// several; base `source` resolves to one. - pub source: Option>, + pub source: Option>, /// Bind one or more names in the current scope (`assign("x", value)`). A /// vector so a multi-binding callee stays expressible; base `assign` and /// `delayedAssign` resolve to one. @@ -78,7 +78,7 @@ pub struct AssignBinding { pub struct EffectsHandlers { pub arguments: Option<&'static dyn EffectHandler>, pub attach: Option<&'static dyn EffectHandler>, - pub source: Option<&'static dyn EffectHandler>>, + pub source: Option<&'static dyn EffectHandler>>, pub assign: Option<&'static dyn AssignHandler>, } @@ -357,21 +357,40 @@ impl EffectHandler for ArgumentsAnnotation { } } -/// Declares how a source function (`source()`) names the file it reads, and -/// serves as the default [`EffectHandler`] for it by pulling that path out of a -/// call. +/// A path a source call names, and what that path points at. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct SourcePath { + pub path: String, + pub target: SourceTarget, +} + +/// What a source function's path argument points at. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum SourceTarget { + /// A single file, as base `source()` takes. + File, + /// A directory, every R file in it. The resolver expands the path into one + /// target per file, since listing it needs workspace knowledge a handler + /// doesn't have. + Dir, +} + +/// Declares how a source function (`source()`) names what it reads, and serves +/// as the default [`EffectHandler`] for it by pulling that path out of a call. #[derive(Debug, Clone, Copy)] pub struct SourceAnnotation { /// Which positional argument holds the path, counting only unnamed /// arguments (0 for base `source`). Other source-like functions may put the /// path elsewhere, so it's configured per entry rather than assumed. pub position: usize, + /// Whether that argument names a file or a directory. + pub target: SourceTarget, } impl EffectHandler for SourceAnnotation { - type Output = Vec; + type Output = Vec; - fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option> { + fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option> { let args = call.arguments().ok()?; // The path is matched positionally among unnamed arguments rather than @@ -421,7 +440,12 @@ impl EffectHandler for SourceAnnotation { positional += 1; } - path.map(|resolved| vec![resolved]) + path.map(|path| { + vec![SourcePath { + path, + target: self.target, + }] + }) } } diff --git a/crates/oak_semantic/src/effects/contrib.rs b/crates/oak_semantic/src/effects/contrib.rs index d803ab446..74075a761 100644 --- a/crates/oak_semantic/src/effects/contrib.rs +++ b/crates/oak_semantic/src/effects/contrib.rs @@ -80,7 +80,10 @@ macro_rules! source { effects: $crate::effects::EffectsHandlers { arguments: None, attach: None, - source: Some(&$crate::effects::SourceAnnotation { position: $pos }), + source: Some(&$crate::effects::SourceAnnotation { + position: $pos, + target: $crate::effects::SourceTarget::File, + }), assign: None, }, } diff --git a/crates/oak_semantic/tests/integration/common.rs b/crates/oak_semantic/tests/integration/common.rs index fa2e246fb..b641c8fdf 100644 --- a/crates/oak_semantic/tests/integration/common.rs +++ b/crates/oak_semantic/tests/integration/common.rs @@ -9,6 +9,8 @@ use oak_semantic::effects::CallContext; use oak_semantic::effects::EffectHandler; use oak_semantic::effects::EffectSite; use oak_semantic::effects::RangedAstPtr; +use oak_semantic::effects::SourcePath; +use oak_semantic::effects::SourceTarget; use oak_semantic::semantic_index::DefinitionKind; use oak_semantic::semantic_index::ScopeId; use oak_semantic::semantic_index::SemanticCallKind; @@ -72,10 +74,19 @@ pub(crate) struct CollationHandler; pub(crate) static COLLATION_HANDLER: CollationHandler = CollationHandler; impl EffectHandler for CollationHandler { - type Output = Vec; + type Output = Vec; - fn resolve(&self, _call: &RCall, _ctx: &CallContext<'_>) -> Option> { - Some(vec!["a.R".into(), "b.R".into()]) + fn resolve(&self, _call: &RCall, _ctx: &CallContext<'_>) -> Option> { + Some(vec![ + SourcePath { + path: "a.R".into(), + target: SourceTarget::File, + }, + SourcePath { + path: "b.R".into(), + target: SourceTarget::File, + }, + ]) } } diff --git a/crates/oak_semantic/tests/integration/contrib/base.rs b/crates/oak_semantic/tests/integration/contrib/base.rs index eb9a76aec..ef2ae9a9e 100644 --- a/crates/oak_semantic/tests/integration/contrib/base.rs +++ b/crates/oak_semantic/tests/integration/contrib/base.rs @@ -4,6 +4,7 @@ use biome_rowan::AstNode; use oak_semantic::build_index; use oak_semantic::effects; use oak_semantic::effects::SourceAnnotation; +use oak_semantic::effects::SourceTarget; use oak_semantic::semantic_index::AmbiguityReason; use oak_semantic::semantic_index::AttachRegion; use oak_semantic::semantic_index::DefinitionId; @@ -115,7 +116,10 @@ impl ImportsResolver for MultiFileResolver { /// positional slot, exercising the configurable `position`. struct PositionResolver; -static SOURCE_AT_POSITION_1: SourceAnnotation = SourceAnnotation { position: 1 }; +static SOURCE_AT_POSITION_1: SourceAnnotation = SourceAnnotation { + position: 1, + target: SourceTarget::File, +}; impl ImportsResolver for PositionResolver { fn resolve_source(&mut self, _path: &str) -> Option { From ad9abe904fc086ccbfe953131a35b0ec52740ed3 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 31 Jul 2026 15:18:48 +0200 Subject: [PATCH 2/5] Support `sourceDir()` calls with stopgap approach --- crates/oak_db/src/file_imports.rs | 42 ++++---- crates/oak_db/src/imports.rs | 56 +++++++---- crates/oak_semantic/src/builder/effects.rs | 3 + crates/oak_semantic/src/builder/scan.rs | 58 +++++++---- crates/oak_semantic/src/effects.rs | 20 ++++ crates/oak_semantic/src/resolver.rs | 10 ++ .../tests/integration/contrib/base.rs | 97 +++++++++++++++++++ 7 files changed, 230 insertions(+), 56 deletions(-) diff --git a/crates/oak_db/src/file_imports.rs b/crates/oak_db/src/file_imports.rs index 45b9ccbcf..7f5f88c82 100644 --- a/crates/oak_db/src/file_imports.rs +++ b/crates/oak_db/src/file_imports.rs @@ -375,35 +375,18 @@ impl File { package.files(db).contains(&self) } - /// The collation members of `self`'s own `R/` directory, in load order: - /// sorted by basename, ASCII case-insensitively, the same order a - /// package `R/` with no `Collate:` gets from - /// `oak_scan::packages::order_alphabetically`. + /// The collation members of `self`'s own `R/` directory, in load order. /// /// Path-based only. The scan-time resolver /// ([`SalsaImportsResolver`](crate::imports::SalsaImportsResolver)) calls /// `cross_file_layers` while `self`'s own semantic index is still being /// built. The query can't recurse into the index. - /// - /// Gathers candidates from workspace roots only (`root.scripts(db)` for - /// each). `OrphanRoot`, library roots, and `StaleRoot` don't contribute - /// collation siblings. #[salsa::tracked(returns(ref))] pub(crate) fn collation_siblings(self, db: &dyn Db) -> Vec { let Some(dir) = self.path(db).as_path().and_then(Utf8Path::parent) else { return Vec::new(); }; - - let mut siblings: Vec = db - .workspace_roots() - .roots(db) - .iter() - .flat_map(|root| root.scripts(db).iter().copied()) - .filter(|file| file.path(db).as_path().and_then(Utf8Path::parent) == Some(dir)) - .collect(); - - siblings.sort_by_cached_key(|file| collation_basename_key(*file, db)); - siblings + files_in_directory(db, dir) } } @@ -721,7 +704,26 @@ fn testthat_support_key(file: File, db: &dyn Db) -> Cow<'_, str> { file.path(db).file_name().unwrap_or_default() } -/// Case-insensitive basename sort key for `collation_siblings`, matching +/// The workspace files sitting directly in `dir`, in load order: sorted by +/// basename, ASCII case-insensitively, the same order a package `R/` with no +/// `Collate:` gets from `oak_scan::packages::order_alphabetically`. +/// +/// Gathers from workspace roots only (`root.scripts(db)` for each). +/// `OrphanRoot`, library roots, and `StaleRoot` don't contribute. +pub(crate) fn files_in_directory(db: &dyn Db, dir: &Utf8Path) -> Vec { + let mut files: Vec = db + .workspace_roots() + .roots(db) + .iter() + .flat_map(|root| root.scripts(db).iter().copied()) + .filter(|file| file.path(db).as_path().and_then(Utf8Path::parent) == Some(dir)) + .collect(); + + files.sort_by_cached_key(|file| collation_basename_key(*file, db)); + files +} + +/// Case-insensitive basename sort key for [`files_in_directory`], matching /// `oak_scan::packages::order_alphabetically`'s `basename_key` so a /// non-package `R/` collates the same way as a package `R/` with no /// `Collate:`. diff --git a/crates/oak_db/src/imports.rs b/crates/oak_db/src/imports.rs index 30b3d0959..c600c5f06 100644 --- a/crates/oak_db/src/imports.rs +++ b/crates/oak_db/src/imports.rs @@ -11,6 +11,7 @@ use oak_semantic::SourceResolution; use rustc_hash::FxHashMap; use url::Url; +use crate::file_imports::files_in_directory; use crate::file_imports::CollationView; use crate::file_imports::ImportLayer; use crate::Db; @@ -56,6 +57,34 @@ impl<'db> SalsaImportsResolver<'db> { cache: EffectsCache::default(), } } + + fn source_resolution(&self, file: File) -> SourceResolution { + let names: Vec = file + .exports(self.db) + .iter() + .map(|(name, _)| name.to_string()) + .collect(); + + let packages: Vec = file + .attached_packages(self.db) + .iter() + .map(|name| name.text(self.db).to_string()) + .collect(); + + SourceResolution { + url: file.path(self.db).to_url(), + names, + packages, + } + } + + /// The workspace files a `SourceTarget::Dir` path names, in load order. + /// `None` when the path doesn't anchor. + fn files_in_source_dir(&self, path: &str) -> Option> { + let anchor = anchor_dir(self.db, self.file)?; + let target_path = resolve_relative_to(&anchor, path)?; + Some(files_in_directory(self.db, target_path.as_path()?)) + } } /// Per-build memo for `resolve_effects`, keyed on `(name, attached)`. @@ -97,24 +126,17 @@ impl<'db> ImportsResolver for SalsaImportsResolver<'db> { // TODO(diagnostics): Until we support out-of-workspace sourced files, // should we at least lint so user knows that we can't analyse the file? let file = self.db.file_by_path(&target_path)?; + Some(self.source_resolution(file)) + } - let names: Vec = file - .exports(self.db) - .iter() - .map(|(name, _)| name.to_string()) - .collect(); - - let packages: Vec = file - .attached_packages(self.db) - .iter() - .map(|name| name.text(self.db).to_string()) - .collect(); - - Some(SourceResolution { - url: target_path.to_url(), - names, - packages, - }) + fn resolve_source_dir(&mut self, path: &str) -> Vec { + self.files_in_source_dir(path) + .unwrap_or_default() + .into_iter() + // Exclude sourcing file + .filter(|file| *file != self.file) + .map(|file| self.source_resolution(file)) + .collect() } fn resolve_effects(&mut self, name: &str, attached: &[String]) -> Option { diff --git a/crates/oak_semantic/src/builder/effects.rs b/crates/oak_semantic/src/builder/effects.rs index 9100bea92..e8b3a0d28 100644 --- a/crates/oak_semantic/src/builder/effects.rs +++ b/crates/oak_semantic/src/builder/effects.rs @@ -77,6 +77,9 @@ impl SemanticIndexBuilder { match &func { AnyRExpression::RIdentifier(ident) => { let name = ident.name_text(); + if let Some(effects) = effects::source_dir_idiom(&name) { + return Some(*effects); + } self.resolve_symbol_effects(&name, call.syntax().text_trimmed_range()) }, diff --git a/crates/oak_semantic/src/builder/scan.rs b/crates/oak_semantic/src/builder/scan.rs index 4ae00b0b2..1eb0fcda0 100644 --- a/crates/oak_semantic/src/builder/scan.rs +++ b/crates/oak_semantic/src/builder/scan.rs @@ -30,6 +30,8 @@ use crate::effects::AssignBinding; use crate::effects::ResolvedArgumentEffect; use crate::effects::ResolvedArgumentEffects; use crate::effects::ScopeContext; +use crate::effects::SourcePath; +use crate::effects::SourceTarget; use crate::resolver::ImportsResolver; use crate::resolver::SourceResolution; use crate::semantic_index::AmbiguityReason; @@ -406,16 +408,24 @@ impl SemanticIndexBuilder { if let Some(paths) = source { let range = call.syntax().text_trimmed_range(); for sourced in paths { - let resolution = self.scan_source_call(&sourced.path, range); - self.scan - .call_resolutions - .entry(range) - .or_default() - .source - .push(SourcedFile { + let resolutions = self.scan_source_call(&sourced, range); + let entries = &mut self.scan.call_resolutions.entry(range).or_default().source; + + // Nothing resolved, so keep the path as written for a consumer + // to report. A directory that holds no scanned R files lands + // here too. + if resolutions.is_empty() { + entries.push(SourcedFile { path: sourced.path, - resolution, + resolution: None, }); + continue; + } + + entries.extend(resolutions.into_iter().map(|resolution| SourcedFile { + path: sourced.path.clone(), + resolution: Some(resolution), + })); } } @@ -715,22 +725,34 @@ impl SemanticIndexBuilder { } } - /// Resolve one sourced `path`, bind the names it brings in, and return its - /// resolution for the caller to cache. - /// - /// 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`). Returns `None` when the resolver + /// Resolve one `sourced` path, bind the names it brings in, and return its + /// resolutions for the caller to cache. A directory yields one per R file + /// in it, in load order; a file yields at most one. Empty when the resolver /// can't locate the target. /// /// [`scan_call`]: Self::scan_call fn scan_source_call( &mut self, - path: &str, + sourced: &SourcePath, source_range: TextRange, - ) -> Option { - let resolution = self.resolver.resolve_source(path)?; + ) -> Vec { + let resolutions = match sourced.target { + SourceTarget::File => self + .resolver + .resolve_source(&sourced.path) + .into_iter() + .collect(), + SourceTarget::Dir => self.resolver.resolve_source_dir(&sourced.path), + }; + + for resolution in &resolutions { + self.record_source_resolution(resolution, source_range); + } + resolutions + } + /// Bind contributions of a sourced file. + fn record_source_resolution(&mut self, resolution: &SourceResolution, source_range: TextRange) { // Sourced names originate in another file, so they have no binding site // here. Anchor the overwrite range at the `source()` call instead. for name in &resolution.names { @@ -750,8 +772,6 @@ impl SemanticIndexBuilder { .push(pkg.clone(), source_range.start()); } } - - Some(resolution) } /// Whether the current evaluation frame binds `name` (see [`scan_scope`]). diff --git a/crates/oak_semantic/src/effects.rs b/crates/oak_semantic/src/effects.rs index 0ef237293..2cc846157 100644 --- a/crates/oak_semantic/src/effects.rs +++ b/crates/oak_semantic/src/effects.rs @@ -98,6 +98,26 @@ pub fn annotates(name: &str) -> bool { INDEX.contains_key(name) } +/// HACK: This matches a `sourceDir()` call syntactically. See `?source` for the +/// definition of `sourceDir()` that people copy around: +/// https://github.com/search?q=sourceDir+language%3AR&type=code +/// This is a stopgap workaround until we can infer source effects around a +/// `list.files()` loop. +pub fn source_dir_idiom(name: &str) -> Option<&'static EffectsHandlers> { + if name != "sourceDir" { + return None; + } + Some(&EffectsHandlers { + arguments: None, + attach: None, + source: Some(&SourceAnnotation { + position: 0, + target: SourceTarget::Dir, + }), + assign: None, + }) +} + /// Resolver for an effect of a call. /// /// The single interface behind every effect kind (NSE, attach, source). diff --git a/crates/oak_semantic/src/resolver.rs b/crates/oak_semantic/src/resolver.rs index a3b044208..f60559ea6 100644 --- a/crates/oak_semantic/src/resolver.rs +++ b/crates/oak_semantic/src/resolver.rs @@ -50,6 +50,16 @@ pub trait ImportsResolver { /// Returns `None` when the target can't be located. fn resolve_source(&mut self, path: &str) -> Option; + /// Resolve a directory path to one resolution per R file in it, in the + /// order they load. `path` is anchored the same way [`resolve_source`] + /// anchors a file path. + /// + /// [`resolve_source`]: ImportsResolver::resolve_source + fn resolve_source_dir(&mut self, path: &str) -> Vec { + let _ = path; + Vec::new() + } + /// Resolve a bare callee `name` to its effects. `attached` is the packages /// attached at this point, in flow order. The builder passes it in because /// the resolver can't query our own semantic index without creating a diff --git a/crates/oak_semantic/tests/integration/contrib/base.rs b/crates/oak_semantic/tests/integration/contrib/base.rs index ef2ae9a9e..d14a31752 100644 --- a/crates/oak_semantic/tests/integration/contrib/base.rs +++ b/crates/oak_semantic/tests/integration/contrib/base.rs @@ -88,6 +88,22 @@ impl ImportsResolver for MapResolver { } } +/// Expands any directory to `files`, standing in for the workspace listing. +/// Needs no `resolve_effects`: `sourceDir` is matched on its name. +struct DirResolver { + files: Vec, +} + +impl ImportsResolver for DirResolver { + fn resolve_source(&mut self, _path: &str) -> Option { + None + } + + fn resolve_source_dir(&mut self, _path: &str) -> Vec { + self.files.clone() + } +} + /// Resolves `source` to the multi-file [`CollationHandler`] and maps the /// collated paths through `sources`. struct MultiFileResolver { @@ -1245,6 +1261,87 @@ fn test_source_resolver_multiple_files_each_emitted_and_injected() { } } +#[test] +fn test_source_dir_expands_to_one_call_per_file() { + // A `SourceTarget::Dir` path routes to `resolve_source_dir` and expands to + // one `Source` call per file, in the order the resolver lists them, each + // followed by its own forwarded packages. Every call keeps the directory + // path as written, since no per-file path appears in the source text. + let files = vec![ + SourceResolution { + url: Url::parse("file:///R/a.R").unwrap(), + names: vec!["a_name".into()], + packages: vec!["pkgA".into()], + }, + SourceResolution { + url: Url::parse("file:///R/b.R").unwrap(), + names: vec!["b_name".into()], + packages: vec![], + }, + ]; + let code = "sourceDir(\"R\")\na_name\nb_name\n"; + let index = build_test_index(code, DirResolver { files }); + + assert_eq!(semantic_call_kinds(&index), [ + &SemanticCallKind::Source { + path: "R".into(), + resolved: Some(Url::parse("file:///R/a.R").unwrap()), + }, + &SemanticCallKind::Attach { + package: "pkgA".into(), + region: AttachRegion::Unconditional, + }, + &SemanticCallKind::Source { + path: "R".into(), + resolved: Some(Url::parse("file:///R/b.R").unwrap()), + }, + ]); + + // Both files' names are injected and resolve at their uses. + // Uses: sourceDir(0), a_name(1), b_name(2) + let file = ScopeId::from(0); + let map = index.use_def_map(file); + for use_index in [1, 2] { + let bindings = map.bindings_at_use(UseId::from(use_index)); + assert_eq!(bindings.definitions().len(), 1); + let def = &index.definitions(file)[bindings.definitions()[0]]; + assert!(matches!(def.kind(), DefinitionKind::Import { .. })); + } +} + +#[test] +fn test_source_dir_recognized_despite_a_local_definition() { + // `sourceDir` is the worked example in `?source`, so the caller has pasted + // the definition into their own file. Ordinary resolution reads that as a + // shadowing local binding and drops the effect, which is why the name is + // matched syntactically. + let files = vec![SourceResolution { + url: Url::parse("file:///R/a.R").unwrap(), + names: vec!["a_name".into()], + packages: vec![], + }]; + let code = "sourceDir <- function(path) NULL\nsourceDir(\"R\")\na_name\n"; + let index = build_test_index(code, DirResolver { files }); + + assert_eq!(semantic_call_kinds(&index), [&SemanticCallKind::Source { + path: "R".into(), + resolved: Some(Url::parse("file:///R/a.R").unwrap()), + }]); +} + +#[test] +fn test_source_dir_with_no_files_records_the_path_unresolved() { + // A directory the resolver expands to nothing still records the path the + // user wrote, so a consumer can report it. Same shape as a file path that + // didn't resolve. + let index = build_test_index("sourceDir(\"R\")\n", DirResolver { files: vec![] }); + + assert_eq!(semantic_call_kinds(&index), [&SemanticCallKind::Source { + path: "R".into(), + resolved: None, + }]); +} + #[test] fn test_source_resolver_honors_configured_path_position() { // A `SourceAnnotation` with `position: 1` takes the path from the second From 4a25e2a851da35bfb6795d5c2fe4da023e4827ea Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 31 Jul 2026 15:38:24 +0200 Subject: [PATCH 3/5] Add support for `targets::tar_source()` --- crates/oak_db/src/db.rs | 11 ++ crates/oak_db/src/file_imports.rs | 26 +++- crates/oak_db/src/imports.rs | 41 ++++-- crates/oak_db/src/tests/file_imports.rs | 114 +++++++++++++++ crates/oak_semantic/src/builder/scan.rs | 5 + crates/oak_semantic/src/effects.rs | 20 ++- crates/oak_semantic/src/effects/contrib.rs | 18 ++- .../src/effects/contrib/targets.rs | 13 ++ .../oak_semantic/tests/integration/contrib.rs | 1 + .../tests/integration/contrib/base.rs | 1 + .../tests/integration/contrib/targets.rs | 133 ++++++++++++++++++ 11 files changed, 359 insertions(+), 24 deletions(-) create mode 100644 crates/oak_semantic/src/effects/contrib/targets.rs create mode 100644 crates/oak_semantic/tests/integration/contrib/targets.rs diff --git a/crates/oak_db/src/db.rs b/crates/oak_db/src/db.rs index 081f40f44..233ffbd7d 100644 --- a/crates/oak_db/src/db.rs +++ b/crates/oak_db/src/db.rs @@ -209,6 +209,17 @@ pub fn workspace_files(db: &dyn Db) -> Vec { files } +/// The scripts held directly by workspace roots, in root order. +/// Like [`workspace_files`] but without package files. +#[salsa::tracked(returns(ref))] +pub(crate) fn workspace_scripts(db: &dyn Db) -> Vec { + db.workspace_roots() + .roots(db) + .iter() + .flat_map(|root| root.scripts(db).iter().copied()) + .collect() +} + fn collect_root_files(db: &dyn Db, files: &mut Vec, r: Root) { let owned = |f: File| root_by_file(db, f) == Some(r); files.extend(r.scripts(db).iter().copied().filter(|&f| owned(f))); diff --git a/crates/oak_db/src/file_imports.rs b/crates/oak_db/src/file_imports.rs index 7f5f88c82..1a21ec9af 100644 --- a/crates/oak_db/src/file_imports.rs +++ b/crates/oak_db/src/file_imports.rs @@ -10,6 +10,7 @@ use oak_semantic::semantic_index::SemanticCallKind; use oak_semantic::semantic_index::SemanticIndex; use rustc_hash::FxHashSet; +use crate::db::workspace_scripts; use crate::Db; use crate::File; use crate::Package; @@ -707,15 +708,10 @@ fn testthat_support_key(file: File, db: &dyn Db) -> Cow<'_, str> { /// The workspace files sitting directly in `dir`, in load order: sorted by /// basename, ASCII case-insensitively, the same order a package `R/` with no /// `Collate:` gets from `oak_scan::packages::order_alphabetically`. -/// -/// Gathers from workspace roots only (`root.scripts(db)` for each). -/// `OrphanRoot`, library roots, and `StaleRoot` don't contribute. pub(crate) fn files_in_directory(db: &dyn Db, dir: &Utf8Path) -> Vec { - let mut files: Vec = db - .workspace_roots() - .roots(db) + let mut files: Vec = workspace_scripts(db) .iter() - .flat_map(|root| root.scripts(db).iter().copied()) + .copied() .filter(|file| file.path(db).as_path().and_then(Utf8Path::parent) == Some(dir)) .collect(); @@ -723,6 +719,22 @@ pub(crate) fn files_in_directory(db: &dyn Db, dir: &Utf8Path) -> Vec { files } +/// The workspace files anywhere under `dir`, approximating the order returned by +/// `list.files(dir, recursive = TRUE)`. +pub(crate) fn files_in_directory_recursive(db: &dyn Db, dir: &Utf8Path) -> Vec { + let mut keyed: Vec<(String, File)> = workspace_scripts(db) + .iter() + .copied() + .filter_map(|file| { + let relative = file.path(db).as_path()?.strip_prefix(dir).ok()?; + Some((relative.as_str().to_ascii_lowercase(), file)) + }) + .collect(); + + keyed.sort_by(|(left, _), (right, _)| left.cmp(right)); + keyed.into_iter().map(|(_, file)| file).collect() +} + /// Case-insensitive basename sort key for [`files_in_directory`], matching /// `oak_scan::packages::order_alphabetically`'s `basename_key` so a /// non-package `R/` collates the same way as a package `R/` with no diff --git a/crates/oak_db/src/imports.rs b/crates/oak_db/src/imports.rs index c600c5f06..2b600ccea 100644 --- a/crates/oak_db/src/imports.rs +++ b/crates/oak_db/src/imports.rs @@ -11,7 +11,7 @@ use oak_semantic::SourceResolution; use rustc_hash::FxHashMap; use url::Url; -use crate::file_imports::files_in_directory; +use crate::file_imports::files_in_directory_recursive; use crate::file_imports::CollationView; use crate::file_imports::ImportLayer; use crate::Db; @@ -58,6 +58,8 @@ impl<'db> SalsaImportsResolver<'db> { } } + /// What sourcing `file` brings in. The two reads this makes are the ones + /// described on [`SalsaImportsResolver`]. fn source_resolution(&self, file: File) -> SourceResolution { let names: Vec = file .exports(self.db) @@ -77,14 +79,31 @@ impl<'db> SalsaImportsResolver<'db> { packages, } } +} - /// The workspace files a `SourceTarget::Dir` path names, in load order. - /// `None` when the path doesn't anchor. - fn files_in_source_dir(&self, path: &str) -> Option> { - let anchor = anchor_dir(self.db, self.file)?; - let target_path = resolve_relative_to(&anchor, path)?; - Some(files_in_directory(self.db, target_path.as_path()?)) - } +/// The workspace files a `SourceTarget::Dir` path names, in load order. Empty +/// when the path doesn't anchor. +/// +/// Tracked to give the directory listing a backdating point, the role +/// [`File::collation_siblings`] plays for the `R/` convention. The listing +/// filters every root's scripts, so without a memo here a file appearing +/// anywhere in the workspace would re-run the whole `semantic_index` of every +/// file holding a directory source, `_targets.R` being the one that hurts. +/// +/// Reads only inputs, so it's safe to call while `file`'s own index is being +/// built. +#[salsa::tracked(returns(ref))] +pub(crate) fn source_dir_scripts(db: &dyn Db, file: File, path: String) -> Vec { + let Some(anchor) = anchor_dir(db, file) else { + return Vec::new(); + }; + let Some(target_path) = resolve_relative_to(&anchor, &path) else { + return Vec::new(); + }; + let Some(dir) = target_path.as_path() else { + return Vec::new(); + }; + files_in_directory_recursive(db, dir) } /// Per-build memo for `resolve_effects`, keyed on `(name, attached)`. @@ -130,9 +149,9 @@ impl<'db> ImportsResolver for SalsaImportsResolver<'db> { } fn resolve_source_dir(&mut self, path: &str) -> Vec { - self.files_in_source_dir(path) - .unwrap_or_default() - .into_iter() + source_dir_scripts(self.db, self.file, path.to_string()) + .iter() + .copied() // Exclude sourcing file .filter(|file| *file != self.file) .map(|file| self.source_resolution(file)) diff --git a/crates/oak_db/src/tests/file_imports.rs b/crates/oak_db/src/tests/file_imports.rs index 815de22b7..4d9546d5e 100644 --- a/crates/oak_db/src/tests/file_imports.rs +++ b/crates/oak_db/src/tests/file_imports.rs @@ -1486,3 +1486,117 @@ fn test_body_edit_in_a_sourcing_file_does_not_invalidate_imports() { assert_eq!(db.executions("File::inherited_layers"), 3); assert_eq!(db.executions("File::imports"), 1); } + +// --- `SourceTarget::Dir` invalidation --- + +/// A workspace with `main.R` sourcing an `R/` directory, plus `a.R` and `b.R` +/// in it. Returns `(db, root, main, a, b)`. +fn source_dir_workspace() -> (TestDb, crate::Root, File, File, File) { + let mut db = TestDb::new(); + let root = workspace_root(&db, "ws"); + let main = File::new( + &db, + file_path("ws/main.R"), + FileRevision::zero(), + Some("sourceDir(\"R\")\n".to_string()), + None, + ); + let a = File::new( + &db, + file_path("ws/R/a.R"), + FileRevision::zero(), + Some("a_val <- function() 1\n".to_string()), + None, + ); + let b = File::new( + &db, + file_path("ws/R/b.R"), + FileRevision::zero(), + Some("b_val <- 2\n".to_string()), + None, + ); + root.set_scripts(&mut db).to(vec![main, a, b]); + db.workspace_roots().set_roots(&mut db).to(vec![root]); + (db, root, main, a, b) +} + +#[test] +fn test_source_dir_injects_every_file_in_the_directory() { + let (db, _root, main, _a, _b) = source_dir_workspace(); + let mut names: Vec<&str> = main.exports(&db).iter().map(|(name, _)| name).collect(); + names.sort(); + assert_eq!(names, vec!["a_val", "b_val"]); +} + +#[test] +fn test_source_dir_backdates_on_a_body_edit_in_the_directory() { + let (mut db, _root, main, a, _b) = source_dir_workspace(); + let _ = main.exports(&db); + let before = db.executions("File::semantic_index"); + + // A body edit leaves `a.R`'s top-level names alone, so `exports` backdates + // and nothing demands `main.R`'s index again. + a.set_source_text_override(&mut db) + .to(Some("a_val <- function() 999\n".to_string())); + let _ = main.exports(&db); + + assert_eq!(db.executions("File::semantic_index"), before + 1); +} + +#[test] +fn test_source_dir_backdates_on_a_file_added_outside_the_directory() { + let (mut db, root, main, a, b) = source_dir_workspace(); + let _ = main.exports(&db); + let before = db.executions("File::semantic_index"); + + // The listing reads every root's scripts, so `source_dir_scripts` re-runs. + // It returns the same files, so `main.R`'s index is never demanded again. + let elsewhere = File::new( + &db, + file_path("ws/other/z.R"), + FileRevision::zero(), + Some("z_val <- 1\n".to_string()), + None, + ); + root.set_scripts(&mut db).to(vec![main, a, b, elsewhere]); + let _ = main.exports(&db); + + assert_eq!(db.executions("File::semantic_index"), before); +} + +#[test] +fn test_source_dir_picks_up_a_file_added_inside_the_directory() { + let (mut db, root, main, a, b) = source_dir_workspace(); + let _ = main.exports(&db); + + let added = File::new( + &db, + file_path("ws/R/c.R"), + FileRevision::zero(), + Some("c_val <- 3\n".to_string()), + None, + ); + root.set_scripts(&mut db).to(vec![main, a, b, added]); + + let mut names: Vec<&str> = main.exports(&db).iter().map(|(name, _)| name).collect(); + names.sort(); + assert_eq!(names, vec!["a_val", "b_val", "c_val"]); +} + +#[test] +fn test_source_dir_recurses_into_subdirectories() { + // `tar_source()` recurses, and the listing is shared with `sourceDir()`. + let (mut db, root, main, a, b) = source_dir_workspace(); + let nested = File::new( + &db, + file_path("ws/R/models/fit.R"), + FileRevision::zero(), + Some("fit_val <- 4\n".to_string()), + None, + ); + root.set_scripts(&mut db).to(vec![main, a, b, nested]); + + let mut names: Vec<&str> = main.exports(&db).iter().map(|(name, _)| name).collect(); + names.sort(); + assert_eq!(names, vec!["a_val", "b_val", "fit_val"]); +} diff --git a/crates/oak_semantic/src/builder/scan.rs b/crates/oak_semantic/src/builder/scan.rs index 1eb0fcda0..7d7464e83 100644 --- a/crates/oak_semantic/src/builder/scan.rs +++ b/crates/oak_semantic/src/builder/scan.rs @@ -743,6 +743,11 @@ impl SemanticIndexBuilder { .into_iter() .collect(), SourceTarget::Dir => self.resolver.resolve_source_dir(&sourced.path), + // A file has precedence over a directory of the same name. + SourceTarget::FileOrDir => match self.resolver.resolve_source(&sourced.path) { + Some(resolution) => vec![resolution], + None => self.resolver.resolve_source_dir(&sourced.path), + }, }; for resolution in &resolutions { diff --git a/crates/oak_semantic/src/effects.rs b/crates/oak_semantic/src/effects.rs index 2cc846157..d1924068a 100644 --- a/crates/oak_semantic/src/effects.rs +++ b/crates/oak_semantic/src/effects.rs @@ -113,6 +113,7 @@ pub fn source_dir_idiom(name: &str) -> Option<&'static EffectsHandlers> { source: Some(&SourceAnnotation { position: 0, target: SourceTarget::Dir, + default_path: None, }), assign: None, }) @@ -389,10 +390,11 @@ pub struct SourcePath { pub enum SourceTarget { /// A single file, as base `source()` takes. File, - /// A directory, every R file in it. The resolver expands the path into one - /// target per file, since listing it needs workspace knowledge a handler - /// doesn't have. + /// All R files in a directory (recursively). Dir, + /// Either. Whereas `source()` only supports files, `targets::tar_source()` + /// supports both. + FileOrDir, } /// Declares how a source function (`source()`) names what it reads, and serves @@ -405,6 +407,9 @@ pub struct SourceAnnotation { pub position: usize, /// Whether that argument names a file or a directory. pub target: SourceTarget, + /// Default path if no argument is suppolied (`tar_source()` defaults to + /// `files = "R"`). + pub default_path: Option<&'static str>, } impl EffectHandler for SourceAnnotation { @@ -413,6 +418,15 @@ impl EffectHandler for SourceAnnotation { fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option> { let args = call.arguments().ok()?; + if args.items().iter().next().is_none() { + return self.default_path.map(|path| { + vec![SourcePath { + path: path.to_string(), + target: self.target, + }] + }); + } + // The path is matched positionally among unnamed arguments rather than // through [`CallContext::match_arguments`], for two reasons. We need to // inspect the `local =` value to bail on non-static calls, which diff --git a/crates/oak_semantic/src/effects/contrib.rs b/crates/oak_semantic/src/effects/contrib.rs index 74075a761..988a8f1c4 100644 --- a/crates/oak_semantic/src/effects/contrib.rs +++ b/crates/oak_semantic/src/effects/contrib.rs @@ -5,6 +5,7 @@ mod magrittr; mod rlang; mod s7; mod shiny; +mod targets; mod testthat; mod withr; @@ -71,10 +72,16 @@ macro_rules! quoted { } pub(crate) use quoted; -/// A source entry: `(path-argument position)`. The function reads and evaluates -/// another file, injecting its top-level names into the caller. +/// A source entry: `(path-argument position)`, optionally what that argument +/// names (a [`SourceTarget`] variant, `File` by default) and what the function +/// reads when called with no arguments. +/// +/// [`SourceTarget`]: crate::effects::SourceTarget macro_rules! source { ($func:literal, $pos:literal) => { + $crate::effects::contrib::source!($func, $pos, File, None) + }; + ($func:literal, $pos:literal, $target:ident, $default:expr) => { $crate::effects::contrib::Entry { function: $func, effects: $crate::effects::EffectsHandlers { @@ -82,7 +89,8 @@ macro_rules! source { attach: None, source: Some(&$crate::effects::SourceAnnotation { position: $pos, - target: $crate::effects::SourceTarget::File, + target: $crate::effects::SourceTarget::$target, + default_path: $default, }), assign: None, }, @@ -148,6 +156,10 @@ pub(super) static REGISTRY: &[PackageEntries] = &[ name: "shiny", functions: shiny::ENTRIES, }, + PackageEntries { + name: "targets", + functions: targets::ENTRIES, + }, PackageEntries { name: "testthat", functions: testthat::ENTRIES, diff --git a/crates/oak_semantic/src/effects/contrib/targets.rs b/crates/oak_semantic/src/effects/contrib/targets.rs new file mode 100644 index 000000000..bf0654e41 --- /dev/null +++ b/crates/oak_semantic/src/effects/contrib/targets.rs @@ -0,0 +1,13 @@ +use crate::effects::contrib::source; +use crate::effects::contrib::Entry; + +pub(super) static ENTRIES: &[Entry] = &[ + // `tar_source(files = "R")` runs every R script under `files`, which is how + // a `_targets.R` pipeline sees its helper functions. Each element of + // `files` may be a script or a directory, and the bare `tar_source()` that + // most pipelines write relies on the default. + // + // `files` is a character vector, so `tar_source(c("R", "utils"))` names + // several paths. Only a single literal is read today. + source!("tar_source", 0, FileOrDir, Some("R")), +]; diff --git a/crates/oak_semantic/tests/integration/contrib.rs b/crates/oak_semantic/tests/integration/contrib.rs index d5d2a3743..b31ff517e 100644 --- a/crates/oak_semantic/tests/integration/contrib.rs +++ b/crates/oak_semantic/tests/integration/contrib.rs @@ -3,4 +3,5 @@ mod magrittr; mod rlang; mod s7; mod shiny; +mod targets; mod testthat; diff --git a/crates/oak_semantic/tests/integration/contrib/base.rs b/crates/oak_semantic/tests/integration/contrib/base.rs index d14a31752..6915418fb 100644 --- a/crates/oak_semantic/tests/integration/contrib/base.rs +++ b/crates/oak_semantic/tests/integration/contrib/base.rs @@ -135,6 +135,7 @@ struct PositionResolver; static SOURCE_AT_POSITION_1: SourceAnnotation = SourceAnnotation { position: 1, target: SourceTarget::File, + default_path: None, }; impl ImportsResolver for PositionResolver { diff --git a/crates/oak_semantic/tests/integration/contrib/targets.rs b/crates/oak_semantic/tests/integration/contrib/targets.rs new file mode 100644 index 000000000..d0f0ec416 --- /dev/null +++ b/crates/oak_semantic/tests/integration/contrib/targets.rs @@ -0,0 +1,133 @@ +use aether_parser::parse; +use aether_parser::RParserOptions; +use oak_semantic::build_index; +use oak_semantic::effects; +use oak_semantic::semantic_index::SemanticCallKind; +use oak_semantic::semantic_index::SemanticIndex; +use oak_semantic::EffectsHandlers; +use oak_semantic::ImportsResolver; +use oak_semantic::SourceResolution; +use url::Url; + +use crate::common::semantic_call_kinds; + +/// Resolves `tar_source` against the targets registry entry, and stands in for +/// the workspace listing: `files` are what a directory expands to, `file` is +/// what a path resolving to a script gives. +struct TargetsResolver { + file: Option, + files: Vec, +} + +impl TargetsResolver { + fn with_dir(files: Vec) -> Self { + Self { file: None, files } + } +} + +impl ImportsResolver for TargetsResolver { + fn resolve_source(&mut self, _path: &str) -> Option { + self.file.clone() + } + + fn resolve_source_dir(&mut self, _path: &str) -> Vec { + self.files.clone() + } + + fn resolve_effects(&mut self, name: &str, _: &[String]) -> Option { + effects::lookup("targets", name) + .or_else(|| effects::lookup("base", name)) + .copied() + } +} + +fn index(source: &str, resolver: TargetsResolver) -> SemanticIndex { + let parsed = parse(source, RParserOptions::default()); + if parsed.has_error() { + panic!("source has syntax errors: {source}"); + } + build_index(&parsed.tree(), resolver) +} + +fn resolution(url: &str, name: &str) -> SourceResolution { + SourceResolution { + url: Url::parse(url).unwrap(), + names: vec![name.to_string()], + packages: vec![], + } +} + +fn sourced(path: &str, url: &str) -> SemanticCallKind { + SemanticCallKind::Source { + path: path.to_string(), + resolved: Some(Url::parse(url).unwrap()), + } +} + +#[test] +fn test_tar_source_no_arguments_uses_the_default_directory() { + // The bare `tar_source()` that most `_targets.R` pipelines write relies on + // `files = "R"`, so the default has to stand in for an absent argument. + let files = vec![ + resolution("file:///R/a.R", "a_name"), + resolution("file:///R/b.R", "b_name"), + ]; + let index = index("tar_source()\n", TargetsResolver::with_dir(files)); + + assert_eq!(semantic_call_kinds(&index), [ + &sourced("R", "file:///R/a.R"), + &sourced("R", "file:///R/b.R"), + ]); +} + +#[test] +fn test_tar_source_positional_directory() { + let files = vec![resolution("file:///code/a.R", "a_name")]; + let index = index("tar_source(\"code\")\n", TargetsResolver::with_dir(files)); + + assert_eq!(semantic_call_kinds(&index), [&sourced( + "code", + "file:///code/a.R" + )]); +} + +#[test] +fn test_tar_source_qualified_call_is_recognized() { + let files = vec![resolution("file:///R/a.R", "a_name")]; + let index = index("targets::tar_source()\n", TargetsResolver::with_dir(files)); + + assert_eq!(semantic_call_kinds(&index), [&sourced( + "R", + "file:///R/a.R" + )]); +} + +#[test] +fn test_tar_source_path_naming_a_script_resolves_as_a_file() { + // `files` takes scripts as well as directories, so a `FileOrDir` target + // tries the file first and only falls back to a listing. + let resolver = TargetsResolver { + file: Some(resolution("file:///R/utils.R", "util")), + files: vec![resolution("file:///unused.R", "unused")], + }; + let index = index("tar_source(\"R/utils.R\")\n", resolver); + + assert_eq!(semantic_call_kinds(&index), [&sourced( + "R/utils.R", + "file:///R/utils.R" + )]); +} + +#[test] +fn test_tar_source_named_files_argument_is_not_recognized() { + // `files = "code"` isn't read yet (the path is matched positionally), and + // the default must not step in for it. Inventing `"R"` here would source a + // directory the call explicitly overrode. + let files = vec![resolution("file:///R/a.R", "a_name")]; + let index = index( + "tar_source(files = \"code\")\n", + TargetsResolver::with_dir(files), + ); + + assert_eq!(semantic_call_kinds(&index), Vec::<&SemanticCallKind>::new()); +} From 186f3a747e21d89c263ff41407342db5440ce5a6 Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 31 Jul 2026 16:06:01 +0200 Subject: [PATCH 4/5] Don't warn in LSP log if package is not installed --- crates/ark/src/lsp/diagnostics.rs | 19 ++++++------------- 1 file changed, 6 insertions(+), 13 deletions(-) diff --git a/crates/ark/src/lsp/diagnostics.rs b/crates/ark/src/lsp/diagnostics.rs index 99ff4a010..a38f281de 100644 --- a/crates/ark/src/lsp/diagnostics.rs +++ b/crates/ark/src/lsp/diagnostics.rs @@ -1000,7 +1000,7 @@ fn handle_package_attach_call(node: Node, context: &mut DiagnosticContext) -> an let package_name = package_node.get_identifier_or_string_text(context.contents())?; let attach_pos = node.end_position(); - insert_package_exports(package_name, attach_pos, context)?; + insert_package_exports(package_name, attach_pos, context); // Also attach packages from `Depends` field, if any if let Some(package_names) = context @@ -1009,7 +1009,7 @@ fn handle_package_attach_call(node: Node, context: &mut DiagnosticContext) -> an .and_then(|package| package.depends(context.db).as_ref()) { for package_name in package_names.iter() { - insert_package_exports(package_name, attach_pos, context)?; + insert_package_exports(package_name, attach_pos, context); } } @@ -1055,21 +1055,16 @@ fn handle_package_attach_call(node: Node, context: &mut DiagnosticContext) -> an _ => vec![], }; for package_name in attach_field { - insert_package_exports(package_name, attach_pos, context)?; + insert_package_exports(package_name, attach_pos, context); } Ok(()) } -fn insert_package_exports( - package_name: &str, - attach_pos: Point, - context: &mut DiagnosticContext, -) -> anyhow::Result<()> { +fn insert_package_exports(package_name: &str, attach_pos: Point, context: &mut DiagnosticContext) { let Some(package) = context.db.package_by_name(package_name) else { - return Err(anyhow::anyhow!( - "Can't get exports from package {package_name} because it is not installed." - )); + // Package is not installed. This is linted via another path. + return; }; // Start from explicit `NAMESPACE` exports @@ -1088,8 +1083,6 @@ fn insert_package_exports( .entry(attach_pos) .or_default() .extend(exports); - - Ok(()) } fn recurse_subset_or_subset2( From 7c8892307ab870d0c728d04f4ce55e41cff2b56b Mon Sep 17 00:00:00 2001 From: Lionel Henry Date: Fri, 31 Jul 2026 16:51:34 +0200 Subject: [PATCH 5/5] Fix symbol resolution of predecessors during eager collation --- crates/oak_db/src/file_imports.rs | 29 ++++++++++++++- crates/oak_db/src/tests/file_resolve_at.rs | 43 ++++++++++++++++++++++ 2 files changed, 71 insertions(+), 1 deletion(-) diff --git a/crates/oak_db/src/file_imports.rs b/crates/oak_db/src/file_imports.rs index 1a21ec9af..ce868cc47 100644 --- a/crates/oak_db/src/file_imports.rs +++ b/crates/oak_db/src/file_imports.rs @@ -447,7 +447,18 @@ fn build_inherited_layers( None => ImportLayer::File(source_site), }; - let mut above = vec![source_layer]; + // One Source effect can load several files (`sourceDir()`, `tar_source()`). + // Keep track of already sourced files, since their eager top-level context + // can see what's already been sourced. + let mut above: Vec = match offset { + Some(offset) => loaded_before(db, source_site, file, offset) + .into_iter() + .map(ImportLayer::File) + .collect(), + None => Vec::new(), + }; + + above.push(source_layer); above.extend(own_cross.above.iter().cloned()); above.extend( grandparents @@ -482,6 +493,22 @@ fn source_offset(db: &dyn Db, source_file: File, file: File) -> Option .max() } +/// The files the call at `offset` loaded before `file`, latest first. +fn loaded_before(db: &dyn Db, source_file: File, file: File, offset: TextSize) -> Vec { + let sites = source_file.source_sites(db); + let Some(own) = sites.iter().rposition(|site| site.target() == Some(file)) else { + return Vec::new(); + }; + + sites[..own] + .iter() + .rev() + .take_while(|site| site.offset() == offset) + .filter_map(|site| site.target()) + .filter(|target| *target != file) + .collect() +} + fn package_load_layers( file: File, db: &dyn Db, diff --git a/crates/oak_db/src/tests/file_resolve_at.rs b/crates/oak_db/src/tests/file_resolve_at.rs index 762e851ec..21e6dc39c 100644 --- a/crates/oak_db/src/tests/file_resolve_at.rs +++ b/crates/oak_db/src/tests/file_resolve_at.rs @@ -827,3 +827,46 @@ fn test_narrowing_applies_at_each_hop_of_a_source_chain() { assert!(helpers.resolve_at(&db, TextSize::from(0)).is_empty()); } } + +// --- ordered expansion of a directory source --- + +/// `main.R` sources the whole `R/` directory in one call, which loads `a.R` +/// then `b.R`. Returns `(db, a, b)`. +fn dir_source_workspace(a_contents: &str, b_contents: &str) -> (TestDb, File, File) { + let mut db = TestDb::new(); + let root = workspace_root(&db, "ws"); + let main = make_file(&mut db, "ws/main.R", "sourceDir(\"R\")\n"); + let a = make_file(&mut db, "ws/R/a.R", a_contents); + let b = make_file(&mut db, "ws/R/b.R", b_contents); + root.set_scripts(&mut db).to(vec![main, a, b]); + db.workspace_roots().set_roots(&mut db).to(vec![root]); + (db, a, b) +} + +#[test] +fn test_dir_source_top_level_sees_a_file_loaded_earlier_in_the_call() { + // `a.R` has fully run by the time `b.R` starts, so a top-level use in + // `b.R` resolves. The sourcing file's snapshot is taken before the call + // and holds neither name, so this can only come from the call's own + // ordered target list. + let (db, a, b) = dir_source_workspace("a_val <- 1\n", "use <- a_val\n"); + let def = resolve_one(&db, b, TextSize::from(7)); + assert_eq!(def.file(&db), a); +} + +#[test] +fn test_dir_source_top_level_does_not_see_a_file_loaded_later_in_the_call() { + // The mirror image: `b.R` hasn't run when `a.R`'s top level does, so this + // stays unresolved. Without it the test above would pass just as well on + // an implementation that made every target visible to every other. + let (db, a, _b) = dir_source_workspace("use <- b_val\n", "b_val <- 2\n"); + assert!(a.resolve_at(&db, TextSize::from(7)).is_empty()); +} + +#[test] +fn test_dir_source_function_body_sees_a_file_loaded_later_in_the_call() { + // A body runs after the whole call, so the lazy view has everything. + let (db, a, b) = dir_source_workspace("f <- function() b_val\n", "b_val <- 2\n"); + let def = resolve_one(&db, a, TextSize::from(17)); + assert_eq!(def.file(&db), b); +}