Merge #7123

7123: Refactors to `bundle outdated` r=deivid-rodriguez a=ryanfox1985

### What was the end-user problem that led to this PR?

Code in `bundle outdated` code be simpler and more readable.

### What is your fix for the problem, implemented in this PR?

My fix is to refactor the command.

Co-authored-by: Guillermo Guerrero <wolf.fox1985@gmail.com>
Co-authored-by: David Rodríguez <deivid.rodriguez@riseup.net>

2 files changed, 83 insertions, 101 deletions

diff --git a/lib/bundler/cli/outdated.rb b/lib/bundler/cli/outdated.rb
index 2075fe83c8..5c8cd26d88 100644
--- a/lib/bundler/cli/outdated.rb
+++ b/lib/bundler/cli/outdated.rb
@@ -2,17 +2,32 @@
 
 module Bundler
   class CLI::Outdated
-    attr_reader :options, :gems
+    attr_reader :options, :gems, :options_include_groups, :filter_options_patch, :sources, :strict
+    attr_accessor :outdated_gems_by_groups, :outdated_gems_list
 
     def initialize(options, gems)
       @options = options
       @gems = gems
+      @sources = Array(options[:source])
+
+      @filter_options_patch = options.keys &
+        %w[filter-major filter-minor filter-patch]
+
+      @outdated_gems_by_groups = {}
+      @outdated_gems_list = []
+
+      @options_include_groups = [:group, :groups].any? do |v|
+        options.keys.include?(v.to_s)
+      end
+
+      # the patch level options imply strict is also true. It wouldn't make
+      # sense otherwise.
+      @strict = options["filter-strict"] ||
+        Bundler::CLI::Common.patch_level_options(options).any?
     end
 
     def run
-      check_for_deployment_mode
-
-      sources = Array(options[:source])
+      check_for_deployment_mode!
 
       gems.each do |gem_name|
         Bundler::CLI::Common.select_spec(gem_name)
@@ -20,11 +35,9 @@ module Bundler
 
       Bundler.definition.validate_runtime!
       current_specs = Bundler.ui.silence { Bundler.definition.resolve }
-      current_dependencies = {}
-      Bundler.ui.silence do
-        Bundler.load.dependencies.each do |dep|
-          current_dependencies[dep.name] = dep
-        end
+
+      current_dependencies = Bundler.ui.silence do
+        Bundler.load.dependencies.map {|dep| [dep.name, dep] }.to_h
       end
 
       definition = if gems.empty? && sources.empty?
@@ -39,14 +52,6 @@ module Bundler
         options
       )
 
-      # the patch level options imply strict is also true. It wouldn't make
-      # sense otherwise.
-      strict = options["filter-strict"] ||
-        Bundler::CLI::Common.patch_level_options(options).any?
-
-      filter_options_patch = options.keys &
-        %w[filter-major filter-minor filter-patch]
-
       definition_resolution = proc do
         options[:local] ? definition.resolve_with_cache! : definition.resolve_remotely!
       end
@@ -58,8 +63,6 @@ module Bundler
       end
 
       Bundler.ui.info ""
-      outdated_gems_by_groups = {}
-      outdated_gems_list = []
 
       # Loop through the current specs
       gemfile_specs, dependency_specs = current_specs.partition do |spec|
@@ -76,13 +79,11 @@ module Bundler
         next if !gems.empty? && !gems.include?(current_spec.name)
 
         dependency = current_dependencies[current_spec.name]
-        active_spec = retrieve_active_spec(strict, definition, current_spec)
+        active_spec = retrieve_active_spec(definition, current_spec)
 
         next if active_spec.nil?
-        if filter_options_patch.any?
-          update_present = update_present_via_semver_portions(current_spec, active_spec, options)
-          next unless update_present
-        end
+        next if filter_options_patch.any? &&
+          !update_present_via_semver_portions(current_spec, active_spec, options)
 
         gem_outdated = Gem::Version.new(active_spec.version) > Gem::Version.new(current_spec.version)
         next unless gem_outdated || (current_spec.git_version != active_spec.git_version)
