From 33535c514e6590278718347b41b66fa8de7e7888 Mon Sep 17 00:00:00 2001
From: JT <547158+jntrnr@users.noreply.github.com>
Date: Mon, 26 Jun 2023 05:59:56 +1200
Subject: [PATCH] Better error message if env var is used as var (#9522)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

# Description

This PR improves the error message if an environment variable (that's
visible before the parser begins) is used in the form of `$PATH` instead
of `$env.PATH`.

Before:

```
Error: nu::parser::variable_not_found

  × Variable not found.
   ╭─[entry #31:1:1]
 1 │ echo $PATH
   ·      ──┬──
   ·        ╰── variable not found.
   ╰────
```

After:

```
Error: nu::parser::env_var_not_var

  × Use $env.PATH instead of $PATH.
   ╭─[entry #1:1:1]
 1 │ echo $PATH
   ·      ──┬──
   ·        ╰── use $env.PATH instead of $PATH
   ╰────
```

# User-Facing Changes

Just the improvement to the error message

# 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 -A clippy::result_large_err` to check that
you're using the standard code style
- `cargo test --workspace` to check that all tests pass
- `cargo run -- crates/nu-std/tests/run.nu` to run the tests for the
standard library

> **Note**
> from `nushell` you can also use the `toolkit` as follows
> ```bash
> use toolkit.nu # or use an `env_change` hook to activate it
automatically
> toolkit check pr
> ```
-->

# 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.
-->
---
 crates/nu-parser/src/parser.rs        | 9 +++++++++
 crates/nu-protocol/src/parse_error.rs | 5 +++++
 tests/shell/environment/env.rs        | 8 ++++++++
 3 files changed, 22 insertions(+)

diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs
index 4df5248833..8edc4ae677 100644
--- a/crates/nu-parser/src/parser.rs
+++ b/crates/nu-parser/src/parser.rs
@@ -1817,6 +1817,12 @@ pub fn parse_variable_expr(working_set: &mut StateWorkingSet, span: Span) -> Exp
         };
     }
 
+    let name = if contents.starts_with(b"$") {
+        String::from_utf8_lossy(&contents[1..]).to_string()
+    } else {
+        String::from_utf8_lossy(contents).to_string()
+    };
+
     if let Some(id) = parse_variable(working_set, span) {
         Expression {
             expr: Expr::Var(id),
@@ -1824,6 +1830,9 @@ pub fn parse_variable_expr(working_set: &mut StateWorkingSet, span: Span) -> Exp
             ty: working_set.get_variable(id).ty.clone(),
             custom_completion: None,
         }
+    } else if working_set.get_env_var(&name).is_some() {
+        working_set.error(ParseError::EnvVarNotVar(name, span));
+        garbage(span)
     } else {
         let ws = &*working_set;
         let suggestion = DidYouMean::new(&ws.list_variables(), ws.get_span_contents(span));
diff --git a/crates/nu-protocol/src/parse_error.rs b/crates/nu-protocol/src/parse_error.rs
index 0393a52ee2..e7f3f834ef 100644
--- a/crates/nu-protocol/src/parse_error.rs
+++ b/crates/nu-protocol/src/parse_error.rs
@@ -173,6 +173,10 @@ pub enum ParseError {
     #[diagnostic(code(nu::parser::variable_not_found))]
     VariableNotFound(DidYouMean, #[label = "variable not found. {0}"] Span),
 
+    #[error("Use $env.{0} instead of ${0}.")]
+    #[diagnostic(code(nu::parser::env_var_not_var))]
+    EnvVarNotVar(String, #[label = "use $env.{0} instead of ${0}"] Span),
+
     #[error("Variable name not supported.")]
     #[diagnostic(code(nu::parser::variable_not_valid))]
     VariableNotValid(#[label = "variable name can't contain spaces or quotes"] Span),
@@ -492,6 +496,7 @@ impl ParseError {
             ParseError::IncorrectValue(_, s, _) => *s,
             ParseError::MultipleRestParams(s) => *s,
             ParseError::VariableNotFound(_, s) => *s,
+            ParseError::EnvVarNotVar(_, s) => *s,
             ParseError::VariableNotValid(s) => *s,
             ParseError::AliasNotValid(s) => *s,
             ParseError::CommandDefNotValid(s) => *s,
diff --git a/tests/shell/environment/env.rs b/tests/shell/environment/env.rs
index a62a7b4028..20f607399a 100644
--- a/tests/shell/environment/env.rs
+++ b/tests/shell/environment/env.rs
@@ -175,3 +175,11 @@ fn hides_env_in_block() {
     assert!(actual.err.contains("column_not_found"));
     assert!(actual_repl.err.contains("column_not_found"));
 }
+
+#[test]
+fn env_var_not_var() {
+    let actual = nu!(cwd: ".", r#"
+        echo $CARGO
+        "#);
+    assert!(actual.err.contains("use $env.CARGO instead of $CARGO"));
+}