Merge branch 'master' into 2.0

2 files changed, 39 insertions, 24 deletions

diff --git a/cmd2/cmd2.py b/cmd2/cmd2.py
index aaa45af6..db4ba86f 100644
--- a/cmd2/cmd2.py
+++ b/cmd2/cmd2.py
@@ -260,22 +260,6 @@ class Cmd(cmd.Cmd):
                                                 multiline_commands=multiline_commands,
                                                 shortcuts=shortcuts)
 
-        # Load modular commands
-        self._installed_command_sets = []  # type: List[CommandSet]
-        self._cmd_to_command_sets = {}  # type: Dict[str, CommandSet]
-        if command_sets:
-            for command_set in command_sets:
-                self.register_command_set(command_set)
-
-        if auto_load_commands:
-            self._autoload_commands()
-
-        # Verify commands don't have invalid names (like starting with a shortcut)
-        for cur_cmd in self.get_all_commands():
-            valid, errmsg = self.statement_parser.is_valid_command(cur_cmd)
-            if not valid:
-                raise ValueError("Invalid command name {!r}: {}".format(cur_cmd, errmsg))
-
         # Stores results from the last command run to enable usage of results in a Python script or interactive console
         # Built-in commands don't make use of this.  It is purely there for user-defined commands and convenience.
         self.last_result = None
@@ -413,6 +397,28 @@ class Cmd(cmd.Cmd):
         # If False, then complete() will sort the matches using self.default_sort_key before they are displayed.
         self.matches_sorted = False
 
+        ############################################################################################################
+        # The following code block loads CommandSets, verifies command names, and registers subcommands.
+        # This block should appear after all attributes have been created since the registration code
+        # depends on them and it's possible a module's on_register() method may need to access some.
+        ############################################################################################################
+        # Load modular commands
+        self._installed_command_sets = []  # type: List[CommandSet]
+        self._cmd_to_command_sets = {}  # type: Dict[str, CommandSet]
+        if command_sets:
+            for command_set in command_sets:
+                self.register_command_set(command_set)
+
+        if auto_load_commands:
+            self._autoload_commands()
+
+        # Verify commands don't have invalid names (like starting with a shortcut)
+        for cur_cmd in self.get_all_commands():
+            valid, errmsg = self.statement_parser.is_valid_command(cur_cmd)
+            if not valid:
+                raise ValueError("Invalid command name {!r}: {}".format(cur_cmd, errmsg))
+
+        # Add functions decorated to be subcommands
         self._register_subcommands(self)
 
     def find_commandsets(self, commandset_type: Type[CommandSet], *, subclass_match: bool = False) -> List[CommandSet]:
@@ -632,10 +638,14 @@ class Cmd(cmd.Cmd):
 
         # iterate through all matching methods
         for method_name, method in methods:
-            subcommand_name = getattr(method, constants.SUBCMD_ATTR_NAME)
+            subcommand_name = getattr(method, constants.SUBCMD_ATTR_NAME)  # type: str
             full_command_name = getattr(method, constants.SUBCMD_ATTR_COMMAND)  # type: str
             subcmd_parser = getattr(method, constants.CMD_ATTR_ARGPARSER)
 
+            subcommand_valid, errmsg = self.statement_parser.is_valid_command(subcommand_name, is_subcommand=True)
+            if not subcommand_valid:
+                raise CommandSetRegistrationError('Subcommand {} is not valid: {}'.format(str(subcommand_name), errmsg))
+
             command_tokens = full_command_name.split()
             command_name = command_tokens[0]
             subcommand_names = command_tokens[1:]
diff --git a/cmd2/parsing.py b/cmd2/parsing.py
index a7ee74a1..657db32c 100755
--- a/cmd2/parsing.py
+++ b/cmd2/parsing.py
@@ -277,7 +277,7 @@ class StatementParser:
         expr = r'\A\s*(\S*?)({})'.format(second_group)
         self._command_pattern = re.compile(expr)
 
-    def is_valid_command(self, word: str) -> Tuple[bool, str]:
+    def is_valid_command(self, word: str, *, is_subcommand: bool = False) -> Tuple[bool, str]:
         """Determine whether a word is a valid name for a command.
 
         Commands can not include redirection characters, whitespace,
@@ -285,6 +285,7 @@ class StatementParser:
         shortcut.
 
         :param word: the word to check as a command
+        :param is_subcommand: Flag whether this command name is a subcommand name
         :return: a tuple of a boolean and an error string
 
         If word is not a valid command, return ``False`` and an error string
@@ -297,18 +298,22 @@ class StatementParser:
         """
         valid = False
 
+        if not isinstance(word, str):
+            return False, 'must be a string. Received {} instead'.format(str(type(word)))
+
         if not word:
             return False, 'cannot be an empty string'
 
         if word.startswith(constants.COMMENT_CHAR):
             return False, 'cannot start with the comment character'
 
-        for (shortcut, _) in self.shortcuts:
-            if word.startswith(shortcut):
-                # Build an error string with all shortcuts listed
-                errmsg = 'cannot start with a shortcut: '
-                errmsg += ', '.join(shortcut for (shortcut, _) in self.shortcuts)
-                return False, errmsg
+        if not is_subcommand:
+            for (shortcut, _) in self.shortcuts:
+                if word.startswith(shortcut):
+                    # Build an error string with all shortcuts listed
+                    errmsg = 'cannot start with a shortcut: '
+                    errmsg += ', '.join(shortcut for (shortcut, _) in self.shortcuts)
+                    return False, errmsg
 
         errmsg = 'cannot contain: whitespace, quotes, '
         errchars = []