separate_arguments: refactoring

Source/cmSeparateArgumentsCommand.cxx

@@ -4,9 +4,13 @@
 
 #include <algorithm>
 
+#include <cmext/string_view>
+
+#include "cmArgumentParser.h"
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
 #include "cmProperty.h"
+#include "cmRange.h"
 #include "cmStringAlgorithms.h"
 #include "cmSystemTools.h"
 
@@ -19,86 +23,89 @@ bool cmSeparateArgumentsCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  std::string var;
-  std::string command;
-  enum Mode
-  {
-    ModeOld,
-    ModeUnix,
-    ModeWindows
-  };
-  Mode mode = ModeOld;
-  enum Doing
-  {
-    DoingNone,
-    DoingVariable,
-    DoingMode,
-    DoingCommand
-  };
-  Doing doing = DoingVariable;
-  for (std::string const& arg : args) {
-    if (doing == DoingVariable) {
-      var = arg;
-      doing = DoingMode;
-      // This will always clone one of the other blocks.
-      // NOLINTNEXTLINE(bugprone-branch-clone)
-    } else if (doing == DoingMode && arg == "NATIVE_COMMAND") {
-#ifdef _WIN32
-      mode = ModeWindows;
-#else
-      mode = ModeUnix;
-#endif
-      doing = DoingCommand;
-    } else if (doing == DoingMode && arg == "UNIX_COMMAND") {
-      mode = ModeUnix;
-      doing = DoingCommand;
-    } else if (doing == DoingMode && arg == "WINDOWS_COMMAND") {
-      mode = ModeWindows;
-      doing = DoingCommand;
-    } else if (doing == DoingCommand) {
-      command = arg;
-      doing = DoingNone;
-    } else {
-      status.SetError(cmStrCat("given unknown argument ", arg));
-      return false;
-    }
-  }
+  std::string const& var = args.front();
 
-  if (mode == ModeOld) {
+  if (args.size() == 1) {
     // Original space-replacement version of command.
     if (cmProp def = status.GetMakefile().GetDefinition(var)) {
       std::string value = *def;
       std::replace(value.begin(), value.end(), ' ', ';');
       status.GetMakefile().AddDefinition(var, value);
     }
+
+    return true;
+  }
+
+  struct Arguments
+  {
+    bool UnixCommand = false;
+    bool WindowsCommand = false;
+    bool NativeCommand = false;
+  };
+
+  static auto const parser =
+    cmArgumentParser<Arguments>{}
+      .Bind("UNIX_COMMAND"_s, &Arguments::UnixCommand)
+      .Bind("WINDOWS_COMMAND"_s, &Arguments::WindowsCommand)
+      .Bind("NATIVE_COMMAND"_s, &Arguments::NativeCommand);
+
+  std::vector<std::string> unparsedArguments;
+  Arguments arguments =
+    parser.Parse(cmMakeRange(args).advance(1), &unparsedArguments);
+
+  if (!arguments.UnixCommand && !arguments.WindowsCommand &&
+      !arguments.NativeCommand) {
+    status.SetError("missing required option: 'UNIX_COMMAND' or "
+                    "'WINDOWS_COMMAND' or 'NATIVE_COMMAND'");
+    return false;
+  }
+  if ((arguments.UnixCommand && arguments.WindowsCommand) ||
+      (arguments.UnixCommand && arguments.NativeCommand) ||
+      (arguments.WindowsCommand && arguments.NativeCommand)) {
+    status.SetError("'UNIX_COMMAND', 'WINDOWS_COMMAND' and 'NATIVE_COMMAND' "
+                    "are mutually exclusive");
+    return false;
+  }
+
+  if (unparsedArguments.size() > 1) {
+    status.SetError("given unexpected argument(s)");
+    return false;
+  }
+
+  std::string& command = unparsedArguments.front();
+
+  if (command.empty()) {
+    status.GetMakefile().AddDefinition(var, command);
+    return true;
+  }
+
+  // split command given
+  std::vector<std::string> values;
+
+  if (arguments.NativeCommand) {
+#if defined(_WIN32)
+    arguments.WindowsCommand = true;
+#else
+    arguments.UnixCommand = true;
+#endif
+  }
+
+  if (arguments.UnixCommand) {
+    cmSystemTools::ParseUnixCommandLine(command.c_str(), values);
   } else {
-    // Parse the command line.
-    std::vector<std::string> vec;
-    if (mode == ModeUnix) {
-      cmSystemTools::ParseUnixCommandLine(command.c_str(), vec);
-    } else // if(mode == ModeWindows)
-    {
-      cmSystemTools::ParseWindowsCommandLine(command.c_str(), vec);
-    }
+    cmSystemTools::ParseWindowsCommandLine(command.c_str(), values);
+  }
 
-    // Construct the result list value.
-    std::string value;
-    const char* sep = "";
-    for (std::string const& vi : vec) {
-      // Separate from the previous argument.
-      value += sep;
-      sep = ";";
-
-      // Preserve semicolons.
-      for (char si : vi) {
-        if (si == ';') {
-          value += '\\';
-        }
-        value += si;
-      }
+  // preserve semicolons in arguments
+  std::for_each(values.begin(), values.end(), [](std::string& value) {
+    std::string::size_type pos = 0;
+    while ((pos = value.find_first_of(';', pos)) != std::string::npos) {
+      value.insert(pos, 1, '\\');
+      pos += 2;
     }
-    status.GetMakefile().AddDefinition(var, value);
-  }
+  });
+  auto value = cmJoin(values, ";");
+  status.GetMakefile().AddDefinition(var, value);
 
   return true;
 }

