Script a rubocop corrector using a data file

We generate the data file using ruby-warnings

Adds rubocop:

- Only match if method name matches, not just line number
- Match only file path from root so that files from other machines work

Adds autocorrection

- Add autocorrection by inserting ** before.
- Special correction case for literal hash args

Mark cop as unsafe as relies on data source

Fix correction from splat type to double splat

.gitlab/ci/rails.gitlab-ci.yml

@@ -21,6 +21,7 @@
     RUBY_GC_MALLOC_LIMIT: 67108864
     RUBY_GC_MALLOC_LIMIT_MAX: 134217728
     CRYSTALBALL: "true"
+    RECORD_KEYWORD_WARNINGS: "true"
   needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets"]
   script:
     - *base-script
@@ -37,6 +38,7 @@
       - tmp/capybara/
       - tmp/memory_test/
       - tmp/feature_flags/
+      - tmp/keyword_warn.txt
       - log/*.log
     reports:
       junit: junit_rspec.xml

.rubocop.yml

@@ -47,6 +47,10 @@ Cop/StaticTranslationDefinition:
     - 'spec/**/*'
     - 'ee/spec/**/*'
 
+Lint/LastKeywordArgument:
+  Enabled: true
+  Safe: false
+
 # This cop checks whether some constant value isn't a
 # mutable literal (e.g. array or hash).
 Style/MutableConstant:

Gemfile

@@ -351,6 +351,7 @@ group :development do
 end
 
 group :development, :test do
+  gem 'warning', '~> 1.1', require: false
   gem 'bullet', '~> 6.1.0'
   gem 'pry-byebug', '~> 3.9.0', platform: :mri
   gem 'pry-rails', '~> 0.3.9'

Gemfile.lock

@@ -1222,6 +1222,7 @@ GEM
     vmstat (2.3.0)
     warden (1.2.8)
       rack (>= 2.0.6)
+    warning (1.1.0)
     webauthn (2.3.0)
       android_key_attestation (~> 0.3.0)
       awrence (~> 1.1)
@@ -1518,6 +1519,7 @@ DEPENDENCIES
   validates_hostname (~> 1.0.10)
   version_sorter (~> 2.2.4)
   vmstat (~> 2.3.0)
+  warning (~> 1.1)
   webauthn (~> 2.3)
   webmock (~> 3.9.1)
   wikicloth (= 0.8.1)

