Merge branch 'enable-cops-with-3-offenses' into 'master'

Fix Rubocop offenses for cops with 3 offenses

See merge request gitlab-org/gitlab!23520

.rubocop_todo.yml

@@ -172,12 +172,6 @@ Lint/DuplicateMethods:
     - 'lib/gitlab/git/tree.rb'
     - 'lib/gitlab/git/wiki_page.rb'
 
-# Offense count: 3
-Lint/InterpolationCheck:
-  Exclude:
-    - 'spec/features/issues/filtered_search/filter_issues_spec.rb'
-    - 'spec/services/quick_actions/interpret_service_spec.rb'
-
 # Offense count: 122
 # Configuration parameters: MaximumRangeSize.
 Lint/MissingCopEnableDirective:
@@ -275,13 +269,6 @@ RSpec/ItBehavesLike:
     - 'spec/lib/gitlab/git/repository_spec.rb'
     - 'spec/services/notification_service_spec.rb'
 
-# Offense count: 3
-RSpec/IteratedExpectation:
-  Exclude:
-    - 'spec/features/admin/admin_settings_spec.rb'
-    - 'spec/lib/gitlab/gitlab_import/client_spec.rb'
-    - 'spec/lib/gitlab/legacy_github_import/client_spec.rb'
-
 # Offense count: 68
 # Cop supports --auto-correct.
 RSpec/LetBeforeExamples:
@@ -303,13 +290,6 @@ RSpec/MultipleSubjects:
   Exclude:
     - 'spec/services/merge_requests/create_from_issue_service_spec.rb'
 
-# Offense count: 3
-RSpec/OverwritingSetup:
-  Exclude:
-    - 'spec/models/email_spec.rb'
-    - 'spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb'
-    - 'spec/services/notes/quick_actions_service_spec.rb'
-
 # Offense count: 2018
 # Cop supports --auto-correct.
 # Configuration parameters: Strict, EnforcedStyle, AllowedExplicitMatchers.
@@ -556,13 +536,6 @@ Style/IfUnlessModifier:
 Style/Lambda:
   Enabled: false
 
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/LineEndConcatenation:
-  Exclude:
-    - 'spec/lib/gitlab/gfm/reference_rewriter_spec.rb'
-    - 'spec/lib/gitlab/incoming_email_spec.rb'
-
 # Offense count: 17
 Style/MethodMissingSuper:
   Enabled: false
@@ -659,14 +632,6 @@ Style/PerlBackrefs:
 Style/RaiseArgs:
   Enabled: false
 
-# Offense count: 3
-# Cop supports --auto-correct.
-Style/RedundantBegin:
-  Exclude:
-    - 'app/models/merge_request.rb'
-    - 'app/services/projects/import_service.rb'
-    - 'lib/gitlab/health_checks/base_abstract_check.rb'
-
 # Offense count: 1
 # Cop supports --auto-correct.
 Style/RedundantConditional:
@@ -771,14 +736,6 @@ Style/TernaryParentheses:
     - 'spec/requests/api/pipeline_schedules_spec.rb'
     - 'spec/support/capybara.rb'
 
-# Offense count: 3
-# Cop supports --auto-correct.
-# Configuration parameters: EnforcedStyleForMultiline.
-# SupportedStylesForMultiline: comma, consistent_comma, no_comma
-Style/TrailingCommaInArguments:
-  Exclude:
-    - 'spec/features/markdown/copy_as_gfm_spec.rb'
-
 # Offense count: 2
 # Cop supports --auto-correct.
 # Configuration parameters: EnforcedStyleForMultiline.

app/models/merge_request.rb

@@ -1201,12 +1201,10 @@ class MergeRequest < ApplicationRecord
   end
 
   def in_locked_state
-    begin
-      lock_mr
-      yield
-    ensure
-      unlock_mr
-    end
+    lock_mr
+    yield
+  ensure
+    unlock_mr
   end
 
   def diverged_commits_count

app/services/projects/import_service.rb

