Allow customizing boost::po::command_line_style in NestedCommandLineApp

Summary:
NestedCommandLineApp works by performing two passes over the argument vector:

1. Treat the entire argv as a single command, ignoring subcommands. Consume all options that "match" a global flag here.

2. Find the first positional argument in argv and treat it as a subcommand, then re-parse everything after it as options to the subcommand.

This allows constructs like `cli subcommand --subcommand-flag --global-flag`. However, as a side effect, if `subcommand-flag` happens to be even a prefix of a global flag name, boost "helpfully" matches it with the global flag in the first pass so that the subcommand never even sees it.

Fixing the issue of a subcommand flag conflicting with a global flag would break backwards compatibility; however, we can at least add an option to disable prefix matching so that there would need to be an exact flag name collision for this scenario to occur. It may be possible to extend NestedCommandLineApp in the future to detect flag collisions as well, instead of silently associating arguments with the wrong flag.

Reviewed By: yfeldblum

Differential Revision: D27419341

fbshipit-source-id: 941c0769e16d40c87f96620155f0ab09927cef3d

folly/experimental/NestedCommandLineApp.cpp

@@ -61,7 +61,8 @@ NestedCommandLineApp::NestedCommandLineApp(
       programHelpFooter_(std::move(programHelpFooter)),
       version_(std::move(version)),
       initFunction_(std::move(initFunction)),
-      globalOptions_("Global options") {
+      globalOptions_("Global options"),
+      optionStyle_(po::command_line_style::default_style) {
   addCommand(
       kHelpCommand.str(),
       "[command]",
@@ -117,6 +118,11 @@ void NestedCommandLineApp::addAlias(std::string newName, std::string oldName) {
   aliases_.emplace(std::move(newName), std::move(oldName));
 }
 
+void NestedCommandLineApp::setOptionStyle(
+    boost::program_options::command_line_style::style_t style) {
+  optionStyle_ = style;
+}
+
 void NestedCommandLineApp::displayHelp(
     const po::variables_map& /* globalOptions */,
     const std::vector<std::string>& args) const {
@@ -285,7 +291,7 @@ void NestedCommandLineApp::doRun(const std::vector<std::string>& args) {
     }
   }
 
-  auto parsed = parseNestedCommandLine(cleanArgs, globalOptions_);
+  auto parsed = parseNestedCommandLine(cleanArgs, globalOptions_, optionStyle_);
   po::variables_map vm;
   po::store(parsed.options, vm);
   if (vm.count(kHelpCommand.str())) {
@@ -315,8 +321,10 @@ void NestedCommandLineApp::doRun(const std::vector<std::string>& args) {
   auto& cmd = p.first;
   auto& info = p.second;
 
-  auto cmdOptions =
-      po::command_line_parser(parsed.rest).options(info.options).run();
+  auto cmdOptions = po::command_line_parser(parsed.rest)
+                        .options(info.options)
+                        .style(optionStyle_)
+                        .run();
 
   po::store(cmdOptions, vm);
   po::notify(vm);

folly/experimental/NestedCommandLineApp.h

@@ -46,6 +46,17 @@ class FOLLY_EXPORT ProgramExit : public std::runtime_error {
  * App that uses a nested command line, of the form:
  *
  * program [--global_options...] command [--command_options...] command_args...
+ *
+ * Note: Global options (including GFlags, if added using addGFlags()) are
+ * recognized anywhere in the command line, and are prefix matched with higher
+ * priority than command options. For example, a global option named "--foobar"
+ * would be matched over a command option named "--foo", even if you specify
+ * "--foo" on the command line. You can disable prefix matching with:
+ *
+ * int style = boost::program_options::command_line_style::default_style;
+ * style &= ~boost::program_options::command_line_style::allow_guessing;
+ * app.setOptionStyle(
+ *   static_cast<boost::program_options::command_line_style::style_t>(style));
  */
 class NestedCommandLineApp {
  public:
@@ -120,6 +131,12 @@ class NestedCommandLineApp {
    */
   void addAlias(std::string newName, std::string oldName);
 
+  /**
+   * Sets the style in which options will be accepted by the parser.
+   */
+  void setOptionStyle(
+      boost::program_options::command_line_style::style_t style);
+
   /**
    * Run the command and return; the return code is 0 on success or
    * non-zero on error, so it is idiomatic to call this at the end of main():
@@ -165,6 +182,7 @@ class NestedCommandLineApp {
   std::string version_;
   InitFunction initFunction_;
   boost::program_options::options_description globalOptions_;
+  boost::program_options::command_line_style::style_t optionStyle_;
   std::map<std::string, CommandInfo> commands_;
   std::map<std::string, std::string> aliases_;
   std::set<folly::StringPiece> builtinCommands_;

folly/experimental/ProgramOptions.cpp

@@ -285,10 +285,12 @@ po::options_description getGFlags(ProgramOptionsStyle style) {
 namespace {
 
 NestedCommandLineParseResult doParseNestedCommandLine(
-    po::command_line_parser&& parser, const po::options_description& desc) {
+    po::command_line_parser&& parser,
+    const po::options_description& desc,
+    boost::program_options::command_line_style::style_t style) {
   NestedCommandLineParseResult result;
 
-  result.options = parser.options(desc).allow_unregistered().run();
+  result.options = parser.options(desc).style(style).allow_unregistered().run();
 
   bool setCommand = true;
   for (auto& opt : result.options.options) {
@@ -319,14 +321,20 @@ NestedCommandLineParseResult doParseNestedCommandLine(
 } // namespace
 
 NestedCommandLineParseResult parseNestedCommandLine(
-    int argc, const char* const argv[], const po::options_description& desc) {
-  return doParseNestedCommandLine(po::command_line_parser(argc, argv), desc);
+    int argc,
+    const char* const argv[],
+    const po::options_description& desc,
+    boost::program_options::command_line_style::style_t style) {
+  return doParseNestedCommandLine(
+      po::command_line_parser(argc, argv), desc, style);
 }
 
 NestedCommandLineParseResult parseNestedCommandLine(
     const std::vector<std::string>& cmdline,
-    const po::options_description& desc) {
-  return doParseNestedCommandLine(po::command_line_parser(cmdline), desc);
+    const po::options_description& desc,
+    boost::program_options::command_line_style::style_t style) {
+  return doParseNestedCommandLine(
+      po::command_line_parser(cmdline), desc, style);
 }
 
 } // namespace folly

folly/experimental/ProgramOptions.h

@@ -77,10 +77,14 @@ struct NestedCommandLineParseResult {
 NestedCommandLineParseResult parseNestedCommandLine(
     int argc,
     const char* const argv[],
-    const boost::program_options::options_description& desc);
+    const boost::program_options::options_description& desc,
+    boost::program_options::command_line_style::style_t style =
+        boost::program_options::command_line_style::default_style);
 
 NestedCommandLineParseResult parseNestedCommandLine(
     const std::vector<std::string>& cmdline,
-    const boost::program_options::options_description& desc);
+    const boost::program_options::options_description& desc,
+    boost::program_options::command_line_style::style_t style =
+        boost::program_options::command_line_style::default_style);
 
 } // namespace folly

folly/experimental/test/NestedCommandLineAppTest.cpp

@@ -75,10 +75,10 @@ TEST(ProgramOptionsTest, Errors) {
 TEST(ProgramOptionsTest, Help) {
   // Not actually checking help output, just verifying that help doesn't fail
   callHelper({"--version"});
-  callHelper({"--h"});
-  callHelper({"--h", "foo"});
-  callHelper({"--h", "bar"});
-  callHelper({"--h", "--", "bar"});
+  callHelper({"-h"});
+  callHelper({"-h", "foo"});
+  callHelper({"-h", "bar"});
+  callHelper({"-h", "--", "bar"});
   callHelper({"--help"});
   callHelper({"--help", "foo"});
   callHelper({"--help", "bar"});
@@ -108,6 +108,8 @@ TEST(ProgramOptionsTest, CutArguments) {
       "running foo\n"
       "foo global-foo 43\n"
       "foo local-foo 42\n"
+      "foo conflict-global 42\n"
+      "foo conflict 42\n"
       "foo arg b\n"
       "foo arg --local-foo\n"
       "foo arg 44\n"
@@ -120,13 +122,17 @@ TEST(ProgramOptionsTest, Success) {
   EXPECT_EQ(
       "running foo\n"
       "foo global-foo 42\n"
-      "foo local-foo 42\n",
+      "foo local-foo 42\n"
+      "foo conflict-global 42\n"
+      "foo conflict 42\n",
       callHelper({"foo"}));
 
   EXPECT_EQ(
       "running foo\n"
       "foo global-foo 43\n"
       "foo local-foo 44\n"
+      "foo conflict-global 42\n"
+      "foo conflict 42\n"
       "foo arg a\n"
       "foo arg b\n",
       callHelper({"--global-foo", "43", "foo", "--local-foo", "44", "a", "b"}));
@@ -136,6 +142,8 @@ TEST(ProgramOptionsTest, Success) {
       "running foo\n"
       "foo global-foo 43\n"
       "foo local-foo 44\n"
+      "foo conflict-global 42\n"
+      "foo conflict 42\n"
       "foo arg a\n"
       "foo arg b\n",
       callHelper({"foo", "--global-foo", "43", "--local-foo", "44", "a", "b"}));
@@ -146,6 +154,8 @@ TEST(ProgramOptionsTest, Aliases) {
       "running foo\n"
       "foo global-foo 43\n"
       "foo local-foo 44\n"
+      "foo conflict-global 42\n"
+      "foo conflict 42\n"
       "foo arg a\n"
       "foo arg b\n",
       callHelper({"--global-foo", "43", "bar", "--local-foo", "44", "a", "b"}));
@@ -160,5 +170,33 @@ TEST(ProgramOptionsTest, BuiltinCommand) {
       NestedCommandLineApp::kHelpCommand.str() + "nonsense"));
 }
 
+TEST(ProgramOptionsTest, ConflictingFlags) {
+  EXPECT_EQ(
+      "running foo\n"
+      "foo global-foo 42\n"
+      "foo local-foo 42\n"
+      "foo conflict-global 43\n"
+      "foo conflict 42\n",
+      callHelper({"--conflict-global", "43", "foo"}));
+
+  EXPECT_EQ(
+      "running foo\n"
+      "foo global-foo 42\n"
+      "foo local-foo 42\n"
+      "foo conflict-global 43\n"
+      "foo conflict 42\n",
+      callHelper({"foo", "--conflict-global", "43"}));
+
+  EXPECT_EQ(
+      "running foo\n"
+      "foo global-foo 42\n"
+      "foo local-foo 42\n"
+      "foo conflict-global 42\n"
+      "foo conflict 43\n",
+      callHelper({"foo", "--conflict", "43"}));
+
+  callHelper({"--conflict", "43", "foo"}, 1);
+}
+
 } // namespace test
 } // namespace folly

folly/experimental/test/NestedCommandLineAppTestHelper.cpp

@@ -18,6 +18,7 @@
 #include <folly/portability/GFlags.h>
 
 DEFINE_int32(global_foo, 42, "Global foo");
+DEFINE_int32(conflict_global, 42, "Global conflict");
 
 namespace po = ::boost::program_options;
 
@@ -34,6 +35,8 @@ void foo(
     const po::variables_map& options, const std::vector<std::string>& args) {
   printf("foo global-foo %d\n", options["global-foo"].as<int32_t>());
   printf("foo local-foo %d\n", options["local-foo"].as<int32_t>());
+  printf("foo conflict-global %d\n", options["conflict-global"].as<int32_t>());
+  printf("foo conflict %d\n", options["conflict"].as<int32_t>());
   for (auto& arg : args) {
     printf("foo arg %s\n", arg.c_str());
   }
@@ -43,11 +46,17 @@ void foo(
 
 int main(int argc, char* argv[]) {
   folly::NestedCommandLineApp app("", "0.1", "", "", init);
+
+  int style = po::command_line_style::default_style;
+  style &= ~po::command_line_style::allow_guessing;
+  app.setOptionStyle(static_cast<po::command_line_style::style_t>(style));
+
   app.addGFlags();
   // clang-format off
   app.addCommand("foo", "[args...]", "Do some foo", "Does foo", foo)
     .add_options()
-      ("local-foo", po::value<int32_t>()->default_value(42), "Local foo");
+      ("local-foo", po::value<int32_t>()->default_value(42), "Local foo")
+      ("conflict", po::value<int32_t>()->default_value(42), "Local conflict");
   // clang-format on
   app.addAlias("bar", "foo");
   app.addAlias("baz", "bar");