From 522a3f1cb996e385395f63f61ff0dbb3ba70392c Mon Sep 17 00:00:00 2001
From: Jakob Hellermann <jakob.hellermann@protonmail.com>
Date: Thu, 1 Aug 2019 15:37:41 +0200
Subject: [PATCH] feat: allow [a-zA-Z_-!] in aliases, functions ans variables

---
 src/lib/builtins/variables.rs        |  2 +-
 src/lib/expansion/words/mod.rs       | 12 ++++++------
 src/lib/parser/mod.rs                |  2 +-
 src/lib/parser/statement/mod.rs      |  2 +-
 src/lib/parser/statement/parse.rs    | 14 +++++---------
 src/lib/parser/statement/splitter.rs |  4 ++--
 src/lib/shell/assignments.rs         |  9 +++------
 src/lib/shell/variables.rs           | 12 +++++-------
 8 files changed, 24 insertions(+), 33 deletions(-)

diff --git a/src/lib/builtins/variables.rs b/src/lib/builtins/variables.rs
index 7cdcb1e6..34689c04 100644
--- a/src/lib/builtins/variables.rs
+++ b/src/lib/builtins/variables.rs
@@ -55,7 +55,7 @@ fn parse_alias(args: &str) -> Binding {
         let value: String = char_iter.skip_while(|&x| x == ' ').collect();
         if value.is_empty() {
             Binding::KeyOnly(key)
-        } else if Variables::is_valid_variable_name(&key) {
+        } else if Variables::is_valid_name(&key) {
             Binding::KeyValue(key, value.into())
         } else {
             Binding::InvalidKey(key)
diff --git a/src/lib/expansion/words/mod.rs b/src/lib/expansion/words/mod.rs
index 484b2aba..ec8a1fe9 100644
--- a/src/lib/expansion/words/mod.rs
+++ b/src/lib/expansion/words/mod.rs
@@ -346,8 +346,8 @@ impl<'a> WordIterator<'a> {
                     self.read += 1;
                     return WordToken::ArrayVariable(output, self.quotes == Quotes::Double, None);
                 }
-                // Only alphanumerical and underscores are allowed in variable names
-                0..=47 | 58..=64 | 91..=94 | 96 | 123..=127 => {
+                // Only [a-zA-Z0-9_-!] are allowed in variable names
+                0..=32 | 34..=44 | 46..=47 | 58..=64 | 91..=94 | 96 | 123..=127 => {
                     return WordToken::ArrayVariable(
                         &self.data[start..self.read],
                         self.quotes == Quotes::Double,
@@ -464,8 +464,8 @@ impl<'a> WordIterator<'a> {
                         Some(self.read_selection(iterator)),
                     );
                 }
-                // Only alphanumerical and underscores are allowed in variable names
-                0..=47 | 58..=64 | 91..=94 | 96 | 123..=127 => {
+                // Only [a-zA-Z0-9_-!] are allowed in variable names
+                0..=32 | 34..=44 | 46..=47 | 58..=64 | 91..=94 | 96 | 123..=127 => {
                     return WordToken::ArrayVariable(
                         &self.data[start..self.read],
                         self.quotes == Quotes::Double,
@@ -591,8 +591,8 @@ impl<'a> WordIterator<'a> {
 
                     panic!("ion: fatal error with syntax validation parsing: unterminated method");
                 }
-                // Only alphanumerical and underscores are allowed in variable names
-                0..=47 | 58..=64 | 91..=94 | 96 | 123..=127 => {
+                // Only [a-zA-Z0-9_-!] are allowed in variable names
+                0..=32 | 34..=44 | 46..=47 | 58..=64 | 91..=94 | 96 | 123..=127 => {
                     let variable = &self.data[start..self.read];
 
                     return if character == b'[' {
diff --git a/src/lib/parser/mod.rs b/src/lib/parser/mod.rs
index 9cb75ebd..4b5bd705 100644
--- a/src/lib/parser/mod.rs
+++ b/src/lib/parser/mod.rs
@@ -13,7 +13,7 @@ mod statement;
 
 pub use self::{
     quotes::Terminator,
-    statement::{is_valid_name, parse_and_validate, Error, StatementSplitter},
+    statement::{parse_and_validate, Error, StatementSplitter},
 };
 
 #[cfg(fuzzing)]
diff --git a/src/lib/parser/statement/mod.rs b/src/lib/parser/statement/mod.rs
index 9610c08d..4899d995 100644
--- a/src/lib/parser/statement/mod.rs
+++ b/src/lib/parser/statement/mod.rs
@@ -4,7 +4,7 @@ mod parse;
 mod splitter;
 
 pub use self::{
-    parse::{is_valid_name, parse},
+    parse::parse,
     splitter::{StatementSplitter, StatementVariant},
 };
 use super::{
diff --git a/src/lib/parser/statement/parse.rs b/src/lib/parser/statement/parse.rs
index aa9f5a6a..b1cfb7d9 100644
--- a/src/lib/parser/statement/parse.rs
+++ b/src/lib/parser/statement/parse.rs
@@ -6,18 +6,14 @@ use super::{
 use crate::{
     builtins::BuiltinMap,
     parser::lexers::{assignment_lexer, ArgumentSplitter},
-    shell::flow_control::{Case, ElseIf, ExportAction, IfMode, LocalAction, Statement},
+    shell::{
+        flow_control::{Case, ElseIf, ExportAction, IfMode, LocalAction, Statement},
+        variables::Variables,
+    },
     types,
 };
 use std::char;
 
-/// Check if the given name is valid for functions, aliases & variables
-pub fn is_valid_name(name: &str) -> bool {
-    let mut chars = name.chars();
-    chars.next().map_or(false, |b| char::is_alphabetic(b) || b == '_')
-        && chars.all(|b| b.is_alphanumeric() || b == '_')
-}
-
 pub fn parse<'a>(code: &str, builtins: &BuiltinMap<'a>) -> super::Result<'a> {
     let cmd = code.trim();
     match cmd {
@@ -115,7 +111,7 @@ pub fn parse<'a>(code: &str, builtins: &BuiltinMap<'a>) -> super::Result<'a> {
             let cmd = cmd[3..].trim_start();
             let pos = cmd.find(char::is_whitespace).unwrap_or_else(|| cmd.len());
             let name = &cmd[..pos];
-            if !is_valid_name(name) {
+            if !Variables::is_valid_name(name) {
                 return Err(Error::InvalidFunctionName(name.into()));
             }
 
diff --git a/src/lib/parser/statement/splitter.rs b/src/lib/parser/statement/splitter.rs
index 8c5a4997..3f2d6294 100644
--- a/src/lib/parser/statement/splitter.rs
+++ b/src/lib/parser/statement/splitter.rs
@@ -94,8 +94,8 @@ impl<'a> Iterator for StatementSplitter<'a> {
                     bytes.find(|&(_, c)| c == b'\'');
                 }
                 b'\\' => self.skip = true,
-                // [^A-Za-z0-9_:,}]
-                0..=43 | 45..=47 | 59..=64 | 91..=94 | 96 | 123..=124 | 126..=127
+                // [^A-Za-z0-9_:,-!}]
+                0..=32 | 34..=43 | 46..=47 | 59..=64 | 91..=94 | 96 | 123..=124 | 126..=127
                     if self.vbrace =>
                 {
                     // If we are just ending the braced section continue as normal
diff --git a/src/lib/shell/assignments.rs b/src/lib/shell/assignments.rs
index b38c4f38..02457356 100644
--- a/src/lib/shell/assignments.rs
+++ b/src/lib/shell/assignments.rs
@@ -5,11 +5,8 @@ use super::{
 use crate::{
     assignments::*,
     builtins::Status,
-    parser::{
-        is_valid_name,
-        lexers::assignments::{Key, Operator, Primitive},
-    },
-    shell::{flow_control::Function, Value},
+    parser::lexers::assignments::{Key, Operator, Primitive},
+    shell::{flow_control::Function, Value, Variables},
 };
 use std::{
     env,
@@ -118,7 +115,7 @@ impl<'b> Shell<'b> {
                 return Err(format!("not allowed to set `{}`", key.name));
             }
 
-            if !is_valid_name(key.name) {
+            if !Variables::is_valid_name(key.name) {
                 return Err("invalid variable name\nVariable names may only be (unicode) \
                             alphanumeric or `_`\nThe first character must be alphabetic or `_`"
                     .to_string());
diff --git a/src/lib/shell/variables.rs b/src/lib/shell/variables.rs
index 09f214cb..15a75e94 100644
--- a/src/lib/shell/variables.rs
+++ b/src/lib/shell/variables.rs
@@ -145,13 +145,11 @@ impl<'a> Variables<'a> {
         env::var("PWD").unwrap().replace(&*home, "~").into()
     }
 
-    /// Indicates if the name is valid for a variable
-    pub fn is_valid_variable_name(name: &str) -> bool {
-        name.chars().all(Variables::is_valid_variable_character)
-    }
-
-    fn is_valid_variable_character(c: char) -> bool {
-        c.is_alphanumeric() || c == '_' || c == '?' || c == '.' || c == '-' || c == '+'
+    /// Indicates if name is valid for functions, variables ans aliases
+    pub fn is_valid_name(name: &str) -> bool {
+        let mut iter = name.chars();
+        iter.next().map_or(false, |c| c.is_alphabetic() || c == '_')
+            && iter.all(|c| c.is_alphanumeric() || c == '_' || c == '-' || c == '!')
     }
 
     /// Remove a variable from the current scope. If the value can't be removed (it is outside a
-- 
GitLab