Merge branch 'add-rubocop-check-for-be-success' into 'master'

Add rubocop check for be success

See merge request gitlab-org/gitlab-ce!32091

4 files changed, 126 insertions, 0 deletions

diff --git a/.rubocop.yml b/.rubocop.yml
index 012f4890c33..573f2fbb6c6 100644
--- a/.rubocop.yml
+++ b/.rubocop.yml
@@ -265,3 +265,12 @@ RSpec/EnvAssignment:
     - 'ee/spec/**/rails_helper.rb'
     - 'spec/**/spec_helper.rb'
     - 'ee/spec/**/spec_helper.rb'
+RSpec/BeSuccessMatcher:
+  Enabled: true
+  Include:
+    - 'spec/controllers/**/*'
+    - 'ee/spec/controllers/**/*'
+    - 'spec/support/shared_examples/controllers/**/*'
+    - 'ee/spec/support/shared_examples/controllers/**/*'
+    - 'spec/support/controllers/**/*'
+    - 'ee/spec/support/controllers/**/*'
diff --git a/rubocop/cop/rspec/be_success_matcher.rb b/rubocop/cop/rspec/be_success_matcher.rb
new file mode 100644
index 00000000000..dce9604b3d7
--- /dev/null
+++ b/rubocop/cop/rspec/be_success_matcher.rb
@@ -0,0 +1,53 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module RSpec
+      # This cop checks for `be_success` usage in controller specs
+      #
+      # @example
+      #
+      #   # bad
+      #   it "responds with success" do
+      #     expect(response).to be_success
+      #   end
+      #
+      #   it { is_expected.to be_success }
+      #
+      #   # good
+      #   it "responds with success" do
+      #     expect(response).to be_successful
+      #   end
+      #
+      #   it { is_expected.to be_successful }
+      #
+      class BeSuccessMatcher < RuboCop::Cop::Cop
+        MESSAGE = 'Do not use deprecated `success?` method, use `successful?` instead.'
+
+        def_node_search :expect_to_be_success?, <<~PATTERN
+          (send (send nil? :expect (send nil? ...)) {:to :not_to :to_not} (send nil? :be_success))
+        PATTERN
+
+        def_node_search :is_expected_to_be_success?, <<~PATTERN
+          (send (send nil? :is_expected) {:to :not_to :to_not} (send nil? :be_success))
+        PATTERN
+
+        def be_success_usage?(node)
+          expect_to_be_success?(node) || is_expected_to_be_success?(node)
+        end
+
+        def on_send(node)
+          return unless be_success_usage?(node)
+
+          add_offense(node, location: :expression, message: MESSAGE)
+        end
+
+        def autocorrect(node)
+          lambda do |corrector|
+            corrector.insert_after(node.loc.expression, 'ful')
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/rubocop/rubocop.rb b/rubocop/rubocop.rb
index d1328c4eb38..c342df6d6c9 100644
--- a/rubocop/rubocop.rb
+++ b/rubocop/rubocop.rb
@@ -31,6 +31,7 @@ require_relative 'cop/migration/timestamps'
 require_relative 'cop/migration/update_column_in_batches'
 require_relative 'cop/migration/update_large_table'
 require_relative 'cop/project_path_helper'
+require_relative 'cop/rspec/be_success_matcher'
 require_relative 'cop/rspec/env_assignment'
 require_relative 'cop/rspec/factories_in_migration_specs'
 require_relative 'cop/rspec/top_level_describe_path'
diff --git a/spec/rubocop/cop/rspec/be_success_matcher_spec.rb b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb
new file mode 100644
index 00000000000..12aa7d1643e
--- /dev/null
+++ b/spec/rubocop/cop/rspec/be_success_matcher_spec.rb
@@ -0,0 +1,63 @@
+# frozen_string_literal: true
+
+require 'spec_helper'
+
+require_relative '../../../../rubocop/cop/rspec/be_success_matcher'
+
+describe RuboCop::Cop::RSpec::BeSuccessMatcher do
+  include CopHelper
+
+  let(:source_file) { 'spec/foo_spec.rb' }
+
+  subject(:cop) { described_class.new }
+
+  shared_examples 'cop' do |good:, bad:|
+    context "using #{bad} call" do
+      it 'registers an offense' do
+        inspect_source(bad, source_file)
+
+        expect(cop.offenses.size).to eq(1)
+        expect(cop.offenses.map(&:line)).to eq([1])
+        expect(cop.highlights).to eq([bad])
+      end
+
+      it "autocorrects it to `#{good}`" do
+        autocorrected = autocorrect_source(bad, source_file)
+
+        expect(autocorrected).to eql(good)
+      end
+    end
+
+    context "using #{good} call" do
+      it 'does not register an offense' do
+        inspect_source(good)
+
+        expect(cop.offenses).to be_empty
+      end
+    end
+  end
+
+  include_examples 'cop',
+    bad: 'expect(response).to be_success',
+    good: 'expect(response).to be_successful'
+
+  include_examples 'cop',
+    bad: 'expect(response).to_not be_success',
+    good: 'expect(response).to_not be_successful'
+
+  include_examples 'cop',
+    bad: 'expect(response).not_to be_success',
+    good: 'expect(response).not_to be_successful'
+
+  include_examples 'cop',
+    bad: 'is_expected.to be_success',
+    good: 'is_expected.to be_successful'
+
+  include_examples 'cop',
+    bad: 'is_expected.to_not be_success',
+    good: 'is_expected.to_not be_successful'
+
+  include_examples 'cop',
+    bad: 'is_expected.not_to be_success',
+    good: 'is_expected.not_to be_successful'
+end