cmCMakePathCommand: Enforce non-empty string arguments via binding type

3 files changed, 12 insertions, 39 deletions

diff --git a/Source/cmCMakePathCommand.cxx b/Source/cmCMakePathCommand.cxx
index b955bcf..7755082 100644
--- a/Source/cmCMakePathCommand.cxx
+++ b/Source/cmCMakePathCommand.cxx
@@ -15,6 +15,7 @@
 #include <cmext/string_view>
 
 #include "cmArgumentParser.h"
+#include "cmArgumentParserTypes.h"
 #include "cmCMakePath.h"
 #include "cmExecutionStatus.h"
 #include "cmMakefile.h"
@@ -82,22 +83,11 @@ public:
     return this->CMakePathArgumentParser<Result>::template Parse<Advance>(
       args);
   }
-
-  bool checkOutputVariable(const Result& arguments,
-                           cmExecutionStatus& status) const
-  {
-    if (arguments.Output && arguments.Output->empty()) {
-      status.SetError("Invalid name for output variable.");
-      return false;
-    }
-
-    return true;
-  }
 };
 
 struct OutputVariable : public ArgumentParser::ParseResult
 {
-  cm::optional<std::string> Output;
+  cm::optional<ArgumentParser::NonEmpty<std::string>> Output;
 };
 // Usable when OUTPUT_VARIABLE is the only option
 class OutputVariableParser
@@ -270,9 +260,6 @@ bool HandleAppendCommand(std::vector<std::string> const& args,
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   cmCMakePath path(status.GetMakefile().GetSafeDefinition(args[1]));
   for (const auto& input : parser.GetInputs()) {
@@ -295,9 +282,6 @@ bool HandleAppendStringCommand(std::vector<std::string> const& args,
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   std::string inputPath;
   if (!getInputPath(args[1], status, inputPath)) {
@@ -325,9 +309,6 @@ bool HandleRemoveFilenameCommand(std::vector<std::string> const& args,
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   if (!parser.GetInputs().empty()) {
     status.SetError("REMOVE_FILENAME called with unexpected arguments.");
@@ -358,9 +339,6 @@ bool HandleReplaceFilenameCommand(std::vector<std::string> const& args,
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   if (parser.GetInputs().size() > 1) {
     status.SetError("REPLACE_FILENAME called with unexpected arguments.");
@@ -387,7 +365,7 @@ bool HandleRemoveExtensionCommand(std::vector<std::string> const& args,
 {
   struct Arguments : public ArgumentParser::ParseResult
   {
-    cm::optional<std::string> Output;
+    cm::optional<ArgumentParser::NonEmpty<std::string>> Output;
     bool LastOnly = false;
   };
 
@@ -400,9 +378,6 @@ bool HandleRemoveExtensionCommand(std::vector<std::string> const& args,
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   if (!parser.GetInputs().empty()) {
     status.SetError("REMOVE_EXTENSION called with unexpected arguments.");
@@ -433,7 +408,7 @@ bool HandleReplaceExtensionCommand(std::vector<std::string> const& args,
 {
   struct Arguments : public ArgumentParser::ParseResult
   {
-    cm::optional<std::string> Output;
+    cm::optional<ArgumentParser::NonEmpty<std::string>> Output;
     bool LastOnly = false;
   };
 
@@ -446,9 +421,6 @@ bool HandleReplaceExtensionCommand(std::vector<std::string> const& args,
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   if (parser.GetInputs().size() > 1) {
     status.SetError("REPLACE_EXTENSION called with unexpected arguments.");
@@ -486,9 +458,6 @@ bool HandleNormalPathCommand(std::vector<std::string> const& args,
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   if (!parser.GetInputs().empty()) {
     status.SetError("NORMAL_PATH called with unexpected arguments.");
@@ -516,7 +485,7 @@ bool HandleTransformPathCommand(
 {
   struct Arguments : public ArgumentParser::ParseResult
   {
-    cm::optional<std::string> Output;
+    cm::optional<ArgumentParser::NonEmpty<std::string>> Output;
     cm::optional<std::string> BaseDirectory;
     bool Normalize = false;
   };
@@ -532,9 +501,6 @@ bool HandleTransformPathCommand(
   if (arguments.MaybeReportError(status.GetMakefile())) {
     return true;
   }
-  if (!parser.checkOutputVariable(arguments, status)) {
-    return false;
-  }
 
   if (!parser.GetInputs().empty()) {
     status.SetError(cmStrCat(args[0], " called with unexpected arguments."));
diff --git a/Tests/RunCMake/cmake_path/OUTPUT_VARIABLE-empty-stderr.txt b/Tests/RunCMake/cmake_path/OUTPUT_VARIABLE-empty-stderr.txt
new file mode 100644
index 0000000..f1b52cc
--- /dev/null
+++ b/Tests/RunCMake/cmake_path/OUTPUT_VARIABLE-empty-stderr.txt
@@ -0,0 +1,4 @@
+CMake Error at .+/call-cmake_path.cmake:[0-9]+ \(cmake_path\):
+  Error after keyword "OUTPUT_VARIABLE":
+
+    empty string not allowed
diff --git a/Tests/RunCMake/cmake_path/RunCMakeTest.cmake b/Tests/RunCMake/cmake_path/RunCMakeTest.cmake
index 8a2dd95..1742b06 100644
--- a/Tests/RunCMake/cmake_path/RunCMakeTest.cmake
+++ b/Tests/RunCMake/cmake_path/RunCMakeTest.cmake
@@ -114,6 +114,9 @@ foreach (command IN ITEMS NATIVE_PATH
   run_cmake_command (${command}-invalid-output "${CMAKE_COMMAND}" "-DCMAKE_PATH_ARGUMENTS=${command} path ${extra_args}" -DCHECK_INVALID_OUTPUT=ON -P "${RunCMake_SOURCE_DIR}/call-cmake_path.cmake")
 endforeach()
 
+# OUTPUT_VARIABLE empty name
+set (RunCMake-stderr-file "OUTPUT_VARIABLE-empty-stderr.txt")
+
 foreach (command IN ITEMS APPEND APPEND_STRING REMOVE_FILENAME REPLACE_FILENAME
                           REMOVE_EXTENSION REPLACE_EXTENSION NORMAL_PATH
                           RELATIVE_PATH ABSOLUTE_PATH)