Merge specs checking CLI flags and subcommands are documented

We had them duplicated, but with slightly different features:

* The ones in `other/cli_man_pages.rb` enforced a specific structure to
  document CLI options, so were less likely to have false positives.

* The ones in `quality_spec.rb` were able to check subcommands and their
  flags.

This commit merges both and preserves the best of both.

Author: deivid-rodriguez

bundler/spec/other/cli_man_pages_spec.rb

@@ -1,49 +1,72 @@
 # frozen_string_literal: true
 
 RSpec.describe "bundle commands" do
-  it "expects all commands to have a man page" do
-    Bundler::CLI.all_commands.each_key do |command_name|
-      next if command_name == "cli_help"
+  it "expects all commands to have all options and subcommands documented" do
+    check_commands!(Bundler::CLI)
 
-      expect(man_page(command_name)).to exist
+    Bundler::CLI.subcommand_classes.each_value do |klass|
+      check_commands!(klass)
     end
   end
 
-  it "expects all commands to have all options documented" do
-    Bundler::CLI.all_commands.each do |command_name, command|
-      next if command_name == "cli_help"
+  private
+
+  def check_commands!(command_class)
+    command_class.commands.each do |command_name, command|
+      next if command.is_a?(Bundler::Thor::HiddenCommand)
+
+      if command_class == Bundler::CLI
+        man_page = man_page(command_name)
+        expect(man_page).to exist
+
+        check_options!(command, man_page)
+      else
+        man_page = man_page(command.ancestor_name)
+        expect(man_page).to exist
 
-      man_page_content = man_page(command_name).read
+        check_options!(command, man_page)
+        check_subcommand!(command_name, man_page)
+      end
+    end
+  end
 
-      command.options.each do |_, option|
-        aliases = option.aliases
-        formatted_aliases = aliases.sort.map {|name| "`#{name}`" }.join(", ") if aliases
+  def check_options!(command, man_page)
+    command.options.each do |_, option|
+      check_option!(option, man_page)
+    end
+  end
 
-        help = if option.type == :boolean
-          "* #{append_aliases("`#{option.switch_name}`", formatted_aliases)}:"
-        elsif option.enum
-          formatted_aliases = "`#{option.switch_name}`" if aliases.empty? && option.lazy_default
-          "* #{prepend_aliases(option.enum.sort.map {|enum| "`#{option.switch_name}=#{enum}`" }.join(", "), formatted_aliases)}:"
-        else
-          names = [option.switch_name, *aliases]
-          value =
-            case option.type
-            when :array then "<list>"
-            when :numeric then "<number>"
-            else option.name.upcase
-            end
+  def check_option!(option, man_page)
+    man_page_content = man_page.read
 
-          value = option.type != :numeric && option.lazy_default ? "[=#{value}]" : "=#{value}"
+    aliases = option.aliases
+    formatted_aliases = aliases.sort.map {|name| "`#{name}`" }.join(", ") if aliases
 
-          "* #{names.map {|name| "`#{name}#{value}`" }.join(", ")}:"
+    help = if option.type == :boolean
+      "* #{append_aliases("`#{option.switch_name}`", formatted_aliases)}:"
+    elsif option.enum
+      formatted_aliases = "`#{option.switch_name}`" if aliases.empty? && option.lazy_default
+      "* #{prepend_aliases(option.enum.sort.map {|enum| "`#{option.switch_name}=#{enum}`" }.join(", "), formatted_aliases)}:"
+    else
+      names = [option.switch_name, *aliases]
+      value =
+        case option.type
+        when :array then "<list>"
+        when :numeric then "<number>"
+        else option.name.upcase
         end
 
-        expect(man_page_content).to include(help)
-      end
+      value = option.type != :numeric && option.lazy_default ? "[=#{value}]" : "=#{value}"
+
+      "* #{names.map {|name| "`#{name}#{value}`" }.join(", ")}:"
     end
+
+    expect(man_page_content).to include(help)
   end
 
-  private
+  def check_subcommand!(name, man_page)
+    expect(man_page.read).to match(name)
+  end
 
   def append_aliases(text, aliases)
     return text if aliases.empty?
@@ -57,6 +80,10 @@ def prepend_aliases(text, aliases)
     "#{aliases}, #{text}"
   end
 
+  def man_page_content(command_name)
+    man_page(command_name).read
+  end
+
   def man_page(command_name)
     source_root.join("lib/bundler/man/bundle-#{command_name}.1.ronn")
   end

bundler/spec/quality_spec.rb

@@ -251,58 +251,9 @@ def check_for_specific_pronouns(filename)
     expect(lib_code).to eq(spec_code)
   end
 
-  it "documents all cli command options in their associated man pages" do
-    commands = normalize_commands_and_options(Bundler::CLI)
-    cli_and_man_pages_in_sync!(commands)
-
-    Bundler::CLI.subcommand_classes.each do |_, klass|
-      subcommands = normalize_commands_and_options(klass)
-
-      cli_and_man_pages_in_sync!(subcommands)
-    end
-  end
-
   private
 
   def each_line(filename, &block)
     File.readlines(filename, encoding: "UTF-8").each_with_index(&block)
   end
-
-  def normalize_commands_and_options(command_class)
-    commands = {}
-
-    command_class.commands.each do |command_name, command|
-      next if command.is_a?(Bundler::Thor::HiddenCommand)
-
-      key = command.ancestor_name || command_name
-      commands[key] ||= []
-      # Verify that all subcommands are documented in the main command's man page.
-      commands[key] << command_name unless command_class == Bundler::CLI
-
-      command.options.each do |_, option|
-        commands[key] << option.switch_name
-      end
-    end
-
-    commands
-  end
-
-  def cli_and_man_pages_in_sync!(commands)
-    commands.each do |command_name, opts|
-      man_page_path = man_tracked_files.find {|f| File.basename(f) == "bundle-#{command_name}.1.ronn" }
-      expect(man_page_path).to_not be_nil, "The command #{command_name} has no associated man page."
-
-      next if opts.empty?
-
-      man_page_content = File.read(man_page_path)
-      opts.each do |option_name|
-        error_msg = <<~EOM
-          The command #{command_name} has no mention of the option or subcommand `#{option_name}` in its man page.
-          Document the `#{option_name}` in the man page to discard this error.
-        EOM
-
-        expect(man_page_content).to match(option_name), error_msg
-      end
-    end
-  end
 end