Make --help, help usage help messages consistent

 * `help` printed an "Error:" at the top for no reason
 * `help [command]` with a non-existent command did not offer a suggestion
   like an attempt to invoke a non-existent command would
 * Exit codes were not consistent
 * `help --list-commands` had line break issues

With this change,

  * `help` is consistent with --help
  * `help [command]` is consistent with `[command] --help`
  * If a command is not found, either during the execution flow
    or `help [commnad]`, we consistently attempt a Jaro distance suggestion
  * Successful or effectively successful exits from the `help`
    command do not produce any error messages at the top

7 files changed, 133 insertions, 78 deletions

diff --git a/deps/rabbitmq_cli/lib/rabbitmq/cli/auto_complete.ex b/deps/rabbitmq_cli/lib/rabbitmq/cli/auto_complete.ex
index fbae95beb4..8a49cce4f1 100644
--- a/deps/rabbitmq_cli/lib/rabbitmq/cli/auto_complete.ex
+++ b/deps/rabbitmq_cli/lib/rabbitmq/cli/auto_complete.ex
@@ -13,9 +13,12 @@
 ## The Initial Developer of the Original Code is GoPivotal, Inc.
 ## Copyright (c) 2007-2020 VMware, Inc. or its affiliates.  All rights reserved.
 
-defmodule Rabbitmq.CLI.AutoComplete do
+defmodule RabbitMQ.CLI.AutoComplete do
   alias RabbitMQ.CLI.Core.{CommandModules, Parser}
 
+  # Use the same jaro distance limit as in Elixir's "did you mean?"
+  @jaro_distance_limit 0.77
+
   @spec complete(String.t(), [String.t()]) :: [String.t()]
   def complete(_, []) do
     []
@@ -31,6 +34,26 @@ defmodule Rabbitmq.CLI.AutoComplete do
     end
   end
 
+  def suggest_command(_cmd_name, empty) when empty == %{} do
+    nil
+  end
+  def suggest_command(typed, module_map) do
+    suggestion =
+      module_map
+      |> Map.keys()
+      |> Enum.map(fn existing ->
+        {existing, String.jaro_distance(existing, typed)}
+      end)
+      |> Enum.max_by(fn {_, distance} -> distance end)
+
+    case suggestion do
+      {cmd, distance} when distance >= @jaro_distance_limit ->
+        {:suggest, cmd}
+      _ ->
+        nil
+    end
+  end
+
   defp complete(tokens) do
     {command, command_name, _, _, _} = Parser.parse(tokens)
     last_token = List.last(tokens)
diff --git a/deps/rabbitmq_cli/lib/rabbitmq/cli/core/parser.ex b/deps/rabbitmq_cli/lib/rabbitmq/cli/core/parser.ex
index 16a5981733..fe91e3da88 100644
--- a/deps/rabbitmq_cli/lib/rabbitmq/cli/core/parser.ex
+++ b/deps/rabbitmq_cli/lib/rabbitmq/cli/core/parser.ex
@@ -17,9 +17,6 @@ defmodule RabbitMQ.CLI.Core.Parser do
   alias RabbitMQ.CLI.{CommandBehaviour, FormatterBehaviour}
   alias RabbitMQ.CLI.Core.{CommandModules, Config}
 