Tests/RunCMake/separate_arguments/MultipleArguments-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/separate_arguments/MultipleArguments-stderr.txt

@@ -0,0 +1,4 @@
+CMake Error at MultipleArguments.cmake:[0-9]+ \(separate_arguments\):
+  separate_arguments given unexpected argument\(s\)
+Call Stack \(most recent call first\):
+  CMakeLists.txt:[0-9]+ \(include\)

Tests/RunCMake/separate_arguments/MultipleArguments.cmake

@@ -0,0 +1,2 @@
+
+separate_arguments (var UNIX_COMMAND arg1 arg2)

Tests/RunCMake/separate_arguments/MultipleCommands-result.txt

@@ -0,0 +1 @@
+1

Tests/RunCMake/separate_arguments/MultipleCommands-stderr.txt

@@ -0,0 +1,19 @@
+CMake Error at MultipleCommands.cmake:[0-9]+ \(separate_arguments\):
+  separate_arguments 'UNIX_COMMAND', 'WINDOWS_COMMAND' and 'NATIVE_COMMAND'
+  are mutually exclusive
+Call Stack \(most recent call first\):
+  CMakeLists.txt:[0-9]+ \(include\)
+
+
+CMake Error at MultipleCommands.cmake:[0-9]+ \(separate_arguments\):
+  separate_arguments 'UNIX_COMMAND', 'WINDOWS_COMMAND' and 'NATIVE_COMMAND'
+  are mutually exclusive
+Call Stack \(most recent call first\):
+  CMakeLists.txt:[0-9]+ \(include\)
+
+
+CMake Error at MultipleCommands.cmake:[0-9]+ \(separate_arguments\):
+  separate_arguments 'UNIX_COMMAND', 'WINDOWS_COMMAND' and 'NATIVE_COMMAND'
+  are mutually exclusive
+Call Stack \(most recent call first\):
+  CMakeLists.txt:[0-9]+ \(include\)

Tests/RunCMake/separate_arguments/MultipleCommands.cmake

@@ -0,0 +1,6 @@
+
+separate_arguments(var UNIX_COMMAND WINDOWS_COMMAND)
+
+separate_arguments(var UNIX_COMMAND NATIVE_COMMAND)
+
+separate_arguments(var WINDOWS_COMMAND NATIVE_COMMAND)

Tests/RunCMake/separate_arguments/RunCMakeTest.cmake

@@ -1,5 +1,8 @@
 include(RunCMake)
 
+run_cmake(MultipleCommands)
+run_cmake(MultipleArguments)
+
 run_cmake(EmptyCommand)
 run_cmake(PlainCommand)
 run_cmake(UnixCommand)