added checks to `rabbit_misc:get_options/3'.

See comments on top of the new definition (now its arity is 4).
I still need to update the tests.

3 files changed, 175 insertions, 76 deletions

diff --git a/src/rabbit_control.erl b/src/rabbit_control.erl
index af59bb3061..1920a3142c 100644
--- a/src/rabbit_control.erl
+++ b/src/rabbit_control.erl
@@ -26,6 +26,57 @@
 -define(NODE_OPT, "-n").
 -define(VHOST_OPT, "-p").
 
+-define(GLOBAL_OPTS, [?QUIET_OPT, ?NODE_OPT]).
+
+-define(COMMANDS,
+        [stop,
+         stop_app,
+         start_app,
+         wait,
+         reset,
+         force_reset,
+         rotate_logs,
+         
+         cluster,
+         force_cluster,
+         cluster_status,
+         
+         add_user,
+         delete_user,
+         change_password,
+         clear_password,
+         set_user_tags,
+         list_users,
+         
+         add_vhost,
+         delete_vhost,
+         list_vhosts,
+         {set_permissions, [?VHOST_OPT]},
+         {clear_permissions, [?VHOST_OPT]},
+         {list_permissions, [?VHOST_OPT]},
+         {list_user_permissions, [?VHOST_OPT]},
+         
+         set_parameter,
+         clear_parameter,
+         list_parameters,
+         
+         {list_queues, [?VHOST_OPT]},
+         {list_exchanges, [?VHOST_OPT]},
+         {list_bindings, [?VHOST_OPT]},
+         {list_connections, [?VHOST_OPT]},
+         list_channels,
+         {list_consumers, [?VHOST_OPT]},
+         status,
+         environment,
+         report,
+         eval,
+         
+         close_connection,
+         {trace_on, [?VHOST_OPT]},
+         {trace_off, [?VHOST_OPT]},
+         set_vm_memory_high_watermark
+        ]).
+
 -define(GLOBAL_QUERIES,
         [{"Connections", rabbit_networking, connection_info_all,
           connection_info_keys},
@@ -39,12 +90,6 @@
          {"Permissions", rabbit_auth_backend_internal, list_vhost_permissions,
           vhost_perms_info_keys}]).
 
--define(OPTS_COMMANDS,
-        [{?VHOST_OPT, [set_permissions, clear_permissions, list_permissions,
-                       list_user_permissions, list_queues, list_bindings,
-                       list_connections, list_channels, list_consumers,
-                       trace_on, trace_off]}]).
-
 %%----------------------------------------------------------------------------
 
 -ifdef(use_specs).
@@ -63,19 +108,22 @@
 
 start() ->
     {ok, [[NodeStr|_]|_]} = init:get_argument(nodename),
-    {[Command0 | Args], Opts} =
-        case rabbit_misc:get_options([{flag, ?QUIET_OPT},
-                                      {option, ?NODE_OPT, NodeStr},
-                                      {option, ?VHOST_OPT, "/"}],
-                                     init:get_plain_arguments()) of
-            {[], _Opts}    -> usage();
-            CmdArgsAndOpts -> CmdArgsAndOpts
+    {Command, Opts, Args} =
+        case rabbit_misc:get_options(?COMMANDS,
+                                     ?GLOBAL_OPTS,
+                                     [{?QUIET_OPT, flag},
+                                      {?NODE_OPT, {option, NodeStr}},
+                                      {?VHOST_OPT, {option, "/"}}],
+                                     init:get_plain_arguments())
+        of
+            {ok, Res}      -> Res;
+            {invalid, Err} -> rabbit_misc:handle_invalid_arguments(Err),
+                              usage()
         end,
     Opts1 = [case K of
                  ?NODE_OPT -> {?NODE_OPT, rabbit_nodes:make(V)};
                  _         -> {K, V}
              end || {K, V} <- Opts],
-    Command = list_to_atom(Command0),
     Quiet = proplists:get_bool(?QUIET_OPT, Opts1),
     Node = proplists:get_value(?NODE_OPT, Opts1),
     Inform = case Quiet of
@@ -84,20 +132,6 @@ start() ->
                                   io:format(Format ++ " ...~n", Args1)
                           end
              end,
-    PrintInvalidCommandError =
-        fun () ->
-                print_error("invalid command '~s'",
-                            [string:join([atom_to_list(Command) | Args], " ")])
-        end,
-
-    lists:foreach(fun ({Opt, Commands}) ->
-                          case {proplists:lookup(Opt, Opts),
-                                lists:member(Command, Commands)} of
-                              {{_, _}, false} -> PrintInvalidCommandError(),
-                                                 usage();
-                              _               -> ok
-                          end
-                  end, ?OPTS_COMMANDS),
 
     %% The reason we don't use a try/catch here is that rpc:call turns
     %% thrown errors into normal return values
@@ -108,12 +142,6 @@ start() ->
                 false -> io:format("...done.~n")
             end,
             rabbit_misc:quit(0);
-        {'EXIT', {function_clause, [{?MODULE, action, _}    | _]}} -> %% < R15
-            PrintInvalidCommandError(),
-            usage();
-        {'EXIT', {function_clause, [{?MODULE, action, _, _} | _]}} -> %% >= R15
-            PrintInvalidCommandError(),
-            usage();
         {'EXIT', {badarg, _}} ->
             print_error("invalid parameter: ~p", [Args]),
             usage();
@@ -210,7 +238,6 @@ action(force_cluster, Node, ClusterNodeSs, _Opts, Inform) ->
 action(wait, Node, [PidFile], _Opts, Inform) ->
     Inform("Waiting for ~p", [Node]),
     wait_for_application(Node, PidFile, rabbit, Inform);
-
 action(wait, Node, [PidFile, App], _Opts, Inform) ->
     Inform("Waiting for ~p on ~p", [App, Node]),
     wait_for_application(Node, PidFile, list_to_atom(App), Inform);
diff --git a/src/rabbit_misc.erl b/src/rabbit_misc.erl
index 706de8359b..8540bfc6d8 100644
--- a/src/rabbit_misc.erl
+++ b/src/rabbit_misc.erl
@@ -49,7 +49,7 @@
 -export([version_compare/2, version_compare/3]).
 -export([dict_cons/3, orddict_cons/3, gb_trees_cons/3]).
 -export([gb_trees_fold/3, gb_trees_foreach/2]).
--export([get_options/2]).
+-export([get_options/4, handle_invalid_arguments/1]).
 -export([all_module_attributes/1, build_acyclic_graph/3]).
 -export([now_ms/0]).
 -export([const_ok/0, const/1]).
@@ -72,7 +72,9 @@
 -type(ok_or_error() :: rabbit_types:ok_or_error(any())).
 -type(thunk(T) :: fun(() -> T)).
 -type(resource_name() :: binary()).
--type(optdef() :: {flag, string()} | {option, string(), any()}).
+-type(optdef() :: {flag, string()} | {option, string(), string()}).
+-type(invalid_arguments() :: 'command_not_found' |
+                             {'invalid_option', atom(), string()}).
 -type(channel_or_connection_exit()
       :: rabbit_types:channel_exit() | rabbit_types:connection_exit()).
 -type(digraph_label() :: term()).
@@ -182,8 +184,11 @@
 -spec(gb_trees_fold/3 :: (fun ((any(), any(), A) -> A), A, gb_tree()) -> A).
 -spec(gb_trees_foreach/2 ::
         (fun ((any(), any()) -> any()), gb_tree()) -> 'ok').
--spec(get_options/2 :: ([optdef()], [string()])
-                       -> {[string()], [{string(), any()}]}).
+-spec(get_options/4 ::
+        ([{atom(), string()} | atom()], [string()], [optdef()], [string()])
+        -> {'ok', {atom(), [{string(), string()}], [string()]}} |
+           {'invalid', invalid_arguments()}).
+-spec(handle_invalid_arguments/1 :: (invalid_arguments()) -> 'ok').
 -spec(all_module_attributes/1 :: (atom()) -> [{atom(), [term()]}]).
 -spec(build_acyclic_graph/3 ::
         (graph_vertex_fun(), graph_edge_fun(), [{atom(), [term()]}])
@@ -736,23 +741,85 @@ gb_trees_fold1(Fun, Acc, {Key, Val, It}) ->
 gb_trees_foreach(Fun, Tree) ->
     gb_trees_fold(fun (Key, Val, Acc) -> Fun(Key, Val), Acc end, ok, Tree).
 
-%% Separate flags and options from arguments.
-%% get_options([{flag, "-q"}, {option, "-p", "/"}],
-%%             ["set_permissions","-p","/","guest",
-%%              "-q",".*",".*",".*"])
-%% == {["set_permissions","guest",".*",".*",".*"],
-%%     [{"-q",true},{"-p","/"}]}
-get_options(Defs, As) ->
-    lists:foldl(fun(Def, {AsIn, RsIn}) ->
-                        {K, {AsOut, V}} =
-                            case Def of
-                                {flag, Key} ->
-                                    {Key, get_flag(Key, AsIn)};
-                                {option, Key, Default} ->
-                                    {Key, get_option(Key, Default, AsIn)}
-                            end,
-                        {AsOut, [{K, V} | RsIn]}
-                end, {As, []}, Defs).
+%% Takes:
+%%    * A list of [{atom(), [string()]} | atom()], where the atom()s are
+%%      the accepted commands and the optional [string()] is the list of
+%%      accepted options for that command
+%%    * A list [string()] of options valid for all commands
+%%    * A list [optdef()] to determine what is a flag and what is an option
+%%    * The list of arguments given by the user
+%% 
+%% Returns either {ok, {atom(), [{string(), string()}], [string()]} which are
+%% respectively the command, the key-value pairs of the options and the leftover
+%% arguments; or {invalid, Reason} if something went wrong.
+get_options(CommandsOpts0, GlobalOpts, Defs, As0) ->
+    DefsDict = dict:from_list(Defs),
+    CommandsOpts = lists:map(fun ({C, Os}) -> {atom_to_list(C), Os};
+                                 (C)       -> {atom_to_list(C), []}
+                             end, CommandsOpts0),
+    %% For each command, drop all the options/flag that are available in that
+    %% command and see what remains
+    case lists:foldl(fun ({C, Os}, not_found) ->
+                             case drop_opts(DefsDict, GlobalOpts ++ Os, As0) of
+                                 [C | _] -> {found, {C, Os}};
+                                 _       -> not_found
+                             end;
+                         (_, {found, {C, Os}}) ->
+                             {found, {C, Os}}
+                     end, not_found, CommandsOpts)
+    of
+        not_found        -> {invalid, command_not_found};
+        {found, {C, Os}} -> get_options1(C, sets:from_list(GlobalOpts ++ Os),
+                                         Defs, As0 -- [C])
+    end.
+
+drop_opts(Defs0, Opts0, As) ->
+    Opts = sets:from_list(Opts0),
+    Defs = dict:filter(fun (K, _) -> sets:is_element(K, Opts) end, Defs0),
+    drop_opts(Defs, As).
+
+drop_opts(_, [])          -> [];
+drop_opts(Opts, [A | As]) ->
+    case dict:find(A, Opts) of
+        {ok, flag}        -> drop_opts(Opts, As);
+        {ok, {option, _}} -> case As of
+                                 []        -> [];
+                                 [_ | As1] -> drop_opts(Opts, As1)
+                             end;
+        error             -> [A | As]
+    end.
+
+get_options1(Command, Opts, Defs, As) ->
+    %% Check that each flag is OK with this command, and get its value
+    case lists:foldl(fun (_, {invalid_option, Opt}) ->
+                             {invalid_option, Opt};
+                         (Def, {ok, {Accum, As1}}) ->
+                             K = case Def of
+                                     {K0, flag}        -> K0;
+                                     {K0, {option, _}} -> K0
+                                 end,
+                             case {lists:member(K, As1),
+                                   sets:is_element(K, Opts)}
+                             of
+                                 %% We have a flag that shouldn't be there
+                                 {true, false}  -> {invalid_option,
+                                                    list_to_atom(Command), K};
+                                 %% We don't care about this flag
+                                 {false, false} -> {ok, {Accum, As1}};
+                                 %% We need the flag
+                                 {_, true}      -> {As2, V} = get_def(Def, As1),
+                                                   {ok, {[{K, V} | Accum], As2}}
+                             end
+                     end, {ok, {[], As}}, Defs)
+    of
+        {ok, {Ks, Arguments}} ->
+            {ok, {list_to_atom(Command), Ks, Arguments}};
+        {invalid_option, Command, K} ->
+            {invalid, {invalid_option, Command, K}}
+    end.
+    
+get_def({Flag, flag}, As)             -> get_flag(Flag, As);
+get_def({Opt, {option, Default}}, As) -> get_option(Opt, Default, As).
 
 get_option(K, _Default, [K, V | As]) ->
     {As, V};
@@ -770,6 +837,12 @@ get_flag(K, [Nk | As]) ->
 get_flag(_, []) ->
     {[], false}.
 
+handle_invalid_arguments(command_not_found) ->
+    rabbit_misc:format_stderr("could not recognise command~n", []);
+handle_invalid_arguments({invalid_function, Command, Opt}) ->
+    rabbit_misc:format_stderr("invalid option '~s' for command '~s'~n",
+                              [Opt, Command]).
+
 now_ms() ->
     timer:now_diff(now(), {0,0,0}) div 1000.
 
diff --git a/src/rabbit_plugins.erl b/src/rabbit_plugins.erl
index 00880fb2e2..ab2298dad5 100644
--- a/src/rabbit_plugins.erl
+++ b/src/rabbit_plugins.erl
@@ -25,6 +25,13 @@
 -define(ENABLED_OPT, "-E").
 -define(ENABLED_ALL_OPT, "-e").
 
+-define(GLOBAL_OPTS, []).
+
+-define(COMMANDS,
+        [{list, [?VERBOSE_OPT, ?MINIMAL_OPT, ?ENABLED_OPT, ?ENABLED_ALL_OPT]},
+         enable,
+         disable]).
+
 %%----------------------------------------------------------------------------
 
 -ifdef(use_specs).
@@ -45,31 +52,23 @@ start() ->
     {ok, [[PluginsFile|_]|_]} =
         init:get_argument(enabled_plugins_file),
     {ok, [[PluginsDir|_]|_]} = init:get_argument(plugins_dist_dir),
-    {[Command0 | Args], Opts} =
-        case rabbit_misc:get_options([{flag, ?VERBOSE_OPT},
-                                      {flag, ?MINIMAL_OPT},
-                                      {flag, ?ENABLED_OPT},
-                                      {flag, ?ENABLED_ALL_OPT}],
-                                     init:get_plain_arguments()) of
-            {[], _Opts}    -> usage();
-            CmdArgsAndOpts -> CmdArgsAndOpts
-        end,
-    Command = list_to_atom(Command0),
-    PrintInvalidCommandError =
-        fun () ->
-                print_error("invalid command '~s'",
-                            [string:join([atom_to_list(Command) | Args], " ")])
+    {Command, Opts, Args} =
+        case rabbit_misc:get_options(?COMMANDS,
+                                     ?GLOBAL_OPTS,
+                                     [{?VERBOSE_OPT, flag},
+                                      {?MINIMAL_OPT, flag},
+                                      {?ENABLED_OPT, flag},
+                                      {?ENABLED_ALL_OPT, flag}],
+                                     init:get_plain_arguments())
+        of
+            {ok, Res}      -> Res;
+            {invalid, Err} -> rabbit_misc:handle_invalid_arguments(Err),
+                              usage()
         end,
 
     case catch action(Command, Args, Opts, PluginsFile, PluginsDir) of
         ok ->
             rabbit_misc:quit(0);
-        {'EXIT', {function_clause, [{?MODULE, action, _} | _]}} ->
-            PrintInvalidCommandError(),
-            usage();
-        {'EXIT', {function_clause, [{?MODULE, action, _, _} | _]}} ->
-            PrintInvalidCommandError(),
-            usage();
         {error, Reason} ->
             print_error("~p", [Reason]),
             rabbit_misc:quit(2);