-  # Use the same jaro distance limit as in Elixir `did_you_mean`
-  @jaro_distance_limit 0.77
-
   def default_switches() do
     [
       node: :atom,
@@ -90,6 +87,13 @@ defmodule RabbitMQ.CLI.Core.Parser do
     end
   end
 
+  def command_suggestion(_cmd_name, empty) when empty == %{} do
+    nil
+  end
+  def command_suggestion(typed, module_map) do
+    RabbitMQ.CLI.AutoComplete.suggest_command(typed, module_map)
+  end
+
   defp look_up_command(parsed_args, options) do
     case parsed_args do
       [cmd_name | arguments] ->
@@ -164,27 +168,6 @@ defmodule RabbitMQ.CLI.Core.Parser do
     end
   end
 
-  defp command_suggestion(_cmd_name, empty) when empty == %{} do
-    nil
-  end
-
-  defp command_suggestion(typed, module_map) do
-    suggestion =
-      module_map
-      |> Map.keys()
-      |> Enum.map(fn existing ->
-        {existing, String.jaro_distance(existing, typed)}
-      end)
-      |> Enum.max_by(fn {_, distance} -> distance end)
-
-    case suggestion do
-      {cmd, distance} when distance >= @jaro_distance_limit ->
-        {:suggest, cmd}
-      _ ->
-        nil
-    end
-  end
-
   defp parse_alias(input, command_name, module, alias_content, options) do
     {pre_command_options, tail, invalid} = parse_global_head(input)
     [^command_name | other] = tail
diff --git a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/help_command.ex b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/help_command.ex
index 14c73ad8bb..dd09d4d5ba 100644
--- a/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/help_command.ex
+++ b/deps/rabbitmq_cli/lib/rabbitmq/cli/ctl/commands/help_command.ex
@@ -29,32 +29,52 @@ defmodule RabbitMQ.CLI.Ctl.Commands.HelpCommand do
   def distribution(_), do: :none
   use RabbitMQ.CLI.Core.MergesNoDefaults
 
-  def validate(_, _), do: :ok
+  def validate([], _), do: :ok
+  def validate([_command], _), do: :ok
+  def validate(args, _) when length(args) > 1 do
+    {:validation_failure, :too_many_args}
+  end
 
   def run([command_name | _], opts) do
     CommandModules.load(opts)
 
-    case CommandModules.module_map()[command_name] do
+    module_map = CommandModules.module_map()
+    case module_map[command_name] do
       nil ->
-        all_usage(opts)
+        # command not found
+        # {:error, all_usage(opts)}
+        case RabbitMQ.CLI.AutoComplete.suggest_command(command_name, module_map) do
+          {:suggest, suggested} ->
+            suggest_message = "\nCommand '#{command_name}' not found. \n" <>
+              "Did you mean '#{suggested}'? \n"
+            {:error, ExitCodes.exit_usage(), suggest_message}
+          nil ->
+            {:error, ExitCodes.exit_usage(), "\nCommand '#{command_name}' not found."}
+        end
 
       command ->
-        command_usage(command, opts)
+        {:ok, command_usage(command, opts)}
     end
   end
 
-  def run(_, opts) do
+  def run([], opts) do
     CommandModules.load(opts)
 
     case opts[:list_commands] do
-      true -> commands_description()
-      _ -> all_usage(opts)
+      true ->
+        {:ok, commands_description()}
+      _ ->
+        {:ok, all_usage(opts)}
     end
   end
 
-  def output(result, _) do
-    {:error, ExitCodes.exit_ok(), result}
+  def output({:ok, result}, _) do
+    {:ok, result}
   end
+  def output({:error, result}, _) do
+    {:error, ExitCodes.exit_usage(), result}
+  end
+  use RabbitMQ.CLI.DefaultOutput
 
   def banner(_, _), do: nil
 
@@ -77,12 +97,9 @@ defmodule RabbitMQ.CLI.Ctl.Commands.HelpCommand do
 
   def all_usage(opts) do
     tool_name = program_name(opts)
-    Enum.join(
-      tool_usage(tool_name) ++
-        [Enum.join(["Available commands:"] ++ commands_description(), "\n")] ++
-        help_footer(tool_name),
-      "\n\n"
-    ) <> "\n"
+    tool_usage(tool_name) ++
+        ["\n\nAvailable commands:\n"] ++ commands_description() ++
+        help_footer(tool_name)
   end
 
   def command_usage(command, opts) do
@@ -212,7 +229,7 @@ short            | long          | description
         end
       end)
 
