Merge pull request #391 from python-cmd2/ignore_identchars

Remove check on self.identchars in do_alias()

4 files changed, 208 insertions, 51 deletions

diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 5c58977b..ee6beb98 100755
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -2392,16 +2392,17 @@ Usage:  Usage: alias [name] | [<name> <value>]
             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 """
diff --git a/cmd2/parsing.py b/cmd2/parsing.py
index f2c86ea8..3a9b390b 100644
--- a/cmd2/parsing.py
+++ b/cmd2/parsing.py
@@ -141,15 +141,67 @@ class StatementParser:
             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 @@ class StatementParser:
 
         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 @@ class StatementParser:
             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)
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 0da7e9d5..bc76505f 100644
--- a/tests/test_cmd2.py
+++ b/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...'
diff --git a/tests/test_parsing.py b/tests/test_parsing.py
index 19237f6e..bfb55b23 100644
--- a/tests/test_parsing.py
+++ b/tests/test_parsing.py
@@ -16,7 +16,7 @@ from cmd2 import utils
 def parser():
     parser = StatementParser(
         allow_redirection=True,
-        terminators=[';'],
+        terminators=[';', '&'],
         multiline_commands=['multiline'],
         aliases={'helpalias': 'help',
                  '42': 'theanswer',
@@ -38,7 +38,13 @@ def test_parse_empty_string(parser):
     ('command /* with some comment */ arg', ['command', 'arg']),
     ('command arg1 arg2 # comment at the end', ['command', 'arg1', 'arg2']),
     ('42 arg1 arg2', ['theanswer', 'arg1', 'arg2']),
-    ('l', ['shell', 'ls', '-al'])
+    ('l', ['shell', 'ls', '-al']),
+    ('termbare ; > /tmp/output', ['termbare', ';', '>', '/tmp/output']),
+    ('termbare; > /tmp/output', ['termbare', ';', '>', '/tmp/output']),
+    ('termbare & > /tmp/output', ['termbare', '&', '>', '/tmp/output']),
+    ('termbare& > /tmp/output', ['termbare', '&', '>', '/tmp/output']),
+    ('help|less', ['help', '|', 'less']),
+    ('l|less', ['shell', 'ls', '-al', '|', 'less']),
 ])
 def test_tokenize(parser, line, tokens):
     tokens_to_test = parser.tokenize(line)
@@ -46,7 +52,7 @@ def test_tokenize(parser, line, tokens):
 
 def test_tokenize_unclosed_quotes(parser):
     with pytest.raises(ValueError):
-        tokens = parser.tokenize('command with "unclosed quotes')
+        _ = parser.tokenize('command with "unclosed quotes')
 
 @pytest.mark.parametrize('tokens,command,args', [
     ([], None, None),
@@ -69,18 +75,28 @@ def test_parse_single_word(parser, line):
     assert not statement.args
     assert statement.argv == [utils.strip_quotes(line)]
 
-def test_parse_word_plus_terminator(parser):
-    line = 'termbare;'
+@pytest.mark.parametrize('line,terminator', [
+    ('termbare;', ';'),
+    ('termbare ;', ';'),
+    ('termbare&', '&'),
+    ('termbare &', '&'),
+])
+def test_parse_word_plus_terminator(parser, line, terminator):
     statement = parser.parse(line)
     assert statement.command == 'termbare'
-    assert statement.terminator == ';'
+    assert statement.terminator == terminator
     assert statement.argv == ['termbare']
 
-def test_parse_suffix_after_terminator(parser):
-    line = 'termbare; suffx'
+@pytest.mark.parametrize('line,terminator', [
+    ('termbare;  suffx', ';'),
+    ('termbare ;suffx', ';'),
+    ('termbare&  suffx', '&'),
+    ('termbare &suffx', '&'),
+])
+def test_parse_suffix_after_terminator(parser, line, terminator):
     statement = parser.parse(line)
     assert statement.command == 'termbare'
-    assert statement.terminator == ';'
+    assert statement.terminator == terminator
     assert statement.suffix == 'suffx'
     assert statement.argv == ['termbare']
 
@@ -134,8 +150,12 @@ def test_parse_what_if_quoted_strings_seem_to_start_comments(parser):
     assert not statement.pipe_to
     assert statement.argv == ['what', 'if', 'quoted strings /* seem to ', 'start', 'comments?']
 
-def test_parse_simple_piped(parser):
-    statement = parser.parse('simple | piped')
+@pytest.mark.parametrize('line',[
+    'simple | piped',
+    'simple|piped',
+])
+def test_parse_simple_pipe(parser, line):
+    statement = parser.parse(line)
     assert statement.command == 'simple'
     assert not statement.args
     assert statement.argv == ['simple']
@@ -150,16 +170,29 @@ def test_parse_double_pipe_is_not_a_pipe(parser):
     assert not statement.pipe_to
 
 def test_parse_complex_pipe(parser):
-    line = 'command with args, terminator;sufx | piped'
+    line = 'command with args, terminator&sufx | piped'
     statement = parser.parse(line)
     assert statement.command == 'command'
     assert statement.args == "with args, terminator"
     assert statement.argv == ['command', 'with', 'args,', 'terminator']
-    assert statement.terminator == ';'
+    assert statement.terminator == '&'
     assert statement.suffix == 'sufx'
     assert statement.pipe_to == 'piped'
 
-def test_parse_output_redirect(parser):
+@pytest.mark.parametrize('line,output', [
+    ('help > out.txt', '>'),
+    ('help>out.txt', '>'),
+    ('help >> out.txt', '>>'),
+    ('help>>out.txt', '>>'),
+])
+def test_parse_redirect(parser,line, output):
+    statement = parser.parse(line)
+    assert statement.command == 'help'
+    assert not statement.args
+    assert statement.output == output
+    assert statement.output_to == 'out.txt'
+
+def test_parse_redirect_with_args(parser):
     line = 'output into > afile.txt'
     statement = parser.parse(line)
     assert statement.command == 'output'
@@ -168,7 +201,7 @@ def test_parse_output_redirect(parser):
     assert statement.output == '>'
     assert statement.output_to == 'afile.txt'
 
-def test_parse_output_redirect_with_dash_in_path(parser):
+def test_parse_redirect_with_dash_in_path(parser):
     line = 'output into > python-cmd2/afile.txt'
     statement = parser.parse(line)
     assert statement.command == 'output'
@@ -177,7 +210,7 @@ def test_parse_output_redirect_with_dash_in_path(parser):
     assert statement.output == '>'
     assert statement.output_to == 'python-cmd2/afile.txt'
 
-def test_parse_output_redirect_append(parser):
+def test_parse_redirect_append(parser):
     line = 'output appended to >> /tmp/afile.txt'
     statement = parser.parse(line)
     assert statement.command == 'output'
@@ -225,13 +258,16 @@ def test_parse_unfinished_multiliine_command(parser):
     assert statement.argv == ['multiline', 'has', '>', 'inside', 'an', 'unfinished', 'command']
     assert not statement.terminator
 
-def test_parse_multiline_command_ignores_redirectors_within_it(parser):
-    line = 'multiline has > inside;'
+@pytest.mark.parametrize('line,terminator',[
+    ('multiline has > inside;', ';'),
+    ('multiline has > inside &', '&'),
+])
+def test_parse_multiline_command_ignores_redirectors_within_it(parser, line, terminator):
     statement = parser.parse(line)
     assert statement.multiline_command == 'multiline'
     assert statement.args == 'has > inside'
     assert statement.argv == ['multiline', 'has', '>', 'inside']
-    assert statement.terminator == ';'
+    assert statement.terminator == terminator
 
 def test_parse_multiline_with_incomplete_comment(parser):
     """A terminator within a comment will be ignored and won't terminate a multiline command.
