Merge pull request #705 from freerange/rubocop-improvements

Rubocop-related improvements

.circleci/config.yml

@@ -53,14 +53,14 @@ jobs:
             - run: MOCHA_GENERATE_DOCS=1 rake yardoc
   lint:
     docker:
-      - image: ruby:2.6
+      - image: ruby:3.3
     steps:
       - checkout
       - run: ruby --version
       - run: gem --version
       - run: bundle --version
-      - run: bundle install --gemfile=Gemfile
-      - run: bundle exec rake lint
+      - run: bundle install --gemfile=gemfiles/Gemfile.rubocop
+      - run: bundle exec --gemfile=gemfiles/Gemfile.rubocop rake lint
 
 workflows:
   build-all:

.rubocop.yml

@@ -1,3 +1,6 @@
+require:
+  - rubocop-rake
+
 inherit_from: .rubocop_todo.yml
 
 inherit_mode:
@@ -13,6 +16,8 @@ AllCops:
   Exclude:
     - '**/Gemfile*.lock'
 
+  NewCops: enable
+
 # Even the reference in the documentation suggests that you should prefer
 # `alias_method` vs `alias`, so I don't understand why that isn't the default.
 Style/Alias:
@@ -60,9 +65,45 @@ Layout/AccessModifierIndentation:
   Enabled: false
 
 # Allow long comment lines, e.g. YARD documentation
-Metrics/LineLength:
-  IgnoredPatterns: ['\A\s*#']
+Layout/LineLength:
+  AllowedPatterns: ['\A\s*#']
 
 # It's not possible to set TargetRubyVersion to Ruby < v2.2
 Gemspec/RequiredRubyVersion:
   Enabled: false
+
+# It can be useful to violate this cop in tests in order to be more explicit
+Lint/UselessTimes:
+  Exclude:
+    - 'test/**/*.rb'
+
+# It can be useful to violate this cop in tests
+Lint/EmptyClass:
+  Exclude:
+    - 'test/**/*.rb'
+
+# It can be useful to violate this cop in tests when testing methods that accept a block
+Lint/EmptyBlock:
+  Exclude:
+    - 'test/**/*.rb'
+
+# There are a mix of styles in the tests and so I don't think it's worth enforcing for now
+Naming/VariableNumber:
+  Exclude:
+    - 'test/**/*.rb'
+
+# These methods from Ruby core are legitimately overridden in the tests
+Style/OptionalBooleanParameter:
+  AllowedMethods:
+    - respond_to?
+    - public_methods
+    - protected_methods
+    - private_methods
+    - public_instance_methods
+    - protected_instance_methods
+    - private_instance_methods
+
+# This cop is useful for required environment variables, but not for optional ones
+Style/FetchEnvVar:
+  AllowedVars:
+    - MOCHA_RUN_INTEGRATION_TESTS

.rubocop_todo.yml

@@ -1,27 +1,39 @@
 # This configuration was generated by
 # `rubocop --auto-gen-config`
-# on 2024-12-22 11:51:08 +0000 using RuboCop version 0.58.2.
+# on 2024-12-24 23:06:22 UTC using RuboCop version 1.69.2.
 # 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: 63
+# Offense count: 34
+# Configuration parameters: AllowedMethods, AllowedPatterns, CountRepeatedAttributes.
 Metrics/AbcSize:
-  Max: 26
+  Max: 27
 
-# Offense count: 29
-# Configuration parameters: CountComments.
+# Offense count: 28
+# Configuration parameters: CountComments, CountAsOne.
 Metrics/ClassLength:
   Max: 381
 
-# Offense count: 200
-# Configuration parameters: CountComments.
+# Offense count: 2
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/CyclomaticComplexity:
+  Max: 11
+
+# Offense count: 199
+# Configuration parameters: CountComments, CountAsOne, AllowedMethods, AllowedPatterns.
 Metrics/MethodLength:
   Max: 27
 
-# Offense count: 699
-# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, IgnoredPatterns.
+# Offense count: 3
+# Configuration parameters: AllowedMethods, AllowedPatterns.
+Metrics/PerceivedComplexity:
+  Max: 13
+
+# Offense count: 68
+# This cop supports safe autocorrection (--autocorrect).
+# Configuration parameters: AllowHeredoc, AllowURI, URISchemes, IgnoreCopDirectives, AllowedPatterns, SplitStrings.
 # URISchemes: http, https
-Metrics/LineLength:
+Layout/LineLength:
   Max: 173

Gemfile

@@ -3,25 +3,8 @@ source 'https://rubygems.org'
 gemspec
 
 gem 'introspection', '~> 0.0.1'
-gem 'jaro_winkler', '>= 1.5.5'
 gem 'minitest'
 gem 'rake'
