diff --git a/crates/nu-command/src/env/let_env.rs b/crates/nu-command/src/env/let_env.rs index f26736081f..a24889c3d4 100644 --- a/crates/nu-command/src/env/let_env.rs +++ b/crates/nu-command/src/env/let_env.rs @@ -33,6 +33,7 @@ impl Command for LetEnv { call: &Call, input: PipelineData, ) -> Result { + // TODO: find and require the crossplatform restrictions on environment names let env_var = call.req(engine_state, stack, 0)?; let keyword_expr = call diff --git a/crates/nu-parser/src/parser.rs b/crates/nu-parser/src/parser.rs index 577b649e5d..fa03031fad 100644 --- a/crates/nu-parser/src/parser.rs +++ b/crates/nu-parser/src/parser.rs @@ -44,7 +44,21 @@ pub fn garbage_pipeline(spans: &[Span]) -> Pipeline { } fn is_identifier_byte(b: u8) -> bool { - b != b'.' && b != b'[' && b != b'(' && b != b'{' + b != b'.' + && b != b'[' + && b != b'(' + && b != b'{' + && b != b'+' + && b != b'-' + && b != b'*' + && b != b'^' + && b != b'/' + && b != b'=' + && b != b'!' + && b != b'<' + && b != b'>' + && b != b'&' + && b != b'|' } pub fn is_math_expression_like( @@ -2899,11 +2913,19 @@ pub fn parse_var_with_opt_type( let ty = parse_type(working_set, type_bytes); - let id = working_set.add_variable( - bytes[0..(bytes.len() - 1)].to_vec(), - spans[*spans_idx - 1], - ty.clone(), - ); + let var_name = bytes[0..(bytes.len() - 1)].to_vec(); + + if !is_variable(&var_name) { + return ( + garbage(spans[*spans_idx]), + Some(ParseError::Expected( + "valid variable name".into(), + spans[*spans_idx], + )), + ); + } + + let id = working_set.add_variable(var_name, spans[*spans_idx - 1], ty.clone()); ( Expression { @@ -2915,11 +2937,19 @@ pub fn parse_var_with_opt_type( None, ) } else { - let id = working_set.add_variable( - bytes[0..(bytes.len() - 1)].to_vec(), - spans[*spans_idx], - Type::Any, - ); + let var_name = bytes[0..(bytes.len() - 1)].to_vec(); + + if !is_variable(&var_name) { + return ( + garbage(spans[*spans_idx]), + Some(ParseError::Expected( + "valid variable name".into(), + spans[*spans_idx], + )), + ); + } + + let id = working_set.add_variable(var_name, spans[*spans_idx], Type::Any); ( Expression { expr: Expr::VarDecl(id), @@ -2931,8 +2961,23 @@ pub fn parse_var_with_opt_type( ) } } else { - let id = - working_set.add_variable(bytes, span(&spans[*spans_idx..*spans_idx + 1]), Type::Any); + let var_name = bytes; + + if !is_variable(&var_name) { + return ( + garbage(spans[*spans_idx]), + Some(ParseError::Expected( + "valid variable name".into(), + spans[*spans_idx], + )), + ); + } + + let id = working_set.add_variable( + var_name, + span(&spans[*spans_idx..*spans_idx + 1]), + Type::Any, + ); ( Expression { @@ -3130,7 +3175,23 @@ pub fn parse_signature_helper( contents.split(|x| x == &b'(').map(|x| x.to_vec()).collect(); let long = String::from_utf8_lossy(&flags[0][2..]).to_string(); - let variable_name = flags[0][2..].to_vec(); + let mut variable_name = flags[0][2..].to_vec(); + // Replace the '-' in a variable name with '_' + (0..variable_name.len()).for_each(|idx| { + if variable_name[idx] == b'-' { + variable_name[idx] = b'_'; + } + }); + + if !is_variable(&variable_name) { + error = error.or_else(|| { + Some(ParseError::Expected( + "valid variable name".into(), + span, + )) + }) + } + let var_id = working_set.add_variable(variable_name, span, Type::Any); @@ -3166,6 +3227,15 @@ pub fn parse_signature_helper( let chars: Vec = short_flag.chars().collect(); let long = String::from_utf8_lossy(&flags[0][2..]).to_string(); let variable_name = flags[0][2..].to_vec(); + if !is_variable(&variable_name) { + error = error.or_else(|| { + Some(ParseError::Expected( + "valid variable name".into(), + span, + )) + }) + } + let var_id = working_set.add_variable(variable_name, span, Type::Any); @@ -3201,6 +3271,15 @@ pub fn parse_signature_helper( let mut encoded_var_name = vec![0u8; 4]; let len = chars[0].encode_utf8(&mut encoded_var_name).len(); let variable_name = encoded_var_name[0..len].to_vec(); + if !is_variable(&variable_name) { + error = error.or_else(|| { + Some(ParseError::Expected( + "valid variable name".into(), + span, + )) + }) + } + let var_id = working_set.add_variable(variable_name, span, Type::Any); @@ -3260,6 +3339,15 @@ pub fn parse_signature_helper( let contents: Vec<_> = contents[..(contents.len() - 1)].into(); let name = String::from_utf8_lossy(&contents).to_string(); + if !is_variable(&contents) { + error = error.or_else(|| { + Some(ParseError::Expected( + "valid variable name".into(), + span, + )) + }) + } + let var_id = working_set.add_variable(contents, span, Type::Any); // Positional arg, optional @@ -3276,6 +3364,14 @@ pub fn parse_signature_helper( } else if let Some(contents) = contents.strip_prefix(b"...") { let name = String::from_utf8_lossy(contents).to_string(); let contents_vec: Vec = contents.to_vec(); + if !is_variable(&contents_vec) { + error = error.or_else(|| { + Some(ParseError::Expected( + "valid variable name".into(), + span, + )) + }) + } let var_id = working_set.add_variable(contents_vec, span, Type::Any); @@ -3291,6 +3387,15 @@ pub fn parse_signature_helper( let name = String::from_utf8_lossy(contents).to_string(); let contents_vec = contents.to_vec(); + if !is_variable(&contents_vec) { + error = error.or_else(|| { + Some(ParseError::Expected( + "valid variable name".into(), + span, + )) + }) + } + let var_id = working_set.add_variable(contents_vec, span, Type::Any); @@ -4650,7 +4755,10 @@ pub fn parse_variable( (None, None) } } else { - (None, Some(ParseError::Expected("variable".into(), span))) + ( + None, + Some(ParseError::Expected("valid variable name".into(), span)), + ) } } diff --git a/src/tests/test_custom_commands.rs b/src/tests/test_custom_commands.rs index f961438b2a..4530e31345 100644 --- a/src/tests/test_custom_commands.rs +++ b/src/tests/test_custom_commands.rs @@ -67,7 +67,7 @@ fn do_rest_args() -> TestResult { #[test] fn custom_switch1() -> TestResult { run_test( - r#"def florb [ --dry-run: bool ] { if ($dry-run) { "foo" } else { "bar" } }; florb --dry-run"#, + r#"def florb [ --dry-run: bool ] { if ($dry_run) { "foo" } else { "bar" } }; florb --dry-run"#, "foo", ) } @@ -75,7 +75,7 @@ fn custom_switch1() -> TestResult { #[test] fn custom_switch2() -> TestResult { run_test( - r#"def florb [ --dry-run: bool ] { if ($dry-run) { "foo" } else { "bar" } }; florb"#, + r#"def florb [ --dry-run: bool ] { if ($dry_run) { "foo" } else { "bar" } }; florb"#, "bar", ) } @@ -83,7 +83,7 @@ fn custom_switch2() -> TestResult { #[test] fn custom_switch3() -> TestResult { run_test( - r#"def florb [ --dry-run ] { if ($dry-run) { "foo" } else { "bar" } }; florb --dry-run"#, + r#"def florb [ --dry-run ] { if ($dry_run) { "foo" } else { "bar" } }; florb --dry-run"#, "foo", ) } @@ -91,7 +91,7 @@ fn custom_switch3() -> TestResult { #[test] fn custom_switch4() -> TestResult { run_test( - r#"def florb [ --dry-run ] { if ($dry-run) { "foo" } else { "bar" } }; florb"#, + r#"def florb [ --dry-run ] { if ($dry_run) { "foo" } else { "bar" } }; florb"#, "bar", ) } diff --git a/src/tests/test_parser.rs b/src/tests/test_parser.rs index 3f75a0956c..6346b3550a 100644 --- a/src/tests/test_parser.rs +++ b/src/tests/test_parser.rs @@ -114,6 +114,11 @@ fn bad_var_name() -> TestResult { fail_test(r#"let $"foo bar" = 4"#, "can't contain") } +#[test] +fn bad_var_name2() -> TestResult { + fail_test(r#"let $foo-bar = 4"#, "valid variable") +} + #[test] fn long_flag() -> TestResult { run_test(