Add parse error for external commands used in assignment without caret (#13585)

# Description

As per our Wednesday meeting, this adds a parse error when something
that would be parsed as an external call is present at the top level,
unless the head of the external call begins with a caret (to make it
explicit).

I tried to make the error quite descriptive about what should be done.

# User-Facing Changes
These now cause a parse error:

```nushell
$foo = bar
$foo = `bar`
```

These would have been interpreted as strings before this version, but
now they'd be interpreted as external calls. This behavior is consistent
with `let`/`mut` (which is unaffected by this change).

Here is an example of the error:

```
Error:   × External command calls must be explicit in assignments
   ╭─[entry #3:1:8]
 1 │ $foo = bar
   ·        ─┬─
   ·         ╰── add a caret (^) before the command name if you intended to run and capture its output
   ╰────
  help: the parsing of assignments was changed in 0.97.0, and this would have previously been treated as a string.
        Alternatively, quote the string with single or double quotes to avoid it being interpreted as a command name. This
        restriction may be removed in a future release.
```

# Tests + Formatting

Tests added to cover the change. Note made about it being temporary.
This commit is contained in:
Devyn Cairns 2024-08-12 01:24:23 -07:00 committed by GitHub
parent 983014cc40
commit 18772b73b3
No known key found for this signature in database
GPG key ID: B5690EEEBB952194
2 changed files with 50 additions and 0 deletions

View file

@ -4904,6 +4904,30 @@ pub fn parse_assignment_expression(
trace!("parsing: assignment right-hand side subexpression"); trace!("parsing: assignment right-hand side subexpression");
let rhs_block = parse_block(working_set, &rhs_tokens, rhs_span, false, true); let rhs_block = parse_block(working_set, &rhs_tokens, rhs_span, false, true);
let rhs_ty = rhs_block.output_type(); let rhs_ty = rhs_block.output_type();
// TEMP: double-check that if the RHS block starts with an external call, it must start with a
// caret. This is to mitigate the change in assignment parsing introduced in 0.97.0 which could
// result in unintentional execution of commands.
if let Some(Expr::ExternalCall(head, ..)) = rhs_block
.pipelines
.first()
.and_then(|pipeline| pipeline.elements.first())
.map(|element| &element.expr.expr)
{
let contents = working_set.get_span_contents(Span {
start: head.span.start - 1,
end: head.span.end,
});
if !contents.starts_with(b"^") {
working_set.parse_errors.push(ParseError::LabeledErrorWithHelp {
error: "External command calls must be explicit in assignments".into(),
label: "add a caret (^) before the command name if you intended to run and capture its output".into(),
help: "the parsing of assignments was changed in 0.97.0, and this would have previously been treated as a string. Alternatively, quote the string with single or double quotes to avoid it being interpreted as a command name. This restriction may be removed in a future release.".into(),
span: head.span,
});
}
}
let rhs_block_id = working_set.add_block(Arc::new(rhs_block)); let rhs_block_id = working_set.add_block(Arc::new(rhs_block));
let mut rhs = Expression::new( let mut rhs = Expression::new(
working_set, working_set,

View file

@ -311,6 +311,32 @@ fn append_assign_takes_pipeline() -> TestResult {
) )
} }
#[test]
fn assign_bare_external_fails() {
let result = nu!("$env.FOO = nu --testbin cococo");
assert!(!result.status.success());
assert!(result.err.contains("must be explicit"));
}
#[test]
fn assign_bare_external_with_caret() {
let result = nu!("$env.FOO = ^nu --testbin cococo");
assert!(result.status.success());
}
#[test]
fn assign_backtick_quoted_external_fails() {
let result = nu!("$env.FOO = `nu` --testbin cococo");
assert!(!result.status.success());
assert!(result.err.contains("must be explicit"));
}
#[test]
fn assign_backtick_quoted_external_with_caret() {
let result = nu!("$env.FOO = ^`nu` --testbin cococo");
assert!(result.status.success());
}
#[test] #[test]
fn string_interpolation_paren_test() -> TestResult { fn string_interpolation_paren_test() -> TestResult {
run_test(r#"$"('(')(')')""#, "()") run_test(r#"$"('(')(')')""#, "()")