Fix parse-time pipeline type checking to support multiple output types for same input type (#16111)

# Description
Fixes #15485

This PR changes pipeline checking to keep track of all possible output
types instead of only first type matching input type which appears in
the input/output types. For example, in this command:
```nushell
def foo []: [int -> string, int -> record] {
  # ...
}
```
An `int` input to the command may result in a string or a record to be
output. Before this PR, Nushell would always assume that an `int` input
would cause a `string` output because it's the first matching
input/output type pair. This would cause issues during type checking
where the parser would incorrectly determine the output type. After this
PR, Nushell considers the command to output either a string or a record.

# User-Facing Changes
* Parse-time pipeline type checking now properly supports commands with
multiple pipeline output types for the same pipeline input type

# Tests + Formatting
Added a couple tests

# After Submitting
N/A

---------

Co-authored-by: Bahex <[email protected]>

Author: 132ikl

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

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()),

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::<Vec<String>>();
-
-    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(", ")
-        }
-    };
+    let input_types: Vec<Type> = io_types.iter().map(|(input, _)| input.clone()).collect();
+    let expected_string = combined_type_string(&input_types, "and");
 
-    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(),
+        }),
     }
 }
 

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<Vec<ParseError>>) {
-    let mut current_type = input_type;
+) -> (Vec<Type>, Option<Vec<ParseError>>) {
+    let mut current_types: Vec<Type>;
+    let mut new_types: Vec<Type> = vec![input_type];
 
     let mut output_errors: Option<Vec<ParseError>> = 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, &current_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(&current_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<ParseError> {
@@ -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, &current_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(&current_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,
             ))
         }

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,
     ),
 

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<String> {
+    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;

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",
+    )
+}