Have CommandObjectParsed check for "commands that take no arguments".

This is currently being done in an ad hoc way, and so for some
commands it isn't being checked.  We have the info to make this check,
since commands are supposed to add their arguments to the m_arguments
field of the CommandObject.  This change uses that info to check whether
the command received arguments in error.

A handful of commands weren't defining their argument types, I also had
to fix them.  And a bunch of commands were checking for arguments by
hand, so I removed those checks in favor of the CommandObject one.  That
also meant I had to change some tests that were checking for the ad hoc
error outputs.

Differential Revision: https://reviews.llvm.org/D128453

Author: jimingham

lldb/include/lldb/lldb-enumerations.h

@@ -603,6 +603,11 @@ enum CommandArgumentType {
   eArgTypeModuleUUID,
   eArgTypeSaveCoreStyle,
   eArgTypeLogHandler,
+  eArgTypeSEDStylePair,
+  eArgTypeRecognizerID,
+  eArgTypeConnectURL,
+  eArgTypeTargetID,
+  eArgTypeStopHookID,
   eArgTypeLastArg // Always keep this entry as the last entry in this
                   // enumeration!!
 };

lldb/source/API/SBCommandInterpreter.cpp

@@ -47,6 +47,10 @@ class CommandPluginInterfaceImplementation : public CommandObjectParsed {
         auto_repeat_command == nullptr
             ? llvm::None
             : llvm::Optional<std::string>(auto_repeat_command);
+    // We don't know whether any given command coming from this interface takes
+    // arguments or not so here we're just disabling the basic args check.
+    CommandArgumentData none_arg{eArgTypeNone, eArgRepeatStar};
+    m_arguments.push_back({none_arg});
   }
 
   bool IsRemovable() const override { return true; }

lldb/source/Commands/CommandObjectCommands.cpp

@@ -824,6 +824,8 @@ a number follows 'f':"
         R"(
 
     (lldb) command regex f s/^$/finish/ 's/([0-9]+)/frame select %1/')");
+    CommandArgumentData thread_arg{eArgTypeSEDStylePair, eArgRepeatOptional};
+    m_arguments.push_back({thread_arg});
   }
 
   ~CommandObjectCommandsAddRegex() override = default;
