From dd56c813f99a47541220d5e04044277930571c2a Mon Sep 17 00:00:00 2001 From: Solomon Date: Thu, 20 Mar 2025 18:20:28 +0000 Subject: [PATCH] preserve variable capture spans in blocks (#15334) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #15160 # User-Facing Changes Certain "variable not found" errors no longer highlight the surrounding block. Before: ```nushell do { match foo { _ => $in } } Error: nu::shell::variable_not_found × Variable not found ╭─[entry #1:1:1] 1 │ ╭─▶ do { 2 │ │ match foo { 3 │ │ _ => $in 4 │ │ } 5 │ ├─▶ } · ╰──── variable not found ``` After: ```nushell Error: nu::shell::variable_not_found × Variable not found ╭─[entry #1:3:10] 2 │ match foo { 3 │ _ => $in · ─┬─ · ╰── variable not found ``` --- crates/nu-engine/src/eval.rs | 10 +++++----- crates/nu-engine/src/eval_ir.rs | 2 +- crates/nu-parser/src/parser.rs | 9 ++++----- crates/nu-protocol/src/ast/block.rs | 2 +- crates/nu-protocol/src/ast/expression.rs | 5 ++++- crates/nu-protocol/src/engine/stack.rs | 4 ++-- 6 files changed, 17 insertions(+), 15 deletions(-) diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 01f7b69f7a..41d39241f8 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -642,17 +642,17 @@ impl Eval for EvalRuntime { .get_block(block_id) .captures .iter() - .map(|&id| { + .map(|(id, span)| { stack - .get_var(id, span) + .get_var(*id, *span) .or_else(|_| { engine_state - .get_var(id) + .get_var(*id) .const_val .clone() - .ok_or(ShellError::VariableNotFoundAtRuntime { span }) + .ok_or(ShellError::VariableNotFoundAtRuntime { span: *span }) }) - .map(|var| (id, var)) + .map(|var| (*id, var)) }) .collect::>()?; diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 6ca4d2e8ca..20a0bda94d 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -857,7 +857,7 @@ fn literal_value( let captures = block .captures .iter() - .map(|var_id| get_var(ctx, *var_id, span).map(|val| (*var_id, val))) + .map(|(var_id, span)| get_var(ctx, *var_id, *span).map(|val| (*var_id, val))) .collect::, ShellError>>()?; Value::closure( Closure { diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 715afb5bad..ef181a6a3f 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -6589,9 +6589,9 @@ pub fn discover_captures_in_expr( None => { let block = working_set.get_block(block_id); if !block.captures.is_empty() { - for capture in &block.captures { + for (capture, span) in &block.captures { if !seen.contains(capture) { - output.push((*capture, call.head)); + output.push((*capture, *span)); } } } else { @@ -6905,8 +6905,7 @@ pub fn parse( &mut captures, ) { Ok(_) => { - Arc::make_mut(&mut output).captures = - captures.into_iter().map(|(var_id, _)| var_id).collect(); + Arc::make_mut(&mut output).captures = captures; } Err(err) => working_set.error(err), } @@ -6961,7 +6960,7 @@ pub fn parse( && block_id.get() >= working_set.permanent_state.num_blocks() { let block = working_set.get_block_mut(block_id); - block.captures = captures.into_iter().map(|(var_id, _)| var_id).collect(); + block.captures = captures; } } diff --git a/crates/nu-protocol/src/ast/block.rs b/crates/nu-protocol/src/ast/block.rs index a6e640cc15..ab603da0da 100644 --- a/crates/nu-protocol/src/ast/block.rs +++ b/crates/nu-protocol/src/ast/block.rs @@ -6,7 +6,7 @@ use serde::{Deserialize, Serialize}; pub struct Block { pub signature: Box, pub pipelines: Vec, - pub captures: Vec, + pub captures: Vec<(VarId, Span)>, pub redirect_env: bool, /// The block compiled to IR instructions. Not available for subexpressions. pub ir_block: Option, diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 7d08f62533..5ffa3aab8b 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -111,7 +111,10 @@ impl Expression { Expr::UnaryNot(expr) => expr.has_in_variable(working_set), Expr::Block(block_id) | Expr::Closure(block_id) => { let block = working_set.get_block(*block_id); - block.captures.contains(&IN_VARIABLE_ID) + block + .captures + .iter() + .any(|(var_id, _)| var_id == &IN_VARIABLE_ID) || block .pipelines .iter() diff --git a/crates/nu-protocol/src/engine/stack.rs b/crates/nu-protocol/src/engine/stack.rs index 0e8df6b654..2c6422f503 100644 --- a/crates/nu-protocol/src/engine/stack.rs +++ b/crates/nu-protocol/src/engine/stack.rs @@ -322,12 +322,12 @@ impl Stack { } } - pub fn gather_captures(&self, engine_state: &EngineState, captures: &[VarId]) -> Stack { + pub fn gather_captures(&self, engine_state: &EngineState, captures: &[(VarId, Span)]) -> Stack { let mut vars = Vec::with_capacity(captures.len()); let fake_span = Span::new(0, 0); - for capture in captures { + for (capture, _) in captures { // Note: this assumes we have calculated captures correctly and that commands // that take in a var decl will manually set this into scope when running the blocks if let Ok(value) = self.get_var(*capture, fake_span) {