cmArgumentParser: Model maybe-missing string with wrapper type

Bindings to `std::string` require one value.  Some clients have been
filtering `keywordsMissingValue` to support keywords that tolerate a
missing value.  Offer them a type-safe way to achieve this instead.

5 files changed, 32 insertions, 24 deletions

diff --git a/Source/cmArgumentParser.cxx b/Source/cmArgumentParser.cxx
index 1bb9051..f0c4cfc 100644
--- a/Source/cmArgumentParser.cxx
+++ b/Source/cmArgumentParser.cxx
@@ -46,6 +46,13 @@ void Instance::Bind(std::string& val)
   this->ExpectValue = true;
 }
 
+void Instance::Bind(Maybe<std::string>& val)
+{
+  this->CurrentString = &val;
+  this->CurrentList = nullptr;
+  this->ExpectValue = false;
+}
+
 void Instance::Bind(MaybeEmpty<std::vector<std::string>>& val)
 {
   this->CurrentString = nullptr;
diff --git a/Source/cmArgumentParser.h b/Source/cmArgumentParser.h
index 2f0a76d..26148d9 100644
--- a/Source/cmArgumentParser.h
+++ b/Source/cmArgumentParser.h
@@ -39,6 +39,7 @@ public:
 
   void Bind(bool& val);
   void Bind(std::string& val);
+  void Bind(Maybe<std::string>& val);
   void Bind(MaybeEmpty<std::vector<std::string>>& val);
   void Bind(NonEmpty<std::vector<std::string>>& val);
   void Bind(std::vector<std::vector<std::string>>& val);
diff --git a/Source/cmArgumentParserTypes.h b/Source/cmArgumentParserTypes.h
index 7d4ce6f..9afa5c7 100644
--- a/Source/cmArgumentParserTypes.h
+++ b/Source/cmArgumentParserTypes.h
@@ -7,6 +7,11 @@
 namespace ArgumentParser {
 
 template <typename T>
+struct Maybe : public T
+{
+};
+
+template <typename T>
 struct MaybeEmpty : public T
 {
 };
diff --git a/Source/cmFileCommand.cxx b/Source/cmFileCommand.cxx
index 22ab5f7..d2aa63c 100644
--- a/Source/cmFileCommand.cxx
+++ b/Source/cmFileCommand.cxx
@@ -28,7 +28,6 @@
 
 #include "cm_sys_stat.h"
 
-#include "cmAlgorithms.h"
 #include "cmArgumentParser.h"
 #include "cmArgumentParserTypes.h"
 #include "cmCMakePath.h"
@@ -3211,7 +3210,8 @@ bool HandleConfigureCommand(std::vector<std::string> const& args,
     cm::optional<std::string> Content;
     bool EscapeQuotes = false;
     bool AtOnly = false;
-    std::string NewlineStyle;
+    // "NEWLINE_STYLE" requires one value, but we use a custom check below.
+    ArgumentParser::Maybe<std::string> NewlineStyle;
   };
 
   static auto const parser =
@@ -3236,15 +3236,10 @@ bool HandleConfigureCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  // Arguments that are allowed to be empty lists.  Keep entries sorted!
-  static const std::vector<cm::string_view> LIST_ARGS = {
-    "NEWLINE_STYLE"_s, // Filter here so we can issue a custom error below.
-  };
-  auto kwbegin = keywordsMissingValues.cbegin();
-  auto kwend = cmRemoveMatching(keywordsMissingValues, LIST_ARGS);
-  if (kwend != kwbegin) {
-    status.SetError(cmStrCat("CONFIGURE keywords missing values:\n  ",
-                             cmJoin(cmMakeRange(kwbegin, kwend), "\n  ")));
+  if (!keywordsMissingValues.empty()) {
+    status.SetError(
+      cmStrCat("CONFIGURE keywords missing values:\n  ",
+               cmJoin(cmMakeRange(keywordsMissingValues), "\n  ")));
     cmSystemTools::SetFatalErrorOccurred();
     return false;
   }
@@ -3347,7 +3342,10 @@ bool HandleArchiveCreateCommand(std::vector<std::string> const& args,
     std::string Format;
     std::string Compression;
     std::string CompressionLevel;
-    std::string MTime;
+    // "MTIME" should require one value, but it has long been accidentally
+    // accepted without one and treated as if an empty value were given.
+    // Fixing this would require a policy.
+    ArgumentParser::Maybe<std::string> MTime;
     bool Verbose = false;
     // "PATHS" requires at least one value, but use a custom check below.
     ArgumentParser::MaybeEmpty<std::vector<std::string>> Paths;
@@ -3375,18 +3373,10 @@ bool HandleArchiveCreateCommand(std::vector<std::string> const& args,
     return false;
   }
 
-  // Arguments that are allowed to be empty lists.  Keep entries sorted!
-  static const std::vector<cm::string_view> LIST_ARGS = {
-    "MTIME"_s, // "MTIME" should not be in this list because it requires one
-               // value, but it has long been accidentally accepted without
-               // one and treated as if an empty value were given.
-               // Fixing this would require a policy.
-  };
-  auto kwbegin = keywordsMissingValues.cbegin();
-  auto kwend = cmRemoveMatching(keywordsMissingValues, LIST_ARGS);
-  if (kwend != kwbegin) {
-    status.SetError(cmStrCat("Keywords missing values:\n  ",
-                             cmJoin(cmMakeRange(kwbegin, kwend), "\n  ")));
+  if (!keywordsMissingValues.empty()) {
+    status.SetError(
+      cmStrCat("Keywords missing values:\n  ",
+               cmJoin(cmMakeRange(keywordsMissingValues), "\n  ")));
     cmSystemTools::SetFatalErrorOccurred();
     return false;
   }
diff --git a/Tests/CMakeLib/testArgumentParser.cxx b/Tests/CMakeLib/testArgumentParser.cxx
index 85650ed..ecfc5fc 100644
--- a/Tests/CMakeLib/testArgumentParser.cxx
+++ b/Tests/CMakeLib/testArgumentParser.cxx
@@ -23,6 +23,7 @@ struct Result
   std::string String1;
   cm::optional<std::string> String2;
   cm::optional<std::string> String3;
+  ArgumentParser::Maybe<std::string> String4;
 
   ArgumentParser::NonEmpty<std::vector<std::string>> List1;
   ArgumentParser::NonEmpty<std::vector<std::string>> List2;
@@ -44,6 +45,7 @@ std::initializer_list<cm::string_view> const args = {
   "STRING_1",                // string arg missing value
   "STRING_2", "foo", "bar",  // string arg + unparsed value, presence captured
   // "STRING_3",             // string arg that is not present
+  "STRING_4",                // string arg allowed to be missing value
   "LIST_1",                  // list arg missing values
   "LIST_2", "foo", "bar",    // list arg with 2 elems
   "LIST_3", "bar",           // list arg ...
@@ -83,6 +85,7 @@ bool verifyResult(Result const& result,
   ASSERT_TRUE(result.String2);
   ASSERT_TRUE(*result.String2 == "foo");
   ASSERT_TRUE(!result.String3);
+  ASSERT_TRUE(result.String4.empty());
 
   ASSERT_TRUE(result.List1.empty());
   ASSERT_TRUE(result.List2 == foobar);
@@ -122,6 +125,7 @@ bool testArgumentParserDynamic()
     .Bind("STRING_1"_s, result.String1)
     .Bind("STRING_2"_s, result.String2)
     .Bind("STRING_3"_s, result.String3)
+    .Bind("STRING_4"_s, result.String4)
     .Bind("LIST_1"_s, result.List1)
     .Bind("LIST_2"_s, result.List2)
     .Bind("LIST_3"_s, result.List3)
@@ -146,6 +150,7 @@ bool testArgumentParserStatic()
       .Bind("STRING_1"_s, &Result::String1)
       .Bind("STRING_2"_s, &Result::String2)
       .Bind("STRING_3"_s, &Result::String3)
+      .Bind("STRING_4"_s, &Result::String4)
       .Bind("LIST_1"_s, &Result::List1)
       .Bind("LIST_2"_s, &Result::List2)
       .Bind("LIST_3"_s, &Result::List3)