@@ -1664,11 +1666,6 @@ class CommandObjectCommandsScriptList : public CommandObjectParsed {
   ~CommandObjectCommandsScriptList() override = default;
 
   bool DoExecute(Args &command, CommandReturnObject &result) override {
-    if (command.GetArgumentCount() != 0) {
-      result.AppendError("'command script list' doesn't take any arguments");
-      return false;
-    }
-
     m_interpreter.GetHelp(result, CommandInterpreter::eCommandTypesUserDef);
 
     result.SetStatus(eReturnStatusSuccessFinishResult);
@@ -1689,11 +1686,6 @@ class CommandObjectCommandsScriptClear : public CommandObjectParsed {
 
 protected:
   bool DoExecute(Args &command, CommandReturnObject &result) override {
-    if (command.GetArgumentCount() != 0) {
-      result.AppendError("'command script clear' doesn't take any arguments");
-      return false;
-    }
-
     m_interpreter.RemoveAllUser();
 
     result.SetStatus(eReturnStatusSuccessFinishResult);

lldb/source/Commands/CommandObjectFrame.cpp

@@ -924,7 +924,11 @@ class CommandObjectFrameRecognizerDelete : public CommandObjectParsed {
 public:
   CommandObjectFrameRecognizerDelete(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "frame recognizer delete",
-                            "Delete an existing frame recognizer.", nullptr) {}
+                            "Delete an existing frame recognizer by id.",
+                            nullptr) {
+    CommandArgumentData thread_arg{eArgTypeRecognizerID, eArgRepeatPlain};
+    m_arguments.push_back({thread_arg});
+  }
 
   ~CommandObjectFrameRecognizerDelete() override = default;
 

lldb/source/Commands/CommandObjectGUI.cpp

@@ -26,22 +26,18 @@ CommandObjectGUI::~CommandObjectGUI() = default;
 
 bool CommandObjectGUI::DoExecute(Args &args, CommandReturnObject &result) {
 #if LLDB_ENABLE_CURSES
-  if (args.GetArgumentCount() == 0) {
-    Debugger &debugger = GetDebugger();
-
-    File &input = debugger.GetInputFile();
-    File &output = debugger.GetOutputFile();
-    if (input.GetStream() && output.GetStream() && input.GetIsRealTerminal() &&
-        input.GetIsInteractive()) {
-      IOHandlerSP io_handler_sp(new IOHandlerCursesGUI(debugger));
-      if (io_handler_sp)
-        debugger.RunIOHandlerAsync(io_handler_sp);
-      result.SetStatus(eReturnStatusSuccessFinishResult);
-    } else {
-      result.AppendError("the gui command requires an interactive terminal.");
-    }
+  Debugger &debugger = GetDebugger();
+
+  File &input = debugger.GetInputFile();
+  File &output = debugger.GetOutputFile();
+  if (input.GetStream() && output.GetStream() && input.GetIsRealTerminal() &&
+      input.GetIsInteractive()) {
+    IOHandlerSP io_handler_sp(new IOHandlerCursesGUI(debugger));
+    if (io_handler_sp)
+      debugger.RunIOHandlerAsync(io_handler_sp);
+    result.SetStatus(eReturnStatusSuccessFinishResult);
   } else {
-    result.AppendError("the gui command takes no arguments.");
+    result.AppendError("the gui command requires an interactive terminal.");
   }
   return true;
 #else

lldb/source/Commands/CommandObjectPlatform.cpp

@@ -150,6 +150,8 @@ class CommandObjectPlatformSelect : public CommandObjectParsed {
   {
     m_option_group.Append(&m_platform_options, LLDB_OPT_SET_ALL, 1);
     m_option_group.Finalize();
+    CommandArgumentData platform_arg{eArgTypePlatform, eArgRepeatPlain};
+    m_arguments.push_back({platform_arg});
   }
 
   ~CommandObjectPlatformSelect() override = default;
@@ -271,7 +273,10 @@ class CommandObjectPlatformConnect : public CommandObjectParsed {
       : CommandObjectParsed(
             interpreter, "platform connect",
             "Select the current platform by providing a connection URL.",
-            "platform connect <connect-url>", 0) {}
+            "platform connect <connect-url>", 0) {
+    CommandArgumentData platform_arg{eArgTypeConnectURL, eArgRepeatPlain};
+    m_arguments.push_back({platform_arg});
+  }
 
   ~CommandObjectPlatformConnect() override = default;
 
@@ -415,7 +420,10 @@ class CommandObjectPlatformMkDir : public CommandObjectParsed {
   CommandObjectPlatformMkDir(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "platform mkdir",
                             "Make a new directory on the remote end.", nullptr,
-                            0) {}
+                            0) {
+    CommandArgumentData thread_arg{eArgTypePath, eArgRepeatPlain};
+    m_arguments.push_back({thread_arg});
+  }
 
   ~CommandObjectPlatformMkDir() override = default;
 
@@ -461,7 +469,10 @@ class CommandObjectPlatformFOpen : public CommandObjectParsed {
 public:
   CommandObjectPlatformFOpen(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "platform file open",
-                            "Open a file on the remote end.", nullptr, 0) {}
+                            "Open a file on the remote end.", nullptr, 0) {
+    CommandArgumentData path_arg{eArgTypePath, eArgRepeatPlain};
+    m_arguments.push_back({path_arg});
+  }
 
   ~CommandObjectPlatformFOpen() override = default;
 
@@ -521,7 +532,10 @@ class CommandObjectPlatformFClose : public CommandObjectParsed {
 public:
   CommandObjectPlatformFClose(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "platform file close",
-                            "Close a file on the remote end.", nullptr, 0) {}
+                            "Close a file on the remote end.", nullptr, 0) {
+    CommandArgumentData path_arg{eArgTypeUnsignedInteger, eArgRepeatPlain};
+    m_arguments.push_back({path_arg});
+  }
 
   ~CommandObjectPlatformFClose() override = default;
 
@@ -562,7 +576,10 @@ class CommandObjectPlatformFRead : public CommandObjectParsed {
   CommandObjectPlatformFRead(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "platform file read",
                             "Read data from a file on the remote end.", nullptr,
-                            0) {}
+                            0) {
+    CommandArgumentData path_arg{eArgTypeUnsignedInteger, eArgRepeatPlain};
+    m_arguments.push_back({path_arg});
+  }
 
   ~CommandObjectPlatformFRead() override = default;
 
@@ -655,7 +672,10 @@ class CommandObjectPlatformFWrite : public CommandObjectParsed {
   CommandObjectPlatformFWrite(CommandInterpreter &interpreter)
       : CommandObjectParsed(interpreter, "platform file write",
                             "Write data to a file on the remote end.", nullptr,
-                            0) {}
+                            0) {
+    CommandArgumentData path_arg{eArgTypeUnsignedInteger, eArgRepeatPlain};
+    m_arguments.push_back({path_arg});
+  }
 
   ~CommandObjectPlatformFWrite() override = default;
 
@@ -1070,6 +1090,10 @@ class CommandObjectPlatformPutFile : public CommandObjectParsed {
     Relative source file paths are resolved against lldb's local working directory.
 
     Omitting the destination places the file in the platform working directory.)");
+    CommandArgumentData source_arg{eArgTypePath, eArgRepeatPlain};
+    CommandArgumentData path_arg{eArgTypePath, eArgRepeatOptional};
+    m_arguments.push_back({source_arg});
+    m_arguments.push_back({path_arg});
   }
 
   ~CommandObjectPlatformPutFile() override = default;
@@ -1121,6 +1145,8 @@ class CommandObjectPlatformProcessLaunch : public CommandObjectParsed {
                             eCommandRequiresTarget | eCommandTryTargetAPILock) {
     m_all_options.Append(&m_options);
     m_all_options.Finalize();
+    CommandArgumentData run_arg_arg{eArgTypeRunArgs, eArgRepeatStar};
+    m_arguments.push_back({run_arg_arg});
   }
 
   ~CommandObjectPlatformProcessLaunch() override = default;
@@ -1229,83 +1255,78 @@ class CommandObjectPlatformProcessList : public CommandObjectParsed {
 
     if (platform_sp) {
       Status error;
-      if (args.GetArgumentCount() == 0) {
-        if (platform_sp) {
-          Stream &ostrm = result.GetOutputStream();
-
-          lldb::pid_t pid =
-              m_options.match_info.GetProcessInfo().GetProcessID();
-          if (pid != LLDB_INVALID_PROCESS_ID) {
-            ProcessInstanceInfo proc_info;
-            if (platform_sp->GetProcessInfo(pid, proc_info)) {
-              ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args,
-                                                   m_options.verbose);
-              proc_info.DumpAsTableRow(ostrm, platform_sp->GetUserIDResolver(),
-                                       m_options.show_args, m_options.verbose);
-              result.SetStatus(eReturnStatusSuccessFinishResult);
-            } else {
-              result.AppendErrorWithFormat(
-                  "no process found with pid = %" PRIu64 "\n", pid);
-            }
+      if (platform_sp) {
+        Stream &ostrm = result.GetOutputStream();
+
+        lldb::pid_t pid = m_options.match_info.GetProcessInfo().GetProcessID();
+        if (pid != LLDB_INVALID_PROCESS_ID) {
+          ProcessInstanceInfo proc_info;
+          if (platform_sp->GetProcessInfo(pid, proc_info)) {
+            ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args,
+                                                 m_options.verbose);
+            proc_info.DumpAsTableRow(ostrm, platform_sp->GetUserIDResolver(),
+                                     m_options.show_args, m_options.verbose);
+            result.SetStatus(eReturnStatusSuccessFinishResult);
           } else {
-            ProcessInstanceInfoList proc_infos;
-            const uint32_t matches =
-                platform_sp->FindProcesses(m_options.match_info, proc_infos);
-            const char *match_desc = nullptr;
-            const char *match_name =
-                m_options.match_info.GetProcessInfo().GetName();
-            if (match_name && match_name[0]) {
-              switch (m_options.match_info.GetNameMatchType()) {
-              case NameMatch::Ignore:
-                break;
-              case NameMatch::Equals:
-                match_desc = "matched";
-                break;
-              case NameMatch::Contains:
-                match_desc = "contained";
-                break;
-              case NameMatch::StartsWith:
-                match_desc = "started with";
-                break;
-              case NameMatch::EndsWith:
-                match_desc = "ended with";
-                break;
-              case NameMatch::RegularExpression:
-                match_desc = "matched the regular expression";
-                break;
-              }
+            result.AppendErrorWithFormat(
+                "no process found with pid = %" PRIu64 "\n", pid);
+          }
+        } else {
+          ProcessInstanceInfoList proc_infos;
+          const uint32_t matches =
+              platform_sp->FindProcesses(m_options.match_info, proc_infos);
+          const char *match_desc = nullptr;
+          const char *match_name =
+              m_options.match_info.GetProcessInfo().GetName();
+          if (match_name && match_name[0]) {
+            switch (m_options.match_info.GetNameMatchType()) {
+            case NameMatch::Ignore:
+              break;
+            case NameMatch::Equals:
+              match_desc = "matched";
+              break;
+            case NameMatch::Contains:
+              match_desc = "contained";
+              break;
+            case NameMatch::StartsWith:
+              match_desc = "started with";
+              break;
+            case NameMatch::EndsWith:
+              match_desc = "ended with";
+              break;
+            case NameMatch::RegularExpression:
+              match_desc = "matched the regular expression";
+              break;
             }
+          }
 
-            if (matches == 0) {
-              if (match_desc)
-                result.AppendErrorWithFormatv(
-                    "no processes were found that {0} \"{1}\" on the \"{2}\" "
-                    "platform\n",
-                    match_desc, match_name, platform_sp->GetName());
-              else
-                result.AppendErrorWithFormatv(
-                    "no processes were found on the \"{0}\" platform\n",
-                    platform_sp->GetName());
-            } else {
-              result.AppendMessageWithFormatv(
-                  "{0} matching process{1} found on \"{2}\"", matches,
-                  matches > 1 ? "es were" : " was", platform_sp->GetName());
-              if (match_desc)
-                result.AppendMessageWithFormat(" whose name %s \"%s\"",
-                                               match_desc, match_name);
-              result.AppendMessageWithFormat("\n");
-              ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args,
-                                                   m_options.verbose);
-              for (uint32_t i = 0; i < matches; ++i) {
-                proc_infos[i].DumpAsTableRow(
-                    ostrm, platform_sp->GetUserIDResolver(),
-                    m_options.show_args, m_options.verbose);
-              }
+          if (matches == 0) {
+            if (match_desc)
+              result.AppendErrorWithFormatv(
+                  "no processes were found that {0} \"{1}\" on the \"{2}\" "
+                  "platform\n",
+                  match_desc, match_name, platform_sp->GetName());
+            else
+              result.AppendErrorWithFormatv(
+                  "no processes were found on the \"{0}\" platform\n",
+                  platform_sp->GetName());
+          } else {
+            result.AppendMessageWithFormatv(
+                "{0} matching process{1} found on \"{2}\"", matches,
+                matches > 1 ? "es were" : " was", platform_sp->GetName());
+            if (match_desc)
+              result.AppendMessageWithFormat(" whose name %s \"%s\"",
+                                             match_desc, match_name);
+            result.AppendMessageWithFormat("\n");
+            ProcessInstanceInfo::DumpTableHeader(ostrm, m_options.show_args,
+                                                 m_options.verbose);
+            for (uint32_t i = 0; i < matches; ++i) {
+              proc_infos[i].DumpAsTableRow(
+                  ostrm, platform_sp->GetUserIDResolver(), m_options.show_args,
+                  m_options.verbose);
             }
           }
         }
-      } else {
-        result.AppendError("invalid args: process list takes only options\n");
       }
     } else {
       result.AppendError("no platform is selected\n");
@@ -1737,7 +1758,10 @@ class CommandObjectPlatformShell : public CommandObjectRaw {
   CommandObjectPlatformShell(CommandInterpreter &interpreter)
       : CommandObjectRaw(interpreter, "platform shell",
                          "Run a shell command on the current platform.",
-                         "platform shell <shell-command>", 0) {}
+                         "platform shell <shell-command>", 0) {
+    CommandArgumentData thread_arg{eArgTypeNone, eArgRepeatStar};
+    m_arguments.push_back({thread_arg});
+  }
 
   ~CommandObjectPlatformShell() override = default;
 
@@ -1824,7 +1848,12 @@ class CommandObjectPlatformInstall : public CommandObjectParsed {
       : CommandObjectParsed(
             interpreter, "platform target-install",
             "Install a target (bundle or executable file) to the remote end.",
-            "platform target-install <local-thing> <remote-sandbox>", 0) {}
+            "platform target-install <local-thing> <remote-sandbox>", 0) {
+    CommandArgumentData local_arg{eArgTypePath, eArgRepeatPlain};
+    CommandArgumentData remote_arg{eArgTypePath, eArgRepeatPlain};
+    m_arguments.push_back({local_arg});
+    m_arguments.push_back({remote_arg});
+  }
 
   ~CommandObjectPlatformInstall() override = default;