From 0cfd5fbece6f25b54ab9dc417a9e06af9d83f282 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jakub=20=C5=BD=C3=A1dn=C3=ADk?= Date: Wed, 3 Jul 2024 09:00:52 +0300 Subject: [PATCH] Span ID Refactor (Step 2): Make Call SpanId-friendly (#13268) # Description Part of https://github.com/nushell/nushell/issues/12963, step 2. This PR refactors Call and related argument structures to remove their dependency on `Expression::span` which will be removed in the future. # User-Facing Changes Should be none. If you see some error messages that look broken, please report. # Tests + Formatting # After Submitting --- .../nu-cli/src/commands/keybindings_list.rs | 2 +- .../src/completions/custom_completions.rs | 29 +-- crates/nu-cli/src/syntax_highlight.rs | 2 +- crates/nu-command/src/bytes/build_.rs | 2 +- crates/nu-command/src/debug/explain.rs | 4 +- crates/nu-command/src/debug/profile.rs | 6 +- crates/nu-command/src/filesystem/open.rs | 7 +- crates/nu-command/src/filesystem/save.rs | 2 +- crates/nu-command/src/filesystem/util.rs | 2 +- crates/nu-command/src/formats/from/csv.rs | 2 +- crates/nu-command/src/generators/cal.rs | 4 +- crates/nu-command/src/misc/tutor.rs | 2 +- crates/nu-command/src/network/http/client.rs | 2 +- crates/nu-command/src/platform/is_terminal.rs | 7 +- crates/nu-command/src/system/run_external.rs | 8 +- crates/nu-command/src/viewers/table.rs | 17 +- crates/nu-engine/src/call_ext.rs | 2 +- crates/nu-engine/src/documentation.rs | 19 +- crates/nu-engine/src/eval.rs | 13 +- crates/nu-parser/src/flatten.rs | 2 +- crates/nu-parser/src/known_external.rs | 4 +- crates/nu-parser/src/lib.rs | 5 +- crates/nu-parser/src/parse_keywords.rs | 65 ++++-- crates/nu-parser/src/parser.rs | 202 ++++++++++------- crates/nu-parser/tests/test_parser.rs | 100 ++++++++- .../nu-plugin-protocol/src/evaluated_call.rs | 7 +- crates/nu-protocol/src/ast/call.rs | 203 +++++++----------- crates/nu-protocol/src/ast/expression.rs | 4 +- .../src/engine/state_working_set.rs | 36 +++- crates/nu-protocol/src/eval_const.rs | 2 +- .../nu-protocol/src/pipeline/pipeline_data.rs | 2 +- 31 files changed, 468 insertions(+), 296 deletions(-) diff --git a/crates/nu-cli/src/commands/keybindings_list.rs b/crates/nu-cli/src/commands/keybindings_list.rs index f4450c0c23770..b6375be8a106f 100644 --- a/crates/nu-cli/src/commands/keybindings_list.rs +++ b/crates/nu-cli/src/commands/keybindings_list.rs @@ -62,7 +62,7 @@ impl Command for KeybindingsList { .collect() } else { call.named_iter() - .flat_map(|(argument, _, _)| get_records(argument.item.as_str(), call.head)) + .flat_map(|(argument, _, _, _)| get_records(argument.item.as_str(), call.head)) .collect() }; diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs index f14971d4663f5..5d45bb708e63e 100644 --- a/crates/nu-cli/src/completions/custom_completions.rs +++ b/crates/nu-cli/src/completions/custom_completions.rs @@ -7,7 +7,7 @@ use nu_protocol::{ ast::{Argument, Call, Expr, Expression}, debugger::WithoutDebug, engine::{Stack, StateWorkingSet}, - PipelineData, Span, Type, Value, + PipelineData, Span, Spanned, Type, Value, }; use nu_utils::IgnoreCaseExt; use std::collections::HashMap; @@ -51,18 +51,21 @@ impl Completer for CustomCompletion { &Call { decl_id: self.decl_id, head: span, - arguments: vec![ - Argument::Positional(Expression::new_unknown( - Expr::String(self.line.clone()), - Span::unknown(), - Type::String, - )), - Argument::Positional(Expression::new_unknown( - Expr::Int(line_pos as i64), - Span::unknown(), - Type::Int, - )), - ], + arguments: Spanned { + item: vec![ + Argument::Positional(Expression::new_unknown( + Expr::String(self.line.clone()), + Span::unknown(), + Type::String, + )), + Argument::Positional(Expression::new_unknown( + Expr::Int(line_pos as i64), + Span::unknown(), + Type::Int, + )), + ], + span: Span::unknown(), + }, parser_info: HashMap::new(), }, PipelineData::empty(), diff --git a/crates/nu-cli/src/syntax_highlight.rs b/crates/nu-cli/src/syntax_highlight.rs index 41ef168390fdd..9fccf5e4090cd 100644 --- a/crates/nu-cli/src/syntax_highlight.rs +++ b/crates/nu-cli/src/syntax_highlight.rs @@ -400,7 +400,7 @@ fn find_matching_block_end_in_expr( } } - Expr::Call(call) => call.arguments.iter().find_map(|arg| { + Expr::Call(call) => call.arguments.item.iter().find_map(|arg| { arg.expr().and_then(|expr| { find_matching_block_end_in_expr( line, diff --git a/crates/nu-command/src/bytes/build_.rs b/crates/nu-command/src/bytes/build_.rs index f6b1327621556..b6a98d1204bbf 100644 --- a/crates/nu-command/src/bytes/build_.rs +++ b/crates/nu-command/src/bytes/build_.rs @@ -49,7 +49,7 @@ impl Command for BytesBuild { _input: PipelineData, ) -> Result { let mut output = vec![]; - for val in call.rest_iter_flattened(0, |expr| { + for val in call.rest_iter_flattened(&engine_state, 0, |expr| { let eval_expression = get_eval_expression(engine_state); eval_expression(engine_state, stack, expr) })? { diff --git a/crates/nu-command/src/debug/explain.rs b/crates/nu-command/src/debug/explain.rs index b451d6916a9c0..d30d41faa8e36 100644 --- a/crates/nu-command/src/debug/explain.rs +++ b/crates/nu-command/src/debug/explain.rs @@ -111,11 +111,11 @@ fn get_arguments( ) -> Vec { let mut arg_value = vec![]; let span = Span::test_data(); - for arg in &call.arguments { + for arg in &call.arguments.item { match arg { // I think the second argument to Argument::Named is the short name, but I'm not really sure. // Please fix it if it's wrong. :) - Argument::Named((name, short, opt_expr)) => { + Argument::Named((name, short, opt_expr, _)) => { let arg_type = "named"; let arg_value_name = name.item.clone(); let arg_value_name_span_start = name.span.start as i64; diff --git a/crates/nu-command/src/debug/profile.rs b/crates/nu-command/src/debug/profile.rs index 0cef979094faf..86136279f7139 100644 --- a/crates/nu-command/src/debug/profile.rs +++ b/crates/nu-command/src/debug/profile.rs @@ -104,7 +104,7 @@ confusing the id/parent_id hierarchy. The --expr flag is helpful for investigati collect_values, collect_exprs, collect_lines, - call.span(), + call.span(&engine_state), ); let lock_err = |_| ShellError::GenericError { @@ -125,12 +125,12 @@ confusing the id/parent_id hierarchy. The --expr flag is helpful for investigati let pipeline_data = result?; // Collect the output - let _ = pipeline_data.into_value(call.span()); + let _ = pipeline_data.into_value(call.span(&engine_state)); Ok(engine_state .deactivate_debugger() .map_err(lock_err)? - .report(engine_state, call.span())? + .report(engine_state, call.span(&engine_state))? .into_pipeline_data()) } diff --git a/crates/nu-command/src/filesystem/open.rs b/crates/nu-command/src/filesystem/open.rs index 90003594500f1..59caf283ba816 100644 --- a/crates/nu-command/src/filesystem/open.rs +++ b/crates/nu-command/src/filesystem/open.rs @@ -177,7 +177,12 @@ impl Command for Open { let block = engine_state.get_block(block_id); eval_block(engine_state, stack, block, stream) } else { - decl.run(engine_state, stack, &Call::new(call_span), stream) + decl.run( + engine_state, + stack, + &Call::new(call_span, call.arguments_span()), + stream, + ) }; output.push(command_output.map_err(|inner| { ShellError::GenericError{ diff --git a/crates/nu-command/src/filesystem/save.rs b/crates/nu-command/src/filesystem/save.rs index ab5257fb019b8..fa14572987df1 100644 --- a/crates/nu-command/src/filesystem/save.rs +++ b/crates/nu-command/src/filesystem/save.rs @@ -398,7 +398,7 @@ fn convert_to_extension( let eval_block = get_eval_block(engine_state); eval_block(engine_state, stack, block, input) } else { - decl.run(engine_state, stack, &Call::new(span), input) + decl.run(engine_state, stack, &Call::new(span, span.past()), input) } } else { Ok(input) diff --git a/crates/nu-command/src/filesystem/util.rs b/crates/nu-command/src/filesystem/util.rs index 1b755875bda09..8c1dfcf85f639 100644 --- a/crates/nu-command/src/filesystem/util.rs +++ b/crates/nu-command/src/filesystem/util.rs @@ -103,7 +103,7 @@ pub fn get_rest_for_glob_pattern( let mut output = vec![]; let eval_expression = get_eval_expression(engine_state); - for result in call.rest_iter_flattened(starting_pos, |expr| { + for result in call.rest_iter_flattened(&engine_state, starting_pos, |expr| { let result = eval_expression(engine_state, stack, expr); match result { Err(e) => Err(e), diff --git a/crates/nu-command/src/formats/from/csv.rs b/crates/nu-command/src/formats/from/csv.rs index b00dd5022b1e4..3db4da62dbd3f 100644 --- a/crates/nu-command/src/formats/from/csv.rs +++ b/crates/nu-command/src/formats/from/csv.rs @@ -141,7 +141,7 @@ fn from_csv( } else { return Err(ShellError::NonUtf8Custom { msg: "separator should be a single char or a 4-byte unicode".into(), - span: call.span(), + span: call.span(&engine_state), }); } } diff --git a/crates/nu-command/src/generators/cal.rs b/crates/nu-command/src/generators/cal.rs index a257f3ab7c907..215469030f088 100644 --- a/crates/nu-command/src/generators/cal.rs +++ b/crates/nu-command/src/generators/cal.rs @@ -3,6 +3,7 @@ use nu_color_config::StyleComputer; use nu_engine::command_prelude::*; use nu_protocol::ast::{Expr, Expression}; +use nu_protocol::engine::UNKNOWN_SPAN_ID; use std::collections::VecDeque; #[derive(Clone)] @@ -143,7 +144,7 @@ pub fn cal( style_computer, )?; - let mut table_no_index = Call::new(Span::unknown()); + let mut table_no_index = Call::new(Span::unknown(), Span::unknown()); table_no_index.add_named(( Spanned { item: "index".to_string(), @@ -155,6 +156,7 @@ pub fn cal( Span::unknown(), Type::Bool, )), + UNKNOWN_SPAN_ID, )); let cal_table_output = diff --git a/crates/nu-command/src/misc/tutor.rs b/crates/nu-command/src/misc/tutor.rs index 6b1c43534bf4f..361b4ada2789e 100644 --- a/crates/nu-command/src/misc/tutor.rs +++ b/crates/nu-command/src/misc/tutor.rs @@ -412,7 +412,7 @@ fn display(help: &str, engine_state: &EngineState, stack: &mut Stack, span: Span let result = decl.run( engine_state, stack, - &Call::new(span), + &Call::new(span, span.past()), Value::string(item, Span::unknown()).into_pipeline_data(), ); diff --git a/crates/nu-command/src/network/http/client.rs b/crates/nu-command/src/network/http/client.rs index 2c02d7be339f6..f0951a1885030 100644 --- a/crates/nu-command/src/network/http/client.rs +++ b/crates/nu-command/src/network/http/client.rs @@ -547,7 +547,7 @@ fn transform_response_using_content_type( Some(converter_id) => engine_state.get_decl(converter_id).run( engine_state, stack, - &Call::new(span), + &Call::new(span, span.past()), output, ), None => Ok(output), diff --git a/crates/nu-command/src/platform/is_terminal.rs b/crates/nu-command/src/platform/is_terminal.rs index c67329e8396cb..1d4d1628f9565 100644 --- a/crates/nu-command/src/platform/is_terminal.rs +++ b/crates/nu-command/src/platform/is_terminal.rs @@ -58,7 +58,12 @@ impl Command for IsTerminal { _ => { return Err(ShellError::IncompatibleParametersSingle { msg: "Only one stream may be checked".into(), - span: Span::merge_many(call.arguments.iter().map(|arg| arg.span())), + span: Span::merge_many( + call.arguments + .item + .iter() + .map(|arg| arg.span(&engine_state)), + ), }); } }; diff --git a/crates/nu-command/src/system/run_external.rs b/crates/nu-command/src/system/run_external.rs index f82c23a0e06db..398cd4919ac20 100644 --- a/crates/nu-command/src/system/run_external.rs +++ b/crates/nu-command/src/system/run_external.rs @@ -375,8 +375,12 @@ fn write_pipeline_data( Arc::make_mut(&mut engine_state.config).use_ansi_coloring = false; // Invoke the `table` command. - let output = - crate::Table.run(&engine_state, &mut stack, &Call::new(Span::unknown()), data)?; + let output = crate::Table.run( + &engine_state, + &mut stack, + &Call::new(Span::unknown(), Span::unknown()), + data, + )?; // Write the output. for value in output { diff --git a/crates/nu-command/src/viewers/table.rs b/crates/nu-command/src/viewers/table.rs index f0cc90fa9f299..07d761a615b3d 100644 --- a/crates/nu-command/src/viewers/table.rs +++ b/crates/nu-command/src/viewers/table.rs @@ -127,7 +127,7 @@ impl Command for Table { return Ok(val.into_pipeline_data()); } - let cfg = parse_table_config(call, engine_state, stack)?; + let cfg = parse_table_config(engine_state, call, engine_state, stack)?; let input = CmdInput::new(engine_state, stack, call, input); // reset vt processing, aka ansi because illbehaved externals can break it @@ -247,6 +247,7 @@ impl TableConfig { } fn parse_table_config( + engine_state: &EngineState, call: &Call, state: &EngineState, stack: &mut Stack, @@ -269,9 +270,9 @@ fn parse_table_config( flatten_separator, }, }; - let theme = - get_theme_flag(call, state, stack)?.unwrap_or_else(|| get_config(state, stack).table_mode); - let index = get_index_flag(call, state, stack)?; + let theme = get_theme_flag(engine_state, call, state, stack)? + .unwrap_or_else(|| get_config(state, stack).table_mode); + let index = get_index_flag(engine_state, call, state, stack)?; let term_width = get_width_param(width_param); @@ -285,6 +286,7 @@ fn parse_table_config( } fn get_index_flag( + engine_state: &EngineState, call: &Call, state: &EngineState, stack: &mut Stack, @@ -308,7 +310,7 @@ fn get_index_flag( Err(ShellError::UnsupportedInput { msg: String::from("got a negative integer"), input: val.to_string(), - msg_span: call.span(), + msg_span: call.span(&engine_state), input_span: internal_span, }) } else { @@ -319,13 +321,14 @@ fn get_index_flag( _ => Err(ShellError::CantConvert { to_type: String::from("index"), from_type: String::new(), - span: call.span(), + span: call.span(&engine_state), help: Some(String::from("supported values: [bool, int, nothing]")), }), } } fn get_theme_flag( + engine_state: &EngineState, call: &Call, state: &EngineState, stack: &mut Stack, @@ -335,7 +338,7 @@ fn get_theme_flag( TableMode::from_str(&theme).map_err(|err| ShellError::CantConvert { to_type: String::from("theme"), from_type: String::from("string"), - span: call.span(), + span: call.span(&engine_state), help: Some(format!("{}, but found '{}'.", err, theme)), }) }) diff --git a/crates/nu-engine/src/call_ext.rs b/crates/nu-engine/src/call_ext.rs index daedb24a1fc0b..4a577a6103d8c 100644 --- a/crates/nu-engine/src/call_ext.rs +++ b/crates/nu-engine/src/call_ext.rs @@ -113,7 +113,7 @@ impl CallExt for Call { let stack = &mut stack.use_call_arg_out_dest(); let mut output = vec![]; - for result in self.rest_iter_flattened(starting_pos, |expr| { + for result in self.rest_iter_flattened(&engine_state, starting_pos, |expr| { eval_expression::(engine_state, stack, expr) })? { output.push(FromValue::from_value(result)?); diff --git a/crates/nu-engine/src/documentation.rs b/crates/nu-engine/src/documentation.rs index a7d4950036bb2..ccc55ce8be723 100644 --- a/crates/nu-engine/src/documentation.rs +++ b/crates/nu-engine/src/documentation.rs @@ -48,7 +48,7 @@ fn nu_highlight_string(code_string: &str, engine_state: &EngineState, stack: &mu if let Ok(output) = decl.run( engine_state, stack, - &Call::new(Span::unknown()), + &Call::new(Span::unknown(), Span::unknown()), Value::string(code_string, Span::unknown()).into_pipeline_data(), ) { let result = output.into_value(Span::unknown()); @@ -241,7 +241,10 @@ fn get_documentation( &Call { decl_id, head: span, - arguments: vec![], + arguments: Spanned { + item: vec![], + span: span.past(), + }, parser_info: HashMap::new(), }, PipelineData::Value(Value::list(vals, span), None), @@ -273,7 +276,7 @@ fn get_documentation( match decl.run( engine_state, stack, - &Call::new(Span::unknown()), + &Call::new(Span::unknown(), Span::unknown()), Value::string(example.example, Span::unknown()).into_pipeline_data(), ) { Ok(output) => { @@ -296,7 +299,7 @@ fn get_documentation( } if let Some(result) = &example.result { - let mut table_call = Call::new(Span::unknown()); + let mut table_call = Call::new(Span::unknown(), Span::unknown()); if example.example.ends_with("--collapse") { // collapse the result table_call.add_named(( @@ -306,6 +309,7 @@ fn get_documentation( }, None, None, + UNKNOWN_SPAN_ID, )) } else { // expand the result @@ -316,6 +320,7 @@ fn get_documentation( }, None, None, + UNKNOWN_SPAN_ID, )) } let table = engine_state @@ -368,13 +373,17 @@ fn get_ansi_color_for_component_or_default( // Call ansi command using argument if let Some(argument) = argument_opt { if let Some(decl_id) = engine_state.find_decl(b"ansi", &[]) { + let arg_span = argument.span(&engine_state); if let Ok(result) = eval_call::( engine_state, caller_stack, &Call { decl_id, head: span, - arguments: vec![argument], + arguments: Spanned { + item: vec![argument], + span: arg_span, + }, parser_info: HashMap::new(), }, PipelineData::Empty, diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index 044bf86980524..15bcd61f3ed65 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -26,7 +26,7 @@ pub fn eval_call( } let decl = engine_state.get_decl(call.decl_id); - if !decl.is_known_external() && call.named_iter().any(|(flag, _, _)| flag.item == "help") { + if !decl.is_known_external() && call.named_iter().any(|(flag, _, _, _)| flag.item == "help") { let help = get_full_help(decl, engine_state, caller_stack); Ok(Value::string(help, call.head).into_pipeline_data()) } else if let Some(block_id) = decl.block_id() { @@ -100,6 +100,7 @@ pub fn eval_call( let mut rest_items = vec![]; for result in call.rest_iter_flattened( + &engine_state, decl.signature().required_positional.len() + decl.signature().optional_positional.len(), |expr| eval_expression::(engine_state, caller_stack, expr), @@ -213,8 +214,16 @@ fn eval_external( })?; let command = engine_state.get_decl(decl_id); + let spans: Vec = args + .iter() + .map(|arg| match arg { + ExternalArgument::Regular(expr) | ExternalArgument::Spread(expr) => { + expr.span(&engine_state) + } + }) + .collect(); - let mut call = Call::new(head.span(&engine_state)); + let mut call = Call::new(head.span(&engine_state), Span::concat(&spans)); call.add_positional(head.clone()); diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 88638b24755e9..09b48ccaee10c 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -252,7 +252,7 @@ fn flatten_expression_into( } let arg_start = output.len(); - for arg in &call.arguments { + for arg in &call.arguments.item { match arg { Argument::Positional(positional) | Argument::Unknown(positional) => { flatten_expression_into(working_set, positional, output) diff --git a/crates/nu-parser/src/known_external.rs b/crates/nu-parser/src/known_external.rs index 453112aa320df..6b3d024d98984 100644 --- a/crates/nu-parser/src/known_external.rs +++ b/crates/nu-parser/src/known_external.rs @@ -43,7 +43,7 @@ impl Command for KnownExternal { let command = engine_state.get_decl(decl_id); - let mut extern_call = Call::new(head_span); + let mut extern_call = Call::new(head_span, call.arguments_span()); let extern_name = if let Some(name_bytes) = engine_state.find_decl_name(call.decl_id, &[]) { String::from_utf8_lossy(name_bytes) @@ -78,7 +78,7 @@ impl Command for KnownExternal { )); } - for arg in &call.arguments { + for arg in &call.arguments.item { match arg { Argument::Positional(positional) => extern_call.add_positional(positional.clone()), Argument::Named(named) => { diff --git a/crates/nu-parser/src/lib.rs b/crates/nu-parser/src/lib.rs index 89cba8d243cdd..5e3e73987df4d 100644 --- a/crates/nu-parser/src/lib.rs +++ b/crates/nu-parser/src/lib.rs @@ -21,8 +21,9 @@ pub use nu_protocol::parser_path::*; pub use parse_keywords::*; pub use parser::{ - is_math_expression_like, parse, parse_block, parse_expression, parse_external_call, - parse_unit_value, trim_quotes, trim_quotes_str, unescape_unquote_string, DURATION_UNIT_GROUPS, + is_math_expression_like, named_arg_span, parse, parse_block, parse_expression, + parse_external_call, parse_unit_value, trim_quotes, trim_quotes_str, unescape_unquote_string, + DURATION_UNIT_GROUPS, }; #[cfg(feature = "plugin")] diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index 70d217c5649b0..089c0b2fbadef 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -123,8 +123,8 @@ pub fn parse_keyword(working_set: &mut StateWorkingSet, lite_command: &LiteComma // Apply parse keyword side effects let cmd = working_set.get_decl(call.decl_id); // check help flag first. - if call.named_iter().any(|(flag, _, _)| flag.item == "help") { - let call_span = call.span(); + if call.named_iter().any(|(flag, _, _, _)| flag.item == "help") { + let call_span = call.span(working_set); return Pipeline::from_vec(vec![Expression::new( working_set, Expr::Call(call), @@ -1056,7 +1056,7 @@ pub fn parse_alias( // First from comments, if any are present false => working_set.build_usage(&lite_command.comments), // Then from the command itself - true => match alias_call.arguments.get(1) { + true => match alias_call.arguments.item.get(1) { Some(Argument::Positional(Expression { expr: Expr::Keyword(kw), .. @@ -1258,7 +1258,10 @@ pub fn parse_export_in_module( let mut call = Box::new(Call { head: spans[0], decl_id: export_decl_id, - arguments: vec![], + arguments: Spanned { + item: vec![], + span: spans[0].past(), + }, parser_info: HashMap::new(), }); @@ -2263,13 +2266,21 @@ pub fn parse_module( .find_decl(b"module") .expect("internal error: missing module command"); + let args_span = Span::concat(&[ + module_name_or_path_expr.span(working_set), + block_expr.span(working_set), + ]); + let call = Box::new(Call { head: Span::concat(&spans[..split_id]), decl_id: module_decl_id, - arguments: vec![ - Argument::Positional(module_name_or_path_expr), - Argument::Positional(block_expr), - ], + arguments: Spanned { + item: vec![ + Argument::Positional(module_name_or_path_expr), + Argument::Positional(block_expr), + ], + span: args_span, + }, parser_info: HashMap::new(), }); @@ -2701,7 +2712,7 @@ pub fn parse_hide(working_set: &mut StateWorkingSet, lite_command: &LiteCommand) } pub fn parse_overlay_new(working_set: &mut StateWorkingSet, call: Box) -> Pipeline { - let call_span = call.span(); + let call_span = call.span(working_set); let (overlay_name, _) = if let Some(expr) = call.positional_nth(0) { match eval_constant(working_set, expr) { @@ -2750,7 +2761,7 @@ pub fn parse_overlay_new(working_set: &mut StateWorkingSet, call: Box) -> } pub fn parse_overlay_use(working_set: &mut StateWorkingSet, call: Box) -> Pipeline { - let call_span = call.span(); + let call_span = call.span(working_set); let (overlay_name, overlay_name_span) = if let Some(expr) = call.positional_nth(0) { match eval_constant(working_set, expr) { @@ -2973,7 +2984,7 @@ pub fn parse_overlay_use(working_set: &mut StateWorkingSet, call: Box) -> } pub fn parse_overlay_hide(working_set: &mut StateWorkingSet, call: Box) -> Pipeline { - let call_span = call.span(); + let call_span = call.span(working_set); let (overlay_name, overlay_name_span) = if let Some(expr) = call.positional_nth(0) { match eval_constant(working_set, expr) { @@ -3113,10 +3124,16 @@ pub fn parse_let(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline } } + let args_span = + Span::concat(&[lvalue.span(working_set), rvalue.span(working_set)]); + let call = Box::new(Call { decl_id, head: spans[0], - arguments: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + arguments: Spanned { + item: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + span: args_span, + }, parser_info: HashMap::new(), }); @@ -3259,10 +3276,16 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin } } + let args_span = + Span::concat(&[lvalue.span(working_set), rvalue.span(working_set)]); + let call = Box::new(Call { decl_id, head: spans[0], - arguments: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + arguments: Spanned { + item: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + span: args_span, + }, parser_info: HashMap::new(), }); @@ -3378,10 +3401,16 @@ pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline } } + let args_span = + Span::concat(&[lvalue.span(working_set), rvalue.span(working_set)]); + let call = Box::new(Call { decl_id, head: spans[0], - arguments: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + arguments: Spanned { + item: vec![Argument::Positional(lvalue), Argument::Positional(rvalue)], + span: args_span, + }, parser_info: HashMap::new(), }); @@ -3884,8 +3913,8 @@ pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box) -> P let plugin_config = call .named_iter() - .find(|(arg_name, _, _)| arg_name.item == "plugin-config") - .map(|(_, _, expr)| { + .find(|(arg_name, _, _, _)| arg_name.item == "plugin-config") + .map(|(_, _, expr, _)| { let expr = expr .as_ref() .expect("--plugin-config arg should have been checked already"); @@ -3961,7 +3990,7 @@ pub fn parse_plugin_use(working_set: &mut StateWorkingSet, call: Box) -> P working_set.error(err); } - let call_span = call.span(); + let call_span = call.span(working_set); Pipeline::from_vec(vec![Expression::new( working_set, @@ -4147,6 +4176,6 @@ fn detect_params_in_name( /// Run has_flag_const and push possible error to working_set fn has_flag_const(working_set: &mut StateWorkingSet, call: &Call, name: &str) -> Result { call.has_flag_const(working_set, name).map_err(|err| { - working_set.error(err.wrap(working_set, call.span())); + working_set.error(err.wrap(working_set, call.span(working_set))); }) } diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index c3d18b55aca38..d5131d0137880 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -160,7 +160,7 @@ pub(crate) fn check_call( call: &Call, ) { // Allow the call to pass if they pass in the help flag - if call.named_iter().any(|(n, _, _)| n.item == "help") { + if call.named_iter().any(|(n, _, _, _)| n.item == "help") { return; } @@ -180,7 +180,7 @@ pub(crate) fn check_call( if let Some(last) = call.positional_iter().last() { working_set.error(ParseError::MissingPositional( argument.name.clone(), - Span::new(last.span.end, last.span.end), + Span::new(last.span(working_set).end, last.span(working_set).end), sig.call_signature(), )); return; @@ -199,7 +199,7 @@ pub(crate) fn check_call( if let Some(last) = call.positional_iter().last() { working_set.error(ParseError::MissingPositional( missing.name.clone(), - Span::new(last.span.end, last.span.end), + Span::new(last.span(working_set).end, last.span(working_set).end), sig.call_signature(), )) } else { @@ -211,7 +211,10 @@ pub(crate) fn check_call( } } else { for req_flag in sig.named.iter().filter(|x| x.required) { - if call.named_iter().all(|(n, _, _)| n.item != req_flag.long) { + if call + .named_iter() + .all(|(n, _, _, _)| n.item != req_flag.long) + { working_set.error(ParseError::MissingRequiredFlag( req_flag.long.clone(), command, @@ -486,18 +489,19 @@ fn ensure_flag_arg_type( arg_shape: &SyntaxShape, long_name_span: Span, ) -> (Spanned, Expression) { + let arg_span = arg.span(working_set); if !type_compatible(&arg.ty, &arg_shape.to_type()) { working_set.error(ParseError::TypeMismatch( arg_shape.to_type(), arg.ty, - arg.span, + arg_span, )); ( Spanned { item: arg_name, span: long_name_span, }, - Expression::garbage(working_set, arg.span), + Expression::garbage(working_set, arg_span), ) } else { ( @@ -916,6 +920,25 @@ pub struct ParsedInternalCall { pub output: Type, } +// moved from call.rs since span creation must be done at parse time now +pub fn named_arg_span( + working_set: &StateWorkingSet, + named: &Spanned, + short: &Option>, + expr: &Option, +) -> Span { + let start = named.span.start; + let end = if let Some(expr) = expr { + expr.span(&working_set).end + } else if let Some(short) = short { + short.span.end + } else { + named.span.end + }; + + Span::new(start, end) +} + pub fn parse_internal_call( working_set: &mut StateWorkingSet, command_span: Span, @@ -924,7 +947,7 @@ pub fn parse_internal_call( ) -> ParsedInternalCall { trace!("parsing: internal call (decl id: {})", decl_id); - let mut call = Call::new(command_span); + let mut call = Call::new(command_span, Span::concat(spans)); call.decl_id = decl_id; call.head = command_span; let _ = working_set.add_span(call.head); @@ -1002,7 +1025,9 @@ pub fn parse_internal_call( call.add_unknown(arg); } else { - call.add_named((long_name, None, arg)); + let named_span = named_arg_span(working_set, &long_name, &None, &arg); + let named_span_id = working_set.add_span(named_span); + call.add_named((long_name, None, arg, named_span_id)); } spans_idx += 1; @@ -1053,27 +1078,30 @@ pub fn parse_internal_call( if flag.long.is_empty() { if let Some(short) = flag.short { - call.add_named(( - Spanned { - item: String::new(), - span: spans[spans_idx], - }, - Some(Spanned { - item: short.to_string(), - span: spans[spans_idx], - }), - Some(arg), - )); + let named = Spanned { + item: String::new(), + span: spans[spans_idx], + }; + let short = Some(Spanned { + item: short.to_string(), + span: spans[spans_idx], + }); + let expr = Some(arg); + let named_span = + named_arg_span(working_set, &named, &short, &expr); + let named_span_id = working_set.add_span(named_span); + call.add_named((named, short, expr, named_span_id)); } } else { - call.add_named(( - Spanned { - item: flag.long.clone(), - span: spans[spans_idx], - }, - None, - Some(arg), - )); + let named = Spanned { + item: flag.long.clone(), + span: spans[spans_idx], + }; + let short = None; + let expr = Some(arg); + let named_span = named_arg_span(working_set, &named, &short, &expr); + let named_span_id = working_set.add_span(named_span); + call.add_named((named, short, expr, named_span_id)); } spans_idx += 1; } else { @@ -1084,27 +1112,29 @@ pub fn parse_internal_call( } } else if flag.long.is_empty() { if let Some(short) = flag.short { - call.add_named(( - Spanned { - item: String::new(), - span: spans[spans_idx], - }, - Some(Spanned { - item: short.to_string(), - span: spans[spans_idx], - }), - None, - )); + let named = Spanned { + item: String::new(), + span: spans[spans_idx], + }; + let short = Some(Spanned { + item: short.to_string(), + span: spans[spans_idx], + }); + let expr = None; + let named_span = named_arg_span(working_set, &named, &short, &expr); + let named_span_id = working_set.add_span(named_span); + call.add_named((named, short, expr, named_span_id)); } } else { - call.add_named(( - Spanned { - item: flag.long.clone(), - span: spans[spans_idx], - }, - None, - None, - )); + let named = Spanned { + item: flag.long.clone(), + span: spans[spans_idx], + }; + let short = None; + let expr = None; + let named_span = named_arg_span(working_set, &named, &short, &expr); + let named_span_id = working_set.add_span(named_span); + call.add_named((named, short, expr, named_span_id)); } } } @@ -1184,12 +1214,13 @@ pub fn parse_internal_call( ); let arg = if !type_compatible(&positional.shape.to_type(), &arg.ty) { + let arg_ty = arg.ty.clone(); working_set.error(ParseError::TypeMismatch( positional.shape.to_type(), - arg.ty, - arg.span, + arg_ty, + arg.span(working_set), )); - Expression::garbage(working_set, arg.span) + Expression::garbage(working_set, arg.span(working_set)) } else { arg }; @@ -1311,7 +1342,7 @@ pub fn parse_call(working_set: &mut StateWorkingSet, spans: &[Span], head: Span) trace!("parsing: alias of external call"); let mut head = head.clone(); - head.span = spans[0]; // replacing the spans preserves syntax highlighting + head.span_id = working_set.add_span(spans[0]); // replacing the spans preserves syntax highlighting let mut final_args = args.clone().into_vec(); for arg_span in &spans[1..] { @@ -3006,13 +3037,15 @@ pub fn parse_import_pattern(working_set: &mut StateWorkingSet, spans: &[Span]) - for item in list { match item { ListItem::Item(expr) => { - let contents = working_set.get_span_contents(expr.span); - output.push((trim_quotes(contents).to_vec(), expr.span)); + let contents = + working_set.get_span_contents(expr.span(working_set)); + output + .push((trim_quotes(contents).to_vec(), expr.span(working_set))); } ListItem::Spread(_, spread) => { working_set.error(ParseError::WrongImportPattern( "cannot spread in an import pattern".into(), - spread.span, + spread.span(working_set), )) } } @@ -3022,7 +3055,7 @@ pub fn parse_import_pattern(working_set: &mut StateWorkingSet, spans: &[Span]) - .members .push(ImportPatternMember::List { names: output }); } else { - working_set.error(ParseError::ExportNotFound(result.span)); + working_set.error(ParseError::ExportNotFound(result.span(working_set))); return Expression::new( working_set, Expr::ImportPattern(Box::new(import_pattern)), @@ -3779,7 +3812,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> format!( "expected default value to be `{var_type}`" ), - expression.span, + expression.span(working_set), ), ) } @@ -3792,7 +3825,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> Some(constant) } else { working_set.error(ParseError::NonConstantDefaultValue( - expression.span, + expression.span(working_set), )); None }; @@ -3806,7 +3839,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> working_set.error(ParseError::AssignmentMismatch( "Rest parameter was given a default value".into(), "can't have default value".into(), - expression.span, + expression.span(working_set), )) } Arg::Flag { @@ -3819,7 +3852,7 @@ pub fn parse_signature_helper(working_set: &mut StateWorkingSet, span: Span) -> }, type_annotated, } => { - let expression_span = expression.span; + let expression_span = expression.span(working_set); *default_value = if let Ok(value) = eval_constant(working_set, &expression) @@ -4065,7 +4098,7 @@ fn parse_table_row( list.into_iter() .map(|item| match item { ListItem::Item(expr) => Ok(expr), - ListItem::Spread(_, spread) => Err(spread.span), + ListItem::Spread(_, spread) => Err(spread.span(working_set)), }) .collect::>() .map(|exprs| (exprs, span)) @@ -4140,7 +4173,7 @@ fn parse_table_expression(working_set: &mut StateWorkingSet, span: Span) -> Expr } Ordering::Greater => { let span = { - let start = list[head.len()].span.start; + let start = list[head.len()].span(working_set).start; let end = span.end; Span::new(start, end) }; @@ -4179,7 +4212,7 @@ fn parse_table_expression(working_set: &mut StateWorkingSet, span: Span) -> Expr }; let ty = if working_set.parse_errors.len() == errors { - let (ty, errs) = table_type(&head, &rows); + let (ty, errs) = table_type(working_set, &head, &rows); working_set.parse_errors.extend(errs); ty } else { @@ -4194,7 +4227,11 @@ fn parse_table_expression(working_set: &mut StateWorkingSet, span: Span) -> Expr Expression::new(working_set, Expr::Table(table), span, ty) } -fn table_type(head: &[Expression], rows: &[Vec]) -> (Type, Vec) { +fn table_type( + working_set: &StateWorkingSet, + head: &[Expression], + rows: &[Vec], +) -> (Type, Vec) { let mut errors = vec![]; let mut rows = rows.to_vec(); let mut mk_ty = || -> Type { @@ -4224,7 +4261,7 @@ fn table_type(head: &[Expression], rows: &[Vec]) -> (Type, Vec Ex let expr = Expr::Call(Box::new(Call { head: Span::unknown(), decl_id, - arguments, + arguments: Spanned { + item: arguments, + span: Span::concat(spans), + }, parser_info: HashMap::new(), })); @@ -6094,7 +6134,7 @@ pub fn discover_captures_in_expr( } } - for arg in &call.arguments { + for arg in &call.arguments.item { match arg { Argument::Named(named) => { if let Some(arg) = &named.2 { @@ -6249,7 +6289,7 @@ pub fn discover_captures_in_expr( } Expr::Var(var_id) => { if (*var_id > ENV_VARIABLE_ID || *var_id == IN_VARIABLE_ID) && !seen.contains(var_id) { - output.push((*var_id, expr.span)); + output.push((*var_id, expr.span(&working_set))); } } Expr::VarDecl(var_id) => { @@ -6294,7 +6334,7 @@ fn wrap_element_with_collect( } fn wrap_expr_with_collect(working_set: &mut StateWorkingSet, expr: &Expression) -> Expression { - let span = expr.span; + let span = expr.span(working_set); if let Some(decl_id) = working_set.find_decl(b"collect") { let mut output = vec![]; @@ -6324,14 +6364,15 @@ fn wrap_expr_with_collect(working_set: &mut StateWorkingSet, expr: &Expression) Type::Any, ))); - output.push(Argument::Named(( - Spanned { - item: "keep-env".to_string(), - span: Span::new(0, 0), - }, - None, - None, - ))); + let named = Spanned { + item: "keep-env".to_string(), + span: Span::unknown(), + }; + let short = None; + let expr = None; + let named_span = named_arg_span(working_set, &named, &short, &expr); + let named_span_id = working_set.add_span(named_span); + output.push(Argument::Named((named, short, expr, named_span_id))); // The containing, synthetic call to `collect`. // We don't want to have a real span as it will confuse flattening @@ -6339,8 +6380,11 @@ fn wrap_expr_with_collect(working_set: &mut StateWorkingSet, expr: &Expression) Expression::new( working_set, Expr::Call(Box::new(Call { - head: Span::new(0, 0), - arguments: output, + head: Span::unknown(), + arguments: Spanned { + item: output, + span: Span::unknown(), + }, decl_id, parser_info: HashMap::new(), })), diff --git a/crates/nu-parser/tests/test_parser.rs b/crates/nu-parser/tests/test_parser.rs index 0784fe69d4a66..da3cfbf9fdc6a 100644 --- a/crates/nu-parser/tests/test_parser.rs +++ b/crates/nu-parser/tests/test_parser.rs @@ -598,9 +598,12 @@ pub fn parse_call_short_flag_batch_arg_allowed() { if let Expr::Call(call) = &element.expr.expr { assert_eq!(call.decl_id, 0); - assert_eq!(call.arguments.len(), 2); - matches!(call.arguments[0], Argument::Named((_, None, None))); - matches!(call.arguments[1], Argument::Named((_, None, Some(_)))); + assert_eq!(call.arguments.item.len(), 2); + matches!(call.arguments.item[0], Argument::Named((_, None, None, _))); + matches!( + call.arguments.item[1], + Argument::Named((_, None, Some(_), _)) + ); } } @@ -2150,7 +2153,7 @@ mod input_types { assert!(pipeline.elements[3].redirection.is_none()); match &pipeline.elements[3].expr.expr { Expr::Call(call) => { - let arg = &call.arguments[0]; + let arg = &call.arguments.item[0]; match arg { Argument::Positional(a) => match &a.expr { Expr::FullCellPath(path) => match &path.head.expr { @@ -2347,3 +2350,92 @@ mod operator { ); } } + +#[cfg(test)] +mod test { + use nu_parser::named_arg_span; + use nu_protocol::ast::{Argument, Call, Expression}; + use nu_protocol::engine::{EngineState, StateWorkingSet}; + use nu_protocol::{Span, Spanned}; + + #[test] + fn argument_span_named() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let named_span = Span::new(2, 3); + let short_span = Span::new(5, 7); + let expr_span = Span::new(11, 13); + + let _ = working_set.add_span(named_span); + let _ = working_set.add_span(short_span); + let _ = working_set.add_span(expr_span); + + let named = Spanned { + item: "named".to_string(), + span: named_span, + }; + let short = Spanned { + item: "short".to_string(), + span: short_span, + }; + let expr = Expression::garbage(&mut working_set, expr_span); + + let arg_span = named_arg_span(&working_set, &named, &None, &None); + assert_eq!(Span::new(2, 3), arg_span); + + let arg_span = named_arg_span(&working_set, &named, &Some(short.clone()), &None); + assert_eq!(Span::new(2, 7), arg_span); + + let arg_span = named_arg_span(&working_set, &named, &None, &Some(expr.clone())); + assert_eq!(Span::new(2, 13), arg_span); + + let arg_span = named_arg_span( + &working_set, + &named, + &Some(short.clone()), + &Some(expr.clone()), + ); + assert_eq!(Span::new(2, 13), arg_span); + } + + #[test] + fn argument_span_positional() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let span = Span::new(2, 3); + let _ = working_set.add_span(span); + let expr = Expression::garbage(&mut working_set, span); + let arg = Argument::Positional(expr); + + assert_eq!(span, arg.span(&working_set)); + } + + #[test] + fn argument_span_unknown() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let span = Span::new(2, 3); + let _ = working_set.add_span(span); + let expr = Expression::garbage(&mut working_set, span); + let arg = Argument::Unknown(expr); + + assert_eq!(span, arg.span(&working_set)); + } + + #[test] + fn call_arguments_span() { + let engine_state = EngineState::new(); + let mut working_set = StateWorkingSet::new(&engine_state); + + let arg1_span = Span::new(2, 3); + let arg2_span = Span::new(5, 7); + let mut call = Call::new(Span::new(0, 1), Span::concat(&[arg1_span, arg2_span])); + call.add_positional(Expression::garbage(&mut working_set, arg1_span)); + call.add_positional(Expression::garbage(&mut working_set, arg2_span)); + + assert_eq!(Span::new(2, 7), call.arguments_span()); + } +} diff --git a/crates/nu-plugin-protocol/src/evaluated_call.rs b/crates/nu-plugin-protocol/src/evaluated_call.rs index 19f9049340b3b..1aa0ded2bffbb 100644 --- a/crates/nu-plugin-protocol/src/evaluated_call.rs +++ b/crates/nu-plugin-protocol/src/evaluated_call.rs @@ -34,11 +34,12 @@ impl EvaluatedCall { stack: &mut Stack, eval_expression_fn: fn(&EngineState, &mut Stack, &Expression) -> Result, ) -> Result { - let positional = - call.rest_iter_flattened(0, |expr| eval_expression_fn(engine_state, stack, expr))?; + let positional = call.rest_iter_flattened(&engine_state, 0, |expr| { + eval_expression_fn(engine_state, stack, expr) + })?; let mut named = Vec::with_capacity(call.named_len()); - for (string, _, expr) in call.named_iter() { + for (string, _, expr, _) in call.named_iter() { let value = match expr { None => None, Some(expr) => Some(eval_expression_fn(engine_state, stack, expr)?), diff --git a/crates/nu-protocol/src/ast/call.rs b/crates/nu-protocol/src/ast/call.rs index 6e8cc64a0576c..bc1c163e4fc43 100644 --- a/crates/nu-protocol/src/ast/call.rs +++ b/crates/nu-protocol/src/ast/call.rs @@ -4,42 +4,47 @@ use serde::{Deserialize, Serialize}; use crate::{ ast::Expression, engine::StateWorkingSet, eval_const::eval_constant, DeclId, FromValue, - ShellError, Span, Spanned, Value, + GetSpan, ShellError, Span, SpanId, Spanned, Value, }; #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub enum Argument { Positional(Expression), - Named((Spanned, Option>, Option)), + Named( + ( + Spanned, + Option>, + Option, + SpanId, + ), + ), Unknown(Expression), // unknown argument used in "fall-through" signatures Spread(Expression), // a list spread to fill in rest arguments } impl Argument { - /// The span for an argument - pub fn span(&self) -> Span { + pub fn span_id(&self) -> SpanId { match self { - Argument::Positional(e) => e.span, - Argument::Named((named, short, expr)) => { - let start = named.span.start; - let end = if let Some(expr) = expr { - expr.span.end - } else if let Some(short) = short { - short.span.end - } else { - named.span.end - }; + Argument::Positional(e) => e.span_id, + Argument::Named((_, _, _, span_id)) => *span_id, + Argument::Unknown(e) => e.span_id, + Argument::Spread(e) => e.span_id, + } + } - Span::new(start, end) - } - Argument::Unknown(e) => e.span, - Argument::Spread(e) => e.span, + /// The span for an argument + pub fn span(&self, state: &impl GetSpan) -> Span { + match self { + Argument::Positional(e) => e.span(state), + Argument::Named((_, _, _, span_id)) => state.get_span(*span_id), + Argument::Unknown(e) => e.span(state), + Argument::Spread(e) => e.span(state), } } pub fn expr(&self) -> Option<&Expression> { match self { - Argument::Named((_, _, expr)) => expr.as_ref(), + Argument::Named((_, _, expr, _)) => expr.as_ref(), Argument::Positional(expr) | Argument::Unknown(expr) | Argument::Spread(expr) => { Some(expr) } @@ -58,17 +63,20 @@ pub struct Call { /// identifier of the declaration to call pub decl_id: DeclId, pub head: Span, - pub arguments: Vec, + pub arguments: Spanned>, /// this field is used by the parser to pass additional command-specific information pub parser_info: HashMap, } impl Call { - pub fn new(head: Span) -> Call { + pub fn new(head: Span, args_span: Span) -> Call { Self { decl_id: 0, head, - arguments: vec![], + arguments: Spanned { + item: vec![], + span: args_span, + }, parser_info: HashMap::new(), } } @@ -80,23 +88,20 @@ impl Call { /// If there are one or more arguments the span encompasses the start of the first argument to /// end of the last argument pub fn arguments_span(&self) -> Span { - let past = self.head.past(); - - let start = self - .arguments - .first() - .map(|a| a.span()) - .unwrap_or(past) - .start; - let end = self.arguments.last().map(|a| a.span()).unwrap_or(past).end; - - Span::new(start, end) + self.arguments.span } pub fn named_iter( &self, - ) -> impl Iterator, Option>, Option)> { - self.arguments.iter().filter_map(|arg| match arg { + ) -> impl Iterator< + Item = &( + Spanned, + Option>, + Option, + SpanId, + ), + > { + self.arguments.item.iter().filter_map(|arg| match arg { Argument::Named(named) => Some(named), Argument::Positional(_) => None, Argument::Unknown(_) => None, @@ -106,9 +111,15 @@ impl Call { pub fn named_iter_mut( &mut self, - ) -> impl Iterator, Option>, Option)> - { - self.arguments.iter_mut().filter_map(|arg| match arg { + ) -> impl Iterator< + Item = &mut ( + Spanned, + Option>, + Option, + SpanId, + ), + > { + self.arguments.item.iter_mut().filter_map(|arg| match arg { Argument::Named(named) => Some(named), Argument::Positional(_) => None, Argument::Unknown(_) => None, @@ -122,25 +133,31 @@ impl Call { pub fn add_named( &mut self, - named: (Spanned, Option>, Option), + named: ( + Spanned, + Option>, + Option, + SpanId, + ), ) { - self.arguments.push(Argument::Named(named)); + self.arguments.item.push(Argument::Named(named)); } pub fn add_positional(&mut self, positional: Expression) { - self.arguments.push(Argument::Positional(positional)); + self.arguments.item.push(Argument::Positional(positional)); } pub fn add_unknown(&mut self, unknown: Expression) { - self.arguments.push(Argument::Unknown(unknown)); + self.arguments.item.push(Argument::Unknown(unknown)); } pub fn add_spread(&mut self, args: Expression) { - self.arguments.push(Argument::Spread(args)); + self.arguments.item.push(Argument::Spread(args)); } pub fn positional_iter(&self) -> impl Iterator { self.arguments + .item .iter() .take_while(|arg| match arg { Argument::Spread(_) => false, // Don't include positional arguments given to rest parameter @@ -168,6 +185,7 @@ impl Call { // todo maybe rewrite to be more elegant or something let args = self .arguments + .item .iter() .filter_map(|arg| match arg { Argument::Named(_) => None, @@ -258,9 +276,9 @@ impl Call { ) -> Result, ShellError> { let mut output = vec![]; - for result in - self.rest_iter_flattened(starting_pos, |expr| eval_constant(working_set, expr))? - { + for result in self.rest_iter_flattened(&working_set, starting_pos, |expr| { + eval_constant(working_set, expr) + })? { output.push(FromValue::from_value(result)?); } @@ -269,6 +287,7 @@ impl Call { pub fn rest_iter_flattened( &self, + state: &impl GetSpan, start: usize, mut eval: F, ) -> Result, ShellError> @@ -282,7 +301,11 @@ impl Call { if spread { match result { Value::List { mut vals, .. } => output.append(&mut vals), - _ => return Err(ShellError::CannotSpreadAsList { span: expr.span }), + _ => { + return Err(ShellError::CannotSpreadAsList { + span: expr.span(state), + }) + } } } else { output.push(result); @@ -310,23 +333,23 @@ impl Call { } } - pub fn span(&self) -> Span { + pub fn span(&self, state: &impl GetSpan) -> Span { let mut span = self.head; for positional in self.positional_iter() { - if positional.span.end > span.end { - span.end = positional.span.end; + if positional.span(state).end > span.end { + span.end = positional.span(state).end; } } - for (named, _, val) in self.named_iter() { + for (named, _, val, _) in self.named_iter() { if named.span.end > span.end { span.end = named.span.end; } if let Some(val) = &val { - if val.span.end > span.end { - span.end = val.span.end; + if val.span(state).end > span.end { + span.end = val.span(state).end; } } } @@ -334,77 +357,3 @@ impl Call { span } } - -#[cfg(test)] -mod test { - use super::*; - use crate::engine::EngineState; - - #[test] - fn argument_span_named() { - let engine_state = EngineState::new(); - let mut working_set = StateWorkingSet::new(&engine_state); - - let named = Spanned { - item: "named".to_string(), - span: Span::new(2, 3), - }; - let short = Spanned { - item: "short".to_string(), - span: Span::new(5, 7), - }; - let expr = Expression::garbage(&mut working_set, Span::new(11, 13)); - - let arg = Argument::Named((named.clone(), None, None)); - - assert_eq!(Span::new(2, 3), arg.span()); - - let arg = Argument::Named((named.clone(), Some(short.clone()), None)); - - assert_eq!(Span::new(2, 7), arg.span()); - - let arg = Argument::Named((named.clone(), None, Some(expr.clone()))); - - assert_eq!(Span::new(2, 13), arg.span()); - - let arg = Argument::Named((named.clone(), Some(short.clone()), Some(expr.clone()))); - - assert_eq!(Span::new(2, 13), arg.span()); - } - - #[test] - fn argument_span_positional() { - let engine_state = EngineState::new(); - let mut working_set = StateWorkingSet::new(&engine_state); - - let span = Span::new(2, 3); - let expr = Expression::garbage(&mut working_set, span); - let arg = Argument::Positional(expr); - - assert_eq!(span, arg.span()); - } - - #[test] - fn argument_span_unknown() { - let engine_state = EngineState::new(); - let mut working_set = StateWorkingSet::new(&engine_state); - - let span = Span::new(2, 3); - let expr = Expression::garbage(&mut working_set, span); - let arg = Argument::Unknown(expr); - - assert_eq!(span, arg.span()); - } - - #[test] - fn call_arguments_span() { - let engine_state = EngineState::new(); - let mut working_set = StateWorkingSet::new(&engine_state); - - let mut call = Call::new(Span::new(0, 1)); - call.add_positional(Expression::garbage(&mut working_set, Span::new(2, 3))); - call.add_positional(Expression::garbage(&mut working_set, Span::new(5, 7))); - - assert_eq!(Span::new(2, 7), call.arguments_span()); - } -} diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 040e140cab113..e03f7e81e9159 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -173,7 +173,7 @@ impl Expression { Expr::Binary(_) => false, Expr::Bool(_) => false, Expr::Call(call) => { - for arg in &call.arguments { + for arg in &call.arguments.item { match arg { Argument::Positional(expr) | Argument::Unknown(expr) @@ -366,7 +366,7 @@ impl Expression { if replaced.contains_span(call.head) { call.head = new_span; } - for arg in call.arguments.iter_mut() { + for arg in call.arguments.item.iter_mut() { match arg { Argument::Positional(expr) | Argument::Unknown(expr) diff --git a/crates/nu-protocol/src/engine/state_working_set.rs b/crates/nu-protocol/src/engine/state_working_set.rs index af950b832141b..61fd5982a290c 100644 --- a/crates/nu-protocol/src/engine/state_working_set.rs +++ b/crates/nu-protocol/src/engine/state_working_set.rs @@ -1023,16 +1023,32 @@ impl<'a> StateWorkingSet<'a> { impl<'a> GetSpan for &'a StateWorkingSet<'a> { fn get_span(&self, span_id: SpanId) -> Span { - let num_permanent_spans = self.permanent_state.num_spans(); - if span_id.0 < num_permanent_spans { - self.permanent_state.get_span(span_id) - } else { - *self - .delta - .spans - .get(span_id.0 - num_permanent_spans) - .expect("internal error: missing span") - } + get_span(self, span_id) + } +} + +impl<'a> GetSpan for &'a mut StateWorkingSet<'a> { + fn get_span(&self, span_id: SpanId) -> Span { + get_span(self, span_id) + } +} + +impl<'a> GetSpan for StateWorkingSet<'a> { + fn get_span(&self, span_id: SpanId) -> Span { + get_span(self, span_id) + } +} + +fn get_span(working_set: &StateWorkingSet, span_id: SpanId) -> Span { + let num_permanent_spans = working_set.permanent_state.num_spans(); + if span_id.0 < num_permanent_spans { + working_set.permanent_state.get_span(span_id) + } else { + *working_set + .delta + .spans + .get(span_id.0 - num_permanent_spans) + .expect("internal error: missing span") } } diff --git a/crates/nu-protocol/src/eval_const.rs b/crates/nu-protocol/src/eval_const.rs index 87913e4ee3c08..0b0859c23b75d 100644 --- a/crates/nu-protocol/src/eval_const.rs +++ b/crates/nu-protocol/src/eval_const.rs @@ -301,7 +301,7 @@ fn eval_const_call( return Err(ShellError::NotAConstCommand { span: call.head }); } - if !decl.is_known_external() && call.named_iter().any(|(flag, _, _)| flag.item == "help") { + if !decl.is_known_external() && call.named_iter().any(|(flag, _, _, _)| flag.item == "help") { // It would require re-implementing get_full_help() for const evaluation. Assuming that // getting help messages at parse-time is rare enough, we can simply disallow it. return Err(ShellError::NotAConstHelp { span: call.head }); diff --git a/crates/nu-protocol/src/pipeline/pipeline_data.rs b/crates/nu-protocol/src/pipeline/pipeline_data.rs index 03fbc64d21bc0..7d971fb518d0a 100644 --- a/crates/nu-protocol/src/pipeline/pipeline_data.rs +++ b/crates/nu-protocol/src/pipeline/pipeline_data.rs @@ -567,7 +567,7 @@ impl PipelineData { if command.block_id().is_some() { self.write_all_and_flush(engine_state, no_newline, to_stderr) } else { - let call = Call::new(Span::new(0, 0)); + let call = Call::new(Span::unknown(), Span::unknown()); let table = command.run(engine_state, stack, &call, self)?; table.write_all_and_flush(engine_state, no_newline, to_stderr) }