Repository: python-cmd2/cmd2

Issue:
Alias and Macro names

It has been brought to my attention by another developer that allowing aliases and macros to share names with commands could cause issues with users being unwise. Specifically he said:

> being able to create an alias that overrides/hides a command is bad for usability

Currently we have the following restrictions:

1. Alias cannot share its name with a macro
2. Macro cannot share its name with an alias or a multiline command

When I first wrote aliases, I did prevent them from sharing their name with a command. Then I decided to remove this restriction to be like bash. However, in bash you can still call the original command if you know its path. (e.g. /bin/ls). **cmd2** does not work in the same way since our commands are not executable files somewhere on disk.

I'm OK adding the restriction back, but I would like some consensus on the issue. And I really don't want the answer to be, "add a flag".

@kotfu @tleonhardt @anselor What are your thoughts?


Discussion:

Patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 50d72395..57237b47 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -8,6 +8,8 @@
     * Added `set_choices_function()`, `set_choices_method()`, `set_completer_function()`, and `set_completer_method()`
     to support cases where this functionality needs to be added to an argparse action outside of the normal
     `parser.add_argument()` call.
+* Breaking Changes
+    * Aliases and macros can no longer have the same name as a command
 
 ## 0.9.15 (July 24, 2019)
 * Bug Fixes
diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index 514d8a05..d82e58c9 100755
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -1672,13 +1672,10 @@ class Cmd(cmd.Cmd):
         statement = self.statement_parser.parse_command_only(line)
         return statement.command, statement.args, statement.command_and_args
 
-    def onecmd_plus_hooks(self, line: str, *, expand: bool = True, add_to_history: bool = True,
-                          py_bridge_call: bool = False) -> bool:
+    def onecmd_plus_hooks(self, line: str, *, add_to_history: bool = True, py_bridge_call: bool = False) -> bool:
         """Top-level function called by cmdloop() to handle parsing a line and running the command and all of its hooks.
 
         :param line: command line to run
-        :param expand: If True, then aliases, macros, and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :param add_to_history: If True, then add this command to history. Defaults to True.
         :param py_bridge_call: This should only ever be set to True by PyBridge to signify the beginning
                                of an app() call from Python. It is used to enable/disable the storage of the
@@ -1689,7 +1686,7 @@ class Cmd(cmd.Cmd):
 
         stop = False
         try:
-            statement = self._input_line_to_statement(line, expand=expand)
+            statement = self._input_line_to_statement(line)
         except EmptyStatement:
             return self._run_cmdfinalization_hooks(stop, None)
         except ValueError as ex:
@@ -1804,15 +1801,12 @@ class Cmd(cmd.Cmd):
         except Exception as ex:
             self.pexcept(ex)
 
-    def runcmds_plus_hooks(self, cmds: List[Union[HistoryItem, str]], *,
-                           expand: bool = True, add_to_history: bool = True) -> bool:
+    def runcmds_plus_hooks(self, cmds: List[Union[HistoryItem, str]], *, add_to_history: bool = True) -> bool:
         """
         Used when commands are being run in an automated fashion like text scripts or history replays.
         The prompt and command line for each command will be printed if echo is True.
 
         :param cmds: commands to run
-        :param expand: If True, then aliases, macros, and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :param add_to_history: If True, then add these commands to history. Defaults to True.
         :return: True if running of commands should stop
         """
@@ -1823,12 +1817,12 @@ class Cmd(cmd.Cmd):
             if self.echo:
                 self.poutput('{}{}'.format(self.prompt, line))
 
-            if self.onecmd_plus_hooks(line, expand=expand, add_to_history=add_to_history):
+            if self.onecmd_plus_hooks(line, add_to_history=add_to_history):
                 return True
 
         return False
 
-    def _complete_statement(self, line: str, *, expand: bool = True) -> Statement:
+    def _complete_statement(self, line: str) -> Statement:
         """Keep accepting lines of input until the command is complete.
 
         There is some pretty hacky code here to handle some quirks of