@@ -97,31 +98,19 @@ module Bundler
                                 :groups => groups }
 
         outdated_gems_by_groups[groups] ||= []
-        outdated_gems_by_groups[groups] << { :active_spec => active_spec,
-                                             :current_spec => current_spec,
-                                             :dependency => dependency,
-                                             :groups => groups }
+        outdated_gems_by_groups[groups] << outdated_gems_list[-1]
       end
 
       if outdated_gems_list.empty?
-        display_nothing_outdated_message(filter_options_patch)
+        display_nothing_outdated_message
       else
         unless options[:parseable]
-          if options[:pre]
-            Bundler.ui.info "Outdated gems included in the bundle (including " \
-              "pre-releases):"
-          else
-            Bundler.ui.info "Outdated gems included in the bundle:"
-          end
+          Bundler.ui.info(header_outdated_message)
         end
 
-        options_include_groups = [:group, :groups].select do |v|
-          options.keys.include?(v.to_s)
-        end
-
-        if options_include_groups.any?
+        if options_include_groups
           ordered_groups = outdated_gems_by_groups.keys.compact.sort
-          [nil, ordered_groups].flatten.each do |groups|
+          ordered_groups.insert(0, nil).each do |groups|
             gems = outdated_gems_by_groups[groups]
             contains_group = if groups
               groups.split(", ").include?(options[:group])
@@ -132,33 +121,13 @@ module Bundler
             next if (!options[:groups] && !contains_group) || gems.nil?
 
             unless options[:parseable]
-              if groups
-                Bundler.ui.info "===== #{groups_text("Group", groups)} ====="
-              else
-                Bundler.ui.info "===== Without group ====="
-              end
+              Bundler.ui.info(header_group_message(groups))
             end
 
-            gems.each do |gem|
-              print_gem(
-                gem[:current_spec],
-                gem[:active_spec],
-                gem[:dependency],
-                groups,
-                options_include_groups.any?
-              )
-            end
+            print_gems(gems)
           end
         else
-          outdated_gems_list.each do |gem|
-            print_gem(
-              gem[:current_spec],
-              gem[:active_spec],
-              gem[:dependency],
-              gem[:groups],
-              options_include_groups.any?
-            )
-          end
+          print_gems(outdated_gems_list)
         end
 
         exit 1
@@ -171,7 +140,35 @@ module Bundler
       "#{group_text}#{groups.split(",").size > 1 ? "s" : ""} \"#{groups}\""
     end
 
-    def retrieve_active_spec(strict, definition, current_spec)
+    def header_outdated_message
+      if options[:pre]
+        "Outdated gems included in the bundle (including pre-releases):"
+      else
+        "Outdated gems included in the bundle:"
+      end
+    end
+
+    def header_group_message(groups)
+      if groups
+        "===== #{groups_text("Group", groups)} ====="
+      else
+        "===== Without group ====="
+      end
+    end
+
+    def nothing_outdated_message
+      if filter_options_patch.any?
+        display = filter_options_patch.map do |o|
+          o.sub("filter-", "")
+        end.join(" or ")
+
+        "No #{display} updates to display.\n"
+      else
+        "Bundle up to date!\n"
+      end
+    end
+
+    def retrieve_active_spec(definition, current_spec)
       return unless current_spec.match_platform(Bundler.local_platform)
 
       if strict
@@ -187,21 +184,24 @@ module Bundler
       active_spec
     end
 
-    def display_nothing_outdated_message(filter_options_patch)
+    def display_nothing_outdated_message
       unless options[:parseable]
-        if filter_options_patch.any?
-          display = filter_options_patch.map do |o|
-            o.sub("filter-", "")
-          end.join(" or ")
+        Bundler.ui.info(nothing_outdated_message)
+      end
+    end
 
