mirror of
https://github.com/nushell/nushell.git
synced 2024-11-07 17:14:23 +01:00
prevent crash when use redirection with let/mut (#10139)
Fix #9992 I mistakenly messed up https://github.com/nushell/nushell/pull/10118 and this is a cleaned version. # Description * This pr changes the panic to errors while parsing `let`, now user will get the following errors: <img width="395" alt="scr" src="https://github.com/nushell/nushell/assets/1991933/4b39ac14-cd1f-47b3-9490-81009ca42717"> <img width="394" alt="scr" src="https://github.com/nushell/nushell/assets/1991933/71ce33ad-f4d0-4132-828f-9674b9603556"> <img width="440" alt="scr" src="https://github.com/nushell/nushell/assets/1991933/257eab4d-1a72-42db-b09e-f42bef33d2ec"> * `out+err>` is cached by `parse_expression` but not this, which may be a potential problem. * `Commond(None, ..)` remains panic for future bug report because I don't actually know when/how does it happen # User-Facing Changes Nushell won't crash when user typing `let a = 1 err> ...` # Tests + Formatting - `cargo fmt --all -- --check` : OK - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used` : OK - `cargo test --workspace` : OK - `cargo run -- -c "use std testing; testing run-tests --path crates/nu-std"` : OK # After Submitting None Co-authored-by: Horasal <horsal@horsal.dev>
This commit is contained in:
parent
fedd879b2e
commit
dac32557cd
@ -5439,6 +5439,17 @@ pub fn parse_pipeline(
|
|||||||
new_command.comments.extend_from_slice(&command.comments);
|
new_command.comments.extend_from_slice(&command.comments);
|
||||||
new_command.parts.extend_from_slice(&command.parts);
|
new_command.parts.extend_from_slice(&command.parts);
|
||||||
}
|
}
|
||||||
|
LiteElement::Redirection(span, ..) => {
|
||||||
|
working_set.error(ParseError::RedirectionInLetMut(*span, None))
|
||||||
|
}
|
||||||
|
LiteElement::SeparateRedirection { out, err } => {
|
||||||
|
working_set.error(ParseError::RedirectionInLetMut(
|
||||||
|
out.0.min(err.0),
|
||||||
|
Some(out.0.max(err.0)),
|
||||||
|
))
|
||||||
|
}
|
||||||
|
LiteElement::SameTargetRedirection { redirection, .. } => working_set
|
||||||
|
.error(ParseError::RedirectionInLetMut(redirection.0, None)),
|
||||||
_ => panic!("unsupported"),
|
_ => panic!("unsupported"),
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -781,6 +781,30 @@ fn test_nothing_comparison_eq() {
|
|||||||
)
|
)
|
||||||
))
|
))
|
||||||
}
|
}
|
||||||
|
#[rstest]
|
||||||
|
#[case(b"let a = 1 err> /dev/null", "RedirectionInLetMut")]
|
||||||
|
#[case(b"let a = 1 out> /dev/null", "RedirectionInLetMut")]
|
||||||
|
#[case(b"mut a = 1 err> /dev/null", "RedirectionInLetMut")]
|
||||||
|
#[case(b"mut a = 1 out> /dev/null", "RedirectionInLetMut")]
|
||||||
|
// This two cases cause AssignInPipeline instead of RedirectionInLetMut
|
||||||
|
#[case(b"let a = 1 out+err> /dev/null", "AssignInPipeline")]
|
||||||
|
#[case(b"mut a = 1 out+err> /dev/null", "AssignInPipeline")]
|
||||||
|
fn test_redirection_with_letmut(#[case] phase: &[u8], #[case] expected: &str) {
|
||||||
|
let engine_state = EngineState::new();
|
||||||
|
let mut working_set = StateWorkingSet::new(&engine_state);
|
||||||
|
let _block = parse(&mut working_set, None, phase, true);
|
||||||
|
match expected {
|
||||||
|
"RedirectionInLetMut" => assert!(matches!(
|
||||||
|
working_set.parse_errors.first(),
|
||||||
|
Some(ParseError::RedirectionInLetMut(_, _))
|
||||||
|
)),
|
||||||
|
"AssignInPipeline" => assert!(matches!(
|
||||||
|
working_set.parse_errors.first(),
|
||||||
|
Some(ParseError::AssignInPipeline(_, _, _, _))
|
||||||
|
)),
|
||||||
|
_ => panic!("unexpected pattern"),
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn test_nothing_comparison_neq() {
|
fn test_nothing_comparison_neq() {
|
||||||
|
@ -466,6 +466,13 @@ pub enum ParseError {
|
|||||||
#[label("{label}")]
|
#[label("{label}")]
|
||||||
span: Span,
|
span: Span,
|
||||||
},
|
},
|
||||||
|
|
||||||
|
#[error("Redirection can not be used with let/mut.")]
|
||||||
|
#[diagnostic()]
|
||||||
|
RedirectionInLetMut(
|
||||||
|
#[label("Not allowed here")] Span,
|
||||||
|
#[label("...and here")] Option<Span>,
|
||||||
|
),
|
||||||
}
|
}
|
||||||
|
|
||||||
impl ParseError {
|
impl ParseError {
|
||||||
@ -550,6 +557,7 @@ impl ParseError {
|
|||||||
ParseError::UnknownOperator(_, _, s) => *s,
|
ParseError::UnknownOperator(_, _, s) => *s,
|
||||||
ParseError::InvalidLiteral(_, _, s) => *s,
|
ParseError::InvalidLiteral(_, _, s) => *s,
|
||||||
ParseError::LabeledErrorWithHelp { span: s, .. } => *s,
|
ParseError::LabeledErrorWithHelp { span: s, .. } => *s,
|
||||||
|
ParseError::RedirectionInLetMut(s, _) => *s,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
Loading…
Reference in New Issue
Block a user