@@ -1837,13 +1831,11 @@ class Cmd(cmd.Cmd):
         backwards compatibility with the standard library version of cmd.
 
         :param line: the line being parsed
-        :param expand: If True, then aliases and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :return: the completed Statement
         """
         while True:
             try:
-                statement = self.statement_parser.parse(line, expand=expand)
+                statement = self.statement_parser.parse(line)
                 if statement.multiline_command and statement.terminator:
                     # we have a completed multiline command, we are done
                     break
@@ -1854,7 +1846,7 @@ class Cmd(cmd.Cmd):
             except ValueError:
                 # we have unclosed quotation marks, lets parse only the command
                 # and see if it's a multiline
-                statement = self.statement_parser.parse_command_only(line, expand=expand)
+                statement = self.statement_parser.parse_command_only(line)
                 if not statement.multiline_command:
                     # not a multiline command, so raise the exception
                     raise
@@ -1891,13 +1883,11 @@ class Cmd(cmd.Cmd):
             raise EmptyStatement()
         return statement
 
-    def _input_line_to_statement(self, line: str, *, expand: bool = True) -> Statement:
+    def _input_line_to_statement(self, line: str) -> Statement:
         """
         Parse the user's input line and convert it to a Statement, ensuring that all macros are also resolved
 
         :param line: the line being parsed
-        :param expand: If True, then aliases, macros, and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :return: parsed command line as a Statement
         """
         used_macros = []
@@ -1906,14 +1896,14 @@ class Cmd(cmd.Cmd):
         # Continue until all macros are resolved
         while True:
             # Make sure all input has been read and convert it to a Statement
-            statement = self._complete_statement(line, expand=expand)
+            statement = self._complete_statement(line)
 
             # Save the fully entered line if this is the first loop iteration
             if orig_line is None:
                 orig_line = statement.raw
 
             # Check if this command matches a macro and wasn't already processed to avoid an infinite loop
-            if expand and statement.command in self.macros.keys() and statement.command not in used_macros:
+            if statement.command in self.macros.keys() and statement.command not in used_macros:
                 used_macros.append(statement.command)
                 line = self._resolve_macro(statement)
                 if line is None:
@@ -2127,22 +2117,19 @@ class Cmd(cmd.Cmd):
         return target if callable(getattr(self, target, None)) else ''
 
     # noinspection PyMethodOverriding
-    def onecmd(self, statement: Union[Statement, str], *,
-               expand: bool = True, add_to_history: bool = True) -> bool:
+    def onecmd(self, statement: Union[Statement, str], *, add_to_history: bool = True) -> bool:
         """ This executes the actual do_* method for a command.
 
         If the command provided doesn't exist, then it executes default() instead.
 
         :param statement: intended to be a Statement instance parsed command from the input stream, alternative
                           acceptance of a str is present only for backward compatibility with cmd
-        :param expand: If True, then aliases, macros, and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :param add_to_history: If True, then add this command to history. Defaults to True.
         :return: a flag indicating whether the interpretation of commands should stop
         """
         # For backwards compatibility with cmd, allow a str to be passed in
         if not isinstance(statement, Statement):
-            statement = self._input_line_to_statement(statement, expand=expand)
+            statement = self._input_line_to_statement(statement)
 
         func = self.cmd_func(statement.command)
         if func:
@@ -2331,6 +2318,10 @@ class Cmd(cmd.Cmd):
             self.perror("Invalid alias name: {}".format(errmsg))
             return
 
+        if args.name in self.get_all_commands():
+            self.perror("Alias cannot have the same name as a command")
+            return
+
         if args.name in self.macros:
             self.perror("Alias cannot have the same name as a macro")
             return
@@ -2460,8 +2451,8 @@ class Cmd(cmd.Cmd):
             self.perror("Invalid macro name: {}".format(errmsg))
             return
 
-        if args.name in self.statement_parser.multiline_commands:
-            self.perror("Macro cannot have the same name as a multiline command")
+        if args.name in self.get_all_commands():
+            self.perror("Macro cannot have the same name as a command")
             return
 
         if args.name in self.aliases:
diff --git a/cmd2/parsing.py b/cmd2/parsing.py
index 84b2468e..4e690b0b 100755
--- a/cmd2/parsing.py
+++ b/cmd2/parsing.py
@@ -356,20 +356,17 @@ class StatementParser:
                 errmsg = ''
         return valid, errmsg
 
-    def tokenize(self, line: str, *, expand: bool = True) -> List[str]:
+    def tokenize(self, line: str) -> List[str]:
         """
         Lex a string into a list of tokens. Shortcuts and aliases are expanded and comments are removed
 
         :param line: the command line being lexed