-          Bundler.ui.info "No #{display} updates to display.\n"
-        else
-          Bundler.ui.info "Bundle up to date!\n"
-        end
+    def print_gems(gems_list)
+      gems_list.each do |gem|
+        print_gem(
+          gem[:current_spec],
+          gem[:active_spec],
+          gem[:dependency],
+          gem[:groups],
+        )
       end
     end
 
-    def print_gem(current_spec, active_spec, dependency, groups, options_include_groups)
+    def print_gem(current_spec, active_spec, dependency, groups)
       spec_version = "#{active_spec.version}#{active_spec.git_version}"
       spec_version += " (from #{active_spec.loaded_from})" if Bundler.ui.debug? && active_spec.loaded_from
       current_version = "#{current_spec.version}#{current_spec.git_version}"
@@ -224,7 +224,7 @@ module Bundler
       Bundler.ui.info output_message.rstrip
     end
 
-    def check_for_deployment_mode
+    def check_for_deployment_mode!
       return unless Bundler.frozen_bundle?
       suggested_command = if Bundler.settings.locations("frozen")[:global]
         "bundle config unset frozen"
@@ -266,7 +266,7 @@ module Bundler
 
     def get_version_semver_portion_value(spec, version_portion_index)
       version_section = spec.version.segments[version_portion_index, 1]
-      version_section.nil? ? 0 : (version_section.first || 0)
+      version_section.to_a[0].to_i
     end
   end
 end
diff --git a/spec/commands/outdated_spec.rb b/spec/commands/outdated_spec.rb
index 5a60b564b8..ab54925756 100644
--- a/spec/commands/outdated_spec.rb
+++ b/spec/commands/outdated_spec.rb
@@ -76,7 +76,7 @@ RSpec.describe "bundle outdated" do
   end
 
   describe "with --group option" do
-    def test_group_option(group = nil, gems_list_size = 1)
+    before do
       install_gemfile <<-G
         source "#{file_uri_for(gem_repo2)}"
 
@@ -87,7 +87,9 @@ RSpec.describe "bundle outdated" do
           gem 'activesupport', '2.3.5'
         end
       G
+    end
 
+    def test_group_option(group = nil, gems_list_size = 1)
       update_repo2 do
         build_gem "activesupport", "3.0"
         build_gem "terranova", "9"
@@ -103,17 +105,6 @@ RSpec.describe "bundle outdated" do
     end
 
     it "not outdated gems" do
-      install_gemfile <<-G
-        source "#{file_uri_for(gem_repo2)}"
-
-        gem "weakling", "~> 0.0.1"
-        gem "terranova", '8'
-        group :development, :test do
-          gem 'activesupport', '2.3.5'
-          gem "duradura", '7.0'
-        end
-      G
-
       bundle "outdated --group"
       expect(out).to include("Bundle up to date!")
     end
@@ -153,7 +144,7 @@ RSpec.describe "bundle outdated" do
   end
 
   describe "with --groups option" do
-    it "not outdated gems" do
+    before do
       install_gemfile <<-G
         source "#{file_uri_for(gem_repo2)}"
 
@@ -164,23 +155,14 @@ RSpec.describe "bundle outdated" do
           gem "duradura", '7.0'
         end
       G
+    end
 
+    it "not outdated gems" do
       bundle "outdated --groups"
       expect(out).to include("Bundle up to date!")
     end
 
     it "returns a sorted list of outdated gems by groups" do
-      install_gemfile <<-G
-        source "#{file_uri_for(gem_repo2)}"
-
-        gem "weakling", "~> 0.0.1"
-        gem "terranova", '8'
-        group :development, :test do
-          gem 'activesupport', '2.3.5'
-          gem "duradura", '7.0'
-        end
-      G
-
       update_repo2 do
         build_gem "activesupport", "3.0"
         build_gem "terranova", "9"