fix: fix Rubocop Metrics/ClassLength offense (refactor Git::Log)

jcouball · jcouball · commit 1aae57a631aa · 2025-07-06T16:00:38.000-07:00
The Git::Log class had grown to over 150 lines, making it difficult
to read and maintain. This refactoring simplifies the internal
implementation to improve its structure and reduce its size to under
100 lines, without altering the public API.

The primary motivations were to enhance readability and make the
class easier to extend in the future.

The key changes include:

- Consolidated all query parameters from individual instance
  variables into a single `@options` hash to simplify state
  management.
- Replaced repetitive builder methods with a concise `set_option`
  private helper, reducing code duplication.
- Centralized the command execution logic into a single
  `run_log_if_dirty` method to ensure consistent behavior.
- Simplified the deprecated Enumerable methods by using a helper
  for warnings and safe navigation (`&amp;.`) for cleaner error handling.
- Modernized the nested `Git::Log::Result` class to use
  `Data.define`, leveraging Ruby 3.2+ features to create a more
  concise and immutable result object.
diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml
@@ -1,12 +1,12 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2025-07-06 20:05:03 UTC using RuboCop version 1.77.0.
+# on 2025-07-06 21:08:14 UTC using RuboCop version 1.77.0.
 # The point is for the user to remove these configuration records
 # one by one as the offenses are removed from the code base.
 # Note that changes in the inspected code, or installation of new
 # versions of RuboCop, may require this file to be generated again.
 
-# Offense count: 3
+# Offense count: 2
 # Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
   Max: 1032
diff --git a/lib/git/log.rb b/lib/git/log.rb
@@ -1,78 +1,34 @@
 # frozen_string_literal: true
 
 module Git
-  # Return the last n commits that match the specified criteria
+  # Builds and executes a `git log` query.
   #
-  # @example The last (default number) of commits
-  #   git = Git.open('.')
-  #   Git::Log.new(git).execute #=> Enumerable of the last 30 commits
+  # This class provides a fluent interface for building complex `git log` queries.
+  # The query is lazily executed when results are requested either via the modern
+  # `#execute` method or the deprecated Enumerable methods.
   #
-  # @example The last n commits
-  #   Git::Log.new(git).max_commits(50).execute #=> Enumerable of last 50 commits
-  #
-  # @example All commits returned by `git log`
-  #   Git::Log.new(git).max_count(:all).execute #=> Enumerable of all commits
-  #
-  # @example All commits that match complex criteria
-  #   Git::Log.new(git)
-  #     .max_count(:all)
-  #     .object('README.md')
-  #     .since('10 years ago')
-  #     .between('v1.0.7', 'HEAD')
-  #     .execute
+  # @example Using the modern `execute` API
+  #   log = git.log.max_count(50).between('v1.0', 'v1.1').author('Scott')
+  #   results = log.execute
+  #   puts "Found #{results.size} commits."
+  #   results.each { |commit| puts commit.sha }
   #
   # @api public
   #
   class Log
     include Enumerable
 
-    # An immutable collection of commits returned by Git::Log#execute
-    #
-    # This object is an Enumerable that contains Git::Object::Commit objects.
-    # It provides methods to access the commit data without executing any
-    # further git commands.
-    #
+    # An immutable, Enumerable collection of `Git::Object::Commit` objects.
+    # Returned by `Git::Log#execute`.
     # @api public
-    class Result
+    Result = Data.define(:commits) do
       include Enumerable
 
-      # @private
-      def initialize(commits)
-        @commits = commits
-      end
-
-      # @return [Integer] the number of commits in the result set
-      def size
-        @commits.size
-      end
-
-      # Iterates over each commit in the result set
-      #
-      # @yield [Git::Object::Commit]
-      def each(&)
-        @commits.each(&)
-      end
-
-      # @return [Git::Object::Commit, nil] the first commit in the result set
-      def first
-        @commits.first
-      end
-
-      # @return [Git::Object::Commit, nil] the last commit in the result set
-      def last
-        @commits.last
-      end
-
-      # @param index [Integer] the index of the commit to return
-      # @return [Git::Object::Commit, nil] the commit at the given index
-      def [](index)
-        @commits[index]
-      end
-
-      # @return [String] a string representation of the log
-      def to_s
-        map(&:to_s).join("\n")
-      end
+      def each(&block) = commits.each(&block)
+      def last = commits.last
+      def [](index) = commits[index]
+      def to_s = map(&:to_s).join("\n")
+      def size = commits.size
     end
 
     # Create a new Git::Log object
@@ -88,12 +44,29 @@ def to_s
     #   Passing max_count to {#initialize} is equivalent to calling {#max_count} on the object.
     #
     def initialize(base, max_count = 30)
-      dirty_log
       @base = base
-      max_count(max_count)
+      @options = {}
+      @dirty = true
+      self.max_count(max_count)
     end
 
-    # Executes the git log command and returns an immutable result object.
+    # Set query options using a fluent interface.
+    # Each method returns `self` to allow for chaining.
+    #
+    def max_count(num)      = set_option(:count, num == :all ? nil : num)
+    def all                 = set_option(:all, true)
+    def object(objectish)   = set_option(:object, objectish)
+    def author(regex)       = set_option(:author, regex)
+    def grep(regex)         = set_option(:grep, regex)
+    def path(path)          = set_option(:path_limiter, path)
+    def skip(num)           = set_option(:skip, num)
+    def since(date)         = set_option(:since, date)
+    def until(date)         = set_option(:until, date)
+    def between(val1, val2 = nil) = set_option(:between, [val1, val2])
+    def cherry              = set_option(:cherry, true)
+    def merges              = set_option(:merges, true)
+
+    # Executes the git log command and returns an immutable result object
     #
     # This is the preferred way to get log data. It separates the query
     # building from the execution, making the API more predictable.