-        :param expand: If True, then aliases and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :return: A list of tokens
         :raises ValueError if there are unclosed quotation marks.
         """
 
         # expand shortcuts and aliases
-        if expand:
-            line = self._expand(line)
+        line = self._expand(line)
 
         # check if this line is a comment
         if line.lstrip().startswith(constants.COMMENT_CHAR):
@@ -382,15 +379,13 @@ class StatementParser:
         tokens = self.split_on_punctuation(tokens)
         return tokens
 
-    def parse(self, line: str, *, expand: bool = True) -> Statement:
+    def parse(self, line: str) -> Statement:
         """
         Tokenize the input and parse it into a Statement object, stripping
         comments, expanding aliases and shortcuts, and extracting output
         redirection directives.
 
         :param line: the command line being parsed
-        :param expand: If True, then aliases and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :return: the created Statement
         :raises ValueError if there are unclosed quotation marks
         """
@@ -407,7 +402,7 @@ class StatementParser:
         arg_list = []
 
         # lex the input into a list of tokens
-        tokens = self.tokenize(line, expand=expand)
+        tokens = self.tokenize(line)
 
         # of the valid terminators, find the first one to occur in the input
         terminator_pos = len(tokens) + 1
@@ -529,7 +524,7 @@ class StatementParser:
                               output_to=output_to)
         return statement
 
-    def parse_command_only(self, rawinput: str, *, expand: bool = True) -> Statement:
+    def parse_command_only(self, rawinput: str) -> Statement:
         """Partially parse input into a Statement object.
 
         The command is identified, and shortcuts and aliases are expanded.
@@ -554,15 +549,12 @@ class StatementParser:
         whitespace.
 
         :param rawinput: the command line as entered by the user
-        :param expand: If True, then aliases and shortcuts will be expanded.
-                       Set this to False if the command token should not be altered. Defaults to True.
         :return: the created Statement
         """
         line = rawinput
 
         # expand shortcuts and aliases
-        if expand:
-            line = self._expand(rawinput)
+        line = self._expand(rawinput)
 
         command = ''
         args = ''
@@ -616,7 +608,7 @@ class StatementParser:
         """
         # Check if to_parse needs to be converted to a Statement
         if not isinstance(to_parse, Statement):
-            to_parse = self.parse(command_name + ' ' + to_parse, expand=False)
+            to_parse = self.parse(command_name + ' ' + to_parse)
 
         if preserve_quotes:
             return to_parse, to_parse.arg_list
diff --git a/docs/features/shortcuts_aliases_macros.rst b/docs/features/shortcuts_aliases_macros.rst
index 0f06fe08..5628dc66 100644
--- a/docs/features/shortcuts_aliases_macros.rst
+++ b/docs/features/shortcuts_aliases_macros.rst
@@ -65,6 +65,7 @@ Use ``alias delete`` to remove aliases
 
 For more details run: ``help alias delete``
 
+Note: Aliases cannot have the same name as a command or macro
 
 Macros
 ------
@@ -93,3 +94,5 @@ sessions.
 For more details on listing macros run: ``help macro list``
 
 For more details on deleting macros run: ``help macro delete``
+
+Note: Macros cannot have the same name as a command or alias
\ No newline at end of file


