simplify parse_expression function code. (#8149)

# Description

When reading parser code to see how it works, I found that
`parse_expression` function contains some duplicate code about function
call, we can match several values at once to simplify code

# User-Facing Changes

None

# 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:
WindSoilder 2023-02-22 20:14:20 +08:00 committed by GitHub
parent 4482862a40
commit 62652cf8c1
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 14 additions and 178 deletions

View File

@ -122,35 +122,15 @@ pub enum ParseError {
)]
BuiltinCommandInPipeline(String, #[label("not allowed in pipeline")] Span),
#[error("Let statement used in pipeline.")]
#[error("{0} statement used in pipeline.")]
#[diagnostic(
code(nu::parser::unexpected_keyword),
url(docsrs),
help(
"Assigning '{0}' to '{1}' does not produce a value to be piped. If the pipeline result is meant to be assigned to '{1}', use 'let {1} = ({0} | ...)'."
"Assigning '{1}' to '{2}' does not produce a value to be piped. If the pipeline result is meant to be assigned to '{2}', use '{0} {2} = ({1} | ...)'."
)
)]
LetInPipeline(String, String, #[label("let in pipeline")] Span),
#[error("Const statement used in pipeline.")]
#[diagnostic(
code(nu::parser::unexpected_keyword),
url(docsrs),
help(
"Assigning '{0}' to '{1}' does not produce a value to be piped. If the pipeline result is meant to be assigned to '{1}', use 'const {1} = ({0} | ...)'."
)
)]
ConstInPipeline(String, String, #[label("const in pipeline")] Span),
#[error("Mut statement used in pipeline.")]
#[diagnostic(
code(nu::parser::unexpected_keyword),
url(docsrs),
help(
"Assigning '{0}' to '{1}' does not produce a value to be piped. If the pipeline result is meant to be assigned to '{1}', use 'mut {1} = ({0} | ...)'."
)
)]
MutInPipeline(String, String, #[label("mut in pipeline")] Span),
AssignInPipeline(String, String, String, #[label("'{0}' in pipeline")] Span),
#[error("Let used with builtin variable name.")]
#[diagnostic(
@ -465,9 +445,7 @@ impl ParseError {
ParseError::ExpectedKeyword(_, s) => *s,
ParseError::UnexpectedKeyword(_, s) => *s,
ParseError::BuiltinCommandInPipeline(_, s) => *s,
ParseError::LetInPipeline(_, _, s) => *s,
ParseError::MutInPipeline(_, _, s) => *s,
ParseError::ConstInPipeline(_, _, s) => *s,
ParseError::AssignInPipeline(_, _, _, s) => *s,
ParseError::LetBuiltinVar(_, s) => *s,
ParseError::MutBuiltinVar(_, s) => *s,
ParseError::ConstBuiltinVar(_, s) => *s,

View File

@ -5015,22 +5015,12 @@ pub fn parse_expression(
{
parse_math_expression(working_set, &spans[pos..], None, expand_aliases_denylist)
} else {
let bytes = working_set.get_span_contents(spans[pos]);
let bytes = working_set.get_span_contents(spans[pos]).to_vec();
// For now, check for special parses of certain keywords
match bytes {
b"def" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::BuiltinCommandInPipeline("def".into(), spans[0])),
),
b"extern" => (
match bytes.as_slice() {
b"def" | b"extern" | b"for" | b"module" | b"use" | b"source" | b"alias" | b"export"
| b"hide" => (
parse_call(
working_set,
&spans[pos..],
@ -5040,11 +5030,12 @@ pub fn parse_expression(
)
.0,
Some(ParseError::BuiltinCommandInPipeline(
"extern".into(),
String::from_utf8(bytes)
.expect("builtin commands bytes should be able to convert to string"),
spans[0],
)),
),
b"for" => (
b"let" | b"const" | b"mut" => (
parse_call(
working_set,
&spans[pos..],
@ -5053,18 +5044,9 @@ pub fn parse_expression(
is_subexpression,
)
.0,
Some(ParseError::BuiltinCommandInPipeline("for".into(), spans[0])),
),
b"let" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::LetInPipeline(
Some(ParseError::AssignInPipeline(
String::from_utf8(bytes)
.expect("builtin commands bytes should be able to convert to string"),
String::from_utf8_lossy(match spans.len() {
1 | 2 | 3 => b"value",
_ => working_set.get_span_contents(spans[3]),
@ -5078,91 +5060,6 @@ pub fn parse_expression(
spans[0],
)),
),
b"const" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::ConstInPipeline(
String::from_utf8_lossy(match spans.len() {
1 | 2 | 3 => b"value",
_ => working_set.get_span_contents(spans[3]),
})
.to_string(),
String::from_utf8_lossy(match spans.len() {
1 => b"variable",
_ => working_set.get_span_contents(spans[1]),
})
.to_string(),
spans[0],
)),
),
b"mut" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::MutInPipeline(
String::from_utf8_lossy(match spans.len() {
1 | 2 | 3 => b"value",
_ => working_set.get_span_contents(spans[3]),
})
.to_string(),
String::from_utf8_lossy(match spans.len() {
1 => b"variable",
_ => working_set.get_span_contents(spans[1]),
})
.to_string(),
spans[0],
)),
),
b"alias" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::BuiltinCommandInPipeline(
"alias".into(),
spans[0],
)),
),
b"module" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::BuiltinCommandInPipeline(
"module".into(),
spans[0],
)),
),
b"use" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::BuiltinCommandInPipeline("use".into(), spans[0])),
),
b"overlay" => {
if spans.len() > 1 && working_set.get_span_contents(spans[1]) == b"list" {
// whitelist 'overlay list'
@ -5190,45 +5087,6 @@ pub fn parse_expression(
)
}
}
b"source" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::BuiltinCommandInPipeline(
"source".into(),
spans[0],
)),
),
b"export" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::UnexpectedKeyword("export".into(), spans[0])),
),
b"hide" => (
parse_call(
working_set,
&spans[pos..],
spans[0],
expand_aliases_denylist,
is_subexpression,
)
.0,
Some(ParseError::BuiltinCommandInPipeline(
"hide".into(),
spans[0],
)),
),
b"where" => parse_where_expr(working_set, &spans[pos..], expand_aliases_denylist),
#[cfg(feature = "plugin")]
b"register" => (