-gem 'rubocop', '> 0.56', '<= 0.58.2'
-
-# Avoid breaking change in psych v4 (https://bugs.ruby-lang.org/issues/17866)
-if RUBY_VERSION >= '3.1.0'
-  gem 'psych', '< 4'
-end
-
-# Avoid base64 gem warning about it not being available in Ruby v3.4
-if RUBY_VERSION >= '3.3.0'
-  gem 'base64'
-end
-
-# Avoid ostruct gem warning about it not being available in Ruby v3.5
-if RUBY_VERSION >= '3.4.0'
-  gem 'ostruct'
-end
 
 if RUBY_ENGINE == 'jruby'
   # Workaround for https://github.com/jruby/jruby/issues/8488

Rakefile

@@ -6,9 +6,10 @@ require 'bundler/setup'
 
 require 'rake/testtask'
 begin
-  # Only available with default Gemfile
+  # Only available with `gemfiles/Gemfile.rubocop`
   require 'rubocop/rake_task'
-rescue LoadError # rubocop:disable Lint/HandleExceptions
+rescue LoadError
+  warn "Unable to load 'rubocop/rake_task', but continuing anyway" if $DEBUG
 end
 
 desc 'Run all linters and tests'
@@ -81,6 +82,7 @@ namespace 'test' do # rubocop:disable Metrics/BlockLength
   end
 end
 
+desc 'Run linters'
 task 'lint' do
   if defined?(RuboCop::RakeTask)
     RuboCop::RakeTask.new
@@ -90,7 +92,6 @@ task 'lint' do
   end
 end
 
-# rubocop:disable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
 def benchmark_test_case(klass, iterations)
   require 'benchmark'
   require 'mocha/detection/minitest'
@@ -119,8 +120,6 @@ def benchmark_test_case(klass, iterations)
     Benchmark.realtime { iterations.times { Test::Unit::UI::Console::TestRunner.run(klass, @silent_option) } }
   end
 end
-# rubocop:enable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
-
 if ENV['MOCHA_GENERATE_DOCS']
   require 'yard'
 
@@ -134,10 +133,12 @@ if ENV['MOCHA_GENERATE_DOCS']
     task.options = ['--title', "Mocha #{Mocha::VERSION}", '--fail-on-warning']
   end
 
+  desc 'Ensure custom domain remains in place for docs on GitHub Pages'
   task 'checkout_docs_cname' do
     `git checkout docs/CNAME`
   end
 
+  desc 'Ensure custom JavaScript files remain in place for docs on GitHub Pages'
   task 'checkout_docs_js' do
     `git checkout docs/js/app.js`
     `git checkout docs/js/jquery.js`

gemfiles/Gemfile.rubocop

@@ -0,0 +1,7 @@
+source 'https://rubygems.org'
+
+gemspec path: '../'
+
+gem 'rake'
+gem 'rubocop'
+gem 'rubocop-rake'

lib/mocha/api.rb

@@ -37,8 +37,8 @@ module API
 
     # @private
     def self.included(_mod)
-      Object.send(:include, Mocha::ObjectMethods)
-      Class.send(:include, Mocha::ClassMethods)
+      Object.include Mocha::ObjectMethods
+      Class.include Mocha::ClassMethods
     end
 
     # @private

lib/mocha/argument_iterator.rb

@@ -4,13 +4,11 @@ def initialize(argument)
       @argument = argument
     end
 
-    def each
+    def each(&block)
       if @argument.is_a?(Hash)
-        @argument.each do |method_name, return_value|
-          yield method_name, return_value
-        end
+        @argument.each(&block)
       else
-        yield @argument
+        block.call(@argument)
       end
     end
   end

lib/mocha/cardinality.rb

@@ -58,7 +58,6 @@ def used?
       @invocations.any? || maximum.zero?
     end
 
-    # rubocop:disable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
     def anticipated_times
       if allowed_any_number_of_times?
         'allowed any number of times'
@@ -74,7 +73,6 @@ def anticipated_times
         "expected between #{required} and #{count(maximum)}"
       end
     end
-    # rubocop:enable Metrics/CyclomaticComplexity,Metrics/PerceivedComplexity
 
     def invoked_times
       "invoked #{count(@invocations.size)}"

lib/mocha/central.rb

@@ -23,12 +23,14 @@ def initialize
 
     def stub(method)
       return if stubba_methods.detect { |m| m.matches?(method) }
+
       method.stub
       stubba_methods.push(method)
     end
 
     def unstub(method)
       return unless (existing = stubba_methods.detect { |m| m.matches?(method) })
+
       existing.unstub
       stubba_methods.delete(existing)
     end

lib/mocha/class_methods.rb

@@ -10,7 +10,7 @@ def initialize(klass)
         @stubba_object = klass
       end
 