Test patch:
diff --git a/tests/test_cmd2.py b/tests/test_cmd2.py
index 8389b9ed..1ba10f6b 100755
--- a/tests/test_cmd2.py
+++ b/tests/test_cmd2.py
@@ -1625,6 +1625,10 @@ def test_alias_create_invalid_name(base_app, alias_name, capsys):
     out, err = run_cmd(base_app, 'alias create {} help'.format(alias_name))
     assert "Invalid alias name" in err[0]
 
+def test_alias_create_with_command_name(base_app):
+    out, err = run_cmd(base_app, 'alias create help stuff')
+    assert "Alias cannot have the same name as a command" in err[0]
+
 def test_alias_create_with_macro_name(base_app):
     macro = "my_macro"
     run_cmd(base_app, 'macro create {} help'.format(macro))
@@ -1713,19 +1717,16 @@ def test_macro_create_invalid_name(base_app, macro_name):
     out, err = run_cmd(base_app, 'macro create {} help'.format(macro_name))
     assert "Invalid macro name" in err[0]
 
+def test_macro_create_with_command_name(base_app):
+    out, err = run_cmd(base_app, 'macro create help stuff')
+    assert "Macro cannot have the same name as a command" in err[0]
+
 def test_macro_create_with_alias_name(base_app):
     macro = "my_macro"
     run_cmd(base_app, 'alias create {} help'.format(macro))
     out, err = run_cmd(base_app, 'macro create {} help'.format(macro))
     assert "Macro cannot have the same name as an alias" in err[0]
 
-def test_macro_create_with_command_name(multiline_app):
-    out, err = run_cmd(multiline_app, 'macro create help stuff')
-    assert out == normalize("Macro 'help' created")
-
-    out, err = run_cmd(multiline_app, 'macro create orate stuff')
-    assert "Macro cannot have the same name as a multiline command" in err[0]
-
 def test_macro_create_with_args(base_app):
     # Create the macro
     out, err = run_cmd(base_app, 'macro create fake {1} {2}')
@@ -1843,37 +1844,6 @@ def test_nonexistent_macro(base_app):
 
     assert exception is not None
 
-def test_input_line_to_statement_expand(base_app):
-    # Enable/Disable expansion of shortcuts
-    line = '!ls'
-    statement = base_app._input_line_to_statement(line, expand=True)
-    assert statement.command == 'shell'
-
-    statement = base_app._input_line_to_statement(line, expand=False)
-    assert statement.command == '!ls'
-
-    # Enable/Disable expansion of aliases
-    run_cmd(base_app, 'alias create help macro')
-
-    line = 'help'
-    statement = base_app._input_line_to_statement(line, expand=True)
-    assert statement.command == 'macro'
-
-    statement = base_app._input_line_to_statement(line, expand=False)
-    assert statement.command == 'help'
-
-    run_cmd(base_app, 'alias delete help')
-
-    # Enable/Disable expansion of macros
-    run_cmd(base_app, 'macro create help alias')
-
-    line = 'help'
-    statement = base_app._input_line_to_statement(line, expand=True)
-    assert statement.command == 'alias'
-
-    statement = base_app._input_line_to_statement(line, expand=False)
-    assert statement.command == 'help'
-
 def test_ppaged(outsim_app):
     msg = 'testing...'
     end = '\n'
diff --git a/tests/test_parsing.py b/tests/test_parsing.py
index ac540183..2114bfaa 100755
--- a/tests/test_parsing.py
+++ b/tests/test_parsing.py
@@ -604,19 +604,11 @@ def test_empty_statement_raises_exception():
     ('l', 'shell', 'ls -al')
 ])
 def test_parse_alias_and_shortcut_expansion(parser, line, command, args):
-    # Test first with expansion
     statement = parser.parse(line)
     assert statement.command == command
     assert statement == args
     assert statement.args == statement
 
-    # Now allow no expansion
-    tokens = shlex_split(line)
-    statement = parser.parse(line, expand=False)
-    assert statement.command == tokens[0]
-    assert shlex_split(statement) == tokens[1:]
-    assert statement.args == statement
-
 def test_parse_alias_on_multiline_command(parser):
     line = 'anothermultiline has > inside an unfinished command'
     statement = parser.parse(line)