Forbid reserved variable names for function arguments (#11169)

Works for all arguments and flags. Because the signature parsing doesn't
give the spans, it is flags the entire signature.

Also added a constant with reserved variable names.

Fix #11158.

crates/nu-parser/src/parse_keywords.rs

@@ -40,6 +40,8 @@ use crate::{
 /// These parser keywords can be aliased
 pub const ALIASABLE_PARSER_KEYWORDS: &[&[u8]] = &[b"overlay hide", b"overlay new", b"overlay use"];
 
+pub const RESERVED_VARIABLE_NAMES: [&str; 3] = ["in", "nu", "env"];
+
 /// These parser keywords cannot be aliased (either not possible, or support not yet added)
 pub const UNALIASABLE_PARSER_KEYWORDS: &[&[u8]] = &[
     b"export",
@@ -350,6 +352,13 @@ pub fn parse_for(working_set: &mut StateWorkingSet, spans: &[Span]) -> Expressio
     }
 }
 
+/// If `name` is a keyword, emit an error.
+fn verify_not_reserved_variable_name(working_set: &mut StateWorkingSet, name: &str, span: Span) {
+    if RESERVED_VARIABLE_NAMES.contains(&name) {
+        working_set.error(ParseError::NameIsBuiltinVar(name.to_string(), span))
+    }
+}
+
 // Returns also the parsed command name and ID
 pub fn parse_def(
     working_set: &mut StateWorkingSet,
@@ -515,6 +524,19 @@ pub fn parse_def(
     let mut result = None;
 
     if let (Some(mut signature), Some(block_id)) = (sig.as_signature(), block.as_block()) {
+        for arg_name in &signature.required_positional {
+            verify_not_reserved_variable_name(working_set, &arg_name.name, sig.span);
+        }
+        for arg_name in &signature.optional_positional {
+            verify_not_reserved_variable_name(working_set, &arg_name.name, sig.span);
+        }
+        for arg_name in &signature.rest_positional {
+            verify_not_reserved_variable_name(working_set, &arg_name.name, sig.span);
+        }
+        for flag_name in &signature.get_names() {
+            verify_not_reserved_variable_name(working_set, flag_name, sig.span);
+        }
+
         if has_wrapped {
             if let Some(rest) = &signature.rest_positional {
                 if let Some(var_id) = rest.var_id {
@@ -2997,7 +3019,7 @@ pub fn parse_let(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline
                             .trim_start_matches('$')
                             .to_string();
 
-                    if ["in", "nu", "env"].contains(&var_name.as_str()) {
+                    if RESERVED_VARIABLE_NAMES.contains(&var_name.as_str()) {
                         working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span))
                     }
 
@@ -3104,8 +3126,7 @@ pub fn parse_const(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipelin
                             .trim_start_matches('$')
                             .to_string();
 
-                    // TODO: Remove the hard-coded variables, too error-prone
+                    if RESERVED_VARIABLE_NAMES.contains(&var_name.as_str()) {
-                    if ["in", "nu", "env"].contains(&var_name.as_str()) {
                         working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span))
                     }
 
@@ -3246,7 +3267,7 @@ pub fn parse_mut(working_set: &mut StateWorkingSet, spans: &[Span]) -> Pipeline
                             .trim_start_matches('$')
                             .to_string();
 
-                    if ["in", "nu", "env"].contains(&var_name.as_str()) {
+                    if RESERVED_VARIABLE_NAMES.contains(&var_name.as_str()) {
                         working_set.error(ParseError::NameIsBuiltinVar(var_name, lvalue.span))
                     }
 

tests/parsing/mod.rs

@@ -304,6 +304,21 @@ fn parse_function_signature(#[case] phrase: &str) {
     assert!(actual.err.is_empty());
 }
 
+#[rstest]
+#[case("def test [ in ] {}")]
+#[case("def test [ in: string ] {}")]
+#[case("def test [ nu: int ] {}")]
+#[case("def test [ env: record<> ] {}")]
+#[case("def test [ --env ] {}")]
+#[case("def test [ --nu: int ] {}")]
+#[case("def test [ --in (-i): list<any> ] {}")]
+#[case("def test [ a: string, b: int, in: table<a: int b: int> ] {}")]
+#[case("def test [ env, in, nu ] {}")]
+fn parse_function_signature_name_is_builtin_var(#[case] phrase: &str) {
+    let actual = nu!(phrase);
+    assert!(actual.err.contains("nu::parser::name_is_builtin_var"))
+}
+
 #[rstest]
 #[case("let a: int = 1")]
 #[case("let a: string = 'qwe'")]