diff --git a/crates/nu-command/src/conversions/into/datetime.rs b/crates/nu-command/src/conversions/into/datetime.rs index 862dbeaef2..525601764a 100644 --- a/crates/nu-command/src/conversions/into/datetime.rs +++ b/crates/nu-command/src/conversions/into/datetime.rs @@ -82,9 +82,6 @@ impl Command for IntoDatetime { (Type::List(Box::new(Type::String)), Type::List(Box::new(Type::Date))), (Type::table(), Type::table()), (Type::Nothing, Type::table()), - // FIXME: https://github.com/nushell/nushell/issues/15485 - // 'record -> any' was added as a temporary workaround to avoid type inference issues. The Any arm needs to be appear first. - (Type::record(), Type::Any), (Type::record(), Type::record()), (Type::record(), Type::Date), // FIXME Type::Any input added to disable pipeline input type checking, as run-time checks can raise undesirable type errors diff --git a/crates/nu-command/src/conversions/into/duration.rs b/crates/nu-command/src/conversions/into/duration.rs index 12e16b8644..4785d7b7c6 100644 --- a/crates/nu-command/src/conversions/into/duration.rs +++ b/crates/nu-command/src/conversions/into/duration.rs @@ -53,9 +53,6 @@ impl Command for IntoDuration { (Type::Float, Type::Duration), (Type::String, Type::Duration), (Type::Duration, Type::Duration), - // FIXME: https://github.com/nushell/nushell/issues/15485 - // 'record -> any' was added as a temporary workaround to avoid type inference issues. The Any arm needs to be appear first. - (Type::record(), Type::Any), (Type::record(), Type::record()), (Type::record(), Type::Duration), (Type::table(), Type::table()), diff --git a/crates/nu-engine/src/eval_ir.rs b/crates/nu-engine/src/eval_ir.rs index 6f7a90713d..106ac03ca8 100644 --- a/crates/nu-engine/src/eval_ir.rs +++ b/crates/nu-engine/src/eval_ir.rs @@ -6,6 +6,7 @@ use nu_protocol::{ PipelineData, PipelineMetadata, PositionalArg, Range, Record, RegId, ShellError, Signals, Signature, Span, Spanned, Type, Value, VarId, ast::{Bits, Block, Boolean, CellPath, Comparison, Math, Operator}, + combined_type_string, debugger::DebugContext, engine::{ Argument, Closure, EngineState, ErrorHandler, Matcher, Redirection, Stack, StateWorkingSet, @@ -1315,37 +1316,21 @@ fn check_input_types( return Ok(()); } - let mut input_types = io_types - .iter() - .map(|(input, _)| input.to_string()) - .collect::>(); + let input_types: Vec = io_types.iter().map(|(input, _)| input.clone()).collect(); + let expected_string = combined_type_string(&input_types, "and"); - let expected_string = match input_types.len() { - 0 => { - return Err(ShellError::NushellFailed { - msg: "Command input type strings is empty, despite being non-zero earlier" - .to_string(), - }); - } - 1 => input_types.swap_remove(0), - 2 => input_types.join(" and "), - _ => { - input_types - .last_mut() - .expect("Vector with length >2 has no elements") - .insert_str(0, "and "); - input_types.join(", ") - } - }; - - match input { - PipelineData::Empty => Err(ShellError::PipelineEmpty { dst_span: head }), - _ => Err(ShellError::OnlySupportsThisInputType { + match (input, expected_string) { + (PipelineData::Empty, _) => Err(ShellError::PipelineEmpty { dst_span: head }), + (_, Some(expected_string)) => Err(ShellError::OnlySupportsThisInputType { exp_input_type: expected_string, wrong_type: input.get_type().to_string(), dst_span: head, src_span: input.span().unwrap_or(Span::unknown()), }), + // expected_string didn't generate properly, so we can't show the proper error + (_, None) => Err(ShellError::NushellFailed { + msg: "Command input type strings is empty, despite being non-zero earlier".to_string(), + }), } } diff --git a/crates/nu-parser/src/type_check.rs b/crates/nu-parser/src/type_check.rs index a532919394..46c7a57a7b 100644 --- a/crates/nu-parser/src/type_check.rs +++ b/crates/nu-parser/src/type_check.rs @@ -1,6 +1,7 @@ use nu_protocol::{ ParseError, Span, Type, ast::{Assignment, Block, Comparison, Expr, Expression, Math, Operator, Pipeline, Range}, + combined_type_string, engine::StateWorkingSet, }; @@ -757,65 +758,71 @@ pub fn math_result_type( } } +/// Determine the possible output types of a pipeline. +/// +/// Output is union of types in the `Vec`. pub fn check_pipeline_type( working_set: &StateWorkingSet, pipeline: &Pipeline, input_type: Type, -) -> (Type, Option>) { - let mut current_type = input_type; +) -> (Vec, Option>) { + let mut current_types: Vec; + let mut new_types: Vec = vec![input_type]; let mut output_errors: Option> = None; - 'elem: for elem in &pipeline.elements { + for elem in &pipeline.elements { + current_types = std::mem::take(&mut new_types); + if elem.redirection.is_some() { - current_type = Type::Any; - } else if let Expr::Call(call) = &elem.expr.expr { + new_types = vec![Type::Any]; + continue; + } + if let Expr::Call(call) = &elem.expr.expr { let decl = working_set.get_decl(call.decl_id); - - if current_type == Type::Any { - let mut new_current_type = None; - for (_, call_output) in decl.signature().input_output_types { - if let Some(inner_current_type) = &new_current_type { - if inner_current_type == &Type::Any { - break; - } else if inner_current_type != &call_output { - // Union unequal types to Any for now - new_current_type = Some(Type::Any) - } - } else { - new_current_type = Some(call_output.clone()) - } - } - - if let Some(new_current_type) = new_current_type { - current_type = new_current_type - } else { - current_type = Type::Any; - } - continue 'elem; + let io_types = decl.signature().input_output_types; + if new_types.contains(&Type::Any) { + // if input type is any, then output type could be any of the valid output types + new_types = io_types.into_iter().map(|(_, out_type)| out_type).collect(); } else { - for (call_input, call_output) in decl.signature().input_output_types { - if type_compatible(&call_input, ¤t_type) { - current_type = call_output.clone(); - continue 'elem; - } - } + // any current type which matches an input type is a possible output type + new_types = io_types + .into_iter() + .filter(|(in_type, _)| { + current_types.iter().any(|ty| type_compatible(in_type, ty)) + }) + .map(|(_, out_type)| out_type) + .collect(); } - if !decl.signature().input_output_types.is_empty() { - if let Some(output_errors) = &mut output_errors { - output_errors.push(ParseError::InputMismatch(current_type, call.head)) - } else { - output_errors = Some(vec![ParseError::InputMismatch(current_type, call.head)]); - } + if !new_types.is_empty() { + continue; } - current_type = Type::Any; + + if decl.signature().input_output_types.is_empty() { + new_types = vec![Type::Any]; + continue; + } + + let Some(types_string) = combined_type_string(¤t_types, "or") else { + output_errors + .get_or_insert_default() + .push(ParseError::InternalError( + "Pipeline has no type at this point".to_string(), + elem.expr.span, + )); + continue; + }; + + output_errors + .get_or_insert_default() + .push(ParseError::InputMismatch(types_string, call.head)); } else { - current_type = elem.expr.ty.clone(); + new_types = vec![elem.expr.ty.clone()]; } } - (current_type, output_errors) + (new_types, output_errors) } pub fn check_block_input_output(working_set: &StateWorkingSet, block: &Block) -> Vec { @@ -824,21 +831,29 @@ pub fn check_block_input_output(working_set: &StateWorkingSet, block: &Block) -> for (input_type, output_type) in &block.signature.input_output_types { let mut current_type = input_type.clone(); - let mut current_output_type = Type::Nothing; + let mut current_output_types = vec![]; for pipeline in &block.pipelines { - let (checked_output_type, err) = + let (checked_output_types, err) = check_pipeline_type(working_set, pipeline, current_type); - current_output_type = checked_output_type; + current_output_types = checked_output_types; current_type = Type::Nothing; if let Some(err) = err { output_errors.extend_from_slice(&err); } } - if !type_compatible(output_type, ¤t_output_type) - && output_type != &Type::Any - && current_output_type != Type::Any + if block.pipelines.is_empty() { + current_output_types = vec![Type::Nothing]; + } + + if output_type == &Type::Any || current_output_types.contains(&Type::Any) { + continue; + } + + if !current_output_types + .iter() + .any(|ty| type_compatible(output_type, ty)) { let span = if block.pipelines.is_empty() { if let Some(span) = block.span { @@ -858,9 +873,17 @@ pub fn check_block_input_output(working_set: &StateWorkingSet, block: &Block) -> .span }; + let Some(current_ty_string) = combined_type_string(¤t_output_types, "or") else { + output_errors.push(ParseError::InternalError( + "Block has no type at this point".to_string(), + span, + )); + continue; + }; + output_errors.push(ParseError::OutputMismatch( output_type.clone(), - current_output_type.clone(), + current_ty_string, span, )) } diff --git a/crates/nu-protocol/src/errors/parse_error.rs b/crates/nu-protocol/src/errors/parse_error.rs index 4044bc39e8..662905b203 100644 --- a/crates/nu-protocol/src/errors/parse_error.rs +++ b/crates/nu-protocol/src/errors/parse_error.rs @@ -60,13 +60,13 @@ pub enum ParseError { #[error("Command does not support {0} input.")] #[diagnostic(code(nu::parser::input_type_mismatch))] - InputMismatch(Type, #[label("command doesn't support {0} input")] Span), + InputMismatch(String, #[label("command doesn't support {0} input")] Span), #[error("Command output doesn't match {0}.")] #[diagnostic(code(nu::parser::output_type_mismatch))] OutputMismatch( Type, - Type, + String, #[label("expected {0}, but command outputs {1}")] Span, ), diff --git a/crates/nu-protocol/src/ty.rs b/crates/nu-protocol/src/ty.rs index 617606d238..33f2d3e024 100644 --- a/crates/nu-protocol/src/ty.rs +++ b/crates/nu-protocol/src/ty.rs @@ -215,6 +215,26 @@ impl Display for Type { } } +/// Get a string nicely combining multiple types +/// +/// Helpful for listing types in errors +pub fn combined_type_string(types: &[Type], join_word: &str) -> Option { + use std::fmt::Write as _; + match types { + [] => None, + [one] => Some(one.to_string()), + [one, two] => Some(format!("{one} {join_word} {two}")), + [initial @ .., last] => { + let mut out = String::new(); + for ele in initial { + let _ = write!(out, "{ele}, "); + } + let _ = write!(out, "{join_word} {last}"); + Some(out) + } + } +} + #[cfg(test)] mod tests { use super::Type; diff --git a/tests/repl/test_type_check.rs b/tests/repl/test_type_check.rs index 2fbc5a7842..c39ae5653c 100644 --- a/tests/repl/test_type_check.rs +++ b/tests/repl/test_type_check.rs @@ -1,4 +1,4 @@ -use crate::repl::tests::{TestResult, fail_test, run_test}; +use crate::repl::tests::{TestResult, fail_test, run_test, run_test_contains}; use rstest::rstest; #[test] @@ -178,3 +178,62 @@ fn in_oneof_block_expected_type(#[case] input: &str) -> TestResult { fn in_oneof_block_expected_block() -> TestResult { fail_test("match 1 { 0 => { try 3 } }", "expected block") } + +#[test] +fn pipeline_multiple_types() -> TestResult { + // https://github.com/nushell/nushell/issues/15485 + run_test_contains("{year: 2019} | into datetime | date humanize", "years ago") +} + +const MULTIPLE_TYPES_DEFS: &str = " +def foo []: [int -> int, int -> string] { + if $in > 2 { 'hi' } else 4 +} +def bar []: [int -> filesize, string -> string] { + if $in == 'hi' { 'meow' } else { into filesize } +} +"; + +#[test] +fn pipeline_multiple_types_custom() -> TestResult { + run_test( + &format!( + "{MULTIPLE_TYPES_DEFS} + 5 | foo | str trim" + ), + "hi", + ) +} + +#[test] +fn pipeline_multiple_types_propagate_string() -> TestResult { + run_test( + &format!( + "{MULTIPLE_TYPES_DEFS} + 5 | foo | bar | str trim" + ), + "meow", + ) +} + +#[test] +fn pipeline_multiple_types_propagate_int() -> TestResult { + run_test( + &format!( + "{MULTIPLE_TYPES_DEFS} + 2 | foo | bar | format filesize B" + ), + "4 B", + ) +} + +#[test] +fn pipeline_multiple_types_propagate_error() -> TestResult { + fail_test( + &format!( + "{MULTIPLE_TYPES_DEFS} + 2 | foo | bar | values" + ), + "parser::input_type_mismatch", + ) +}