mirror of
https://github.com/nushell/nushell.git
synced 2025-08-09 15:25:06 +02:00
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 <Bahex@users.noreply.github.com>
This commit is contained in:
@ -82,9 +82,6 @@ impl Command for IntoDatetime {
|
|||||||
(Type::List(Box::new(Type::String)), Type::List(Box::new(Type::Date))),
|
(Type::List(Box::new(Type::String)), Type::List(Box::new(Type::Date))),
|
||||||
(Type::table(), Type::table()),
|
(Type::table(), Type::table()),
|
||||||
(Type::Nothing, 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::record()),
|
||||||
(Type::record(), Type::Date),
|
(Type::record(), Type::Date),
|
||||||
// FIXME Type::Any input added to disable pipeline input type checking, as run-time checks can raise undesirable type errors
|
// FIXME Type::Any input added to disable pipeline input type checking, as run-time checks can raise undesirable type errors
|
||||||
|
@ -53,9 +53,6 @@ impl Command for IntoDuration {
|
|||||||
(Type::Float, Type::Duration),
|
(Type::Float, Type::Duration),
|
||||||
(Type::String, Type::Duration),
|
(Type::String, Type::Duration),
|
||||||
(Type::Duration, 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::record()),
|
||||||
(Type::record(), Type::Duration),
|
(Type::record(), Type::Duration),
|
||||||
(Type::table(), Type::table()),
|
(Type::table(), Type::table()),
|
||||||
|
@ -6,6 +6,7 @@ use nu_protocol::{
|
|||||||
PipelineData, PipelineMetadata, PositionalArg, Range, Record, RegId, ShellError, Signals,
|
PipelineData, PipelineMetadata, PositionalArg, Range, Record, RegId, ShellError, Signals,
|
||||||
Signature, Span, Spanned, Type, Value, VarId,
|
Signature, Span, Spanned, Type, Value, VarId,
|
||||||
ast::{Bits, Block, Boolean, CellPath, Comparison, Math, Operator},
|
ast::{Bits, Block, Boolean, CellPath, Comparison, Math, Operator},
|
||||||
|
combined_type_string,
|
||||||
debugger::DebugContext,
|
debugger::DebugContext,
|
||||||
engine::{
|
engine::{
|
||||||
Argument, Closure, EngineState, ErrorHandler, Matcher, Redirection, Stack, StateWorkingSet,
|
Argument, Closure, EngineState, ErrorHandler, Matcher, Redirection, Stack, StateWorkingSet,
|
||||||
@ -1315,37 +1316,21 @@ fn check_input_types(
|
|||||||
return Ok(());
|
return Ok(());
|
||||||
}
|
}
|
||||||
|
|
||||||
let mut input_types = io_types
|
let input_types: Vec<Type> = io_types.iter().map(|(input, _)| input.clone()).collect();
|
||||||
.iter()
|
let expected_string = combined_type_string(&input_types, "and");
|
||||||
.map(|(input, _)| input.to_string())
|
|
||||||
.collect::<Vec<String>>();
|
|
||||||
|
|
||||||
let expected_string = match input_types.len() {
|
match (input, expected_string) {
|
||||||
0 => {
|
(PipelineData::Empty, _) => Err(ShellError::PipelineEmpty { dst_span: head }),
|
||||||
return Err(ShellError::NushellFailed {
|
(_, Some(expected_string)) => Err(ShellError::OnlySupportsThisInputType {
|
||||||
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 {
|
|
||||||
exp_input_type: expected_string,
|
exp_input_type: expected_string,
|
||||||
wrong_type: input.get_type().to_string(),
|
wrong_type: input.get_type().to_string(),
|
||||||
dst_span: head,
|
dst_span: head,
|
||||||
src_span: input.span().unwrap_or(Span::unknown()),
|
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(),
|
||||||
|
}),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1,6 +1,7 @@
|
|||||||
use nu_protocol::{
|
use nu_protocol::{
|
||||||
ParseError, Span, Type,
|
ParseError, Span, Type,
|
||||||
ast::{Assignment, Block, Comparison, Expr, Expression, Math, Operator, Pipeline, Range},
|
ast::{Assignment, Block, Comparison, Expr, Expression, Math, Operator, Pipeline, Range},
|
||||||
|
combined_type_string,
|
||||||
engine::StateWorkingSet,
|
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(
|
pub fn check_pipeline_type(
|
||||||
working_set: &StateWorkingSet,
|
working_set: &StateWorkingSet,
|
||||||
pipeline: &Pipeline,
|
pipeline: &Pipeline,
|
||||||
input_type: Type,
|
input_type: Type,
|
||||||
) -> (Type, Option<Vec<ParseError>>) {
|
) -> (Vec<Type>, Option<Vec<ParseError>>) {
|
||||||
let mut current_type = input_type;
|
let mut current_types: Vec<Type>;
|
||||||
|
let mut new_types: Vec<Type> = vec![input_type];
|
||||||
|
|
||||||
let mut output_errors: Option<Vec<ParseError>> = None;
|
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() {
|
if elem.redirection.is_some() {
|
||||||
current_type = Type::Any;
|
new_types = vec![Type::Any];
|
||||||
} else if let Expr::Call(call) = &elem.expr.expr {
|
continue;
|
||||||
|
}
|
||||||
|
if let Expr::Call(call) = &elem.expr.expr {
|
||||||
let decl = working_set.get_decl(call.decl_id);
|
let decl = working_set.get_decl(call.decl_id);
|
||||||
|
let io_types = decl.signature().input_output_types;
|
||||||
if current_type == Type::Any {
|
if new_types.contains(&Type::Any) {
|
||||||
let mut new_current_type = None;
|
// if input type is any, then output type could be any of the valid output types
|
||||||
for (_, call_output) in decl.signature().input_output_types {
|
new_types = io_types.into_iter().map(|(_, out_type)| out_type).collect();
|
||||||
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 {
|
} else {
|
||||||
new_current_type = Some(call_output.clone())
|
// 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 !new_types.is_empty() {
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
|
||||||
|
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 {
|
||||||
|
new_types = vec![elem.expr.ty.clone()];
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if let Some(new_current_type) = new_current_type {
|
(new_types, output_errors)
|
||||||
current_type = new_current_type
|
|
||||||
} else {
|
|
||||||
current_type = Type::Any;
|
|
||||||
}
|
|
||||||
continue 'elem;
|
|
||||||
} 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;
|
|
||||||
}
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
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)]);
|
|
||||||
}
|
|
||||||
}
|
|
||||||
current_type = Type::Any;
|
|
||||||
} else {
|
|
||||||
current_type = elem.expr.ty.clone();
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
(current_type, output_errors)
|
|
||||||
}
|
}
|
||||||
|
|
||||||
pub fn check_block_input_output(working_set: &StateWorkingSet, block: &Block) -> Vec<ParseError> {
|
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 {
|
for (input_type, output_type) in &block.signature.input_output_types {
|
||||||
let mut current_type = input_type.clone();
|
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 {
|
for pipeline in &block.pipelines {
|
||||||
let (checked_output_type, err) =
|
let (checked_output_types, err) =
|
||||||
check_pipeline_type(working_set, pipeline, current_type);
|
check_pipeline_type(working_set, pipeline, current_type);
|
||||||
current_output_type = checked_output_type;
|
current_output_types = checked_output_types;
|
||||||
current_type = Type::Nothing;
|
current_type = Type::Nothing;
|
||||||
if let Some(err) = err {
|
if let Some(err) = err {
|
||||||
output_errors.extend_from_slice(&err);
|
output_errors.extend_from_slice(&err);
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
if !type_compatible(output_type, ¤t_output_type)
|
if block.pipelines.is_empty() {
|
||||||
&& output_type != &Type::Any
|
current_output_types = vec![Type::Nothing];
|
||||||
&& current_output_type != Type::Any
|
}
|
||||||
|
|
||||||
|
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() {
|
let span = if block.pipelines.is_empty() {
|
||||||
if let Some(span) = block.span {
|
if let Some(span) = block.span {
|
||||||
@ -858,9 +873,17 @@ pub fn check_block_input_output(working_set: &StateWorkingSet, block: &Block) ->
|
|||||||
.span
|
.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_errors.push(ParseError::OutputMismatch(
|
||||||
output_type.clone(),
|
output_type.clone(),
|
||||||
current_output_type.clone(),
|
current_ty_string,
|
||||||
span,
|
span,
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
@ -60,13 +60,13 @@ pub enum ParseError {
|
|||||||
|
|
||||||
#[error("Command does not support {0} input.")]
|
#[error("Command does not support {0} input.")]
|
||||||
#[diagnostic(code(nu::parser::input_type_mismatch))]
|
#[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}.")]
|
#[error("Command output doesn't match {0}.")]
|
||||||
#[diagnostic(code(nu::parser::output_type_mismatch))]
|
#[diagnostic(code(nu::parser::output_type_mismatch))]
|
||||||
OutputMismatch(
|
OutputMismatch(
|
||||||
Type,
|
Type,
|
||||||
Type,
|
String,
|
||||||
#[label("expected {0}, but command outputs {1}")] Span,
|
#[label("expected {0}, but command outputs {1}")] Span,
|
||||||
),
|
),
|
||||||
|
|
||||||
|
@ -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)]
|
#[cfg(test)]
|
||||||
mod tests {
|
mod tests {
|
||||||
use super::Type;
|
use super::Type;
|
||||||
|
@ -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;
|
use rstest::rstest;
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@ -178,3 +178,62 @@ fn in_oneof_block_expected_type(#[case] input: &str) -> TestResult {
|
|||||||
fn in_oneof_block_expected_block() -> TestResult {
|
fn in_oneof_block_expected_block() -> TestResult {
|
||||||
fail_test("match 1 { 0 => { try 3 } }", "expected block")
|
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",
|
||||||
|
)
|
||||||
|
}
|
||||||
|
Reference in New Issue
Block a user