@@ -66,23 +66,21 @@ module Projects
     end
 
     def import_repository
-      begin
-        refmap = importer_class.try(:refmap) if has_importer?
-
-        if refmap
-          project.ensure_repository
-          project.repository.fetch_as_mirror(project.import_url, refmap: refmap)
-        else
-          gitlab_shell.import_project_repository(project)
-        end
-      rescue Gitlab::Shell::Error => e
-        # Expire cache to prevent scenarios such as:
-        # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true
-        # 2. Retried import, repo is broken or not imported but +exists?+ still returns true
-        project.repository.expire_content_cache if project.repository_exists?
+      refmap = importer_class.try(:refmap) if has_importer?
 
-        raise Error, e.message
+      if refmap
+        project.ensure_repository
+        project.repository.fetch_as_mirror(project.import_url, refmap: refmap)
+      else
+        gitlab_shell.import_project_repository(project)
       end
+    rescue Gitlab::Shell::Error => e
+      # Expire cache to prevent scenarios such as:
+      # 1. First import failed, but the repo was imported successfully, so +exists?+ returns true
+      # 2. Retried import, repo is broken or not imported but +exists?+ still returns true
+      project.repository.expire_content_cache if project.repository_exists?
+
+      raise Error, e.message
     end
 
     def download_lfs_objects

lib/gitlab/health_checks/base_abstract_check.rb

@@ -32,11 +32,9 @@ module Gitlab
       end
 
       def catch_timeout(seconds, &block)
-        begin
-          Timeout.timeout(seconds.to_i, &block)
-        rescue Timeout::Error => ex
-          ex
-        end
+        Timeout.timeout(seconds.to_i, &block)
+      rescue Timeout::Error => ex
+        ex
       end
     end
   end

spec/features/admin/admin_settings_spec.rb

@@ -228,9 +228,7 @@ describe 'Admin updates settings', :clean_gitlab_redis_shared_state, :do_not_moc
 
         click_link 'Slack notifications'
 
-        page.all('input[type=checkbox]').each do |checkbox|
-          expect(checkbox).to be_checked
-        end
+        expect(page.all('input[type=checkbox]')).to all(be_checked)
         expect(find_field('Webhook').value).to eq 'http://localhost'
         expect(find_field('Username').value).to eq 'test_user'
         expect(find('#service_push_channel').value).to eq '#test_channel'

spec/features/issues/filtered_search/filter_issues_spec.rb

@@ -192,7 +192,7 @@ describe 'Filter issues', :js do
       end
 
       it 'filters issues by label containing special characters' do
-        special_label = create(:label, project: project, title: '!@#{$%^&*()-+[]<>?/:{}|\}')
+        special_label = create(:label, project: project, title: '!@#$%^&*()-+[]<>?/:{}|\\')
         special_issue = create(:issue, title: "Issue with special character label", project: project)
         special_issue.labels << special_label
 
@@ -204,7 +204,7 @@ describe 'Filter issues', :js do
       end
 
       it 'filters issues by label not containing special characters' do
-        special_label = create(:label, project: project, title: '!@#{$%^&*()-+[]<>?/:{}|\}')
+        special_label = create(:label, project: project, title: '!@#$%^&*()-+[]<>?/:{}|\\')
         special_issue = create(:issue, title: "Issue with special character label", project: project)
         special_issue.labels << special_label
 

spec/features/markdown/copy_as_gfm_spec.rb

@@ -624,7 +624,7 @@ describe 'Copy as GFM', :js do
         GFM
 
         # table with empty heading
-        <<~GFM,
+        <<~GFM
           |  | x | y |
           |--|---|---|
           | a | 1 | 0 |
