diff --git a/crates/ark/src/lsp/diagnostics.rs b/crates/ark/src/lsp/diagnostics.rs index a66653477..6a81adb1f 100644 --- a/crates/ark/src/lsp/diagnostics.rs +++ b/crates/ark/src/lsp/diagnostics.rs @@ -995,7 +995,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 @@ -1004,7 +1004,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); } } @@ -1050,21 +1050,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 @@ -1083,8 +1078,6 @@ fn insert_package_exports( .entry(attach_pos) .or_default() .extend(exports); - - Ok(()) } fn recurse_subset_or_subset2( diff --git a/crates/oak_db/src/db.rs b/crates/oak_db/src/db.rs index 96a7d07b9..7f691d4b3 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 34eb4ec6b..2882d18ee 100644 --- a/crates/oak_db/src/file_imports.rs +++ b/crates/oak_db/src/file_imports.rs @@ -11,6 +11,7 @@ use oak_semantic::semantic_index::SemanticCall; use oak_semantic::semantic_index::SemanticCallKind; use oak_semantic::semantic_index::SemanticIndex; +use crate::db::workspace_scripts; use crate::Db; use crate::File; use crate::Package; @@ -449,35 +450,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) } } @@ -528,7 +512,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 offsets.as_deref() { + Some(offsets) => loaded_before(db, source_site, file, offsets) + .into_iter() + .map(ImportLayer::File) + .collect(), + None => Vec::new(), + }; + + above.push(source_layer); above.extend(own_cross.above.iter().cloned()); above.extend( grandparents @@ -576,6 +571,36 @@ fn source_offsets(db: &dyn Db, sourcing_file: File, file: File) -> Option Vec { + let sites = source_file.source_sites(db); + let mut loaded: Vec = Vec::new(); + + // Descending, so the latest call's targets rank first. + for &offset in offsets.iter().rev() { + let targets: Vec = sites + .iter() + .filter(|site| site.offset() == offset) + .filter_map(|site| site.target()) + .collect(); + let Some(own) = targets.iter().position(|target| *target == file) else { + continue; + }; + + for &target in targets[..own].iter().rev() { + if !loaded.contains(&target) { + loaded.push(target); + } + } + } + + loaded +} + fn package_load_layers( file: File, db: &dyn Db, @@ -811,10 +836,44 @@ 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 -/// `oak_scan::packages::order_alphabetically`'s `basename_key` so a -/// non-package `R/` collates the same way as a package `R/` with no -/// `Collate:`. +/// Returns workspace scripts directly under `dir`, in `list.files()` load order. +/// +/// `sourceDir()`, Shiny's `loadSupport()`, and non-package `R/` collation use +/// this order. [`collation_basename_key()`] mirrors their session-locale sort. +pub(crate) fn files_in_directory(db: &dyn Db, dir: &Utf8Path) -> Vec { + let mut files: Vec = workspace_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 +} + +/// Returns workspace scripts below `dir` in `targets::tar_source()` load order. +/// +/// `list.files(recursive = TRUE)` sorts nested scripts by relative path. Case +/// folding matches [`collation_basename_key()`]. +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() +} + +/// ASCII-case-folded basename key approximating `list.files()` session-locale +/// ordering. +/// +/// Package installation instead forces `LC_COLLATE=C`, where raw byte order +/// determines collation. fn collation_basename_key(file: File, db: &dyn Db) -> Option { file.path(db) .file_name() diff --git a/crates/oak_db/src/imports.rs b/crates/oak_db/src/imports.rs index 30b3d0959..28fc71602 100644 --- a/crates/oak_db/src/imports.rs +++ b/crates/oak_db/src/imports.rs @@ -5,12 +5,15 @@ use camino::Utf8Component; use camino::Utf8Path; use camino::Utf8PathBuf; use oak_semantic::effects; +use oak_semantic::effects::DirWalk; use oak_semantic::EffectsHandlers; use oak_semantic::ImportsResolver; 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; @@ -56,6 +59,62 @@ impl<'db> SalsaImportsResolver<'db> { cache: EffectsCache::default(), } } + + /// 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) + .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, + } + } +} + +/// Returns scripts from the workspace directory named by `path`, in load order. +/// `walk` determines whether nested directories are included. Returns no files +/// when `path` cannot resolve to a directory. +/// +/// 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, + walk: DirWalk, +) -> 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(); + }; + match walk { + DirWalk::Shallow => files_in_directory(db, dir), + DirWalk::Recursive => files_in_directory_recursive(db, dir), + } } /// Per-build memo for `resolve_effects`, keyed on `(name, attached)`. @@ -97,24 +156,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) + fn resolve_source_dir(&mut self, path: &str, walk: DirWalk) -> Vec { + source_dir_scripts(self.db, self.file, path.to_string(), walk) .iter() - .map(|name| name.text(self.db).to_string()) - .collect(); - - Some(SourceResolution { - url: target_path.to_url(), - names, - packages, - }) + .copied() + // 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_db/src/package.rs b/crates/oak_db/src/package.rs index 231685a9e..27a5db0c2 100644 --- a/crates/oak_db/src/package.rs +++ b/crates/oak_db/src/package.rs @@ -56,13 +56,13 @@ pub struct Package { /// editing a package's `NAMESPACE` live. #[returns(ref)] pub namespace_override: Option, - /// R source files belonging to this package (the `R/*.R` files), in - /// R's load order. When DESCRIPTION's `Collate:` directive is - /// present, this is exactly the files it lists, in that order; - /// files in `R/` not listed are excluded (matching R's loader, - /// Writing R Extensions §1.1.1). When `Collate:` is absent, files - /// are in case-insensitive alphabetical order. TODO(diagnostics): - /// Lint files missing from collation. + /// R source files belonging to this package (the `R/*.R` files), in R's + /// load order. When DESCRIPTION's `Collate:` directive is present, this is + /// exactly the files it lists, in that order; files in `R/` not listed are + /// excluded (matching R's loader, Writing R Extensions §1.1.1). When + /// `Collate:` is absent, files are in raw basename byte order, matching R's + /// `LC_COLLATE=C` installation collation. + /// TODO(diagnostics): Lint files missing from collation. /// /// Per-package granularity: adding or removing a file in one /// package doesn't invalidate tracked queries reading another diff --git a/crates/oak_db/src/tests/file_diagnostics.rs b/crates/oak_db/src/tests/file_diagnostics.rs index cfac4b597..c609b9409 100644 --- a/crates/oak_db/src/tests/file_diagnostics.rs +++ b/crates/oak_db/src/tests/file_diagnostics.rs @@ -208,17 +208,10 @@ f <- function() reactive({ x <- 1 }) } #[test] -fn test_diagnostic_gap_named_arg_before_block() { - // R matches named arguments first, so `desc = "d"` binds to the `desc` - // formal, and the unnamed block then fills the remaining `code` formal, - // which is formal position 1, even though the block sits at call - // position 0. `match_positional()` in `crates/oak_semantic/src/effects.rs` - // only matches a positional argument to a formal declared at that exact - // call position, so it never finds `code` here and no scope gets pushed - // for `x <- 1`. Confirmed by direct comparison: this source yields one - // scope, versus two for the same call with `code` in its normal - // position, so `x` resolves at file scope instead of inside - // `test_that()`. +fn test_diagnostic_named_arg_before_block_scopes_correctly() { + // After `desc` binds by name, the block fills `code` despite appearing first. + // Its `test_that()` resolution is conditionally shadowed and must still gain + // the `code` scope. let mut db = TestDb::new(); install_packages(&mut db, &["testthat"]); let source = "\ diff --git a/crates/oak_db/src/tests/file_imports.rs b/crates/oak_db/src/tests/file_imports.rs index 815de22b7..05eee5db7 100644 --- a/crates/oak_db/src/tests/file_imports.rs +++ b/crates/oak_db/src/tests/file_imports.rs @@ -1486,3 +1486,160 @@ 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) +} + +/// Creates a bare `tar_source()` pipeline with `R/a.R` and `R/models/fit.R`. +fn tar_source_workspace() -> (TestDb, crate::Root, File) { + let mut db = TestDb::new(); + install_packages(&mut db, &["targets"]); + let root = workspace_root(&db, "ws"); + let pipeline = File::new( + &db, + file_path("ws/_targets.R"), + FileRevision::zero(), + Some("library(targets)\ntar_source()\n".to_string()), + None, + ); + let a = File::new( + &db, + file_path("ws/R/a.R"), + FileRevision::zero(), + Some("a_val <- 1\n".to_string()), + None, + ); + 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![pipeline, a, nested]); + db.workspace_roots().set_roots(&mut db).to(vec![root]); + (db, root, pipeline) +} + +#[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_does_not_recurse_into_subdirectories() { + // `sourceDir()` leaves `list.files()` at its `recursive = FALSE` default, + // so `R/models/fit.R` is excluded. + 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"]); +} + +#[test] +fn test_tar_source_recurses_into_subdirectories() { + // `tar_source()` includes nested scripts because `file_list_files()` calls + // `list.files(recursive = TRUE)` for directories. + let (db, _root, pipeline) = tar_source_workspace(); + + let mut names: Vec<&str> = pipeline.exports(&db).iter().map(|(name, _)| name).collect(); + names.sort(); + assert_eq!(names, vec!["a_val", "fit_val"]); +} diff --git a/crates/oak_db/src/tests/file_imports_at.rs b/crates/oak_db/src/tests/file_imports_at.rs index 07cc897b9..39b291166 100644 --- a/crates/oak_db/src/tests/file_imports_at.rs +++ b/crates/oak_db/src/tests/file_imports_at.rs @@ -862,9 +862,9 @@ fn test_script_r_directory_top_level_sees_only_alphabetic_predecessor() { #[test] fn test_script_r_directory_collation_is_case_insensitive() { - // Matches `oak_scan::packages::order_alphabetically`: basenames sort - // case-insensitively, so `a.R` collates before `Z.R` even though it's - // lexically greater in byte order. + // Non-package `R/` files use `list.files()` collation, so `a.R` precedes + // `Z.R` in a UTF-8 session locale. Package installation's `LC_COLLATE=C` + // reverses them. let mut db = TestDb::new(); let root = workspace_root(&db, "ws"); let z_source = "z_val <- 1\n"; diff --git a/crates/oak_db/src/tests/file_resolve_at.rs b/crates/oak_db/src/tests/file_resolve_at.rs index 1dda06e9c..215872397 100644 --- a/crates/oak_db/src/tests/file_resolve_at.rs +++ b/crates/oak_db/src/tests/file_resolve_at.rs @@ -1003,3 +1003,63 @@ fn test_source_call_in_an_else_arm_does_not_see_the_if_arm() { assert_eq!(resolve_one(&db, a, TextSize::from(0)).file(&db), main); assert!(b.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); +} + +#[test] +fn test_dir_sourced_twice_still_sees_the_files_loaded_earlier_in_each_call() { + // Each `sourceDir()` call is its own group of same-offset targets. `b.R` + // must see `a.R` through the group it belongs to, so the earlier call can't + // be skipped just because a later one also loads both files. + let mut db = TestDb::new(); + let root = workspace_root(&db, "ws"); + let main = make_file(&mut db, "ws/main.R", "sourceDir(\"R\")\nsourceDir(\"R\")\n"); + let a = make_file(&mut db, "ws/R/a.R", "a_val <- 1\n"); + let b = make_file(&mut db, "ws/R/b.R", "use <- a_val\n"); + root.set_scripts(&mut db).to(vec![main, a, b]); + db.workspace_roots().set_roots(&mut db).to(vec![root]); + + let def = resolve_one(&db, b, TextSize::from(7)); + assert_eq!(def.file(&db), a); +} diff --git a/crates/oak_db/src/tests/snapshots/oak_db__tests__file_diagnostics__diagnostic_gap_named_arg_before_block.snap b/crates/oak_db/src/tests/snapshots/oak_db__tests__file_diagnostics__diagnostic_gap_named_arg_before_block.snap deleted file mode 100644 index 879ba7dc2..000000000 --- a/crates/oak_db/src/tests/snapshots/oak_db__tests__file_diagnostics__diagnostic_gap_named_arg_before_block.snap +++ /dev/null @@ -1,6 +0,0 @@ ---- -source: crates/oak_db/src/tests/file_diagnostics.rs -expression: "render(\"a.R\", source, file.diagnostics(&db))" ---- -a.R -(no diagnostics) diff --git a/crates/oak_db/src/tests/snapshots/oak_db__tests__file_diagnostics__diagnostic_named_arg_before_block_scopes_correctly.snap b/crates/oak_db/src/tests/snapshots/oak_db__tests__file_diagnostics__diagnostic_named_arg_before_block_scopes_correctly.snap new file mode 100644 index 000000000..766121e93 --- /dev/null +++ b/crates/oak_db/src/tests/snapshots/oak_db__tests__file_diagnostics__diagnostic_named_arg_before_block_scopes_correctly.snap @@ -0,0 +1,13 @@ +--- +source: crates/oak_db/src/tests/file_diagnostics.rs +expression: "render(\"a.R\", source, file.diagnostics(&db))" +--- +info[ambiguous-effect]: Ambiguous reading of effectful `test_that()`. + A conditional assignment could shadow `test_that` on some paths and change its effect. + --> a.R:5:1 + | +3 | test_that <- identity + | --------- conditional assignment to `test_that` +4 | } +5 | test_that({ x <- 1 }, desc = "d") + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ diff --git a/crates/oak_scan/src/packages.rs b/crates/oak_scan/src/packages.rs index 143efebf7..a79011b7d 100644 --- a/crates/oak_scan/src/packages.rs +++ b/crates/oak_scan/src/packages.rs @@ -192,11 +192,11 @@ fn read_workspace_package(package_dir: &Path) -> Option { } } - // `file_imports()` in `oak_db` reads `package.files` order as the collation - // chain, so a file only sees the files ordered before it. `Collate:` is R's - // explicit load order; without it R loads `R/` in case-insensitive - // alphabetical order. R/ files left out of a `Collate:` directive aren't - // loaded into the namespace, so they move to `scripts` rather than `files`. + // `package.files` supplies cross-file lookup order, so a file sees only its + // predecessors. `Collate:` supplies that order explicitly. Without it, + // `tools:::.install_package_code_files()` forces `LC_COLLATE=C` and lists + // `R/` in byte order. Files omitted from `Collate:` are not loaded into the + // namespace and become `scripts`. let (loadable, leftover) = match package.collation.as_deref() { Some(order) => order_by_collation(files, order), None => order_alphabetically(files), @@ -308,8 +308,7 @@ fn order_by_collation( ) } -/// All files are loadable, in case-insensitive alphabetical order by basename. -/// No leftover: without `Collate:`, R loads every R/ file. +/// Without `Collate:`, every `R/` file is loadable in raw basename byte order. fn order_alphabetically(mut files: Vec<(PathBuf, FileEntry)>) -> (Vec, Vec) { files.sort_by_cached_key(|(path, _)| basename_key(path)); ( @@ -318,9 +317,11 @@ fn order_alphabetically(mut files: Vec<(PathBuf, FileEntry)>) -> (Vec ) } -/// Case-insensitive sort key from a path's basename. +/// Raw basename byte-order key for R's `LC_COLLATE=C` package installation. +/// `AllGenerics.R` precedes `aaa-utils.R`, letting S4 packages load generics +/// before methods. fn basename_key(path: &Path) -> Option { - path.file_name().map(|name| name.to_ascii_lowercase()) + path.file_name().map(|name| name.to_os_string()) } /// Walk a workspace root for its top-level scripts: every `.R` file that isn't diff --git a/crates/oak_scan/src/tests/packages.rs b/crates/oak_scan/src/tests/packages.rs index 299c0326a..5467311e2 100644 --- a/crates/oak_scan/src/tests/packages.rs +++ b/crates/oak_scan/src/tests/packages.rs @@ -51,17 +51,26 @@ fn test_empty_r_directory_yields_no_files() { assert!(scripts.is_empty()); } -/// Without `Collate:`, every R file is loadable, ordered case-insensitively by -/// basename so the result doesn't depend on `read_dir` order. +/// Without `Collate:`, R loads every file in raw basename byte order. +/// Under `LC_COLLATE=C`, `AllGenerics.R` precedes `aaa-utils.R`, so S4 packages +/// define generics before methods. #[test] fn test_without_collation_all_files_are_loadable_and_sorted() { let tmp = tempfile::tempdir().unwrap(); let r = tmp.path().join("R"); - write_r_dir(&r, &[("zebra.R", "1"), ("Apple.R", "1"), ("mango.R", "1")]); + write_r_dir(&r, &[ + ("zebra.R", "1"), + ("aaa-utils.R", "1"), + ("AllGenerics.R", "1"), + ]); let (files, scripts) = read_package_sources(&r, None); - assert_eq!(names(&files), vec!["Apple.R", "mango.R", "zebra.R"]); + assert_eq!(names(&files), vec![ + "AllGenerics.R", + "aaa-utils.R", + "zebra.R" + ]); assert!(scripts.is_empty()); } diff --git a/crates/oak_scan/src/tests/workspace.rs b/crates/oak_scan/src/tests/workspace.rs index 2bb1beea1..c2adf6003 100644 --- a/crates/oak_scan/src/tests/workspace.rs +++ b/crates/oak_scan/src/tests/workspace.rs @@ -633,20 +633,20 @@ fn test_set_workspace_paths_stale_no_duplicates_across_cycles() { #[test] fn test_scan_orders_files_alphabetically_without_collate() { - // Default behavior: case-insensitive alphabetical filename order. - // Matches R's load order for packages without an explicit `Collate:`. + // Package installation uses raw basename byte order under `LC_COLLATE=C` + // when `Collate:` is absent. let tmp = tempfile::tempdir().unwrap(); write_package(&tmp.path().join("pkg"), "pkg", &[ ("zzz.R", "z <- 1\n"), ("aaa.R", "a <- 1\n"), - ("mmm.R", "m <- 1\n"), + ("Mmm.R", "m <- 1\n"), ]); let mut db = OakDatabase::new(); set_workspace_paths(&mut db, &[tmp.path().to_path_buf()], &HashSet::new()); let pkg = db.workspace_roots().roots(&db)[0].packages(&db)[0]; - assert_eq!(file_basenames(&db, pkg), vec!["aaa.R", "mmm.R", "zzz.R"]); + assert_eq!(file_basenames(&db, pkg), vec!["Mmm.R", "aaa.R", "zzz.R"]); } #[test] diff --git a/crates/oak_semantic/src/builder/effects.rs b/crates/oak_semantic/src/builder/effects.rs index b6235f56c..0785777c7 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 ce8b59b8d..35a9b12ff 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; @@ -446,14 +448,25 @@ 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); - self.scan - .call_resolutions - .entry(range) - .or_default() - .source - .push(SourcedFile { path, resolution }); + for sourced in paths { + 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: None, + }); + continue; + } + + entries.extend(resolutions.into_iter().map(|resolution| SourcedFile { + path: sourced.path.clone(), + resolution: Some(resolution), + })); } } @@ -753,22 +766,39 @@ 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(walk) => self.resolver.resolve_source_dir(&sourced.path, walk), + // A file has precedence over a directory of the same name. + SourceTarget::FileOrDir(walk) => match self.resolver.resolve_source(&sourced.path) { + Some(resolution) => vec![resolution], + None => self.resolver.resolve_source_dir(&sourced.path, walk), + }, + }; + + 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 { @@ -788,8 +818,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 472d8a191..cb31471bd 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. @@ -80,7 +80,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>, } @@ -100,6 +100,31 @@ 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. +/// +/// The copied `sourceDir()` idiom leaves `list.files()` at its +/// `recursive = FALSE` default, so nested scripts are excluded. +pub fn source_dir_idiom(name: &str) -> Option<&'static EffectsHandlers> { + if name != "sourceDir" { + return None; + } + Some(&EffectsHandlers { + arguments: None, + attach: None, + source: Some(&SourceAnnotation { + formals: &["path"], + path: "path", + target: SourceTarget::Dir(DirWalk::Shallow), + default_path: None, + }), + assign: None, + }) +} + /// Resolver for an effect of a call. /// /// The single interface behind every effect kind (NSE, attach, source). @@ -200,13 +225,10 @@ impl<'a> CallContext<'a> { self.scope.is_none_or(|scope| scope.is_global()) } - /// Match `call`'s arguments to `formals`, returning for each call argument - /// in order the index into `formals` it bound to. Named arguments match - /// first, then the rest fill by position. - /// - /// A stopgap: without the callee's full formal list, a positional argument - /// only binds a formal declared at that exact position. - pub fn match_arguments(&self, call: &RCall, formals: &[Formal]) -> Vec> { + /// Match `call` arguments to `formals`, returning a formal index for each + /// call argument. Exact named matches consume slots first, then unnamed + /// arguments fill unconsumed slots in signature order. + pub fn match_arguments(&self, call: &RCall, formals: Formals) -> Vec> { let Ok(args) = call.arguments() else { return Vec::new(); }; @@ -227,21 +249,21 @@ impl<'a> CallContext<'a> { // Positional pass. Only unnamed args reach the match, and none of them // were set by the named pass, so no need to re-check `matched[i]`. - let mut position = 0usize; + let mut next_slot = 0usize; for (i, item) in items.iter().enumerate() { - let Ok(arg) = item else { - position += 1; - continue; - }; + let Ok(arg) = item else { continue }; if arg.name_clause().is_some() { - position += 1; continue; } - if let Some(formal_idx) = match_positional(formals, position, &consumed) { - consumed[formal_idx] = true; - matched[i] = Some(formal_idx); + while next_slot < consumed.len() && consumed[next_slot] { + next_slot += 1; } - position += 1; + let Some(formal_idx) = (next_slot < formals.len()).then_some(next_slot) else { + continue; + }; + consumed[formal_idx] = true; + matched[i] = Some(formal_idx); + next_slot += 1; } matched @@ -277,17 +299,10 @@ impl<'a> CallContext<'a> { } } -/// A formal a handler wants to locate in a call, by name and by its position in -/// the callee's signature. -/// -/// TODO(nse): `position` is a stopgap that stems from our annotation registry -/// listing only its scoped formals. Once `match_arguments` is signature-aware -/// it gets the callee's full ordered formals, and this collapses to a list of -/// names where the index is the position. -pub struct Formal { - pub name: &'static str, - pub position: usize, -} +/// The initial formal names needed to match the arguments this handler reads. +/// Include every earlier slot so unnamed arguments bind correctly. Stop before +/// `...`, because later R formals are matched by name rather than position. +pub type Formals = &'static [&'static str]; /// A call's resolved argument effects: for each argument in call order, the /// effect it resolved to, or `None` for a plain (standard-eval) argument. @@ -310,14 +325,13 @@ pub enum ResolvedArgumentEffect { /// default [`EffectHandler`] for it by matching the declaration to a call. #[derive(Debug, Clone, Copy)] pub struct ArgumentsAnnotation { + pub formals: Formals, pub arguments: &'static [Argument], } -/// A single annotated argument: its effect, plus where to find it in a call. #[derive(Debug)] pub struct Argument { pub name: &'static str, - pub position: usize, pub effect: ArgumentEffect, } @@ -349,91 +363,100 @@ impl EffectHandler for ArgumentsAnnotation { type Output = ResolvedArgumentEffects; fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option { - let arguments = self.arguments; - let formals: Vec = arguments - .iter() - .map(|arg| Formal { - name: arg.name, - position: arg.position, - }) - .collect(); - - // The match yields a formal index per call argument - let matched = ctx.match_arguments(call, &formals); + let matched = ctx.match_arguments(call, self.formals); Some( matched .into_iter() - .map(|formal| formal.map(|i| arguments[i].effect.resolve())) + .map(|formal_idx| { + let name = self.formals[formal_idx?]; + self.arguments + .iter() + .find(|argument| argument.name == name) + .map(|argument| argument.effect.resolve()) + }) .collect(), ) } } -/// 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, + /// R files in a directory. [`DirWalk`] determines whether descendants count. + Dir(DirWalk), + /// A file or directory. `source()` takes only files, while + /// `targets::tar_source()` takes both. + FileOrDir(DirWalk), +} + +/// Controls whether directory source targets include descendants. +#[derive(Debug, Clone, Copy, PartialEq, Eq, Hash)] +pub enum DirWalk { + /// Direct children only, matching `list.files()` without `recursive = TRUE`. + Shallow, + /// Every R file below the directory, matching `list.files(recursive = TRUE)`. + Recursive, +} + +/// 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, + pub formals: Formals, + pub path: &'static str, + /// 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 { - type Output = Vec; + type Output = Vec; - fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option> { + fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option> { + let matched = ctx.match_arguments(call, self.formals); let args = call.arguments().ok()?; + let values: Vec> = args + .items() + .iter() + .map(|item| item.ok().and_then(|arg| arg.value())) + .collect(); - // 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 - // argument matching doesn't do. And counting unnamed arguments is robust - // to a named argument coming first (e.g. `source(echo = TRUE, "x.R")`), - // which the call-position matching isn't yet. A named `file =` therefore - // isn't recognized today. - // - // TODO(nse): once `match_arguments` is signature-aware (see `Formal`), - // the leading-named-arg robustness comes for free and this scan could - // fold onto it, keeping only the `local =` bail on top. - let mut path: Option = None; - let mut positional = 0; - - 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 the call isn't - // statically analyzable, so it's not recognized. - _ => return None, - } - } - } - continue; - } + let bound_value = |formal_name: &str| -> Option<&AnyRExpression> { + let formal_idx = self.formals.iter().position(|name| *name == formal_name)?; + let call_idx = matched.iter().position(|idx| *idx == Some(formal_idx))?; + values[call_idx].as_ref() + }; - if positional == self.position { - path = arg - .value() - .and_then(|value| ctx.resolve_static_string(&value)); + if let Some(local) = bound_value("local") { + match local { + AnyRExpression::RTrueExpression(_) | AnyRExpression::RFalseExpression(_) => {}, + // Only literal `TRUE` and `FALSE` make the source scope statically + // known. + _ => return None, } - positional += 1; } - path.map(|resolved| vec![resolved]) + let path = match bound_value(self.path) { + // An explicit dynamic path suppresses the default. + Some(value) => ctx.resolve_static_string(value)?, + None => self.default_path?.to_string(), + }; + + Some(vec![SourcePath { + path, + target: self.target, + }]) } } @@ -460,9 +483,6 @@ impl AssignHandler for AssignAnnotation { // `assign(x, value, ...)`). // // FIXME: A named `value =` isn't captured yet. - // TODO(nse): Fold onto `match_arguments()` once it's signature-aware, - // same as `source` (see `SourceAnnotation::resolve`), keeping only the - // `envir`/`pos` bail and the value-after-name read on top. let mut name: Option<(String, RangedAstPtr)> = None; let mut value_expr: Option> = None; let mut positional = 0; @@ -537,7 +557,7 @@ impl AssignHandler for BindingOperatorHandler { /// formal. /// /// Should we do partial argument matching? Or rely on partial matching being linted? -fn match_named(arg: &RArgument, formals: &[Formal], consumed: &[bool]) -> Option { +fn match_named(arg: &RArgument, formals: Formals, consumed: &[bool]) -> Option { let clause = arg.name_clause()?; let name = clause.name().ok()?; let name_text = match &name { @@ -548,24 +568,6 @@ fn match_named(arg: &RArgument, formals: &[Formal], consumed: &[bool]) -> Option formals .iter() .enumerate() - .find(|(i, formal)| !consumed[*i] && formal.name == name_text.as_str()) - .map(|(i, _)| i) -} - -/// Match an unnamed argument at `position` against `formals`. Returns the index -/// of the matched formal. -/// -/// FIXME: This matches positionally on call-site position only: an unnamed -/// argument at position N matches a formal 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(formals: &[Formal], position: usize, consumed: &[bool]) -> Option { - formals - .iter() - .enumerate() - .find(|(i, formal)| !consumed[*i] && formal.position == position) + .find(|(i, formal_name)| !consumed[*i] && **formal_name == name_text.as_str()) .map(|(i, _)| i) } diff --git a/crates/oak_semantic/src/effects/contrib.rs b/crates/oak_semantic/src/effects/contrib.rs index d1c049b4b..86eb77a0d 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; @@ -21,17 +22,19 @@ pub(crate) struct PackageEntries { pub(super) functions: &'static [Entry], } -/// An NSE entry. Each `(name, position, scope, laziness)` tuple is a scoped -/// argument; list more than one for a function that scopes several. +/// Declares non-standard evaluation effects. macro_rules! nse { - ($func:literal, $(($name:literal, $pos:literal, $scope:expr, $timing:expr)),+ $(,)?) => { + ($func:literal, $(($name:literal, $scope:expr, $timing:expr)),+ $(,)?) => { + $crate::effects::contrib::nse!($func, [$($name),+], $(($name, $scope, $timing)),+) + }; + ($func:literal, [$($formal:literal),+ $(,)?], $(($name:literal, $scope:expr, $timing:expr)),+ $(,)?) => { $crate::effects::contrib::Entry { function: $func, effects: $crate::effects::EffectsHandlers { arguments: Some(&$crate::effects::ArgumentsAnnotation { + formals: &[$($formal),+], arguments: &[$($crate::effects::Argument { name: $name, - position: $pos, effect: $crate::effects::ArgumentEffect::EvalQ { env: $scope, timing: $timing, @@ -47,18 +50,19 @@ macro_rules! nse { } pub(crate) use nse; -/// A quoted entry. Each `(name, position)` names an argument captured -/// unevaluated: its symbols aren't uses and nothing in it runs. `quote`, -/// `bquote`. +/// Declares arguments that remain unevaluated. macro_rules! quoted { - ($func:literal, $(($name:literal, $pos:literal)),+ $(,)?) => { + ($func:literal, $($name:literal),+ $(,)?) => { + $crate::effects::contrib::quoted!($func, [$($name),+], $($name),+) + }; + ($func:literal, [$($formal:literal),+ $(,)?], $($name:literal),+ $(,)?) => { $crate::effects::contrib::Entry { function: $func, effects: $crate::effects::EffectsHandlers { arguments: Some(&$crate::effects::ArgumentsAnnotation { + formals: &[$($formal),+], arguments: &[$($crate::effects::Argument { name: $name, - position: $pos, effect: $crate::effects::ArgumentEffect::Quote, }),+], }), @@ -71,16 +75,33 @@ 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. +/// Declares a source function's signature prefix, path formal, [`SourceTarget`], +/// and no-argument default. The target defaults to [`SourceTarget::File`]. +/// +/// [`SourceTarget`]: crate::effects::SourceTarget +/// [`SourceTarget::File`]: crate::effects::SourceTarget::File macro_rules! source { - ($func:literal, $pos:literal) => { + ($func:literal, [$($formal:literal),+ $(,)?], $path:literal) => { + $crate::effects::contrib::source!( + $func, + [$($formal),+], + $path, + $crate::effects::SourceTarget::File, + None + ) + }; + ($func:literal, [$($formal:literal),+ $(,)?], $path:literal, $target:expr, $default:expr) => { $crate::effects::contrib::Entry { function: $func, effects: $crate::effects::EffectsHandlers { arguments: None, attach: None, - source: Some(&$crate::effects::SourceAnnotation { position: $pos }), + source: Some(&$crate::effects::SourceAnnotation { + formals: &[$($formal),+], + path: $path, + target: $target, + default_path: $default, + }), assign: None, }, } @@ -146,6 +167,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/base.rs b/crates/oak_semantic/src/effects/contrib/base.rs index 20ef45f42..f76318acd 100644 --- a/crates/oak_semantic/src/effects/contrib/base.rs +++ b/crates/oak_semantic/src/effects/contrib/base.rs @@ -19,18 +19,22 @@ use crate::semantic_index::EvalTiming::Lazy; pub(crate) static ENTRIES: &[Entry] = &[ // base NSE - nse!("evalq", ("expr", 0, Current, Eager)), + nse!("evalq", ("expr", Current, Eager)), // `on.exit(expr)` captures `expr` and runs it in the current function's // frame when the function exits. Bindings land in that frame (`Current`) at // an unknown later time (`Lazy`), the same shape as `rlang::on_load()`. - nse!("on.exit", ("expr", 0, Current, Lazy)), - nse!("local", ("expr", 0, Nested, Eager)), - nse!("with", ("expr", 1, Nested, Eager)), - nse!("with.default", ("expr", 1, Nested, Eager)), - nse!("within", ("expr", 1, Nested, Eager)), - nse!("within.data.frame", ("expr", 1, Nested, Eager)), + nse!("on.exit", ("expr", Current, Lazy)), + nse!("local", ("expr", Nested, Eager)), + nse!("with", ["data", "expr"], ("expr", Nested, Eager)), + nse!("with.default", ["data", "expr"], ("expr", Nested, Eager)), + nse!("within", ["data", "expr"], ("expr", Nested, Eager)), + nse!( + "within.data.frame", + ["data", "expr"], + ("expr", Nested, Eager) + ), // base quote - quoted!("quote", ("expr", 0)), + quoted!("quote", "expr"), // `bquote` quotes `expr` too, but its `.()` holes escape to evaluation, so // it needs a handler rather than a static per-argument effect. Entry { @@ -58,7 +62,7 @@ pub(crate) static ENTRIES: &[Entry] = &[ attach_entry("library"), attach_entry("require"), // base source - source!("source", 0), + source!("source", ["file", "local"], "file"), // base assign assign!("assign", 0), assign!("delayedAssign", 0), diff --git a/crates/oak_semantic/src/effects/contrib/base/bquote.rs b/crates/oak_semantic/src/effects/contrib/base/bquote.rs index 3bdc8a40b..a950c2259 100644 --- a/crates/oak_semantic/src/effects/contrib/base/bquote.rs +++ b/crates/oak_semantic/src/effects/contrib/base/bquote.rs @@ -7,7 +7,7 @@ use oak_core::syntax_ext::RIdentifierExt; use crate::effects::CallContext; use crate::effects::EffectHandler; -use crate::effects::Formal; +use crate::effects::Formals; use crate::effects::ResolvedArgumentEffect; use crate::effects::ResolvedArgumentEffects; @@ -22,19 +22,8 @@ impl EffectHandler for BquoteHandler { type Output = ResolvedArgumentEffects; fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option { - // `bquote(expr, where, splice)`: only `expr` (the first positional) is - // quoted. The other arguments are ordinary values. - let formals = [ - Formal { - name: "expr", - position: 0, - }, - Formal { - name: "splice", - position: 2, - }, - ]; - let matched = ctx.match_arguments(call, &formals); + let formals: Formals = &["expr", "where", "splice"]; + let matched = ctx.match_arguments(call, formals); let args = call.arguments().ok()?; let values: Vec> = args @@ -46,7 +35,7 @@ impl EffectHandler for BquoteHandler { // `..()` only splices under `splice = TRUE`. let splice = matched .iter() - .position(|formal| *formal == Some(1)) + .position(|formal| *formal == Some(2)) .and_then(|i| values.get(i)) .and_then(|value| value.as_ref()) .and_then(|value| ctx.resolve_static_bool(value)) diff --git a/crates/oak_semantic/src/effects/contrib/base/library.rs b/crates/oak_semantic/src/effects/contrib/base/library.rs index b63e0831a..44bdfcf73 100644 --- a/crates/oak_semantic/src/effects/contrib/base/library.rs +++ b/crates/oak_semantic/src/effects/contrib/base/library.rs @@ -4,7 +4,7 @@ use biome_rowan::AstSeparatedList; use crate::effects::CallContext; use crate::effects::EffectHandler; -use crate::effects::Formal; +use crate::effects::Formals; /// Handler for `library()` and `require()`. Names the attached package from the /// first argument, read as quoted (the symbol or string as written, so @@ -19,19 +19,8 @@ impl EffectHandler for LibraryHandler { type Output = String; fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option { - // `character.only` sits at signature position 4 in both callees; in - // practice it's passed by name. - let formals = [ - Formal { - name: "package", - position: 0, - }, - Formal { - name: "character.only", - position: 4, - }, - ]; - let matched = ctx.match_arguments(call, &formals); + let formals: Formals = &["package", "help", "pos", "lib.loc", "character.only"]; + let matched = ctx.match_arguments(call, formals); let args = call.arguments().ok()?; let values: Vec> = args @@ -48,7 +37,7 @@ impl EffectHandler for LibraryHandler { let character_only = matched .iter() - .position(|formal| *formal == Some(1)) + .position(|formal| *formal == Some(4)) .and_then(|i| values.get(i)) .and_then(|value| value.as_ref()) .and_then(|value| ctx.resolve_static_bool(value)) diff --git a/crates/oak_semantic/src/effects/contrib/base/substitute.rs b/crates/oak_semantic/src/effects/contrib/base/substitute.rs index 9203a8325..6de2f16e2 100644 --- a/crates/oak_semantic/src/effects/contrib/base/substitute.rs +++ b/crates/oak_semantic/src/effects/contrib/base/substitute.rs @@ -10,7 +10,7 @@ use oak_core::syntax_ext::RIdentifierExt; use crate::effects::CallContext; use crate::effects::EffectHandler; -use crate::effects::Formal; +use crate::effects::Formals; use crate::effects::ResolvedArgumentEffect; use crate::effects::ResolvedArgumentEffects; @@ -28,17 +28,8 @@ impl EffectHandler for SubstituteHandler { fn resolve(&self, call: &RCall, ctx: &CallContext<'_>) -> Option { // `substitute(expr, env)`: only `expr` (formal 0) is quoted, everything // else is a plain value. - let formals = [ - Formal { - name: "expr", - position: 0, - }, - Formal { - name: "env", - position: 1, - }, - ]; - let matched = ctx.match_arguments(call, &formals); + let formals: Formals = &["expr", "env"]; + let matched = ctx.match_arguments(call, formals); let expr_pos = matched.iter().position(|formal| *formal == Some(0))?; // Only the default `env`, the current frame, is one we can query. Any diff --git a/crates/oak_semantic/src/effects/contrib/rlang.rs b/crates/oak_semantic/src/effects/contrib/rlang.rs index aee91ce5e..9f6ef2b00 100644 --- a/crates/oak_semantic/src/effects/contrib/rlang.rs +++ b/crates/oak_semantic/src/effects/contrib/rlang.rs @@ -7,9 +7,9 @@ use crate::semantic_index::EvalTiming::Lazy; pub(crate) static ENTRIES: &[Entry] = &[ assign_op!("%<~%", Write), - nse!("on_load", ("expr", 0, Current, Lazy)), + nse!("on_load", ("expr", Current, Lazy)), // `defer(expr, env = caller_env())` runs `expr` in the caller frame when it // exits. Written in a function, that frame is the function itself, so it's // `Current + Lazy` like `on.exit`. A non-default `env` isn't modeled. - nse!("defer", ("expr", 0, Current, Lazy)), + nse!("defer", ("expr", Current, Lazy)), ]; diff --git a/crates/oak_semantic/src/effects/contrib/shiny.rs b/crates/oak_semantic/src/effects/contrib/shiny.rs index 1d9002429..abbd03199 100644 --- a/crates/oak_semantic/src/effects/contrib/shiny.rs +++ b/crates/oak_semantic/src/effects/contrib/shiny.rs @@ -4,11 +4,11 @@ use crate::semantic_index::EvalEnv::Nested; use crate::semantic_index::EvalTiming::Lazy; pub(crate) static ENTRIES: &[Entry] = &[ - nse!("observe", ("x", 0, Nested, Lazy)), - nse!("reactive", ("x", 0, Nested, Lazy)), - nse!("renderPlot", ("expr", 0, Nested, Lazy)), - nse!("renderPrint", ("expr", 0, Nested, Lazy)), - nse!("renderTable", ("expr", 0, Nested, Lazy)), - nse!("renderText", ("expr", 0, Nested, Lazy)), - nse!("renderUI", ("expr", 0, Nested, Lazy)), + nse!("observe", ("x", Nested, Lazy)), + nse!("reactive", ("x", Nested, Lazy)), + nse!("renderPlot", ("expr", Nested, Lazy)), + nse!("renderPrint", ("expr", Nested, Lazy)), + nse!("renderTable", ("expr", Nested, Lazy)), + nse!("renderText", ("expr", Nested, Lazy)), + nse!("renderUI", ("expr", Nested, Lazy)), ]; 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..c3e2e9000 --- /dev/null +++ b/crates/oak_semantic/src/effects/contrib/targets.rs @@ -0,0 +1,21 @@ +use crate::effects::contrib::source; +use crate::effects::contrib::Entry; +use crate::effects::DirWalk; +use crate::effects::SourceTarget; + +pub(super) static ENTRIES: &[Entry] = &[ + // `tar_source()` loads scripts from `files`, defaulting to `R`. A path may + // name a script or directory. + // + // Directory paths recurse because `file_list_files()` calls + // `list.files(recursive = TRUE)`. + // + // The scanner reads one literal even though `files` is a character vector. + source!( + "tar_source", + ["files"], + "files", + SourceTarget::FileOrDir(DirWalk::Recursive), + Some("R") + ), +]; diff --git a/crates/oak_semantic/src/effects/contrib/testthat.rs b/crates/oak_semantic/src/effects/contrib/testthat.rs index 66e471973..37e3a1467 100644 --- a/crates/oak_semantic/src/effects/contrib/testthat.rs +++ b/crates/oak_semantic/src/effects/contrib/testthat.rs @@ -3,4 +3,5 @@ use crate::effects::contrib::Entry; use crate::semantic_index::EvalEnv::Nested; use crate::semantic_index::EvalTiming::Eager; -pub(crate) static ENTRIES: &[Entry] = &[nse!("test_that", ("code", 1, Nested, Eager))]; +pub(crate) static ENTRIES: &[Entry] = + &[nse!("test_that", ["desc", "code"], ("code", Nested, Eager))]; diff --git a/crates/oak_semantic/src/effects/contrib/withr.rs b/crates/oak_semantic/src/effects/contrib/withr.rs index 27ce932e5..59c0d5daf 100644 --- a/crates/oak_semantic/src/effects/contrib/withr.rs +++ b/crates/oak_semantic/src/effects/contrib/withr.rs @@ -7,5 +7,5 @@ pub(crate) static ENTRIES: &[Entry] = &[ // `defer(expr, envir = parent.frame())` runs `expr` in the caller frame when // it exits, the same `Current + Lazy` shape as `on.exit`. A non-default // `envir` isn't modeled. - nse!("defer", ("expr", 0, Current, Lazy)), + nse!("defer", ("expr", Current, Lazy)), ]; diff --git a/crates/oak_semantic/src/resolver.rs b/crates/oak_semantic/src/resolver.rs index a3b044208..28f8af934 100644 --- a/crates/oak_semantic/src/resolver.rs +++ b/crates/oak_semantic/src/resolver.rs @@ -1,6 +1,7 @@ use url::Url; use crate::effects; +use crate::effects::DirWalk; use crate::effects::EffectsHandlers; /// The result of resolving a `source()` call. Returned by @@ -50,6 +51,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 load order. + /// `path` uses the same anchor as [`resolve_source`]. `walk` determines + /// whether nested directories are included. + /// + /// [`resolve_source`]: ImportsResolver::resolve_source + fn resolve_source_dir(&mut self, path: &str, walk: DirWalk) -> Vec { + let _ = (path, walk); + 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/common.rs b/crates/oak_semantic/tests/integration/common.rs index 4e60e5583..cbf3acb1f 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::effects::TargetAccess; use oak_semantic::semantic_index::DefinitionKind; use oak_semantic::semantic_index::ScopeId; @@ -73,10 +75,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.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 782150efe..d71046c43 100644 --- a/crates/oak_semantic/tests/integration/contrib/base.rs +++ b/crates/oak_semantic/tests/integration/contrib/base.rs @@ -3,7 +3,9 @@ use aether_parser::RParserOptions; use biome_rowan::AstNode; use oak_semantic::build_index; use oak_semantic::effects; +use oak_semantic::effects::DirWalk; 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; @@ -87,6 +89,24 @@ 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, walk: DirWalk) -> Vec { + // `sourceDir()` leaves `list.files()` at its `recursive = FALSE` default. + assert_eq!(walk, DirWalk::Shallow); + self.files.clone() + } +} + /// Resolves `source` to the multi-file [`CollationHandler`] and maps the /// collated paths through `sources`. struct MultiFileResolver { @@ -111,11 +131,15 @@ impl ImportsResolver for MultiFileResolver { } } -/// Resolves `source` to a [`SourceAnnotation`] whose path sits at the second -/// positional slot, exercising the configurable `position`. +/// Provides a `source()` annotation whose `file` path formal is second. struct PositionResolver; -static SOURCE_AT_POSITION_1: SourceAnnotation = SourceAnnotation { position: 1 }; +static SOURCE_PATH_SECOND: SourceAnnotation = SourceAnnotation { + formals: &["ignored", "file"], + path: "file", + target: SourceTarget::File, + default_path: None, +}; impl ImportsResolver for PositionResolver { fn resolve_source(&mut self, _path: &str) -> Option { @@ -127,7 +151,7 @@ impl ImportsResolver for PositionResolver { return Some(EffectsHandlers { arguments: None, attach: None, - source: Some(&SOURCE_AT_POSITION_1), + source: Some(&SOURCE_PATH_SECOND), assign: None, }); } @@ -1242,9 +1266,88 @@ fn test_source_resolver_multiple_files_each_emitted_and_injected() { } #[test] -fn test_source_resolver_honors_configured_path_position() { - // A `SourceAnnotation` with `position: 1` takes the path from the second - // positional argument, not the first. +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_finds_path_formal_not_first() { let index = build_test_index("source(\"ignored\", \"real.R\")", PositionResolver); assert_eq!(semantic_call_kinds(&index), [&SemanticCallKind::Source { path: "real.R".into(), 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..f55fa7e4f --- /dev/null +++ b/crates/oak_semantic/tests/integration/contrib/targets.rs @@ -0,0 +1,163 @@ +use aether_parser::parse; +use aether_parser::RParserOptions; +use oak_semantic::build_index; +use oak_semantic::effects; +use oak_semantic::effects::DirWalk; +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, walk: DirWalk) -> Vec { + // `tar_source()` lists directory arguments with `recursive = TRUE`. + assert_eq!(walk, DirWalk::Recursive); + 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_recognized() { + let files = vec![resolution("file:///code/a.R", "a_name")]; + let index = index( + "tar_source(files = \"code\")\n", + TargetsResolver::with_dir(files), + ); + + assert_eq!(semantic_call_kinds(&index), [&sourced( + "code", + "file:///code/a.R" + )]); +} + +#[test] +fn test_tar_source_change_directory_false_uses_the_default_directory() { + // `change_directory` does not bind `files`, so `files` uses its `"R"` default. + let files = vec![resolution("file:///R/a.R", "a_name")]; + let index = index( + "tar_source(change_directory = FALSE)\n", + TargetsResolver::with_dir(files), + ); + + assert_eq!(semantic_call_kinds(&index), [&sourced( + "R", + "file:///R/a.R" + )]); +} + +#[test] +fn test_tar_source_dynamic_files_argument_is_not_recognized() { + // A dynamic `files` value overrides the `"R"` default but produces no source call. + let files = vec![resolution("file:///R/a.R", "a_name")]; + let index = index( + "tar_source(files = some_var)\n", + TargetsResolver::with_dir(files), + ); + + assert_eq!(semantic_call_kinds(&index), Vec::<&SemanticCallKind>::new()); +}