Merge pull request #391 from python-cmd2/ignore_identchars

Remove check on self.identchars in do_alias()

Author: kotfu

cmd2/cmd2.py

@@ -2392,16 +2392,17 @@ def do_alias(self, arglist):
             name = arglist[0]
             value = ' '.join(arglist[1:])
 
-            # Check for a valid name
-            for cur_char in name:
-                if cur_char not in self.identchars:
-                    self.perror("Alias names can only contain the following characters: {}".format(self.identchars),
-                                traceback_war=False)
-                    return
-
-            # Set the alias
-            self.aliases[name] = value
-            self.poutput("Alias {!r} created".format(name))
+            # Validate the alias to ensure it doesn't include weird characters
+            # like terminators, output redirection, or whitespace
+            valid, invalidchars = self.statement_parser.is_valid_command(name)
+            if valid:
+                # Set the alias
+                self.aliases[name] = value
+                self.poutput("Alias {!r} created".format(name))
+            else:
+                errmsg = "Aliases can not contain: {}".format(invalidchars)
+                self.perror(errmsg, traceback_war=False)
+
 
     def complete_alias(self, text, line, begidx, endidx):
         """ Tab completion for alias """

cmd2/parsing.py

@@ -141,15 +141,67 @@ def __init__(
             re.DOTALL | re.MULTILINE
         )
 
-        # aliases have to be a word, so make a regular expression
-        # that matches the first word in the line. This regex has two
-        # parts, the first parenthesis enclosed group matches one
-        # or more non-whitespace characters (which may be preceeded
-        # by whitespace) and the second group matches either a whitespace
-        # character or the end of the string. We use \A and \Z to ensure
-        # we always match the beginning and end of a string that may have
-        # multiple lines
-        self.command_pattern = re.compile(r'\A\s*(\S+)(\s|\Z)+')
+        # commands have to be a word, so make a regular expression
+        # that matches the first word in the line. This regex has three
+        # parts:
+        #     - the '\A\s*' matches the beginning of the string (even
+        #       if contains multiple lines) and gobbles up any leading
+        #       whitespace
+        #     - the first parenthesis enclosed group matches one
+        #       or more non-whitespace characters with a non-greedy match
+        #       (that's what the '+?' part does). The non-greedy match
+        #       ensures that this first group doesn't include anything
+        #       matched by the second group
+        #     - the second parenthesis group must be dynamically created
+        #       because it needs to match either whitespace, something in
+        #       REDIRECTION_CHARS, one of the terminators, or the end of
+        #       the string (\Z matches the end of the string even if it
+        #       contains multiple lines)
+        #
+        invalid_command_chars = []
+        invalid_command_chars.extend(constants.QUOTES)
+        invalid_command_chars.extend(constants.REDIRECTION_CHARS)
+        invalid_command_chars.extend(terminators)
+        # escape each item so it will for sure get treated as a literal
+        second_group_items = [re.escape(x) for x in invalid_command_chars]
+        # add the whitespace and end of string, not escaped because they
+        # are not literals
+        second_group_items.extend([r'\s', r'\Z'])
+        # join them up with a pipe
+        second_group = '|'.join(second_group_items)
+        # build the regular expression
+        expr = r'\A\s*(\S*?)({})'.format(second_group)
+        self._command_pattern = re.compile(expr)
+
+    def is_valid_command(self, word: str) -> Tuple[bool, str]:
+        """Determine whether a word is a valid alias.
+
+        Aliases can not include redirection characters, whitespace,
+        or termination characters.
+
+        If word is not a valid command, return False and a comma
+        separated string of characters that can not appear in a command.
+        This string is suitable for inclusion in an error message of your
+        choice:
+
+        valid, invalidchars = statement_parser.is_valid_command('>')
+        if not valid:
+            errmsg = "Aliases can not contain: {}".format(invalidchars)
+        """
+        valid = False
+
+        errmsg = 'whitespace, quotes, '
+        errchars = []
+        errchars.extend(constants.REDIRECTION_CHARS)
+        errchars.extend(self.terminators)
+        errmsg += ', '.join([shlex.quote(x) for x in errchars])
+
+        match = self._command_pattern.search(word)
+        if match:
+            if word == match.group(1):
+                valid = True
+                errmsg = None
+        return valid, errmsg
 
     def tokenize(self, line: str) -> List[str]:
         """Lex a string into a list of tokens.
@@ -324,16 +376,24 @@ def parse_command_only(self, rawinput: str) -> Statement:
 
         command = None
         args = None
-        match = self.command_pattern.search(line)
+        match = self._command_pattern.search(line)
         if match:
             # we got a match, extract the command
             command = match.group(1)
-            # the command_pattern regex is designed to match the spaces
+            # the match could be an empty string, if so, turn it into none
+            if not command:
+                command = None
+            # the _command_pattern regex is designed to match the spaces
             # between command and args with a second match group. Using
             # the end of the second match group ensures that args has
             # no leading whitespace. The rstrip() makes sure there is
             # no trailing whitespace
             args = line[match.end(2):].rstrip()
+            # if the command is none that means the input was either empty
+            # or something wierd like '>'. args should be None if we couldn't
+            # parse a command
+            if not command or not args:
+                args = None
 
         # build the statement
         # string representation of args must be an empty string instead of
@@ -355,11 +415,11 @@ def _expand(self, line: str) -> str:
             for cur_alias in tmp_aliases:
                 keep_expanding = False
                 # apply our regex to line
-                match = self.command_pattern.search(line)
+                match = self._command_pattern.search(line)
                 if match:
                     # we got a match, extract the command
                     command = match.group(1)
-                    if command == cur_alias:
+                    if command and command == cur_alias:
                         # rebuild line with the expanded alias
                         line = self.aliases[cur_alias] + match.group(2) + line[match.end(2):]
                         tmp_aliases.remove(cur_alias)

tests/test_cmd2.py

@@ -1688,12 +1688,6 @@ def test_alias_lookup_invalid_alias(base_app, capsys):
     out, err = capsys.readouterr()
     assert "not found" in err
 
-def test_alias_with_invalid_name(base_app, capsys):
-    run_cmd(base_app, 'alias @ help')
-    out, err = capsys.readouterr()
-    assert "can only contain the following characters" in err
-
-
 def test_unalias(base_app):
     # Create an alias
     run_cmd(base_app, 'alias fake pyscript')
@@ -1711,6 +1705,18 @@ def test_unalias_non_existing(base_app, capsys):
     out, err = capsys.readouterr()
     assert "does not exist" in err
 
+@pytest.mark.parametrize('alias_name', [
+    '">"',
+    '"no>pe"',
+    '"no spaces"',
+    '"nopipe|"',
+    '"noterm;"',
+    'noembedded"quotes',
+])
+def test_create_invalid_alias(base_app, alias_name, capsys):
+    run_cmd(base_app, 'alias {} help'.format(alias_name))
+    out, err = capsys.readouterr()
+    assert "can not contain" in err
 
 def test_ppaged(base_app):
     msg = 'testing...'