-      def mocha(instantiate = true)
+      def mocha(instantiate: true)
         if instantiate
           @mocha ||= Mocha::Mockery.instance.mock_impersonating_any_instance_of(@stubba_object)
         else
@@ -46,17 +46,16 @@ def any_instance
       if frozen?
         raise StubbingError.new("can't stub method on frozen object: #{mocha_inspect}.any_instance", caller)
       end
+
       @any_instance ||= AnyInstance.new(self)
     end
 
     # @private
-    # rubocop:disable Metrics/CyclomaticComplexity
-    def __method_visibility__(method, include_public_methods = true)
+    def __method_visibility__(method, include_public_methods: true)
       (include_public_methods && public_method_defined?(method) && :public) ||
         (protected_method_defined?(method) && :protected) ||
         (private_method_defined?(method) && :private)
     end
-    # rubocop:enable Metrics/CyclomaticComplexity
     alias_method :__method_exists__?, :__method_visibility__
   end
 end

lib/mocha/configuration.rb

@@ -266,6 +266,7 @@ def display_matching_invocations_on_failure?
     #   # This now fails as expected
     def strict_keyword_argument_matching=(value)
       raise 'Strict keyword argument matching requires Ruby 2.7 and above.' unless Mocha::RUBY_V27_PLUS
+
       @options[:strict_keyword_argument_matching] = value
     end
 
@@ -312,15 +313,15 @@ def change_config(action, new_value, &block)
         if block_given?
           temporarily_change_config action, new_value, &block
         else
-          configuration.send("#{action}=".to_sym, new_value)
+          configuration.send(:"#{action}=", new_value)
         end
       end
 
       # @private
       def temporarily_change_config(action, new_value)
         original_configuration = configuration
         new_configuration = configuration.dup
-        new_configuration.send("#{action}=".to_sym, new_value)
+        new_configuration.send(:"#{action}=", new_value)
         @configuration = new_configuration
         yield
       ensure

lib/mocha/deprecation.rb

@@ -9,6 +9,7 @@ def warning(*messages)
         message = messages.join
         @messages << message
         return if mode == :disabled
+
         filter = BacktraceFilter.new
         location = filter.filtered(caller)[0]
         warn "Mocha deprecation warning at #{location}: #{message}"

lib/mocha/detection/test_unit.rb

@@ -14,7 +14,8 @@ def self.version
         if testcase
           begin
             require 'test/unit/version'
-          rescue LoadError # rubocop:disable Lint/HandleExceptions
+          rescue LoadError
+            warn "Unable to load 'test/unit/version', but continuing anyway" if $DEBUG
           end
           if defined?(::Test::Unit::VERSION)
             version = ::Test::Unit::VERSION

lib/mocha/exception_raiser.rb

@@ -9,6 +9,7 @@ def evaluate(invocation)
       invocation.raised(@exception)
       raise @exception, @exception.to_s if @exception.is_a?(Module) && (@exception < Interrupt)
       raise @exception, @message if @message
+
       raise @exception
     end
   end

lib/mocha/inspect.rb

@@ -11,7 +11,7 @@ def mocha_inspect
     end
 
     module ArrayMethods
-      def mocha_inspect(wrapped = true)
+      def mocha_inspect(wrapped: true)
         unwrapped = collect(&:mocha_inspect).join(', ')
         wrapped ? "[#{unwrapped}]" : unwrapped
       end

lib/mocha/integration/minitest/adapter.rb

@@ -35,6 +35,7 @@ def before_setup
         # @private
         def before_teardown
           return unless passed?
+
           assertion_counter = Integration::AssertionCounter.new(self)
           mocha_verify(assertion_counter)
         ensure

lib/mocha/integration/minitest/exception_translation.rb

@@ -5,6 +5,7 @@ module Integration
     module Minitest
       def self.translate(exception)
         return exception unless exception.is_a?(::Mocha::ExpectationError)
+
         translated_exception = ::Minitest::Assertion.new(exception.message)
         translated_exception.set_backtrace(exception.backtrace)
         translated_exception

lib/mocha/integration/test_unit/adapter.rb

@@ -45,6 +45,7 @@ def mocha_test_name
         # @private
         def handle_mocha_expectation_error(exception)
           return false unless exception.is_a?(Mocha::ExpectationError)
+
           problem_occurred
           add_failure(exception.message, exception.backtrace)
           true

lib/mocha/invocation.rb

@@ -21,6 +21,7 @@ def call(yield_parameters = YieldParameters.new, return_values = ReturnValues.ne
       yield_parameters.next_invocation.each do |yield_args|
         @yields << ParametersMatcher.new(yield_args)
         raise LocalJumpError unless @block
+
         @block.call(*yield_args)
       end
       return_values.next(self)