mirror of
https://github.com/nushell/nushell.git
synced 2024-11-28 19:33:47 +01:00
Better errors when bash-like operators are used (#7241)
# Description Adds improved errors for when a user uses a bashism that nu doesn't support. fixes #7237 Examples: ``` Error: nu::parser::shell_andand (link) × The '&&' operator is not supported in Nushell ╭─[entry #1:1:1] 1 │ ls && ls · ─┬ · ╰── instead of '&&', use ';' or 'and' ╰──── help: use ';' instead of the shell '&&', or 'and' instead of the boolean '&&' ``` ``` Error: nu::parser::shell_oror (link) × The '||' operator is not supported in Nushell ╭─[entry #8:1:1] 1 │ ls || ls · ─┬ · ╰── instead of '||', use 'try' or 'or' ╰──── help: use 'try' instead of the shell '||', or 'or' instead of the boolean '||' ``` ``` Error: nu::parser::shell_err (link) × The '2>' shell operation is 'err>' in Nushell. ╭─[entry #9:1:1] 1 │ foo 2> bar.txt · ─┬ · ╰── use 'err>' instead of '2>' in Nushell ╰──── ``` ``` Error: nu::parser::shell_outerr (link) × The '2>&1' shell operation is 'out+err>' in Nushell. ╭─[entry #10:1:1] 1 │ foo 2>&1 bar.txt · ──┬─ · ╰── use 'out+err>' instead of '2>&1' in Nushell ╰──── help: Nushell redirection will write all of stdout before stderr. ``` # User-Facing Changes **BREAKING CHANGES** This removes the `&&` and `||` operators. We previously supported by `&&`/`and` and `||`/`or`. With this change, only `and` and `or` are valid boolean operators. # Tests + Formatting Don't forget to add tests that cover your changes. Make sure you've run and fixed any issues with these commands: - `cargo fmt --all -- --check` to check standard code formatting (`cargo fmt --all` applies these changes) - `cargo clippy --workspace -- -D warnings -D clippy::unwrap_used -A clippy::needless_collect` to check that you're using the standard code style - `cargo test --workspace` to check that all tests pass # After Submitting If your PR had any user-facing changes, update [the documentation](https://github.com/nushell/nushell.github.io) after the PR is merged, if necessary. This will help us keep the docs up to date.
This commit is contained in:
parent
6fc87fad72
commit
379e3d70ca
@ -431,7 +431,7 @@ fn compound_comparison() {
|
|||||||
let actual = nu!(
|
let actual = nu!(
|
||||||
cwd: "tests/fixtures/formats", pipeline(
|
cwd: "tests/fixtures/formats", pipeline(
|
||||||
r#"
|
r#"
|
||||||
4 > 3 && 2 > 1
|
4 > 3 and 2 > 1
|
||||||
"#
|
"#
|
||||||
));
|
));
|
||||||
|
|
||||||
@ -443,7 +443,7 @@ fn compound_comparison2() {
|
|||||||
let actual = nu!(
|
let actual = nu!(
|
||||||
cwd: "tests/fixtures/formats", pipeline(
|
cwd: "tests/fixtures/formats", pipeline(
|
||||||
r#"
|
r#"
|
||||||
4 < 3 || 2 > 1
|
4 < 3 or 2 > 1
|
||||||
"#
|
"#
|
||||||
));
|
));
|
||||||
|
|
||||||
@ -455,7 +455,7 @@ fn compound_where() {
|
|||||||
let actual = nu!(
|
let actual = nu!(
|
||||||
cwd: "tests/fixtures/formats", pipeline(
|
cwd: "tests/fixtures/formats", pipeline(
|
||||||
r#"
|
r#"
|
||||||
echo '[{"a": 1, "b": 1}, {"a": 2, "b": 1}, {"a": 2, "b": 2}]' | from json | where a == 2 && b == 1 | to json -r
|
echo '[{"a": 1, "b": 1}, {"a": 2, "b": 1}, {"a": 2, "b": 2}]' | from json | where a == 2 and b == 1 | to json -r
|
||||||
"#
|
"#
|
||||||
));
|
));
|
||||||
|
|
||||||
@ -467,7 +467,7 @@ fn compound_where_paren() {
|
|||||||
let actual = nu!(
|
let actual = nu!(
|
||||||
cwd: "tests/fixtures/formats", pipeline(
|
cwd: "tests/fixtures/formats", pipeline(
|
||||||
r#"
|
r#"
|
||||||
echo '[{"a": 1, "b": 1}, {"a": 2, "b": 1}, {"a": 2, "b": 2}]' | from json | where ($it.a == 2 && $it.b == 1) || $it.b == 2 | to json -r
|
echo '[{"a": 1, "b": 1}, {"a": 2, "b": 1}, {"a": 2, "b": 2}]' | from json | where ($it.a == 2 and $it.b == 1) or $it.b == 2 | to json -r
|
||||||
"#
|
"#
|
||||||
));
|
));
|
||||||
|
|
||||||
|
@ -9,7 +9,7 @@ export def expect [
|
|||||||
right
|
right
|
||||||
] {
|
] {
|
||||||
$left | zip $right | all {|row|
|
$left | zip $right | all {|row|
|
||||||
$row.name.0 == $row.name.1 && $row.commits.0 == $row.commits.1
|
$row.name.0 == $row.name.1 and $row.commits.0 == $row.commits.1
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
"#;
|
"#;
|
||||||
|
@ -42,6 +42,34 @@ pub enum ParseError {
|
|||||||
#[diagnostic(code(nu::parser::type_mismatch), url(docsrs))]
|
#[diagnostic(code(nu::parser::type_mismatch), url(docsrs))]
|
||||||
Mismatch(String, String, #[label("expected {0}, found {1}")] Span), // expected, found, span
|
Mismatch(String, String, #[label("expected {0}, found {1}")] Span), // expected, found, span
|
||||||
|
|
||||||
|
#[error("The '&&' operator is not supported in Nushell")]
|
||||||
|
#[diagnostic(
|
||||||
|
code(nu::parser::shell_andand),
|
||||||
|
url(docsrs),
|
||||||
|
help("use ';' instead of the shell '&&', or 'and' instead of the boolean '&&'")
|
||||||
|
)]
|
||||||
|
ShellAndAnd(#[label("instead of '&&', use ';' or 'and'")] Span),
|
||||||
|
|
||||||
|
#[error("The '||' operator is not supported in Nushell")]
|
||||||
|
#[diagnostic(
|
||||||
|
code(nu::parser::shell_oror),
|
||||||
|
url(docsrs),
|
||||||
|
help("use 'try' instead of the shell '||', or 'or' instead of the boolean '||'")
|
||||||
|
)]
|
||||||
|
ShellOrOr(#[label("instead of '||', use 'try' or 'or'")] Span),
|
||||||
|
|
||||||
|
#[error("The '2>' shell operation is 'err>' in Nushell.")]
|
||||||
|
#[diagnostic(code(nu::parser::shell_err), url(docsrs))]
|
||||||
|
ShellErrRedirect(#[label("use 'err>' instead of '2>' in Nushell")] Span),
|
||||||
|
|
||||||
|
#[error("The '2>&1' shell operation is 'out+err>' in Nushell.")]
|
||||||
|
#[diagnostic(
|
||||||
|
code(nu::parser::shell_outerr),
|
||||||
|
url(docsrs),
|
||||||
|
help("Nushell redirection will write all of stdout before stderr.")
|
||||||
|
)]
|
||||||
|
ShellOutErrRedirect(#[label("use 'out+err>' instead of '2>&1' in Nushell")] Span),
|
||||||
|
|
||||||
#[error("Types mismatched for operation.")]
|
#[error("Types mismatched for operation.")]
|
||||||
#[diagnostic(
|
#[diagnostic(
|
||||||
code(nu::parser::unsupported_operation),
|
code(nu::parser::unsupported_operation),
|
||||||
@ -421,6 +449,10 @@ impl ParseError {
|
|||||||
ParseError::FileNotFound(_, s) => *s,
|
ParseError::FileNotFound(_, s) => *s,
|
||||||
ParseError::ReadingFile(_, s) => *s,
|
ParseError::ReadingFile(_, s) => *s,
|
||||||
ParseError::LabeledError(_, _, s) => *s,
|
ParseError::LabeledError(_, _, s) => *s,
|
||||||
|
ParseError::ShellAndAnd(s) => *s,
|
||||||
|
ParseError::ShellOrOr(s) => *s,
|
||||||
|
ParseError::ShellErrRedirect(s) => *s,
|
||||||
|
ParseError::ShellOutErrRedirect(s) => *s,
|
||||||
ParseError::UnknownOperator(_, _, s) => *s,
|
ParseError::UnknownOperator(_, _, s) => *s,
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -6,6 +6,7 @@ pub enum TokenContents {
|
|||||||
Item,
|
Item,
|
||||||
Comment,
|
Comment,
|
||||||
Pipe,
|
Pipe,
|
||||||
|
PipePipe,
|
||||||
Semicolon,
|
Semicolon,
|
||||||
OutGreaterThan,
|
OutGreaterThan,
|
||||||
ErrGreaterThan,
|
ErrGreaterThan,
|
||||||
@ -251,6 +252,27 @@ pub fn lex_item(
|
|||||||
},
|
},
|
||||||
None,
|
None,
|
||||||
),
|
),
|
||||||
|
b"&&" => (
|
||||||
|
Token {
|
||||||
|
contents: TokenContents::Item,
|
||||||
|
span,
|
||||||
|
},
|
||||||
|
Some(ParseError::ShellAndAnd(span)),
|
||||||
|
),
|
||||||
|
b"2>" => (
|
||||||
|
Token {
|
||||||
|
contents: TokenContents::Item,
|
||||||
|
span,
|
||||||
|
},
|
||||||
|
Some(ParseError::ShellErrRedirect(span)),
|
||||||
|
),
|
||||||
|
b"2>&1" => (
|
||||||
|
Token {
|
||||||
|
contents: TokenContents::Item,
|
||||||
|
span,
|
||||||
|
},
|
||||||
|
Some(ParseError::ShellOutErrRedirect(span)),
|
||||||
|
),
|
||||||
_ => (
|
_ => (
|
||||||
Token {
|
Token {
|
||||||
contents: TokenContents::Item,
|
contents: TokenContents::Item,
|
||||||
@ -289,7 +311,7 @@ pub fn lex(
|
|||||||
let idx = curr_offset;
|
let idx = curr_offset;
|
||||||
curr_offset += 1;
|
curr_offset += 1;
|
||||||
output.push(Token::new(
|
output.push(Token::new(
|
||||||
TokenContents::Item,
|
TokenContents::PipePipe,
|
||||||
Span::new(span_offset + prev_idx, span_offset + idx + 1),
|
Span::new(span_offset + prev_idx, span_offset + idx + 1),
|
||||||
));
|
));
|
||||||
continue;
|
continue;
|
||||||
|
@ -1212,18 +1212,17 @@ fn parse_binary_with_base(
|
|||||||
|
|
||||||
binary_value.extend_from_slice(contents);
|
binary_value.extend_from_slice(contents);
|
||||||
}
|
}
|
||||||
TokenContents::Pipe => {
|
TokenContents::Pipe
|
||||||
|
| TokenContents::PipePipe
|
||||||
|
| TokenContents::OutGreaterThan
|
||||||
|
| TokenContents::ErrGreaterThan
|
||||||
|
| TokenContents::OutErrGreaterThan => {
|
||||||
return (
|
return (
|
||||||
garbage(span),
|
garbage(span),
|
||||||
Some(ParseError::Expected("binary".into(), span)),
|
Some(ParseError::Expected("binary".into(), span)),
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
TokenContents::Comment
|
TokenContents::Comment | TokenContents::Semicolon | TokenContents::Eol => {}
|
||||||
| TokenContents::Semicolon
|
|
||||||
| TokenContents::Eol
|
|
||||||
| TokenContents::OutGreaterThan
|
|
||||||
| TokenContents::ErrGreaterThan
|
|
||||||
| TokenContents::OutErrGreaterThan => {}
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
@ -4075,19 +4074,12 @@ pub fn parse_closure_expression(
|
|||||||
(Some((signature, signature_span)), amt_to_skip)
|
(Some((signature, signature_span)), amt_to_skip)
|
||||||
}
|
}
|
||||||
Some(Token {
|
Some(Token {
|
||||||
contents: TokenContents::Item,
|
contents: TokenContents::PipePipe,
|
||||||
span,
|
span,
|
||||||
}) => {
|
}) => (
|
||||||
let contents = working_set.get_span_contents(*span);
|
Some((Box::new(Signature::new("closure".to_string())), *span)),
|
||||||
if contents == b"||" {
|
1,
|
||||||
(
|
),
|
||||||
Some((Box::new(Signature::new("closure".to_string())), *span)),
|
|
||||||
1,
|
|
||||||
)
|
|
||||||
} else {
|
|
||||||
(None, 0)
|
|
||||||
}
|
|
||||||
}
|
|
||||||
_ => (None, 0),
|
_ => (None, 0),
|
||||||
};
|
};
|
||||||
|
|
||||||
@ -4456,8 +4448,8 @@ pub fn parse_operator(
|
|||||||
b"bit-shr" => Operator::Bits(Bits::ShiftRight),
|
b"bit-shr" => Operator::Bits(Bits::ShiftRight),
|
||||||
b"starts-with" => Operator::Comparison(Comparison::StartsWith),
|
b"starts-with" => Operator::Comparison(Comparison::StartsWith),
|
||||||
b"ends-with" => Operator::Comparison(Comparison::EndsWith),
|
b"ends-with" => Operator::Comparison(Comparison::EndsWith),
|
||||||
b"&&" | b"and" => Operator::Boolean(Boolean::And),
|
b"and" => Operator::Boolean(Boolean::And),
|
||||||
b"||" | b"or" => Operator::Boolean(Boolean::Or),
|
b"or" => Operator::Boolean(Boolean::Or),
|
||||||
b"xor" => Operator::Boolean(Boolean::Xor),
|
b"xor" => Operator::Boolean(Boolean::Xor),
|
||||||
b"**" => Operator::Math(Math::Pow),
|
b"**" => Operator::Math(Math::Pow),
|
||||||
// WARNING: not actual operators below! Error handling only
|
// WARNING: not actual operators below! Error handling only
|
||||||
@ -5916,8 +5908,15 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option<ParseError>) {
|
|||||||
|
|
||||||
let mut curr_comment: Option<Vec<Span>> = None;
|
let mut curr_comment: Option<Vec<Span>> = None;
|
||||||
|
|
||||||
|
let mut error = None;
|
||||||
|
|
||||||
for token in tokens.iter() {
|
for token in tokens.iter() {
|
||||||
match &token.contents {
|
match &token.contents {
|
||||||
|
TokenContents::PipePipe => {
|
||||||
|
error = error.or(Some(ParseError::ShellOrOr(token.span)));
|
||||||
|
curr_command.push(token.span);
|
||||||
|
last_token = TokenContents::Item;
|
||||||
|
}
|
||||||
TokenContents::Item => {
|
TokenContents::Item => {
|
||||||
// If we have a comment, go ahead and attach it
|
// If we have a comment, go ahead and attach it
|
||||||
if let Some(curr_comment) = curr_comment.take() {
|
if let Some(curr_comment) = curr_comment.take() {
|
||||||
@ -6163,7 +6162,7 @@ pub fn lite_parse(tokens: &[Token]) -> (LiteBlock, Option<ParseError>) {
|
|||||||
)),
|
)),
|
||||||
)
|
)
|
||||||
} else {
|
} else {
|
||||||
(block, None)
|
(block, error)
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
@ -1388,11 +1388,11 @@ mod input_types {
|
|||||||
|
|
||||||
let mut working_set = StateWorkingSet::new(&engine_state);
|
let mut working_set = StateWorkingSet::new(&engine_state);
|
||||||
let inputs = vec![
|
let inputs = vec![
|
||||||
r#"let a = 'b'; ($a == 'b') || ($a == 'b')"#,
|
r#"let a = 'b'; ($a == 'b') or ($a == 'b')"#,
|
||||||
r#"let a = 'b'; ($a == 'b') || ($a == 'b') && ($a == 'b')"#,
|
r#"let a = 'b'; ($a == 'b') or ($a == 'b') and ($a == 'b')"#,
|
||||||
r#"let a = 1; ($a == 1) || ($a == 2) && ($a == 3)"#,
|
r#"let a = 1; ($a == 1) or ($a == 2) and ($a == 3)"#,
|
||||||
r#"let a = 'b'; if ($a == 'b') || ($a == 'b') { true } else { false }"#,
|
r#"let a = 'b'; if ($a == 'b') or ($a == 'b') { true } else { false }"#,
|
||||||
r#"let a = 1; if ($a == 1) || ($a > 0) { true } else { false }"#,
|
r#"let a = 1; if ($a == 1) or ($a > 0) { true } else { false }"#,
|
||||||
];
|
];
|
||||||
|
|
||||||
for input in inputs {
|
for input in inputs {
|
||||||
|
@ -238,12 +238,12 @@ fn datetime_literal() -> TestResult {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn shortcircuiting_and() -> TestResult {
|
fn shortcircuiting_and() -> TestResult {
|
||||||
run_test(r#"false && (5 / 0; false)"#, "false")
|
run_test(r#"false and (5 / 0; false)"#, "false")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn shortcircuiting_or() -> TestResult {
|
fn shortcircuiting_or() -> TestResult {
|
||||||
run_test(r#"true || (5 / 0; false)"#, "true")
|
run_test(r#"true or (5 / 0; false)"#, "true")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
@ -47,12 +47,12 @@ fn sub_bit_shr() -> TestResult {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn and() -> TestResult {
|
fn and() -> TestResult {
|
||||||
run_test("true && false", "false")
|
run_test("true and false", "false")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn or() -> TestResult {
|
fn or() -> TestResult {
|
||||||
run_test("true || false", "true")
|
run_test("true or false", "true")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
@ -112,7 +112,7 @@ fn floating_add() -> TestResult {
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn precedence_of_or_groups() -> TestResult {
|
fn precedence_of_or_groups() -> TestResult {
|
||||||
run_test(r#"4 mod 3 == 0 || 5 mod 5 == 0"#, "true")
|
run_test(r#"4 mod 3 == 0 or 5 mod 5 == 0"#, "true")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
@ -2,7 +2,7 @@ use crate::tests::{fail_test, run_test, TestResult};
|
|||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
fn chained_operator_typecheck() -> TestResult {
|
fn chained_operator_typecheck() -> TestResult {
|
||||||
run_test("1 != 2 && 3 != 4 && 5 != 6", "true")
|
run_test("1 != 2 and 3 != 4 and 5 != 6", "true")
|
||||||
}
|
}
|
||||||
|
|
||||||
#[test]
|
#[test]
|
||||||
|
Loading…
Reference in New Issue
Block a user