Merge branch 'keyword_scripter' into 'master'

Script to generate file for Rubocop to autocorrect keyword warnings

See merge request gitlab-org/gitlab!47720

.gitignore

@@ -77,6 +77,7 @@ eslint-report.html
 /.gitlab_kas_secret
 /webpack-report/
 /crystalball/
+/deprecations/
 /knapsack/
 /rspec_flaky/
 /locale/**/LC_MESSAGES

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

@@ -21,6 +21,7 @@
     RUBY_GC_MALLOC_LIMIT: 67108864
     RUBY_GC_MALLOC_LIMIT_MAX: 134217728
     CRYSTALBALL: "true"
+    RECORD_DEPRECATIONS: "true"
   needs: ["setup-test-env", "retrieve-tests-metadata", "compile-test-assets"]
   script:
     - *base-script
@@ -31,6 +32,7 @@
     paths:
       - coverage/
       - crystalball/
+      - deprecations/
       - knapsack/
       - rspec_flaky/
       - rspec_profiling/

.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 'deprecation_toolkit', '~> 1.5.1', require: false
   gem 'bullet', '~> 6.1.0'
   gem 'pry-byebug', '~> 3.9.0', platform: :mri
   gem 'pry-rails', '~> 0.3.9'

Gemfile.lock

@@ -224,6 +224,8 @@ GEM
     declarative-option (0.1.0)
     default_value_for (3.3.0)
       activerecord (>= 3.2.0, < 6.1)
+    deprecation_toolkit (1.5.1)
+      activesupport (>= 4.2)
     derailed_benchmarks (1.7.0)
       benchmark-ips (~> 2)
       get_process_mem (~> 0)
@@ -1302,6 +1304,7 @@ DEPENDENCIES
   database_cleaner (~> 1.7.0)
   deckar01-task_list (= 2.3.1)
   default_value_for (~> 3.3.0)
+  deprecation_toolkit (~> 1.5.1)
   derailed_benchmarks
   device_detector
   devise (~> 4.7.2)

rubocop/cop/lint/last_keyword_argument.rb

+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Lint
+      # This cop only works if there are files from deprecation_toolkit. You can
+      # generate these files by:
+      #
+      # 1. Running specs with RECORD_DEPRECATIONS=1
+      # 1. Downloading the complete set of deprecations/ files from a CI
+      # pipeline (see https://gitlab.com/gitlab-org/gitlab/-/merge_requests/47720)
+      class LastKeywordArgument < Cop
+        MSG = 'Using the last argument as keyword parameters is deprecated'.freeze
+
+        DEPRECATIONS_GLOB = File.expand_path('../../../deprecations/**/*.yml', __dir__)
+        KEYWORD_DEPRECATION_STR = 'maybe ** should be added to the call'
+
+        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
+          hash = Dir.glob(DEPRECATIONS_GLOB).each_with_object({}) do |file, hash|
+            hash.merge!(YAML.safe_load(File.read(file)))
+          end
+
+          hash.values.flatten.select { |str| str.include?(KEYWORD_DEPRECATION_STR) }.uniq
+        end
+      end
+    end
+  end
+end

spec/deprecation_toolkit_env.rb

+# frozen_string_literal: true
+
+if ENV.key?('RECORD_DEPRECATIONS')
+  require 'deprecation_toolkit'
+  require 'deprecation_toolkit/rspec'
+  DeprecationToolkit::Configuration.test_runner = :rspec
+  DeprecationToolkit::Configuration.deprecation_path = 'deprecations'
+  DeprecationToolkit::Configuration.behavior = DeprecationToolkit::Behaviors::Record
+
+  # Enable ruby deprecations for keywords, it's suppressed by default in Ruby 2.7.2
+  Warning[:deprecated] = true
+
+  kwargs_warnings = [
+    # Taken from https://github.com/jeremyevans/ruby-warning/blob/1.1.0/lib/warning.rb#L18
+    %r{warning: (?:Using the last argument (?:for `.+' )?as keyword parameters is deprecated; maybe \*\* should be added to the call|Passing the keyword argument (?:for `.+' )?as the last hash parameter is deprecated|Splitting the last argument (?:for `.+' )?into positional and keyword parameters is deprecated|The called method (?:`.+' )?is defined here)\n\z}
+  ]
+  DeprecationToolkit::Configuration.warnings_treated_as_deprecation = kwargs_warnings
+end

spec/graphql/mutations/boards/lists/create_spec.rb

@@ -23,12 +23,12 @@ RSpec.describe Mutations::Boards::Lists::Create do
 
   describe '#ready?' do
     it 'raises an error if required arguments are missing' do
-      expect { mutation.ready?({ board_id: 'some id' }) }
+      expect { mutation.ready?(board_id: 'some id') }
         .to raise_error(Gitlab::Graphql::Errors::ArgumentError, /one and only one of/)
     end
 
     it 'raises an error if too many required arguments are specified' do
-      expect { mutation.ready?({ board_id: 'some id', backlog: true, label_id: 'some label' }) }
+      expect { mutation.ready?(board_id: 'some id', backlog: true, label_id: 'some label') }
         .to raise_error(Gitlab::Graphql::Errors::ArgumentError, /one and only one of/)
     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 'deprecation files does not exist' do
+    before do
+      allow(Dir).to receive(:glob).and_return([])
+      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 'deprecation files does exist' do
+    let(:create_spec_yaml) do
+      <<~YAML
+      ---
+      test_mutations/boards/lists/create#resolve_with_proper_permissions_backlog_list_creates_one_and_only_one_backlog:
+      - |
+        DEPRECATION WARNING: /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
+      test_mutations/boards/lists/create#ready?_raises_an_error_if_required_arguments_are_missing:
+      - |
+        DEPRECATION WARNING: /Users/tkuah/code/ee-gdk/gitlab/create_service.rb:1: warning: Using the last argument as keyword parameters is deprecated; maybe ** should be added to the call
+        /Users/tkuah/code/ee-gdk/gitlab/user.rb:17: warning: The called method `call' is defined here
+      YAML
+    end
+
+    let(:projects_spec_yaml) do
+      <<~YAML
+      ---
+      test_api/projects_get_/projects_when_unauthenticated_behaves_like_projects_response_returns_an_array_of_projects:
+      - |
+        DEPRECATION WARNING: /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/state_machines-activerecord-0.6.0/lib/state_machines/integrations/active_record.rb:511: 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/activerecord-6.0.3.3/lib/active_record/suppressor.rb:43: warning: The called method `save' is defined here
+      - |
+        DEPRECATION WARNING: /Users/tkuah/.rbenv/versions/2.7.2/lib/ruby/gems/2.7.0/gems/rack-2.2.3/lib/rack/builder.rb:158: 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/grape-1.4.0/lib/grape/middleware/error.rb:30: warning: The called method `initialize' is defined here
+      YAML
+    end
+
+    before do
+      allow(Dir).to receive(:glob).and_return(['deprecations/service/create_spec.yml', 'deprecations/api/projects_spec.yml'])
+      allow(File).to receive(:read).and_return(create_spec_yaml, projects_spec_yaml)
+    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/spec_helper.rb

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