Browse Source

Improve support for plugin parameters.

--[name]_opt support depended on the plugin being register, and didn't support
working with just --[name]_out directive (where the plugin is found via the
users PATH. This extends the command line handing to allow --[name]_out to
be all it takes for the _opt directive to also be supported.

Fixes https://github.com/google/protobuf/issues/2712
Thomas Van Lenten 8 years ago
parent
commit
fa925b9893

+ 20 - 2
src/google/protobuf/compiler/command_line_interface.cc

@@ -1031,16 +1031,34 @@ CommandLineInterface::ParseArguments(int argc, const char* const argv[]) {
   }
   }
 
 
   // Make sure each plugin option has a matching plugin output.
   // Make sure each plugin option has a matching plugin output.
+  bool foundUnknownPluginOption = false;
   for (map<string, string>::const_iterator i = plugin_parameters_.begin();
   for (map<string, string>::const_iterator i = plugin_parameters_.begin();
        i != plugin_parameters_.end(); ++i) {
        i != plugin_parameters_.end(); ++i) {
-    if (plugins_.find(i->first) == plugins_.end()) {
+    if (plugins_.find(i->first) != plugins_.end()) {
+      continue;
+    }
+    bool foundImplicitPlugin = false;
+    for (std::vector<OutputDirective>::const_iterator j = output_directives_.cbegin();
+         j != output_directives_.cend(); ++j) {
+      if (j->generator == NULL) {
+        string plugin_name = PluginName(plugin_prefix_ , j->name);
+        if (plugin_name == i->first) {
+          foundImplicitPlugin = true;
+          break;
+        }
+      }
+    }
+    if (!foundImplicitPlugin) {
       std::cerr << "Unknown flag: "
       std::cerr << "Unknown flag: "
                 // strip prefix + "gen-" and add back "_opt"
                 // strip prefix + "gen-" and add back "_opt"
                 << "--" + i->first.substr(plugin_prefix_.size() + 4) + "_opt"
                 << "--" + i->first.substr(plugin_prefix_.size() + 4) + "_opt"
                 << std::endl;
                 << std::endl;
-      return PARSE_ARGUMENT_FAIL;
+      foundUnknownPluginOption = true;
     }
     }
   }
   }
+  if (foundUnknownPluginOption) {
+    return PARSE_ARGUMENT_FAIL;
+  }
 
 
   // If no --proto_path was given, use the current working directory.
   // If no --proto_path was given, use the current working directory.
   if (proto_path_.empty()) {
   if (proto_path_.empty()) {

+ 27 - 1
src/google/protobuf/compiler/command_line_interface_unittest.cc

@@ -689,10 +689,36 @@ TEST_F(CommandLineInterfaceTest, UnrecognizedExtraParameters) {
     "message Foo {}\n");
     "message Foo {}\n");
 
 
   Run("protocol_compiler --plug_out=TestParameter:$tmpdir "
   Run("protocol_compiler --plug_out=TestParameter:$tmpdir "
+      "--unknown_plug_a_opt=Foo "
+      "--unknown_plug_b_opt=Bar "
+      "--proto_path=$tmpdir foo.proto");
+
+  ExpectErrorSubstring("Unknown flag: --unknown_plug_a_opt");
+  ExpectErrorSubstring("Unknown flag: --unknown_plug_b_opt");
+}
+
+TEST_F(CommandLineInterfaceTest, ExtraPluginParametersForOutParameters) {
+  // This doesn't rely on the plugin having been registred and instead that
+  // the existence of --[name]_out is enough to make the --[name]_opt valid.
+  // However, running out of process plugins found via the search path (i.e. -
+  // not pre registered with --plugin) isn't support in this test suite, so we
+  // list the options pre/post the _out directive, and then include _opt that
+  // will be unknown, and confirm the failure output is about the expected
+  // unknown directive, which means the other were accepted.
+  // NOTE: UnrecognizedExtraParameters confirms that if two unknown _opt
+  // directives appear, they both are reported.
+
+  CreateTempFile("foo.proto",
+    "syntax = \"proto2\";\n"
+    "message Foo {}\n");
+
+  Run("protocol_compiler --plug_out=TestParameter:$tmpdir "
+      "--xyz_opt=foo=bar --xyz_out=$tmpdir "
+      "--abc_out=$tmpdir --abc_opt=foo=bar "
       "--unknown_plug_opt=Foo "
       "--unknown_plug_opt=Foo "
       "--proto_path=$tmpdir foo.proto");
       "--proto_path=$tmpdir foo.proto");
 
 
-  ExpectErrorSubstring("Unknown flag: --unknown_plug_opt");
+  ExpectErrorText("Unknown flag: --unknown_plug_opt\n");
 }
 }
 
 
 TEST_F(CommandLineInterfaceTest, Insert) {
 TEST_F(CommandLineInterfaceTest, Insert) {