@@ -107,188 +80,64 @@ def initialize(base, max_count = 30)
     #   end
     #
     # @return [Git::Log::Result] an object containing the log results
+    #
     def execute
-      run_log
+      run_log_if_dirty
       Result.new(@commits)
     end
 
-    # The maximum number of commits to return
-    #
-    # @example All commits returned by `git log`
-    #   git = Git.open('.')
-    #   Git::Log.new(git).max_count(:all)
-    #
-    # @param num_or_all [Integer, Symbol, nil] the number of commits to return, or
-    #   `:all` or `nil` to return all
-    #
-    # @return [self]
-    #
-    def max_count(num_or_all)
-      dirty_log
-      @max_count = num_or_all == :all ? nil : num_or_all
-      self
-    end
-
-    # Adds the --all flag to the git log command
-    #
-    # This asks for the logs of all refs (basically all commits reachable by HEAD,
-    # branches, and tags). This does not control the maximum number of commits
-    # returned. To control how many commits are returned, call {#max_count}.
-    #
-    # @example Return the last 50 commits reachable by all refs
-    #   git = Git.open('.')
-    #   Git::Log.new(git).max_count(50).all
-    #
-    # @return [self]
-    #
-    def all
-      dirty_log
-      @all = true
-      self
-    end
-
-    def object(objectish)
-      dirty_log
-      @object = objectish
-      self
-    end
-
-    def author(regex)
-      dirty_log
-      @author = regex
-      self
-    end
-
-    def grep(regex)
-      dirty_log
-      @grep = regex
-      self
-    end
-
-    def path(path)
-      dirty_log
-      @path = path
-      self
-    end
-
-    def skip(num)
-      dirty_log
-      @skip = num
-      self
-    end
-
-    def since(date)
-      dirty_log
-      @since = date
-      self
-    end
-
-    def until(date)
-      dirty_log
-      @until = date
-      self
-    end
-
-    def between(sha1, sha2 = nil)
-      dirty_log
-      @between = [sha1, sha2]
-      self
-    end
-
-    def cherry
-      dirty_log
-      @cherry = true
-      self
-    end
-
-    def merges
-      dirty_log
-      @merges = true
-      self
-    end
-
-    def to_s
-      deprecate_method(__method__)
-      check_log
-      @commits.map(&:to_s).join("\n")
-    end
-
-    # forces git log to run
-
-    def size
-      deprecate_method(__method__)
-      check_log
-      begin
-        @commits.size
-      rescue StandardError
-        nil
-      end
-    end
+    # @!group Deprecated Enumerable Interface
 
+    # @deprecated Use {#execute} and call `each` on the result.
     def each(&)
-      deprecate_method(__method__)
-      check_log
+      deprecate_and_run
       @commits.each(&)
     end
 
-    def first
-      deprecate_method(__method__)
-      check_log
-      begin
-        @commits.first
-      rescue StandardError
-        nil
-      end
+    # @deprecated Use {#execute} and call `size` on the result.
+    def size
+      deprecate_and_run
+      @commits&.size
     end
 
-    def last
-      deprecate_method(__method__)
-      check_log
-      begin
-        @commits.last
-      rescue StandardError
-        nil
-      end
+    # @deprecated Use {#execute} and call `to_s` on the result.
+    def to_s
+      deprecate_and_run
+      @commits&.map(&:to_s)&.join("\n")
     end
 
-    def [](index)
-      deprecate_method(__method__)
-      check_log
-      begin
-        @commits[index]
-      rescue StandardError
-        nil
+    # @deprecated Use {#execute} and call the method on the result.
+    %i[first last []].each do |method_name|
+      define_method(method_name) do |*args|
+        deprecate_and_run
+        @commits&.public_send(method_name, *args)
       end
     end
 
-    private
+    # @!endgroup
 
-    def deprecate_method(method_name)
-      Git::Deprecation.warn(
-        "Calling Git::Log##{method_name} is deprecated and will be removed in a future version. " \
-        "Call #execute and then ##{method_name} on the result object."
-      )
-    end
+    private
 
-    def dirty_log
-      @dirty_flag = true
+    def set_option(key, value)
+      @dirty = true
+      @options[key] = value
+      self
     end
 
-    def check_log
-      return unless @dirty_flag
+    def run_log_if_dirty
+      return unless @dirty
 
-      run_log
-      @dirty_flag = false
+      log_data = @base.lib.full_log_commits(@options)
+      @commits = log_data.map { |c| Git::Object::Commit.new(@base, c['sha'], c) }
+      @dirty = false
     end
 
-    # actually run the 'git log' command
-    def run_log
-      log = @base.lib.full_log_commits(
-        count: @max_count, all: @all, object: @object, path_limiter: @path, since: @since,
-        author: @author, grep: @grep, skip: @skip, until: @until, between: @between,
-        cherry: @cherry, merges: @merges
+    def deprecate_and_run(method = caller_locations(1, 1)[0].label)
+      Git::Deprecation.warn(
+        "Calling Git::Log##{method} is deprecated. " \
+        "Call #execute and then ##{method} on the result object."
       )
-      @commits = log.map { |c| Git::Object::Commit.new(@base, c['sha'], c) }
+      run_log_if_dirty
     end
   end
 end
