Allow $env and mutable records to be mutated by = (closes #7110) (#7318)

# Description

Closes #7110. ~~Note that unlike "real" `mut` vars, $env can be deeply
mutated via stuff like `$env.PYTHON_IO_ENCODING = utf8` or
`$env.config.history.max_size = 2000`. So, it's a slightly awkward
special case, arguably justifiable because of what $env represents (the
environment variables of your system, which is essentially "outside"
normal Nushell regulations).~~
EDIT: Now allows all `mut` vars to be deeply mutated using `=`, on
request.

# User-Facing Changes

See above.

# 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:
Leon 2022-12-07 03:51:55 +10:00 committed by GitHub
parent 48ade4993d
commit 9b41f9ecb8
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 129 additions and 59 deletions

View File

@ -83,6 +83,11 @@ impl Command for Mut {
example: "mut x = 10; $x = 12",
result: None,
},
Example {
description: "Upsert a value inside a mutable data structure",
example: "mut a = {b:{c:1}}; $a.b.c = 2",
result: None,
},
Example {
description: "Set a mutable variable to the result of an expression",
example: "mut x = 10 + 100",

View File

@ -36,18 +36,6 @@ fn capture_of_mutable_var() {
assert!(actual.err.contains("capture of mutable variable"));
}
#[test]
fn mut_a_field() {
let actual = nu!(
cwd: ".", pipeline(
r#"
mut y = {abc: 123}; $y.abc = 456; $y.abc
"#
));
assert_eq!(actual.out, "456");
}
#[test]
fn mut_add_assign() {
let actual = nu!(
@ -95,3 +83,63 @@ fn mut_divide_assign() {
assert_eq!(actual.out, "4");
}
#[test]
fn mut_path_insert() {
let actual = nu!(
cwd: ".", pipeline(
r#"
mut y = {abc: 123}; $y.abc = 456; $y.abc
"#
));
assert_eq!(actual.out, "456");
}
#[test]
fn mut_path_insert_list() {
let actual = nu!(
cwd: ".", pipeline(
r#"
mut a = [0 1 2]; $a.3 = 3; $a | to nuon
"#
));
assert_eq!(actual.out, "[0, 1, 2, 3]");
}
#[test]
fn mut_path_upsert() {
let actual = nu!(
cwd: ".", pipeline(
r#"
mut a = {b:[{c:1}]}; $a.b.0.d = 11; $a.b.0.d
"#
));
assert_eq!(actual.out, "11");
}
#[test]
fn mut_path_upsert_list() {
let actual = nu!(
cwd: ".", pipeline(
r#"
mut a = [[[3] 2] 1]; $a.0.0.1 = 0; $a.0.2 = 0; $a.2 = 0; $a | to nuon
"#
));
assert_eq!(actual.out, "[[[3, 0], 2, 0], 1, 0]");
}
#[test]
fn mut_path_operator_assign() {
let actual = nu!(
cwd: ".", pipeline(
r#"
mut a = {b:1}; $a.b += 3; $a.b -= 2; $a.b *= 10; $a.b /= 4; $a.b
"#
));
assert_eq!(actual.out, "5");
}

View File

@ -491,30 +491,37 @@ pub fn eval_expression(
}
Expr::FullCellPath(cell_path) => match &cell_path.head.expr {
Expr::Var(var_id) | Expr::VarDecl(var_id) => {
if var_id == &ENV_VARIABLE_ID {
// let mut lhs =
// eval_expression(engine_state, stack, &cell_path.head)?;
//lhs.update_data_at_cell_path(&cell_path.tail, rhs)?;
match &cell_path.tail[0] {
PathMember::String { val, .. } => {
stack.add_env_var(val.to_string(), rhs);
}
PathMember::Int { val, .. } => {
stack.add_env_var(val.to_string(), rhs);
// The $env variable is considered "mutable" in Nushell.
// As such, give it special treatment here.
let is_env = var_id == &ENV_VARIABLE_ID;
if is_env || engine_state.get_var(*var_id).mutable {
let mut lhs =
eval_expression(engine_state, stack, &cell_path.head)?;
lhs.upsert_data_at_cell_path(&cell_path.tail, rhs)?;
if is_env {
// The special $env treatment: for something like $env.config.history.max_size = 2000,
// get $env.config (or whichever one it is) AFTER the above mutation, and set it
// as the "config" environment variable.
let vardata = lhs.follow_cell_path(
&[cell_path.tail[0].clone()],
false,
)?;
match &cell_path.tail[0] {
PathMember::String { val, .. } => {
stack.add_env_var(val.to_string(), vardata);
}
// In case someone really wants an integer env-var
PathMember::Int { val, .. } => {
stack.add_env_var(val.to_string(), vardata);
}
}
} else {
stack.vars.insert(*var_id, lhs);
}
Ok(Value::nothing(cell_path.head.span))
} else {
let var_info = engine_state.get_var(*var_id);
if var_info.mutable {
let mut lhs =
eval_expression(engine_state, stack, &cell_path.head)?;
lhs.update_data_at_cell_path(&cell_path.tail, rhs)?;
stack.vars.insert(*var_id, lhs);
Ok(Value::nothing(cell_path.head.span))
} else {
Err(ShellError::AssignmentRequiresMutableVar(lhs.span))
}
Err(ShellError::AssignmentRequiresMutableVar(lhs.span))
}
}
_ => Err(ShellError::AssignmentRequiresVar(lhs.span)),

View File

@ -1997,28 +1997,22 @@ pub fn parse_full_cell_path(
);
error = error.or(err);
if !tail.is_empty() {
(
Expression {
ty: head.ty.clone(), // FIXME. How to access the last type of tail?
expr: Expr::FullCellPath(Box::new(FullCellPath { head, tail })),
span: full_cell_span,
custom_completion: None,
(
Expression {
// FIXME: Get the type of the data at the tail using follow_cell_path() (or something)
ty: if !tail.is_empty() {
// Until the aforementioned fix is implemented, this is necessary to allow mutable list upserts
// such as $a.1 = 2 to work correctly.
Type::Any
} else {
head.ty.clone()
},
error,
)
} else {
let ty = head.ty.clone();
(
Expression {
expr: Expr::FullCellPath(Box::new(FullCellPath { head, tail })),
ty,
span: full_cell_span,
custom_completion: None,
},
error,
)
}
expr: Expr::FullCellPath(Box::new(FullCellPath { head, tail })),
span: full_cell_span,
custom_completion: None,
},
error,
)
} else {
(garbage(span), error)
}

View File

@ -376,6 +376,18 @@ Either make sure {0} is a string, or add a 'to_string' entry for it in ENV_CONVE
#[diagnostic(code(nu::shell::access_beyond_end), url(docsrs))]
AccessBeyondEnd(usize, #[label = "index too large (max: {0})"] Span),
/// You attempted to insert data at a list position higher than the end.
///
/// ## Resolution
///
/// To insert data into a list, assign to the last used index + 1.
#[error("Inserted at wrong row number (should be {0}).")]
#[diagnostic(code(nu::shell::access_beyond_end), url(docsrs))]
InsertAfterNextFreeIndex(
usize,
#[label = "can't insert at index (the next available index is {0})"] Span,
),
/// You attempted to access an index when it's empty.
///
/// ## Resolution

View File

@ -878,10 +878,12 @@ impl Value {
Value::List { vals, .. } => {
if let Some(v) = vals.get_mut(*row_num) {
v.upsert_data_at_cell_path(&cell_path[1..], new_val)?
} else if vals.is_empty() {
return Err(ShellError::AccessEmptyContent(*span));
} else if vals.len() == *row_num && cell_path.len() == 1 {
// If the upsert is at 1 + the end of the list, it's OK.
// Otherwise, it's prohibited.
vals.push(new_val);
} else {
return Err(ShellError::AccessBeyondEnd(vals.len() - 1, *span));
return Err(ShellError::InsertAfterNextFreeIndex(vals.len(), *span));
}
}
v => return Err(ShellError::NotAList(*span, v.span()?)),
@ -1266,10 +1268,12 @@ impl Value {
Value::List { vals, .. } => {
if let Some(v) = vals.get_mut(*row_num) {
v.insert_data_at_cell_path(&cell_path[1..], new_val)?
} else if vals.is_empty() {
return Err(ShellError::AccessEmptyContent(*span));
} else if vals.len() == *row_num && cell_path.len() == 1 {
// If the insert is at 1 + the end of the list, it's OK.
// Otherwise, it's prohibited.
vals.push(new_val);
} else {
return Err(ShellError::AccessBeyondEnd(vals.len() - 1, *span));
return Err(ShellError::InsertAfterNextFreeIndex(vals.len(), *span));
}
}
v => return Err(ShellError::NotAList(*span, v.span()?)),