From 683b912263e9cd8e55b0416af3925fb5207b7863 Mon Sep 17 00:00:00 2001 From: Hristo Filaretov Date: Sat, 9 Apr 2022 04:55:02 +0200 Subject: [PATCH] Track call arguments in a single list (#5125) * Initial implementation of ordered call args * Run cargo fmt * Fix some clippy lints * Add positional len and nth * Cargo fmt * Remove more old nth calls * Good ole rustfmt * Add named len Co-authored-by: Hristo Filaretov --- .../src/completions/custom_completions.rs | 13 +-- crates/nu-command/src/core_commands/for_.rs | 8 +- crates/nu-command/src/core_commands/hide.rs | 8 +- crates/nu-command/src/core_commands/if_.rs | 4 +- crates/nu-command/src/core_commands/let_.rs | 8 +- .../nu-command/src/core_commands/metadata.rs | 2 +- crates/nu-command/src/core_commands/use_.rs | 2 +- crates/nu-command/src/env/let_env.rs | 4 +- crates/nu-command/src/env/with_env.rs | 8 +- crates/nu-command/src/filesystem/cd.rs | 2 +- crates/nu-command/src/filesystem/mkdir.rs | 9 +- crates/nu-command/src/filesystem/touch.rs | 16 ++- crates/nu-command/src/platform/ansi/ansi_.rs | 4 +- .../reedline_commands/keybindings_list.rs | 5 +- crates/nu-command/src/strings/build_string.rs | 3 +- crates/nu-command/src/strings/char_.rs | 9 +- crates/nu-engine/src/call_ext.rs | 12 +- crates/nu-engine/src/eval.rs | 16 +-- crates/nu-parser/src/flatten.rs | 4 +- crates/nu-parser/src/known_external.rs | 108 ++++++++---------- crates/nu-parser/src/parse_keywords.rs | 58 +++++----- crates/nu-parser/src/parser.rs | 55 +++++---- .../nu-plugin/src/protocol/evaluated_call.rs | 7 +- crates/nu-protocol/src/ast/call.rs | 76 +++++++++--- crates/nu-protocol/src/ast/expression.rs | 12 +- 25 files changed, 263 insertions(+), 190 deletions(-) diff --git a/crates/nu-cli/src/completions/custom_completions.rs b/crates/nu-cli/src/completions/custom_completions.rs index d8790d3a55..1f5334b03e 100644 --- a/crates/nu-cli/src/completions/custom_completions.rs +++ b/crates/nu-cli/src/completions/custom_completions.rs @@ -1,7 +1,7 @@ use crate::completions::{Completer, CompletionOptions, SortBy}; use nu_engine::eval_call; use nu_protocol::{ - ast::{Call, Expr, Expression}, + ast::{Argument, Call, Expr, Expression}, engine::{EngineState, Stack, StateWorkingSet}, PipelineData, Span, Type, Value, CONFIG_VARIABLE_ID, }; @@ -92,21 +92,20 @@ impl Completer for CustomCompletion { &Call { decl_id: self.decl_id, head: span, - positional: vec![ - Expression { + arguments: vec![ + Argument::Positional(Expression { span: Span { start: 0, end: 0 }, ty: Type::String, expr: Expr::String(self.line.clone()), custom_completion: None, - }, - Expression { + }), + Argument::Positional(Expression { span: Span { start: 0, end: 0 }, ty: Type::Int, expr: Expr::Int(line_pos as i64), custom_completion: None, - }, + }), ], - named: vec![], redirect_stdout: true, redirect_stderr: true, }, diff --git a/crates/nu-command/src/core_commands/for_.rs b/crates/nu-command/src/core_commands/for_.rs index c92ecfb5e2..e3e431f58b 100644 --- a/crates/nu-command/src/core_commands/for_.rs +++ b/crates/nu-command/src/core_commands/for_.rs @@ -61,11 +61,15 @@ https://www.nushell.sh/book/thinking_in_nushell.html#parsing-and-evaluation-are- _input: PipelineData, ) -> Result { let head = call.head; - let var_id = call.positional[0] + let var_id = call + .positional_nth(0) + .expect("checked through parser") .as_var() .expect("internal error: missing variable"); - let keyword_expr = call.positional[1] + let keyword_expr = call + .positional_nth(1) + .expect("checked through parser") .as_keyword() .expect("internal error: missing keyword"); let values = eval_expression(engine_state, stack, keyword_expr)?; diff --git a/crates/nu-command/src/core_commands/hide.rs b/crates/nu-command/src/core_commands/hide.rs index c35bad3c16..382281ed65 100644 --- a/crates/nu-command/src/core_commands/hide.rs +++ b/crates/nu-command/src/core_commands/hide.rs @@ -44,7 +44,7 @@ https://www.nushell.sh/book/thinking_in_nushell.html#parsing-and-evaluation-are- let import_pattern = if let Some(Expression { expr: Expr::ImportPattern(pat), .. - }) = call.positional.get(0) + }) = call.positional_nth(0) { pat } else { @@ -115,7 +115,11 @@ https://www.nushell.sh/book/thinking_in_nushell.html#parsing-and-evaluation-are- }; if stack.remove_env_var(engine_state, &name).is_none() { - return Err(ShellError::NotFound(call.positional[0].span)); + return Err(ShellError::NotFound( + call.positional_nth(0) + .expect("already checked for present positional") + .span, + )); } } } else if !import_pattern.hidden.contains(&import_pattern.head.name) diff --git a/crates/nu-command/src/core_commands/if_.rs b/crates/nu-command/src/core_commands/if_.rs index a26fbdc784..db15978a25 100644 --- a/crates/nu-command/src/core_commands/if_.rs +++ b/crates/nu-command/src/core_commands/if_.rs @@ -50,9 +50,9 @@ https://www.nushell.sh/book/thinking_in_nushell.html#parsing-and-evaluation-are- call: &Call, input: PipelineData, ) -> Result { - let cond = &call.positional[0]; + let cond = call.positional_nth(0).expect("checked through parser"); let then_block: CaptureBlock = call.req(engine_state, stack, 1)?; - let else_case = call.positional.get(2); + let else_case = call.positional_nth(2); let result = eval_expression(engine_state, stack, cond)?; match &result { diff --git a/crates/nu-command/src/core_commands/let_.rs b/crates/nu-command/src/core_commands/let_.rs index 9327fab1d4..310fe77089 100644 --- a/crates/nu-command/src/core_commands/let_.rs +++ b/crates/nu-command/src/core_commands/let_.rs @@ -42,11 +42,15 @@ https://www.nushell.sh/book/thinking_in_nushell.html#parsing-and-evaluation-are- call: &Call, input: PipelineData, ) -> Result { - let var_id = call.positional[0] + let var_id = call + .positional_nth(0) + .expect("checked through parser") .as_var() .expect("internal error: missing variable"); - let keyword_expr = call.positional[1] + let keyword_expr = call + .positional_nth(1) + .expect("checked through parser") .as_keyword() .expect("internal error: missing keyword"); diff --git a/crates/nu-command/src/core_commands/metadata.rs b/crates/nu-command/src/core_commands/metadata.rs index d84d1a4e89..3697236b9e 100644 --- a/crates/nu-command/src/core_commands/metadata.rs +++ b/crates/nu-command/src/core_commands/metadata.rs @@ -35,7 +35,7 @@ impl Command for Metadata { call: &Call, input: PipelineData, ) -> Result { - let arg = call.positional.get(0); + let arg = call.positional_nth(0); let head = call.head; match arg { diff --git a/crates/nu-command/src/core_commands/use_.rs b/crates/nu-command/src/core_commands/use_.rs index 5a6fe80af5..f0eed22e1c 100644 --- a/crates/nu-command/src/core_commands/use_.rs +++ b/crates/nu-command/src/core_commands/use_.rs @@ -42,7 +42,7 @@ https://www.nushell.sh/book/thinking_in_nushell.html#parsing-and-evaluation-are- let import_pattern = if let Some(Expression { expr: Expr::ImportPattern(pat), .. - }) = call.positional.get(0) + }) = call.positional_nth(0) { pat } else { diff --git a/crates/nu-command/src/env/let_env.rs b/crates/nu-command/src/env/let_env.rs index f9a9e45b44..3657bc8ddf 100644 --- a/crates/nu-command/src/env/let_env.rs +++ b/crates/nu-command/src/env/let_env.rs @@ -35,7 +35,9 @@ impl Command for LetEnv { ) -> Result { let env_var = call.req(engine_state, stack, 0)?; - let keyword_expr = call.positional[1] + let keyword_expr = call + .positional_nth(1) + .expect("checked through parser") .as_keyword() .expect("internal error: missing keyword"); diff --git a/crates/nu-command/src/env/with_env.rs b/crates/nu-command/src/env/with_env.rs index e79c541d3e..fa87e5a9ab 100644 --- a/crates/nu-command/src/env/with_env.rs +++ b/crates/nu-command/src/env/with_env.rs @@ -99,7 +99,9 @@ fn with_env( return Err(ShellError::CantConvert( "string list or single row".into(), x.get_type().to_string(), - call.positional[1].span, + call.positional_nth(1) + .expect("already checked through .req") + .span, )); } } @@ -123,7 +125,9 @@ fn with_env( return Err(ShellError::CantConvert( "string list or single row".into(), x.get_type().to_string(), - call.positional[1].span, + call.positional_nth(1) + .expect("already checked through .req") + .span, )); } }; diff --git a/crates/nu-command/src/filesystem/cd.rs b/crates/nu-command/src/filesystem/cd.rs index 6e20544f32..915940006a 100644 --- a/crates/nu-command/src/filesystem/cd.rs +++ b/crates/nu-command/src/filesystem/cd.rs @@ -28,7 +28,7 @@ impl Command for Cd { call: &Call, _input: PipelineData, ) -> Result { - let raw_path = call.nth(0); + let raw_path = call.positional_nth(0); let path_val: Option = call.opt(engine_state, stack, 0)?; let cwd = current_dir(engine_state, stack)?; diff --git a/crates/nu-command/src/filesystem/mkdir.rs b/crates/nu-command/src/filesystem/mkdir.rs index e5806a4885..25650ae68c 100644 --- a/crates/nu-command/src/filesystem/mkdir.rs +++ b/crates/nu-command/src/filesystem/mkdir.rs @@ -57,13 +57,18 @@ impl Command for Mkdir { } for (i, dir) in directories.enumerate() { - let span = call.positional[i].span; + let span = call + .positional_nth(i) + .expect("already checked through directories") + .span; let dir_res = std::fs::create_dir_all(&dir); if let Err(reason) = dir_res { return Err(ShellError::CreateNotPossible( format!("failed to create directory: {}", reason), - call.positional[i].span, + call.positional_nth(i) + .expect("already checked through directories") + .span, )); } diff --git a/crates/nu-command/src/filesystem/touch.rs b/crates/nu-command/src/filesystem/touch.rs index d9269e0509..bb10fd2be0 100644 --- a/crates/nu-command/src/filesystem/touch.rs +++ b/crates/nu-command/src/filesystem/touch.rs @@ -239,7 +239,9 @@ impl Command for Touch { if let Err(err) = OpenOptions::new().write(true).create(true).open(&item) { return Err(ShellError::CreateNotPossible( format!("Failed to create file: {}", err), - call.positional[index].span, + call.positional_nth(index) + .expect("already checked positional") + .span, )); }; @@ -251,7 +253,9 @@ impl Command for Touch { ) { return Err(ShellError::ChangeModifiedTimeNotPossible( format!("Failed to change the modified time: {}", err), - call.positional[index].span, + call.positional_nth(index) + .expect("already checked positional") + .span, )); }; } @@ -268,7 +272,9 @@ impl Command for Touch { ) { return Err(ShellError::ChangeAccessTimeNotPossible( format!("Failed to change the access time: {}", err), - call.positional[index].span, + call.positional_nth(index) + .expect("already checked positional") + .span, )); }; } else { @@ -279,7 +285,9 @@ impl Command for Touch { ) { return Err(ShellError::ChangeAccessTimeNotPossible( format!("Failed to change the access time: {}", err), - call.positional[index].span, + call.positional_nth(index) + .expect("already checked positional") + .span, )); }; } diff --git a/crates/nu-command/src/platform/ansi/ansi_.rs b/crates/nu-command/src/platform/ansi/ansi_.rs index e5b3088410..3baab4bd9e 100644 --- a/crates/nu-command/src/platform/ansi/ansi_.rs +++ b/crates/nu-command/src/platform/ansi/ansi_.rs @@ -380,7 +380,9 @@ Format: # None => { return Err(ShellError::UnsupportedInput( String::from("Unknown ansi code"), - call.nth(0).expect("Unexpected missing argument").span, + call.positional_nth(0) + .expect("Unexpected missing argument") + .span, )) } } diff --git a/crates/nu-command/src/platform/reedline_commands/keybindings_list.rs b/crates/nu-command/src/platform/reedline_commands/keybindings_list.rs index 1a2b0e02d7..47d9301a80 100644 --- a/crates/nu-command/src/platform/reedline_commands/keybindings_list.rs +++ b/crates/nu-command/src/platform/reedline_commands/keybindings_list.rs @@ -57,15 +57,14 @@ impl Command for KeybindingsList { call: &Call, _input: PipelineData, ) -> Result { - let records = if call.named.is_empty() { + let records = if call.named_len() == 0 { let all_options = vec!["modifiers", "keycodes", "edits", "modes", "events"]; all_options .iter() .flat_map(|argument| get_records(argument, &call.head)) .collect() } else { - call.named - .iter() + call.named_iter() .flat_map(|(argument, _)| get_records(argument.item.as_str(), &call.head)) .collect() }; diff --git a/crates/nu-command/src/strings/build_string.rs b/crates/nu-command/src/strings/build_string.rs index 8046a8f0f9..beda86be71 100644 --- a/crates/nu-command/src/strings/build_string.rs +++ b/crates/nu-command/src/strings/build_string.rs @@ -54,8 +54,7 @@ impl Command for BuildString { ) -> Result { let config = stack.get_config().unwrap_or_default(); let output = call - .positional - .iter() + .positional_iter() .map(|expr| { eval_expression(engine_state, stack, expr).map(|val| val.into_string(", ", &config)) }) diff --git a/crates/nu-command/src/strings/char_.rs b/crates/nu-command/src/strings/char_.rs index 40ad25400a..fe755c2912 100644 --- a/crates/nu-command/src/strings/char_.rs +++ b/crates/nu-command/src/strings/char_.rs @@ -245,7 +245,10 @@ impl Command for Char { } let mut multi_byte = String::new(); for (i, arg) in args.iter().enumerate() { - let span = call.nth(i).expect("Unexpected missing argument").span; + let span = call + .positional_nth(i) + .expect("Unexpected missing argument") + .span; multi_byte.push(string_to_unicode_char(arg, &span)?) } Ok(Value::string(multi_byte, call_span).into_pipeline_data()) @@ -262,7 +265,9 @@ impl Command for Char { } else { Err(ShellError::UnsupportedInput( "error finding named character".into(), - call.nth(0).expect("Unexpected missing argument").span, + call.positional_nth(0) + .expect("Unexpected missing argument") + .span, )) } } diff --git a/crates/nu-engine/src/call_ext.rs b/crates/nu-engine/src/call_ext.rs index 3ea04e0467..78a684b638 100644 --- a/crates/nu-engine/src/call_ext.rs +++ b/crates/nu-engine/src/call_ext.rs @@ -59,7 +59,7 @@ impl CallExt for Call { ) -> Result, ShellError> { let mut output = vec![]; - for expr in self.positional.iter().skip(starting_pos) { + for expr in self.positional_iter().skip(starting_pos) { let result = eval_expression(engine_state, stack, expr)?; output.push(FromValue::from_value(&result)?); } @@ -73,8 +73,8 @@ impl CallExt for Call { stack: &mut Stack, pos: usize, ) -> Result, ShellError> { - if let Some(expr) = self.nth(pos) { - let result = eval_expression(engine_state, stack, &expr)?; + if let Some(expr) = self.positional_nth(pos) { + let result = eval_expression(engine_state, stack, expr)?; FromValue::from_value(&result).map(Some) } else { Ok(None) @@ -87,12 +87,12 @@ impl CallExt for Call { stack: &mut Stack, pos: usize, ) -> Result { - if let Some(expr) = self.nth(pos) { - let result = eval_expression(engine_state, stack, &expr)?; + if let Some(expr) = self.positional_nth(pos) { + let result = eval_expression(engine_state, stack, expr)?; FromValue::from_value(&result) } else { Err(ShellError::AccessBeyondEnd( - self.positional.len(), + self.positional_len(), self.head, )) } diff --git a/crates/nu-engine/src/eval.rs b/crates/nu-engine/src/eval.rs index d3d2241e49..889a6d43ee 100644 --- a/crates/nu-engine/src/eval.rs +++ b/crates/nu-engine/src/eval.rs @@ -32,7 +32,7 @@ pub fn eval_call( ) -> Result { 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 mut signature = decl.signature(); signature.usage = decl.usage().to_string(); signature.extra_usage = decl.extra_usage().to_string(); @@ -59,7 +59,7 @@ pub fn eval_call( .var_id .expect("internal error: all custom parameters must have var_ids"); - if let Some(arg) = call.positional.get(param_idx) { + if let Some(arg) = call.positional_nth(param_idx) { let result = eval_expression(engine_state, caller_stack, arg)?; callee_stack.add_var(var_id, result); } else if let Some(arg) = ¶m.default_value { @@ -73,7 +73,7 @@ pub fn eval_call( if let Some(rest_positional) = decl.signature().rest_positional { let mut rest_items = vec![]; - for arg in call.positional.iter().skip( + for arg in call.positional_iter().skip( decl.signature().required_positional.len() + decl.signature().optional_positional.len(), ) { @@ -101,7 +101,7 @@ pub fn eval_call( for named in decl.signature().named { if let Some(var_id) = named.var_id { let mut found = false; - for call_named in &call.named { + for call_named in call.named_iter() { if call_named.0.item == named.long { if let Some(arg) = &call_named.1 { let result = eval_expression(engine_state, caller_stack, arg)?; @@ -198,14 +198,14 @@ fn eval_external( let mut call = Call::new(head.span); - call.positional.push(head.clone()); + call.add_positional(head.clone()); for arg in args { - call.positional.push(arg.clone()) + call.add_positional(arg.clone()) } if redirect_stdout { - call.named.push(( + call.add_named(( Spanned { item: "redirect-stdout".into(), span: head.span, @@ -215,7 +215,7 @@ fn eval_external( } if redirect_stderr { - call.named.push(( + call.add_named(( Spanned { item: "redirect-stderr".into(), span: head.span, diff --git a/crates/nu-parser/src/flatten.rs b/crates/nu-parser/src/flatten.rs index 93f97e43fd..207f0e356e 100644 --- a/crates/nu-parser/src/flatten.rs +++ b/crates/nu-parser/src/flatten.rs @@ -145,10 +145,10 @@ pub fn flatten_expression( let mut output = vec![(call.head, FlatShape::InternalCall)]; let mut args = vec![]; - for positional in &call.positional { + for positional in call.positional_iter() { args.extend(flatten_expression(working_set, positional)); } - for named in &call.named { + for named in call.named_iter() { args.push((named.0.span, FlatShape::Flag)); if let Some(expr) = &named.1 { args.extend(flatten_expression(working_set, expr)); diff --git a/crates/nu-parser/src/known_external.rs b/crates/nu-parser/src/known_external.rs index a36a5a1ce8..d253449f9a 100644 --- a/crates/nu-parser/src/known_external.rs +++ b/crates/nu-parser/src/known_external.rs @@ -1,7 +1,10 @@ -use nu_protocol::ast::Expr; use nu_protocol::engine::{EngineState, Stack, StateWorkingSet}; -use nu_protocol::{ast::Call, engine::Command, ShellError, Signature}; -use nu_protocol::{PipelineData, Spanned}; +use nu_protocol::{ + ast::{Argument, Call, Expr, Expression}, + engine::Command, + ShellError, Signature, +}; +use nu_protocol::{PipelineData, Spanned, Type}; #[derive(Clone)] pub struct KnownExternal { @@ -34,69 +37,56 @@ impl Command for KnownExternal { call: &Call, input: PipelineData, ) -> Result { - // FIXME: This is a bit of a hack, and it'd be nice for the parser/AST to be able to handle the original - // order of the parameters. Until then, we need to recover the original order. - - // FIXME: This is going to be a bit expensive, but we need to do it to ensure any new block/subexpression - // we find when parsing the external call is handled properly. - let mut engine_state = engine_state.clone(); - let call_span = call.span(); - let contents = engine_state.get_span_contents(&call_span); + let head_span = call.head; + let decl_id = engine_state + .find_decl("run-external".as_bytes()) + .ok_or(ShellError::ExternalNotSupported(head_span))?; - let redirect_stdout = call.redirect_stdout; - let redirect_stderr = call.redirect_stderr; + let command = engine_state.get_decl(decl_id); - let (lexed, _) = crate::lex(contents, call_span.start, &[], &[], true); + let mut extern_call = Call::new(head_span); - let spans: Vec<_> = lexed.into_iter().map(|x| x.span).collect(); - let mut working_set = StateWorkingSet::new(&engine_state); - let (external_call, _) = crate::parse_external_call(&mut working_set, &spans, &[]); - let delta = working_set.render(); - engine_state.merge_delta(delta, None, ".")?; + let working_state = StateWorkingSet::new(engine_state); + let extern_name = working_state.get_span_contents(call.head); + let extern_name = String::from_utf8(extern_name.to_vec()) + .expect("this was already parsed as a command name"); + let arg_extern_name = Expression { + expr: Expr::String(extern_name), + span: call.head, + ty: Type::String, + custom_completion: None, + }; - match external_call.expr { - Expr::ExternalCall(head, args) => { - let decl_id = engine_state - .find_decl("run-external".as_bytes()) - .ok_or(ShellError::ExternalNotSupported(head.span))?; + extern_call.add_positional(arg_extern_name); - let command = engine_state.get_decl(decl_id); - - let mut call = Call::new(head.span); - - call.positional.push(*head); - - for arg in args { - call.positional.push(arg.clone()) - } - - if redirect_stdout { - call.named.push(( - Spanned { - item: "redirect-stdout".into(), - span: call_span, - }, - None, - )) - } - - if redirect_stderr { - call.named.push(( - Spanned { - item: "redirect-stderr".into(), - span: call_span, - }, - None, - )) - } - - command.run(&engine_state, stack, &call, input) - } - x => { - println!("{:?}", x); - panic!("internal error: known external not actually external") + for arg in &call.arguments { + match arg { + Argument::Positional(positional) => extern_call.add_positional(positional.clone()), + Argument::Named(named) => extern_call.add_named(named.clone()), } } + + if call.redirect_stdout { + extern_call.add_named(( + Spanned { + item: "redirect-stdout".into(), + span: call_span, + }, + None, + )) + } + + if call.redirect_stderr { + extern_call.add_named(( + Spanned { + item: "redirect-stderr".into(), + span: call_span, + }, + None, + )) + } + + command.run(engine_state, stack, &extern_call, input) } } diff --git a/crates/nu-parser/src/parse_keywords.rs b/crates/nu-parser/src/parse_keywords.rs index ffe466e016..9e64923cc5 100644 --- a/crates/nu-parser/src/parse_keywords.rs +++ b/crates/nu-parser/src/parse_keywords.rs @@ -1,8 +1,8 @@ use nu_path::canonicalize_with; use nu_protocol::{ ast::{ - Block, Call, Expr, Expression, ImportPattern, ImportPatternHead, ImportPatternMember, - Pipeline, + Argument, Block, Call, Expr, Expression, ImportPattern, ImportPatternHead, + ImportPatternMember, Pipeline, }, engine::StateWorkingSet, span, Exportable, Overlay, PositionalArg, Span, SyntaxShape, Type, CONFIG_VARIABLE_ID, @@ -142,7 +142,7 @@ pub fn parse_for( let sig = decl.signature(); // Let's get our block and make sure it has the right signature - if let Some(arg) = call.positional.get(2) { + if let Some(arg) = call.positional_nth(2) { match arg { Expression { expr: Expr::Block(block_id), @@ -178,8 +178,8 @@ pub fn parse_for( }; // All positional arguments must be in the call positional vector by this point - let var_decl = call.positional.get(0).expect("for call already checked"); - let block = call.positional.get(2).expect("for call already checked"); + let var_decl = call.positional_nth(0).expect("for call already checked"); + let block = call.positional_nth(2).expect("for call already checked"); let error = None; if let (Some(var_id), Some(block_id)) = (&var_decl.as_var(), block.as_block()) { @@ -311,7 +311,7 @@ pub fn parse_def( let sig = decl.signature(); // Let's get our block and make sure it has the right signature - if let Some(arg) = call.positional.get(2) { + if let Some(arg) = call.positional_nth(2) { match arg { Expression { expr: Expr::Block(block_id), @@ -347,9 +347,9 @@ pub fn parse_def( }; // All positional arguments must be in the call positional vector by this point - let name_expr = call.positional.get(0).expect("def call already checked"); - let sig = call.positional.get(1).expect("def call already checked"); - let block = call.positional.get(2).expect("def call already checked"); + let name_expr = call.positional_nth(0).expect("def call already checked"); + let sig = call.positional_nth(1).expect("def call already checked"); + let block = call.positional_nth(2).expect("def call already checked"); let mut error = None; @@ -457,8 +457,8 @@ pub fn parse_extern( (call, call_span) } }; - let name_expr = call.positional.get(0); - let sig = call.positional.get(1); + let name_expr = call.positional_nth(0); + let sig = call.positional_nth(1); if let (Some(name_expr), Some(sig)) = (name_expr, sig) { if let (Some(name), Some(mut signature)) = (&name_expr.as_string(), sig.as_signature()) { @@ -626,8 +626,7 @@ pub fn parse_export( let mut call = Box::new(Call { head: spans[0], decl_id: export_decl_id, - positional: vec![], - named: vec![], + arguments: vec![], redirect_stdout: true, redirect_stderr: false, }); @@ -895,7 +894,7 @@ pub fn parse_export( if let Some(name_span) = spans.get(2) { let (name_expr, err) = parse_string(working_set, *name_span); error = error.or(err); - call.positional.push(name_expr); + call.add_positional(name_expr); if let Some(block_span) = spans.get(3) { let (block_expr, err) = parse_block_expression( @@ -922,7 +921,7 @@ pub fn parse_export( None }; - call.positional.push(block_expr); + call.add_positional(block_expr); exportable } else { @@ -1184,8 +1183,10 @@ pub fn parse_module( let call = Box::new(Call { head: spans[0], decl_id: module_decl_id, - positional: vec![module_name_expr, block_expr], - named: vec![], + arguments: vec![ + Argument::Positional(module_name_expr), + Argument::Positional(block_expr), + ], redirect_stdout: true, redirect_stderr: false, }); @@ -1264,7 +1265,7 @@ pub fn parse_use( } }; - let import_pattern = if let Some(expr) = call.nth(0) { + let import_pattern = if let Some(expr) = call.positional_nth(0) { if let Some(pattern) = expr.as_import_pattern() { pattern } else { @@ -1422,8 +1423,7 @@ pub fn parse_use( let call = Box::new(Call { head: spans[0], decl_id: use_decl_id, - positional: vec![import_pattern_expr], - named: vec![], + arguments: vec![Argument::Positional(import_pattern_expr)], redirect_stdout: true, redirect_stderr: false, }); @@ -1493,7 +1493,7 @@ pub fn parse_hide( } }; - let import_pattern = if let Some(expr) = call.nth(0) { + let import_pattern = if let Some(expr) = call.positional_nth(0) { if let Some(pattern) = expr.as_import_pattern() { pattern } else { @@ -1630,8 +1630,7 @@ pub fn parse_hide( let call = Box::new(Call { head: spans[0], decl_id: hide_decl_id, - positional: vec![import_pattern_expr], - named: vec![], + arguments: vec![Argument::Positional(import_pattern_expr)], redirect_stdout: true, redirect_stderr: false, }); @@ -1715,8 +1714,10 @@ pub fn parse_let( let call = Box::new(Call { decl_id, head: spans[0], - positional: vec![lvalue, rvalue], - named: vec![], + arguments: vec![ + Argument::Positional(lvalue), + Argument::Positional(rvalue), + ], redirect_stdout: true, redirect_stderr: false, }); @@ -1834,7 +1835,7 @@ pub fn parse_source( // Adding this expression to the positional creates a syntax highlighting error // after writing `source example.nu` - call_with_block.positional.push(Expression { + call_with_block.add_positional(Expression { expr: Expr::Int(block_id as i64), span: spans[1], ty: Type::Any, @@ -1947,8 +1948,7 @@ pub fn parse_register( // The ? operator is not used because the error has to be kept to be printed in the shell // For that reason the values are kept in a result that will be passed at the end of this call let arguments = call - .positional - .get(0) + .positional_nth(0) .map(|expr| { let name_expr = working_set.get_span_contents(expr.span); @@ -1992,7 +1992,7 @@ pub fn parse_register( // Signature is an optional value from the call and will be used to decide if // the plugin is called to get the signatures or to use the given signature - let signature = call.positional.get(1).map(|expr| { + let signature = call.positional_nth(1).map(|expr| { let signature = working_set.get_span_contents(expr.span); serde_json::from_slice::(signature).map_err(|_| { ParseError::LabeledError( diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 3a4193f416..97a44d4fe3 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -8,8 +8,9 @@ use crate::{ use nu_protocol::{ ast::{ - Block, Call, CellPath, Expr, Expression, FullCellPath, ImportPattern, ImportPatternHead, - ImportPatternMember, Operator, PathMember, Pipeline, RangeInclusion, RangeOperator, + Argument, Block, Call, CellPath, Expr, Expression, FullCellPath, ImportPattern, + ImportPatternHead, ImportPatternMember, Operator, PathMember, Pipeline, RangeInclusion, + RangeOperator, }, engine::StateWorkingSet, span, BlockId, Flag, PositionalArg, Signature, Span, Spanned, SyntaxShape, Type, Unit, VarId, @@ -127,16 +128,16 @@ pub fn trim_quotes(bytes: &[u8]) -> &[u8] { pub fn check_call(command: Span, sig: &Signature, call: &Call) -> Option { // 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 None; } - if call.positional.len() < sig.required_positional.len() { + if call.positional_len() < sig.required_positional.len() { // Comparing the types of all signature positional arguments against the parsed // expressions found in the call. If one type is not found then it could be assumed // that that positional argument is missing from the parsed call for argument in &sig.required_positional { - let found = call.positional.iter().fold(false, |ac, expr| { + let found = call.positional_iter().fold(false, |ac, expr| { if argument.shape.to_type() == expr.ty || argument.shape == SyntaxShape::Any { true } else { @@ -144,7 +145,7 @@ pub fn check_call(command: Span, sig: &Signature, call: &Call) -> Option Option Option Result { let positional = call - .positional - .iter() + .positional_iter() .map(|expr| eval_expression(engine_state, stack, expr)) .collect::, ShellError>>()?; - let mut named = Vec::with_capacity(call.named.len()); - for (string, expr) in call.named.iter() { + let mut named = Vec::with_capacity(call.named_len()); + for (string, expr) in call.named_iter() { let value = match expr { None => None, Some(expr) => Some(eval_expression(engine_state, stack, expr)?), diff --git a/crates/nu-protocol/src/ast/call.rs b/crates/nu-protocol/src/ast/call.rs index 2d137e17c7..b576101bd1 100644 --- a/crates/nu-protocol/src/ast/call.rs +++ b/crates/nu-protocol/src/ast/call.rs @@ -3,13 +3,18 @@ use serde::{Deserialize, Serialize}; use super::Expression; use crate::{DeclId, Span, Spanned}; +#[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] +pub enum Argument { + Positional(Expression), + Named((Spanned, Option)), +} + #[derive(Debug, Clone, PartialEq, Serialize, Deserialize)] pub struct Call { /// identifier of the declaration to call pub decl_id: DeclId, pub head: Span, - pub positional: Vec, - pub named: Vec<(Spanned, Option)>, + pub arguments: Vec, pub redirect_stdout: bool, pub redirect_stderr: bool, } @@ -19,15 +24,64 @@ impl Call { Self { decl_id: 0, head, - positional: vec![], - named: vec![], + arguments: vec![], redirect_stdout: true, redirect_stderr: false, } } + pub fn named_iter(&self) -> impl Iterator, Option)> { + self.arguments.iter().filter_map(|arg| match arg { + Argument::Named(named) => Some(named), + Argument::Positional(_) => None, + }) + } + + pub fn named_iter_mut( + &mut self, + ) -> impl Iterator, Option)> { + self.arguments.iter_mut().filter_map(|arg| match arg { + Argument::Named(named) => Some(named), + Argument::Positional(_) => None, + }) + } + + pub fn named_len(&self) -> usize { + self.named_iter().count() + } + + pub fn add_named(&mut self, named: (Spanned, Option)) { + self.arguments.push(Argument::Named(named)); + } + + pub fn add_positional(&mut self, positional: Expression) { + self.arguments.push(Argument::Positional(positional)); + } + + pub fn positional_iter(&self) -> impl Iterator { + self.arguments.iter().filter_map(|arg| match arg { + Argument::Named(_) => None, + Argument::Positional(positional) => Some(positional), + }) + } + + pub fn positional_iter_mut(&mut self) -> impl Iterator { + self.arguments.iter_mut().filter_map(|arg| match arg { + Argument::Named(_) => None, + Argument::Positional(positional) => Some(positional), + }) + } + + pub fn positional_nth(&self, i: usize) -> Option<&Expression> { + self.positional_iter().nth(i) + } + + pub fn positional_len(&self) -> usize { + self.positional_iter().count() + } + pub fn has_flag(&self, flag_name: &str) -> bool { - for name in &self.named { + for name in self.named_iter() { if flag_name == name.0.item { return true; } @@ -37,7 +91,7 @@ impl Call { } pub fn get_flag_expr(&self, flag_name: &str) -> Option { - for name in &self.named { + for name in self.named_iter() { if flag_name == name.0.item { return name.1.clone(); } @@ -47,7 +101,7 @@ impl Call { } pub fn get_named_arg(&self, flag_name: &str) -> Option> { - for name in &self.named { + for name in self.named_iter() { if flag_name == name.0.item { return Some(name.0.clone()); } @@ -56,20 +110,16 @@ impl Call { None } - pub fn nth(&self, pos: usize) -> Option { - self.positional.get(pos).cloned() - } - pub fn span(&self) -> Span { let mut span = self.head; - for positional in &self.positional { + for positional in self.positional_iter() { if positional.span.end > span.end { span.end = positional.span.end; } } - for (named, val) in &self.named { + for (named, val) in self.named_iter() { if named.span.end > span.end { span.end = named.span.end; } diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 3fe59a1777..3b243419a8 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -133,12 +133,12 @@ impl Expression { Expr::Binary(_) => false, Expr::Bool(_) => false, Expr::Call(call) => { - for positional in &call.positional { + for positional in call.positional_iter() { if positional.has_in_variable(working_set) { return true; } } - for named in &call.named { + for named in call.named_iter() { if let Some(expr) = &named.1 { if expr.has_in_variable(working_set) { return true; @@ -302,10 +302,10 @@ impl Expression { Expr::Binary(_) => {} Expr::Bool(_) => {} Expr::Call(call) => { - for positional in &mut call.positional { + for positional in call.positional_iter_mut() { positional.replace_in_variable(working_set, new_var_id); } - for named in &mut call.named { + for named in call.named_iter_mut() { if let Some(expr) = &mut named.1 { expr.replace_in_variable(working_set, new_var_id) } @@ -449,10 +449,10 @@ impl Expression { if replaced.contains_span(call.head) { call.head = new_span; } - for positional in &mut call.positional { + for positional in call.positional_iter_mut() { positional.replace_span(working_set, replaced, new_span); } - for named in &mut call.named { + for named in call.named_iter_mut() { if let Some(expr) = &mut named.1 { expr.replace_span(working_set, replaced, new_span) }