diff --git a/crates/oak_semantic/src/builder/builder_nse.rs b/crates/oak_semantic/src/builder/builder_nse.rs index 58d537f21..0b62352c2 100644 --- a/crates/oak_semantic/src/builder/builder_nse.rs +++ b/crates/oak_semantic/src/builder/builder_nse.rs @@ -118,6 +118,12 @@ impl SemanticIndexBuilder { match arg_effects[i] { None => self.scan_expression(&value), + // Quoted argument: captured unevaluated. No effects to scan, no + // names to bind. + Some(Argument { + effect: ArgumentEffect::Quote, + .. + }) => {}, Some(Argument { effect: ArgumentEffect::Nse { scope, timing }, .. @@ -506,9 +512,16 @@ impl SemanticIndexBuilder { let Ok(arg) = item else { continue }; let Some(value) = arg.value() else { continue }; - match arg_effects[i] { - None => self.collect_expression(&value), - Some(argument) => self.collect_nse_argument(argument, &value), + let Some(argument) = arg_effects[i] else { + self.collect_expression(&value); + continue; + }; + match argument.effect { + ArgumentEffect::Nse { scope, timing } => { + self.collect_nse_argument(scope, timing, &value) + }, + // Quoted argument: No uses inside. + ArgumentEffect::Quote => {}, } } } @@ -519,8 +532,7 @@ impl SemanticIndexBuilder { /// already scanned by the descent, so we install its pending names and only /// walk. The remaining lazy bodies are their own scan units that we scan /// here on entry. - fn collect_nse_argument(&mut self, argument: &Argument, value: &AnyRExpression) { - let ArgumentEffect::Nse { scope, timing } = argument.effect; + fn collect_nse_argument(&mut self, scope: NseScope, timing: NseTiming, value: &AnyRExpression) { match (scope, timing) { // Calls like `evalq()` (NseScope::Current, NseTiming::Eager) => { diff --git a/crates/oak_semantic/src/effects.rs b/crates/oak_semantic/src/effects.rs index 733427101..fbc53b6e8 100644 --- a/crates/oak_semantic/src/effects.rs +++ b/crates/oak_semantic/src/effects.rs @@ -181,6 +181,10 @@ pub struct Argument { pub enum ArgumentEffect { /// Quote plus Eval in a controlled scope, fused Nse { scope: NseScope, timing: NseTiming }, + /// Captured unevaluated, so its symbols are not uses and nothing in it runs. + /// `quote`, `bquote`. TODO:`bquote(.(foo))` should unquote `foo`. This + /// requires implementing the `Eval` effect. + Quote, } impl EffectHandler for ArgumentsAnnotation { diff --git a/crates/oak_semantic/src/effects_registry.rs b/crates/oak_semantic/src/effects_registry.rs index d0b46e82c..f467b988f 100644 --- a/crates/oak_semantic/src/effects_registry.rs +++ b/crates/oak_semantic/src/effects_registry.rs @@ -60,6 +60,30 @@ macro_rules! nse { }; } +/// A quoted entry. Each `(name, position)` names an argument captured +/// unevaluated: its symbols aren't uses and nothing in it runs. `quote`, +/// `bquote`. +macro_rules! quoted { + ($pkg:literal, $func:literal, $(($name:literal, $pos:literal)),+ $(,)?) => { + Entry { + package: $pkg, + function: $func, + effects: EffectsHandlers { + arguments: Some(&ArgumentsAnnotation { + arguments: &[$(Argument { + name: $name, + position: $pos, + effect: ArgumentEffect::Quote, + }),+], + }), + attach: None, + source: None, + assign: None, + }, + } + }; +} + /// An attach entry: `(package-argument position, has-`character.only`-flag)`. macro_rules! attach { ($pkg:literal, $func:literal, $pos:literal, $character_only:literal) => { @@ -120,6 +144,9 @@ static REGISTRY: &[Entry] = &[ nse!("base", "with.default", ("expr", 1, Nested, Eager)), nse!("base", "within", ("expr", 1, Nested, Eager)), nse!("base", "within.data.frame", ("expr", 1, Nested, Eager)), + // base quote + quoted!("base", "quote", ("expr", 0)), + quoted!("base", "bquote", ("expr", 0)), // base attach attach!("base", "library", 0, true), attach!("base", "require", 0, true), diff --git a/crates/oak_semantic/tests/integration/builder.rs b/crates/oak_semantic/tests/integration/builder.rs index a31a41bf4..422722170 100644 --- a/crates/oak_semantic/tests/integration/builder.rs +++ b/crates/oak_semantic/tests/integration/builder.rs @@ -859,9 +859,9 @@ fn test_super_assignment_with_use_on_value_side() { // --- NSE / quoting constructs --- // -// Identifiers inside `~`, `quote()`, and `bquote()` are currently recorded -// as uses. This is a known simplification; refining it is deferred as -// future work. These tests document the current behaviour. +// `quote()` and `bquote()` capture their argument unevaluated, so the symbols +// inside are not uses (the tests below). Identifiers inside a formula `~` are +// still recorded as uses, a known simplification left for future work. #[test] fn test_fixme_formula_records_uses() { @@ -898,18 +898,61 @@ fn test_fixme_one_sided_formula_records_uses() { } #[test] -fn test_fixme_quote_records_uses() { - let index = index("quote(x + y)"); +fn test_quote_suppresses_uses() { + // `quote()` captures its argument unevaluated, so the symbols inside are not + // uses. Only the callee `quote` itself is a use. + let index = index_with_base("quote(x + y)"); let file = ScopeId::from(0); assert_eq!( index.symbols(file).get("quote").unwrap().flags(), SymbolFlags::IS_USED ); - assert_eq!( - index.symbols(file).get("x").unwrap().flags(), - SymbolFlags::IS_USED - ); + assert!(index.symbols(file).get("x").is_none()); + assert!(index.symbols(file).get("y").is_none()); + assert_eq!(index.uses(file).len(), 1); +} + +#[test] +fn test_quote_suppresses_assignment() { + // The `x <- 1` inside `quote()` is captured unevaluated, so it binds + // nothing. + let index = index_with_base("quote(x <- 1)"); + let file = ScopeId::from(0); + + assert!(index.symbols(file).get("x").is_none()); + assert_eq!(index.definitions(file).len(), 0); +} + +#[test] +fn test_bquote_suppresses_uses() { + // `bquote()` quotes its argument the same as `quote()`. + let index = index_with_base("bquote(x + y)"); + let file = ScopeId::from(0); + + assert!(index.symbols(file).get("x").is_none()); + assert!(index.symbols(file).get("y").is_none()); + assert_eq!(index.uses(file).len(), 1); +} + +#[test] +fn test_fixme_bquote_unquote_hole_suppressed() { + // TODO(eval): `bquote`'s `.()` holes are evaluated, so `y` here is really a + // use. We treat the whole argument as quoted until the `Eval` effect lands, + // so `y` is suppressed for now. + let index = index_with_base("bquote(x + .(y))"); + let file = ScopeId::from(0); + + assert!(index.symbols(file).get("y").is_none()); +} + +#[test] +fn test_local_quote_definition_shadows() { + // A local `quote` shadows base's, so `quote(y)` is an ordinary call and `y` + // is a use again. + let index = index_with_base("quote <- function(a) a\nquote(y)"); + let file = ScopeId::from(0); + assert_eq!( index.symbols(file).get("y").unwrap().flags(), SymbolFlags::IS_USED @@ -917,13 +960,30 @@ fn test_fixme_quote_records_uses() { } #[test] -fn test_fixme_quote_records_assignment() { - let index = index("quote(x <- 1)"); +fn test_quote_suppresses_attach_effect() { + // A `library()` inside `quote()` is captured unevaluated, so it attaches + // nothing to the search path. + let index = index_with_base("quote(library(dplyr))"); + assert!(semantic_call_kinds(&index).is_empty()); +} + +#[test] +fn test_quote_suppresses_source_effect() { + // A `source()` inside `quote()` is captured unevaluated, so it injects no + // names and records no source call. + let index = index_with_base("quote(source(\"helpers.R\"))"); + assert!(semantic_call_kinds(&index).is_empty()); +} + +#[test] +fn test_quote_suppresses_assign_effect() { + // An `assign()` inside `quote()` is captured unevaluated, so it binds + // nothing. + let index = index_with_base("quote(assign(\"x\", 1))"); let file = ScopeId::from(0); - let x = index.symbols(file).get("x").unwrap(); - assert_eq!(x.flags(), SymbolFlags::IS_BOUND); - assert_eq!(index.definitions(file).len(), 1); + assert!(index.symbols(file).get("x").is_none()); + assert_eq!(index.definitions(file).len(), 0); } #[test]