rubocop/cop/lint/last_keyword_argument.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Lint
+      class LastKeywordArgument < Cop
+        MSG = 'Using the last argument as keyword parameters is deprecated'.freeze
+
+        def on_send(node)
+          arg = node.arguments.last
+          return unless arg
+
+          return unless known_match?(processed_source.file_path, node.first_line, node.method_name.to_s)
+
+          return if arg.children.first.respond_to?(:kwsplat_type?) && arg.children.first&.kwsplat_type?
+
+          # parser thinks `a: :b, c: :d` is hash type, it's actually kwargs
+          return if arg.hash_type? && !arg.source.match(/\A{/)
+
+          add_offense(arg)
+        end
+
+        def autocorrect(arg)
+          lambda do |corrector|
+            if arg.hash_type?
+              kwarg = arg.source.sub(/\A{\s*/, '').sub(/\s*}\z/, '')
+              corrector.replace(arg, kwarg)
+            elsif arg.splat_type?
+              corrector.insert_before(arg, '*')
+            else
+              corrector.insert_before(arg, '**')
+            end
+          end
+        end
+
+        private
+
+        def known_match?(file_path, line_number, method_name)
+          file_path_from_root = file_path.sub(File.expand_path('../../..', __dir__), '')
+
+          self.class.keyword_warnings.any? do |warning|
+            warning.include?("#{file_path_from_root}:#{line_number}") && warning.include?("called method `#{method_name}'")
+          end
+        end
+
+        def self.keyword_warnings
+          @keyword_warnings ||= keywords_list
+        end
+
+        def self.keywords_list
+          return [] unless File.exist?(keywords_file_path)
+
+          File.read(keywords_file_path).split("----\n")
+        end
+
+        def self.keywords_file_path
+          File.expand_path('../../../tmp/keyword_warn.txt', __dir__)
+        end
+      end
+    end
+  end
+end

spec/rubocop/cop/lint/last_keyword_argument_spec.rb

+# frozen_string_literal: true
+
+require 'fast_spec_helper'
+require 'rubocop'
+require_relative '../../../../rubocop/cop/lint/last_keyword_argument'
+
+RSpec.describe RuboCop::Cop::Lint::LastKeywordArgument, type: :rubocop do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  before do
+    described_class.instance_variable_set(:@keyword_warnings, nil)
+  end
+
+  context 'file does not exist' do
+    before do
+      allow(File).to receive(:exist?).and_return(false)
+    end
+
+    it 'does not register an offense' do
+      expect_no_offenses(<<~SOURCE)
+        users.call(params)
+      SOURCE
+    end
+  end
+
+  context 'file does exist' do
+    before do
+      allow(File).to receive(:exist?).and_return(true)
+
+      allow(File).to receive(:read).and_return(<<~DATA)
+----
+create_service.rb:1: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
+user.rb:17: warning: The called method `call' is defined here
+----
+/Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/batch-loader-1.4.0/lib/batch_loader/graphql.rb:38: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
+/Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/batch-loader-1.4.0/lib/batch_loader.rb:26: warning: The called method `batch' is defined here
+      DATA
+    end
+
+    it 'registers an offense' do
+      expect_offense(<<~SOURCE, 'create_service.rb')
+        users.call(params)
+                   ^^^^^^ Using the last argument as keyword parameters is deprecated
+      SOURCE
+
+      expect_correction(<<~SOURCE)
+        users.call(**params)
+      SOURCE
+    end
+
+    it 'registers an offense and corrects by converting hash to kwarg' do
+      expect_offense(<<~SOURCE, 'create_service.rb')
+        users.call(id, { a: :b, c: :d })
+                       ^^^^^^^^^^^^^^^^ Using the last argument as keyword parameters is deprecated
+      SOURCE
+
+      expect_correction(<<~SOURCE)
+        users.call(id, a: :b, c: :d)
+      SOURCE
+    end
+
+    it 'registers an offense and corrects by converting splat to double splat' do
+      expect_offense(<<~SOURCE, 'create_service.rb')
+        users.call(id, *params)
+                       ^^^^^^^ Using the last argument as keyword parameters is deprecated
+      SOURCE
+
+      expect_correction(<<~SOURCE)
+        users.call(id, **params)
+      SOURCE
+    end
+
+    it 'does not register an offense if already a kwarg', :aggregate_failures do
+      expect_no_offenses(<<~SOURCE, 'create_service.rb')
+        users.call(**params)
+      SOURCE
+
+      expect_no_offenses(<<~SOURCE, 'create_service.rb')
+        users.call(id, a: :b, c: :d)
+      SOURCE
+    end
+
+    it 'does not register an offense if the method name does not match' do
+      expect_no_offenses(<<~SOURCE, 'create_service.rb')
+        users.process(params)
+      SOURCE
+    end
+
+    it 'does not register an offense if the line number does not match' do
+      expect_no_offenses(<<~SOURCE, 'create_service.rb')
+        users.process
+        users.call(params)
+      SOURCE
+    end
+
+    it 'does not register an offense if the filename does not match' do
+      expect_no_offenses(<<~SOURCE, 'update_service.rb')
+        users.call(params)
+      SOURCE
+    end
+  end
+end

spec/ruby_keyword_warning.rb

+# frozen_string_literal: true
+
+if ENV['RECORD_KEYWORD_WARNINGS']
+  require 'warning'
+
+  Warning[:deprecated] = true
+
+  # The warnings are emitted with two calls of Warning.warn.
+  # In an attempt to group these two calls we use the `----` separator.
+
+  keyword_regex = /: warning: (?:Using the last argument (?:for `.+' )?as keyword parameters is deprecated; maybe \*\* should be added to the call)\n\z/
+  method_called_regex = /: warning: (?:The called method (?:`.+' )?is defined here)\n\z/
+  actions = {
+    keyword_regex => proc do |warning|
+      File.open(File.expand_path('../tmp/keyword_warn.txt', __dir__), "a") do |file|
+        file.write("----\n")
+        file.write(warning)
+
+        # keep ruby behaviour of warning in stderr
+        $stderr.puts(warning) # rubocop:disable Style/StderrPuts
+      end
+    end,
+    method_called_regex => proc do |warning|
+      File.open(File.expand_path('../tmp/keyword_warn.txt', __dir__), "a") do |file|
+        file.write(warning)
+
+        # keep ruby behaviour of warning in stderr
+        $stderr.puts(warning) # rubocop:disable Style/StderrPuts
+      end
+    end
+  }
+  Warning.process('', actions)
+end

spec/spec_helper.rb

@@ -8,6 +8,8 @@ if $".include?(File.expand_path('fast_spec_helper.rb', __dir__))
   abort 'Aborting...'
 end
 
+require './spec/ruby_keyword_warning'
+
 require './spec/simplecov_env'
 SimpleCovEnv.start!