From b6d765a2d8940ad5017537a61a8d65715ab6bc9a Mon Sep 17 00:00:00 2001 From: Ian Manske Date: Thu, 25 Apr 2024 14:16:12 +0000 Subject: [PATCH] `nu-cmd-lang` cleanup (#12609) # Description This PR does miscellaneous cleanup in some of the commands from `nu-cmd-lang`. # User-Facing Changes None. # After Submitting Cleanup the other commands in `nu-cmd-lang`. --- .../nu-cmd-lang/src/core_commands/collect.rs | 6 +- .../nu-cmd-lang/src/core_commands/hide_env.rs | 6 +- crates/nu-cmd-lang/src/core_commands/if_.rs | 78 +++++-------- crates/nu-cmd-lang/src/core_commands/let_.rs | 10 +- .../nu-cmd-lang/src/core_commands/match_.rs | 67 ++++++----- crates/nu-cmd-lang/src/core_commands/mut_.rs | 10 +- .../nu-cmd-lang/src/core_commands/return_.rs | 16 +-- crates/nu-cmd-lang/src/core_commands/try_.rs | 4 +- .../nu-cmd-lang/src/core_commands/version.rs | 105 +++++++----------- crates/nu-protocol/src/ast/expression.rs | 9 +- 10 files changed, 125 insertions(+), 186 deletions(-) diff --git a/crates/nu-cmd-lang/src/core_commands/collect.rs b/crates/nu-cmd-lang/src/core_commands/collect.rs index 0d06484174..eae41e8690 100644 --- a/crates/nu-cmd-lang/src/core_commands/collect.rs +++ b/crates/nu-cmd-lang/src/core_commands/collect.rs @@ -38,12 +38,12 @@ impl Command for Collect { ) -> Result { let closure: Closure = call.req(engine_state, stack, 0)?; - let block = engine_state.get_block(closure.block_id).clone(); + let block = engine_state.get_block(closure.block_id); let mut stack_captures = stack.captures_to_stack_preserve_out_dest(closure.captures.clone()); let metadata = input.metadata(); - let input: Value = input.into_value(call.head); + let input = input.into_value(call.head); let mut saved_positional = None; if let Some(var) = block.signature.get_positional(0) { @@ -58,7 +58,7 @@ impl Command for Collect { let result = eval_block( engine_state, &mut stack_captures, - &block, + block, input.into_pipeline_data(), ) .map(|x| x.set_metadata(metadata)); diff --git a/crates/nu-cmd-lang/src/core_commands/hide_env.rs b/crates/nu-cmd-lang/src/core_commands/hide_env.rs index 73bff6e18d..43ed830629 100644 --- a/crates/nu-cmd-lang/src/core_commands/hide_env.rs +++ b/crates/nu-cmd-lang/src/core_commands/hide_env.rs @@ -41,11 +41,7 @@ impl Command for HideEnv { for name in env_var_names { if !stack.remove_env_var(engine_state, &name.item) && !ignore_errors { - let all_names: Vec = stack - .get_env_var_names(engine_state) - .iter() - .cloned() - .collect(); + let all_names = stack.get_env_var_names(engine_state); if let Some(closest_match) = did_you_mean(&all_names, &name.item) { return Err(ShellError::DidYouMeanCustom { msg: format!("Environment variable '{}' not found", name.item), diff --git a/crates/nu-cmd-lang/src/core_commands/if_.rs b/crates/nu-cmd-lang/src/core_commands/if_.rs index 259dfaef5a..83808c8e06 100644 --- a/crates/nu-cmd-lang/src/core_commands/if_.rs +++ b/crates/nu-cmd-lang/src/core_commands/if_.rs @@ -59,43 +59,27 @@ impl Command for If { .expect("internal error: missing block"); let else_case = call.positional_nth(2); - let result = eval_constant(working_set, cond)?; - match &result { - Value::Bool { val, .. } => { - if *val { - let block = working_set.get_block(then_block); + if eval_constant(working_set, cond)?.as_bool()? { + let block = working_set.get_block(then_block); + eval_const_subexpression(working_set, block, input, block.span.unwrap_or(call.head)) + } else if let Some(else_case) = else_case { + if let Some(else_expr) = else_case.as_keyword() { + if let Some(block_id) = else_expr.as_block() { + let block = working_set.get_block(block_id); eval_const_subexpression( working_set, block, input, block.span.unwrap_or(call.head), ) - } else if let Some(else_case) = else_case { - if let Some(else_expr) = else_case.as_keyword() { - if let Some(block_id) = else_expr.as_block() { - let block = working_set.get_block(block_id); - eval_const_subexpression( - working_set, - block, - input, - block.span.unwrap_or(call.head), - ) - } else { - eval_constant_with_input(working_set, else_expr, input) - } - } else { - eval_constant_with_input(working_set, else_case, input) - } } else { - Ok(PipelineData::empty()) + eval_constant_with_input(working_set, else_expr, input) } + } else { + eval_constant_with_input(working_set, else_case, input) } - x => Err(ShellError::CantConvert { - to_type: "bool".into(), - from_type: x.get_type().to_string(), - span: result.span(), - help: None, - }), + } else { + Ok(PipelineData::empty()) } } @@ -118,35 +102,23 @@ impl Command for If { let eval_expression_with_input = get_eval_expression_with_input(engine_state); let eval_block = get_eval_block(engine_state); - let result = eval_expression(engine_state, stack, cond)?; - match &result { - Value::Bool { val, .. } => { - if *val { - let block = engine_state.get_block(then_block); + if eval_expression(engine_state, stack, cond)?.as_bool()? { + let block = engine_state.get_block(then_block); + eval_block(engine_state, stack, block, input) + } else if let Some(else_case) = else_case { + if let Some(else_expr) = else_case.as_keyword() { + if let Some(block_id) = else_expr.as_block() { + let block = engine_state.get_block(block_id); eval_block(engine_state, stack, block, input) - } else if let Some(else_case) = else_case { - if let Some(else_expr) = else_case.as_keyword() { - if let Some(block_id) = else_expr.as_block() { - let block = engine_state.get_block(block_id); - eval_block(engine_state, stack, block, input) - } else { - eval_expression_with_input(engine_state, stack, else_expr, input) - .map(|res| res.0) - } - } else { - eval_expression_with_input(engine_state, stack, else_case, input) - .map(|res| res.0) - } } else { - Ok(PipelineData::empty()) + eval_expression_with_input(engine_state, stack, else_expr, input) + .map(|res| res.0) } + } else { + eval_expression_with_input(engine_state, stack, else_case, input).map(|res| res.0) } - x => Err(ShellError::CantConvert { - to_type: "bool".into(), - from_type: x.get_type().to_string(), - span: result.span(), - help: None, - }), + } else { + Ok(PipelineData::empty()) } } diff --git a/crates/nu-cmd-lang/src/core_commands/let_.rs b/crates/nu-cmd-lang/src/core_commands/let_.rs index 949c883b4e..c780954bc6 100644 --- a/crates/nu-cmd-lang/src/core_commands/let_.rs +++ b/crates/nu-cmd-lang/src/core_commands/let_.rs @@ -61,7 +61,7 @@ impl Command for Let { let eval_block = get_eval_block(engine_state); let stack = &mut stack.start_capture(); let pipeline_data = eval_block(engine_state, stack, block, input)?; - let mut value = pipeline_data.into_value(call.head); + let value = pipeline_data.into_value(call.head); // if given variable type is Glob, and our result is string // then nushell need to convert from Value::String to Value::Glob @@ -69,12 +69,12 @@ impl Command for Let { // if we pass it to other commands. let var_type = &engine_state.get_var(var_id).ty; let val_span = value.span(); - match value { + let value = match value { Value::String { val, .. } if var_type == &Type::Glob => { - value = Value::glob(val, false, val_span); + Value::glob(val, false, val_span) } - _ => {} - } + value => value, + }; stack.add_var(var_id, value); Ok(PipelineData::empty()) diff --git a/crates/nu-cmd-lang/src/core_commands/match_.rs b/crates/nu-cmd-lang/src/core_commands/match_.rs index fc4af9dc51..41b5c24702 100644 --- a/crates/nu-cmd-lang/src/core_commands/match_.rs +++ b/crates/nu-cmd-lang/src/core_commands/match_.rs @@ -1,10 +1,7 @@ use nu_engine::{ command_prelude::*, get_eval_block, get_eval_expression, get_eval_expression_with_input, }; -use nu_protocol::{ - ast::{Expr, Expression}, - engine::Matcher, -}; +use nu_protocol::engine::Matcher; #[derive(Clone)] pub struct Match; @@ -38,45 +35,45 @@ impl Command for Match { input: PipelineData, ) -> Result { let value: Value = call.req(engine_state, stack, 0)?; - let block = call.positional_nth(1); + let matches = call + .positional_nth(1) + .expect("checked through parser") + .as_match_block() + .expect("missing match block"); + let eval_expression = get_eval_expression(engine_state); let eval_expression_with_input = get_eval_expression_with_input(engine_state); let eval_block = get_eval_block(engine_state); - if let Some(Expression { - expr: Expr::MatchBlock(matches), - .. - }) = block - { - for match_ in matches { - let mut match_variables = vec![]; - if match_.0.match_value(&value, &mut match_variables) { - // This case does match, go ahead and return the evaluated expression - for match_variable in match_variables { - stack.add_var(match_variable.0, match_variable.1); - } + let mut match_variables = vec![]; + for (pattern, expr) in matches { + if pattern.match_value(&value, &mut match_variables) { + // This case does match, go ahead and return the evaluated expression + for (id, value) in match_variables.drain(..) { + stack.add_var(id, value); + } - let guard_matches = if let Some(guard) = &match_.0.guard { - let Value::Bool { val, .. } = eval_expression(engine_state, stack, guard)? - else { - return Err(ShellError::MatchGuardNotBool { span: guard.span }); - }; - - val - } else { - true + let guard_matches = if let Some(guard) = &pattern.guard { + let Value::Bool { val, .. } = eval_expression(engine_state, stack, guard)? + else { + return Err(ShellError::MatchGuardNotBool { span: guard.span }); }; - if guard_matches { - return if let Some(block_id) = match_.1.as_block() { - let block = engine_state.get_block(block_id); - eval_block(engine_state, stack, block, input) - } else { - eval_expression_with_input(engine_state, stack, &match_.1, input) - .map(|x| x.0) - }; - } + val + } else { + true + }; + + if guard_matches { + return if let Some(block_id) = expr.as_block() { + let block = engine_state.get_block(block_id); + eval_block(engine_state, stack, block, input) + } else { + eval_expression_with_input(engine_state, stack, expr, input).map(|x| x.0) + }; } + } else { + match_variables.clear(); } } diff --git a/crates/nu-cmd-lang/src/core_commands/mut_.rs b/crates/nu-cmd-lang/src/core_commands/mut_.rs index 481f16a7c5..be2d66aff4 100644 --- a/crates/nu-cmd-lang/src/core_commands/mut_.rs +++ b/crates/nu-cmd-lang/src/core_commands/mut_.rs @@ -61,7 +61,7 @@ impl Command for Mut { let eval_block = get_eval_block(engine_state); let stack = &mut stack.start_capture(); let pipeline_data = eval_block(engine_state, stack, block, input)?; - let mut value = pipeline_data.into_value(call.head); + let value = pipeline_data.into_value(call.head); // if given variable type is Glob, and our result is string // then nushell need to convert from Value::String to Value::Glob @@ -69,12 +69,12 @@ impl Command for Mut { // if we pass it to other commands. let var_type = &engine_state.get_var(var_id).ty; let val_span = value.span(); - match value { + let value = match value { Value::String { val, .. } if var_type == &Type::Glob => { - value = Value::glob(val, false, val_span); + Value::glob(val, false, val_span) } - _ => {} - } + value => value, + }; stack.add_var(var_id, value); Ok(PipelineData::empty()) diff --git a/crates/nu-cmd-lang/src/core_commands/return_.rs b/crates/nu-cmd-lang/src/core_commands/return_.rs index 48c04f80f2..969456d005 100644 --- a/crates/nu-cmd-lang/src/core_commands/return_.rs +++ b/crates/nu-cmd-lang/src/core_commands/return_.rs @@ -40,17 +40,11 @@ impl Command for Return { _input: PipelineData, ) -> Result { let return_value: Option = call.opt(engine_state, stack, 0)?; - if let Some(value) = return_value { - Err(ShellError::Return { - span: call.head, - value: Box::new(value), - }) - } else { - Err(ShellError::Return { - span: call.head, - value: Box::new(Value::nothing(call.head)), - }) - } + let value = return_value.unwrap_or(Value::nothing(call.head)); + Err(ShellError::Return { + span: call.head, + value: Box::new(value), + }) } fn examples(&self) -> Vec { diff --git a/crates/nu-cmd-lang/src/core_commands/try_.rs b/crates/nu-cmd-lang/src/core_commands/try_.rs index bfc7a85748..2bf3ee6b8d 100644 --- a/crates/nu-cmd-lang/src/core_commands/try_.rs +++ b/crates/nu-cmd-lang/src/core_commands/try_.rs @@ -49,9 +49,7 @@ impl Command for Try { let try_block = engine_state.get_block(try_block); let eval_block = get_eval_block(engine_state); - let result = eval_block(engine_state, stack, try_block, input); - - match result { + match eval_block(engine_state, stack, try_block, input) { Err(error) => { let error = intercept_block_control(error)?; let err_record = err_to_record(error, call.head); diff --git a/crates/nu-cmd-lang/src/core_commands/version.rs b/crates/nu-cmd-lang/src/core_commands/version.rs index 3455b9c699..7aaa72b38a 100644 --- a/crates/nu-cmd-lang/src/core_commands/version.rs +++ b/crates/nu-cmd-lang/src/core_commands/version.rs @@ -36,7 +36,7 @@ impl Command for Version { call: &Call, _input: PipelineData, ) -> Result { - version(engine_state, call) + version(engine_state, call.head) } fn run_const( @@ -45,7 +45,7 @@ impl Command for Version { call: &Call, _input: PipelineData, ) -> Result { - version(working_set.permanent(), call) + version(working_set.permanent(), call.head) } fn examples(&self) -> Vec { @@ -57,7 +57,13 @@ impl Command for Version { } } -pub fn version(engine_state: &EngineState, call: &Call) -> Result { +fn push_non_empty(record: &mut Record, name: &str, value: &str, span: Span) { + if !value.is_empty() { + record.push(name, Value::string(value, span)) + } +} + +pub fn version(engine_state: &EngineState, span: Span) -> Result { // Pre-allocate the arrays in the worst case (17 items): // - version // - major @@ -69,6 +75,7 @@ pub fn version(engine_state: &EngineState, call: &Call) -> Result Result Result &'static str { diff --git a/crates/nu-protocol/src/ast/expression.rs b/crates/nu-protocol/src/ast/expression.rs index 8766b3f8e5..2f31196871 100644 --- a/crates/nu-protocol/src/ast/expression.rs +++ b/crates/nu-protocol/src/ast/expression.rs @@ -1,5 +1,5 @@ use crate::{ - ast::{Argument, Block, Expr, ExternalArgument, ImportPattern, RecordItem}, + ast::{Argument, Block, Expr, ExternalArgument, ImportPattern, MatchPattern, RecordItem}, engine::StateWorkingSet, BlockId, DeclId, Signature, Span, Type, VarId, IN_VARIABLE_ID, }; @@ -79,6 +79,13 @@ impl Expression { } } + pub fn as_match_block(&self) -> Option<&[(MatchPattern, Expression)]> { + match &self.expr { + Expr::MatchBlock(matches) => Some(matches), + _ => None, + } + } + pub fn as_signature(&self) -> Option> { match &self.expr { Expr::Signature(sig) => Some(sig.clone()),