@@ -784,7 +784,7 @@ describe 'Copy as GFM', :js do
           verify(
             '.line[id="LC9"], .line[id="LC10"]',
 
-            <<~GFM,
+            <<~GFM
               ```ruby
                     raise RuntimeError, "System commands must be given as an array of strings"
                   end
@@ -826,7 +826,7 @@ describe 'Copy as GFM', :js do
           verify(
             '.line[id="LC27"], .line[id="LC28"]',
 
-            <<~GFM,
+            <<~GFM
               ```json
                   "bio": null,
                   "skype": "",

spec/lib/gitlab/gfm/reference_rewriter_spec.rb

@@ -35,7 +35,7 @@ describe Gitlab::Gfm::ReferenceRewriter do
 
       context 'description with ignored elements' do
         let(:text) do
-          "Hi. This references #1, but not `#2`\n" +
+          "Hi. This references #1, but not `#2`\n" \
             '<pre>and not !1</pre>'
         end
 

spec/lib/gitlab/gitlab_import/client_spec.rb

@@ -13,9 +13,7 @@ describe Gitlab::GitlabImport::Client do
   end
 
   it 'all OAuth2 client options are symbols' do
-    client.client.options.keys.each do |key|
-      expect(key).to be_kind_of(Symbol)
-    end
+    expect(client.client.options.keys).to all(be_kind_of(Symbol))
   end
 
   it 'uses membership and simple flags' do

spec/lib/gitlab/incoming_email_spec.rb

@@ -99,8 +99,8 @@ describe Gitlab::IncomingEmail do
 
   context 'self.scan_fallback_references' do
     let(:references) do
-      '<issue_1@localhost>' +
-        ' <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>' +
+      '<issue_1@localhost>' \
+        ' <reply-59d8df8370b7e95c5a49fbf86aeb2c93@localhost>' \
         ',<exchange@microsoft.com>'
     end
 

spec/lib/gitlab/legacy_github_import/client_spec.rb

@@ -13,9 +13,7 @@ describe Gitlab::LegacyGithubImport::Client do
   end
 
   it 'convert OAuth2 client options to symbols' do
-    client.client.options.keys.each do |key|
-      expect(key).to be_kind_of(Symbol)
-    end
+    expect(client.client.options.keys).to all(be_kind_of(Symbol))
   end
 
   it 'does not crash (e.g. Settingslogic::MissingSetting) when verify_ssl config is not present' do

spec/models/email_spec.rb

@@ -15,11 +15,6 @@ describe Email do
   end
 
   describe '#update_invalid_gpg_signatures' do
-    let(:user) do
-      create(:user, email: 'tula.torphy@abshire.ca').tap do |user|
-        user.skip_reconfirmation!
-      end
-    end
     let(:user) { create(:user) }
 
     it 'synchronizes the gpg keys when the email is updated' do

spec/services/merge_requests/add_todo_when_build_fails_service_spec.rb

@@ -4,7 +4,6 @@ require 'spec_helper'
 
 describe MergeRequests::AddTodoWhenBuildFailsService do
   let(:user) { create(:user) }
-  let(:merge_request) { create(:merge_request) }
   let(:project) { create(:project, :repository) }
   let(:sha) { '1234567890abcdef1234567890abcdef12345678' }
   let(:ref) { merge_request.source_branch }

spec/services/notes/quick_actions_service_spec.rb

@@ -176,7 +176,6 @@ describe Notes::QuickActionsService do
   context 'CE restriction for issue assignees' do
     describe '/assign' do
       let(:project) { create(:project) }
-      let(:maintainer) { create(:user).tap { |u| project.add_maintainer(u) } }
       let(:assignee) { create(:user) }
       let(:maintainer) { create(:user) }
       let(:service) { described_class.new(project, maintainer) }

spec/services/quick_actions/interpret_service_spec.rb

@@ -1322,11 +1322,6 @@ describe QuickActions::InterpretService do
         let(:issuable) { issue }
       end
 
-      it_behaves_like 'empty command', _('Failed to mark this issue as a duplicate because referenced issue was not found.') do
-        let(:content) { '/duplicate #{issue.to_reference}' }
-        let(:issuable) { issue }
-      end
-
       it_behaves_like 'empty command' do
         let(:content) { '/lock' }
         let(:issuable) { issue }