-    module_map
+    lines = module_map
     |> Enum.map(
       fn({name, cmd}) ->
         description = CommandBehaviour.description(cmd)
@@ -223,6 +240,7 @@ short            | long          | description
     |> Enum.sort_by(
       fn({help_section, _}) ->
         case help_section do
+          :deprecated -> 999
           :other -> 100
           {:plugin, _} -> 99
           :help -> 1
@@ -241,16 +259,18 @@ short            | long          | description
     |> Enum.map(
       fn({help_section, section_helps}) ->
         [
-          "\n" <> bright(section_head(help_section)) <> ":\n" |
+          "\n" <> bright(section_head(help_section)) <> ":\n\n" |
           Enum.sort(section_helps)
           |> Enum.map(
             fn({name, {description, _}}) ->
-              "   #{String.pad_trailing(name, pad_commands_to)}  #{description}"
+              "   #{String.pad_trailing(name, pad_commands_to)}  #{description}\n"
             end)
         ]
 
       end)
     |> Enum.concat()
+
+    lines ++ ["\n"]
   end
 
   defp section_head(help_section) do
diff --git a/deps/rabbitmq_cli/lib/rabbitmqctl.ex b/deps/rabbitmq_cli/lib/rabbitmqctl.ex
index b3debac317..576a8e2b99 100644
--- a/deps/rabbitmq_cli/lib/rabbitmqctl.ex
+++ b/deps/rabbitmq_cli/lib/rabbitmqctl.ex
@@ -59,7 +59,7 @@ defmodule RabbitMQCtl do
 
     # the user asked for --help and we are displaying it to her,
     # reporting a success
-    {:ok, ExitCodes.exit_ok(), HelpCommand.all_usage(parsed_options)};
+    {:ok, ExitCodes.exit_ok(), Enum.join(HelpCommand.all_usage(parsed_options), "")};
   end
 
   def exec_command(["--version"] = _unparsed_command, opts) do
@@ -80,7 +80,7 @@ defmodule RabbitMQCtl do
 
         usage_string =
           command_not_found_string <>
-            HelpCommand.all_usage(parsed_options)
+            Enum.join(HelpCommand.all_usage(parsed_options), "")
 
         {:error, ExitCodes.exit_usage(), usage_string}
 
@@ -249,7 +249,7 @@ defmodule RabbitMQCtl do
   end
 
   def auto_complete(script_name, args) do
-    Rabbitmq.CLI.AutoComplete.complete(script_name, args)
+    RabbitMQ.CLI.AutoComplete.complete(script_name, args)
     |> Stream.map(&IO.puts/1)
     |> Stream.run()
 
diff --git a/deps/rabbitmq_cli/test/core/auto_complete_test.exs b/deps/rabbitmq_cli/test/core/auto_complete_test.exs
index e813267410..df13251406 100644
--- a/deps/rabbitmq_cli/test/core/auto_complete_test.exs
+++ b/deps/rabbitmq_cli/test/core/auto_complete_test.exs
@@ -17,7 +17,7 @@
 defmodule AutoCompleteTest do
   use ExUnit.Case, async: false
 
-  @subject Rabbitmq.CLI.AutoComplete
+  @subject RabbitMQ.CLI.AutoComplete
 
 
   test "Auto-completes a command" do
diff --git a/deps/rabbitmq_cli/test/ctl/help_command_test.exs b/deps/rabbitmq_cli/test/ctl/help_command_test.exs
index 79abdae549..baa4b51bc1 100644
--- a/deps/rabbitmq_cli/test/ctl/help_command_test.exs
+++ b/deps/rabbitmq_cli/test/ctl/help_command_test.exs
@@ -18,7 +18,7 @@ defmodule HelpCommandTest do
   use ExUnit.Case, async: false
   import TestHelper
 
-  alias RabbitMQ.CLI.Core.{CommandModules, ExitCodes}
+  alias RabbitMQ.CLI.Core.{CommandModules}
 
   @command RabbitMQ.CLI.Ctl.Commands.HelpCommand
 
@@ -27,8 +27,22 @@ defmodule HelpCommandTest do
     :ok
   end
 
+  test "validate: providing no position arguments passes validation" do
+    assert @command.validate([], %{}) == :ok
+  end
+
+  test "validate: providing one position argument passes validation" do
+    assert @command.validate(["status"], %{}) == :ok
+  end
+
+  test "validate: providing two or more position arguments fails validation" do
+    assert @command.validate(["extra1", "extra2"], %{}) ==
+      {:validation_failure, :too_many_args}
+  end
+
   test "run: prints basic usage info" do
-    output = @command.run([], %{})
+    {:ok, lines} = @command.run([], %{})
+    output = Enum.join(lines, "\n")
     assert output =~ ~r/[-n <node>] [-t <timeout>]/
     assert output =~ ~r/commands/i
   end
@@ -49,9 +63,6 @@ defmodule HelpCommandTest do
   end
 
   test "run prints command info" do
-    assert @command.run([], %{}) =~ ~r/commands/i
-
-    # Checks to verify that each module's command appears in the list.
     ctl_commands = CommandModules.module_map
     |> Enum.filter(fn({_name, command_mod}) ->
                      to_string(command_mod) =~ ~r/^RabbitMQ\.CLI\.Ctl\.Commands/
@@ -61,20 +72,14 @@ defmodule HelpCommandTest do
     Enum.each(
       ctl_commands,
       fn(command) ->
-        assert @command.run([], %{}) =~ ~r/\n\s+#{command}.*\n/
+        {:ok, lines} = @command.run([], %{})
+        output = Enum.join(lines, "\n")
+        assert output =~ ~r/\n\s+#{command}.*\n/
       end)
   end
 
-  test "run: exits with code of OK" do
-    assert @command.output("Help string", %{}) ==
-      {:error, ExitCodes.exit_ok, "Help string"}
-  end
-
-  test "run: no arguments print general help" do
-    assert @command.run([], %{}) =~ ~r/usage/i
-  end
-
-  test "run: unrecognised arguments print general help" do
-    assert @command.run(["extra1", "extra2"], %{}) =~ ~r/usage/i
+  test "run: exits with the code of OK" do
+    assert @command.output({:ok, "Help string"}, %{}) ==
+      {:ok, "Help string"}
   end
 end
diff --git a/deps/rabbitmq_cli/test/rabbitmqctl_test.exs b/deps/rabbitmq_cli/test/rabbitmqctl_test.exs
index 396bd46426..514a7772c8 100644
--- a/deps/rabbitmq_cli/test/rabbitmqctl_test.exs
+++ b/deps/rabbitmq_cli/test/rabbitmqctl_test.exs
@@ -29,16 +29,42 @@ defmodule RabbitMQCtlTest do
     :ok
   end
 
-## ------------------------ --help option -------------------------------------
+  #
+  # --help and `help [command]`
+  #
 
-  test "--help option prints help for command and exits normally" do
+  test "--help option prints help for the command and exits with an OK" do
     command = ["status", "--help"]
     assert capture_io(fn ->
       error_check(command, exit_ok())
     end) =~ ~r/Usage/
   end
 
-## ------------------------ Error Messages ------------------------------------
+  test "bare --help prints general help and exits with an OK" do
+    command = ["--help"]
+    assert capture_io(fn ->
+      error_check(command, exit_ok())
+    end) =~ ~r/Usage/
+  end
+
+  test "help [command] prints help for the command and exits with an OK" do
+    command = ["help", "status"]
+    assert capture_io(fn ->
+      error_check(command, exit_ok())
+    end) =~ ~r/Usage/
+  end
+
+  test "bare help command prints general help and exits with an OK" do
+    command = ["help"]
+    assert capture_io(fn ->
+      error_check(command, exit_ok())
+    end) =~ ~r/Usage/
+  end
+
+  #
+  # Validation and Error Handling
+  #
+
   test "print error message on a bad connection" do
     command = ["status", "-n", "sandwich@pastrami"]
     assert capture_io(:stderr, fn ->
@@ -46,14 +72,14 @@ defmodule RabbitMQCtlTest do
     end) =~ ~r/unable to perform an operation on node 'sandwich@pastrami'/
   end
 
-  test "print timeout message when an RPC call times out" do
+  test "when an RPC call times out, prints a timeout message" do
     command = ["list_users", "-t", "0"]
     assert capture_io(:stderr, fn ->
       error_check(command, exit_tempfail())
     end) =~ ~r/Error: operation list_users on node #{get_rabbit_hostname()} timed out. Timeout value used: 0/
   end
 
-  test "print an authentication error message when auth is refused" do
+  test "when authentication fails, prints an authentication error message" do
     add_user "kirk", "khaaaaaan"
     command = ["authenticate_user", "kirk", "makeitso"]
     assert capture_io(:stderr,
@@ -62,8 +88,6 @@ defmodule RabbitMQCtlTest do
     delete_user "kirk"
   end
 
-## ------------------------ Help and Malformed Commands --------------------------------
-
   test "when invoked without arguments, displays a generic usage message and exits with a non-zero code" do
     command = []
     assert capture_io(:stderr, fn ->
@@ -85,7 +109,7 @@ defmodule RabbitMQCtlTest do
     end) =~ ~r/usage/i
   end
 
-  test "Empty command with options shows usage, and exit with usage exit code" do
+  test "when no command name is provided, displays usage" do
     command = ["-n", "sandwich@pastrami"]
     assert capture_io(:stderr, fn ->
       error_check(command, exit_usage())
@@ -97,14 +121,14 @@ defmodule RabbitMQCtlTest do
     capture_io(:stderr, fn -> error_check(command, exit_ok()) end)
   end
 
-  test "Unimplemented command shows usage message and returns error" do
+  test "a non-existent command results in help message displayed" do
     command = ["not_real"]
     assert capture_io(:stderr, fn ->
       error_check(command, exit_usage())
     end) =~ ~r/Usage/
   end
 
-  test "Extraneous arguments return a usage error" do
+  test "a command that's been provided extra arguments exits with a reasonable error code" do
     command = ["status", "extra"]
     output = capture_io(:stderr, fn ->
       error_check(command, exit_usage())
@@ -114,7 +138,7 @@ defmodule RabbitMQCtlTest do
     assert output =~ ~r/status/
   end
 
-  test "Insufficient arguments return a usage error" do
+  test "a command that's been provided insufficient arguments exits with a reasonable error code" do
     command = ["list_user_permissions"]
     output = capture_io(:stderr, fn ->
       error_check(command, exit_usage())
@@ -124,17 +148,17 @@ defmodule RabbitMQCtlTest do
     assert output =~ ~r/list_user_permissions/
   end
 
-  test "A bad argument returns a data error" do
+  test "a command that's provided an invalid argument exits a reasonable error" do
     command = ["set_disk_free_limit", "2097152bytes"]
     capture_io(:stderr, fn -> error_check(command, exit_dataerr()) end)
   end
 
-  test "An errored command returns an error code" do
+  test "a command that fails with an error exits with a reasonable error code" do
     command = ["delete_user", "voldemort"]
     capture_io(:stderr, fn -> error_check(command, exit_nouser()) end)
   end
 
-  test "A malformed command with an option as the first command-line arg fails gracefully" do
+  test "a mcommand with an unsupported option as the first command-line arg fails gracefully" do
     command1 = ["--invalid=true", "list_permissions", "-p", "/"]
     assert capture_io(:stderr, fn ->
       error_check(command1, exit_usage())