@@ -296,7 +332,7 @@ def test_parse_redirect_to_unicode_filename(parser):
 
 def test_parse_unclosed_quotes(parser):
     with pytest.raises(ValueError):
-        tokens = parser.tokenize("command with 'unclosed quotes")
+        _ = parser.tokenize("command with 'unclosed quotes")
 
 def test_empty_statement_raises_exception():
     app = cmd2.Cmd()
@@ -315,12 +351,12 @@ def test_empty_statement_raises_exception():
     ('!ls -al /tmp', 'shell', 'ls -al /tmp'),
     ('l', 'shell', 'ls -al')
 ])
-def test_alias_and_shortcut_expansion(parser, line, command, args):
+def test_parse_alias_and_shortcut_expansion(parser, line, command, args):
     statement = parser.parse(line)
     assert statement.command == command
     assert statement.args == args
 
-def test_alias_on_multiline_command(parser):
+def test_parse_alias_on_multiline_command(parser):
     line = 'anothermultiline has > inside an unfinished command'
     statement = parser.parse(line)
     assert statement.multiline_command == 'multiline'
@@ -328,6 +364,36 @@ def test_alias_on_multiline_command(parser):
     assert statement.args == 'has > inside an unfinished command'
     assert not statement.terminator
 
+@pytest.mark.parametrize('line,output', [
+    ('helpalias > out.txt', '>'),
+    ('helpalias>out.txt', '>'),
+    ('helpalias >> out.txt', '>>'),
+    ('helpalias>>out.txt', '>>'),
+])
+def test_parse_alias_redirection(parser, line, output):
+    statement = parser.parse(line)
+    assert statement.command == 'help'
+    assert not statement.args
+    assert statement.output == output
+    assert statement.output_to == 'out.txt'
+
+@pytest.mark.parametrize('line', [
+    'helpalias | less',
+    'helpalias|less',
+])
+def test_parse_alias_pipe(parser, line):
+    statement = parser.parse(line)
+    assert statement.command == 'help'
+    assert not statement.args
+    assert statement.pipe_to == 'less'
+
+def test_parse_alias_terminator_no_whitespace(parser):
+    line = 'helpalias;'
+    statement = parser.parse(line)
+    assert statement.command == 'help'
+    assert not statement.args
+    assert statement.terminator == ';'
+
 def test_parse_command_only_command_and_args(parser):
     line = 'help history'
     statement = parser.parse_command_only(line)
@@ -373,3 +439,27 @@ def test_parse_command_only_quoted_args(parser):
     assert statement.command == 'shell'
     assert statement.args == 'ls -al "/tmp/directory with spaces/doit.sh"'
     assert statement.command_and_args == line.replace('l', 'shell ls -al')
+
+@pytest.mark.parametrize('line', [
+    'helpalias > out.txt',
+    'helpalias>out.txt',
+    'helpalias >> out.txt',
+    'helpalias>>out.txt',
+    'help|less',
+    'helpalias;',
+])
+def test_parse_command_only_specialchars(parser, line):
+    statement = parser.parse_command_only(line)
+    assert statement.command == 'help'
+
+@pytest.mark.parametrize('line', [
+    ';',
+    '>',
+    "'",
+    '"',
+    '|',
+])
+def test_parse_command_only_none(parser, line):
+    statement = parser.parse_command_only(line)
+    assert statement.command == None
+    assert statement.args == None