Adds Rubocop rule for line break around conditionals

15 files changed, 427 insertions, 0 deletions

diff --git a/spec/factories/protected_branches.rb b/spec/factories/protected_branches.rb
index 39460834d06..60956511834 100644
--- a/spec/factories/protected_branches.rb
+++ b/spec/factories/protected_branches.rb
@@ -53,6 +53,7 @@ FactoryBot.define do
       if evaluator.default_access_level && evaluator.default_push_level
         protected_branch.push_access_levels.new(access_level: Gitlab::Access::MASTER)
       end
+
       if evaluator.default_access_level && evaluator.default_merge_level
         protected_branch.merge_access_levels.new(access_level: Gitlab::Access::MASTER)
       end
diff --git a/spec/features/issues/bulk_assignment_labels_spec.rb b/spec/features/issues/bulk_assignment_labels_spec.rb
index 587ece22ec7..cf283119f36 100644
--- a/spec/features/issues/bulk_assignment_labels_spec.rb
+++ b/spec/features/issues/bulk_assignment_labels_spec.rb
@@ -377,6 +377,7 @@ feature 'Issues > Labels bulk assignment' do
       items.map do |item|
         click_link item
       end
+
       if unmark
         items.map do |item|
           # Make sure we are unmarking the item no matter the state it has currently
diff --git a/spec/requests/api/commit_statuses_spec.rb b/spec/requests/api/commit_statuses_spec.rb
index ffa17d296e8..f246bb79ab7 100644
--- a/spec/requests/api/commit_statuses_spec.rb
+++ b/spec/requests/api/commit_statuses_spec.rb
@@ -142,6 +142,7 @@ describe API::CommitStatuses do
               expect(json_response['ref']).not_to be_empty
               expect(json_response['target_url']).to be_nil
               expect(json_response['description']).to be_nil
+
               if status == 'failed'
                 expect(CommitStatus.find(json_response['id'])).to be_api_failure
               end
diff --git a/spec/requests/api/runner_spec.rb b/spec/requests/api/runner_spec.rb
index 679d391caa5..cb66d23b77c 100644
--- a/spec/requests/api/runner_spec.rb
+++ b/spec/requests/api/runner_spec.rb
@@ -1142,6 +1142,7 @@ describe API::Runner do
                    else
                      { 'file' => file }
                    end
+
           post api("/jobs/#{job.id}/artifacts"), params, headers
         end
       end
diff --git a/spec/rubocop/cop/line_break_around_conditional_block_spec.rb b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb
new file mode 100644
index 00000000000..7ddf9141fcd
--- /dev/null
+++ b/spec/rubocop/cop/line_break_around_conditional_block_spec.rb
@@ -0,0 +1,411 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/line_break_around_conditional_block'
+
+describe RuboCop::Cop::LineBreakAroundConditionalBlock do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  shared_examples 'examples with conditional' do |conditional|
+    it "flags violation for #{conditional} without line break before" do
+      source = <<~RUBY
+          do_something
+          #{conditional} condition
+            do_something_more
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses.size).to eq(1)
+      offense = cop.offenses.first
+
+      expect(offense.line).to eq(2)
+      expect(cop.highlights).to eq(["#{conditional} condition\n  do_something_more\nend"])
+      expect(offense.message).to eq('Add a line break around conditional blocks')
+    end
+
+    it "flags violation for #{conditional} without line break after" do
+      source = <<~RUBY
+          #{conditional} condition
+            do_something
+          end
+          do_something_more
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses.size).to eq(1)
+      offense = cop.offenses.first
+
+      expect(offense.line).to eq(1)
+      expect(cop.highlights).to eq(["#{conditional} condition\n  do_something\nend"])
+      expect(offense.message).to eq('Add a line break around conditional blocks')
+    end
+
+    it "doesn't flag violation for #{conditional} with line break before and after" do
+      source = <<~RUBY
+          #{conditional} condition
+            do_something
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a method definition" do
+      source = <<~RUBY
+          def a_method
+            #{conditional} condition
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a class definition" do
+      source = <<~RUBY
+          class Foo
+            #{conditional} condition
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a module definition" do
+      source = <<~RUBY
+          module Foo
+            #{conditional} condition
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a begin definition" do
+      source = <<~RUBY
+          begin
+            #{conditional} condition
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by an assign/begin definition" do
+      source = <<~RUBY
+          @project ||= begin
+            #{conditional} condition
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a block definition" do
+      source = <<~RUBY
+          on_block(param_a) do |item|
+            #{conditional} condition
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a block definition using brackets" do
+      source = <<~RUBY
+          on_block(param_a) { |item|
+            #{conditional} condition
+              do_something
+            end
+          }
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a comment" do
+      source = <<~RUBY
+          # a short comment
+          #{conditional} condition
+            do_something
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by an assignment" do
+      source = <<~RUBY
+          foo =
+            #{conditional} condition
+              do_something
+            else
+              do_something_more
+            end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a multiline comment" do
+      source = <<~RUBY
+          =begin
+          a multiline comment
+          =end
+          #{conditional} condition
+            do_something
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by another conditional" do
+      source = <<~RUBY
+          #{conditional} condition_a
+            #{conditional} condition_b
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by an else" do
+      source = <<~RUBY
+           if condition_a
+             do_something
+           else
+             #{conditional} condition_b
+               do_something_extra
+             end
+           end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by an elsif" do
+      source = <<~RUBY
+           if condition_a
+             do_something
+           elsif condition_b
+             #{conditional} condition_c
+               do_something_extra
+             end
+           end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by an ensure" do
+      source = <<~RUBY
+           def a_method
+           ensure
+             #{conditional} condition_c
+               do_something_extra
+             end
+           end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} preceded by a when" do
+      source = <<~RUBY
+           case field
+           when value
+             #{conditional} condition_c
+               do_something_extra
+             end
+           end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} followed by an end" do
+      source = <<~RUBY
+          class Foo
+
+            #{conditional} condition
+              do_something
+            end
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} followed by an else" do
+      source = <<~RUBY
+          #{conditional} condition_a
+            #{conditional} condition_b
+              do_something
+            end
+          else
+            do_something_extra
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} followed by a when" do
+      source = <<~RUBY
+          case
+          when condition_a
+            #{conditional} condition_b
+              do_something
+            end
+          when condition_c
+            do_something_extra
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} followed by an elsif" do
+      source = <<~RUBY
+          if condition_a
+            #{conditional} condition_b
+              do_something
+            end
+          elsif condition_c
+            do_something_extra
+          end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "doesn't flag violation for #{conditional} followed by a rescue" do
+      source = <<~RUBY
+            def a_method
+              #{conditional} condition
+                do_something
+              end
+              rescue
+                do_something_extra
+            end
+      RUBY
+      inspect_source(source)
+
+      expect(cop.offenses).to be_empty
+    end
+
+    it "autocorrects #{conditional} without line break before" do
+      source = <<~RUBY
+          do_something
+          #{conditional} condition
+            do_something_more
+          end
+      RUBY
+      autocorrected = autocorrect_source(source)
+
+      expected_source = <<~RUBY
+          do_something
+
+          #{conditional} condition
+            do_something_more
+          end
+      RUBY
+      expect(autocorrected).to eql(expected_source)
+    end
+
+    it "autocorrects #{conditional} without line break after" do
+      source = <<~RUBY
+          #{conditional} condition
+            do_something
+          end
+          do_something_more
+      RUBY
+      autocorrected = autocorrect_source(source)
+
+      expected_source = <<~RUBY
+          #{conditional} condition
+            do_something
+          end
+
+          do_something_more
+      RUBY
+      expect(autocorrected).to eql(expected_source)
+    end
+
+    it "autocorrects #{conditional} without line break before and after" do
+      source = <<~RUBY
+          do_something
+          #{conditional} condition
+            do_something_more
+          end
+          do_something_extra
+      RUBY
+      autocorrected = autocorrect_source(source)
+
+      expected_source = <<~RUBY
+          do_something
+
+          #{conditional} condition
+            do_something_more
+          end
+
+          do_something_extra
+      RUBY
+      expect(autocorrected).to eql(expected_source)
+    end
+  end
+
+  %w[if unless].each do |example|
+    it_behaves_like 'examples with conditional', example
+  end
+
+  it "doesn't flag violation for if with elsif" do
+    source = <<~RUBY
+          if condition
+            do_something
+          elsif another_condition
+            do_something_more
+          end
+    RUBY
+    inspect_source(source)
+
+    expect(cop.offenses).to be_empty
+  end
+end
diff --git a/spec/services/projects/transfer_service_spec.rb b/spec/services/projects/transfer_service_spec.rb
index 39f6388c25e..ef68742a463 100644
--- a/spec/services/projects/transfer_service_spec.rb
+++ b/spec/services/projects/transfer_service_spec.rb
@@ -150,6 +150,7 @@ describe Projects::TransferService do
 
     before do
       group.add_owner(user)
+
       unless gitlab_shell.add_repository(repository_storage, "#{group.full_path}/#{project.path}")
         raise 'failed to add repository'
       end
diff --git a/spec/services/system_note_service_spec.rb b/spec/services/system_note_service_spec.rb
index 4e640a82dfc..965fd39c967 100644
--- a/spec/services/system_note_service_spec.rb
+++ b/spec/services/system_note_service_spec.rb
@@ -727,6 +727,7 @@ describe SystemNoteService do
               else
                 "#{Settings.gitlab.base_url}/#{project.namespace.path}/#{project.path}/merge_requests/#{merge_request.iid}"
               end
+
         link = double(object: { 'url' => url })
         links << link
         expect(link).to receive(:save!)
diff --git a/spec/support/features/discussion_comments_shared_example.rb b/spec/support/features/discussion_comments_shared_example.rb
index c24940393f9..fa94aa2ae3d 100644
--- a/spec/support/features/discussion_comments_shared_example.rb
+++ b/spec/support/features/discussion_comments_shared_example.rb
@@ -113,6 +113,7 @@ shared_examples 'discussion comments' do |resource_name|
         else
           expect(find(submit_selector).value).to eq 'Start discussion'
         end
+
         expect(page).not_to have_selector menu_selector
       end
 
@@ -200,6 +201,7 @@ shared_examples 'discussion comments' do |resource_name|
             else
               expect(find(submit_selector).value).to eq 'Comment'
             end
+
             expect(page).not_to have_selector menu_selector
           end
 
diff --git a/spec/support/filtered_search_helpers.rb b/spec/support/filtered_search_helpers.rb
index 05021ea9054..f3f96bd1f0a 100644
--- a/spec/support/filtered_search_helpers.rb
+++ b/spec/support/filtered_search_helpers.rb
@@ -61,9 +61,11 @@ module FilteredSearchHelpers
         token_emoji = tokens[index][:emoji_name]
 
         expect(el.find('.name')).to have_content(token_name)
+
         if token_value
           expect(el.find('.value')).to have_content(token_value)
         end
+
         # gl-emoji content is blank when the emoji unicode is not supported
         if token_emoji
           selector = %(gl-emoji[data-name="#{token_emoji}"])
diff --git a/spec/support/generate-seed-repo-rb b/spec/support/generate-seed-repo-rb
index 4ee33f9725b..876b3b8242d 100755
--- a/spec/support/generate-seed-repo-rb
+++ b/spec/support/generate-seed-repo-rb
@@ -24,6 +24,7 @@ def main
     unless system(*%W[git clone --bare #{SOURCE} #{REPO_NAME}], chdir: dir)
       abort "git clone failed"
     end
+
     repo = File.join(dir, REPO_NAME)
     erb = ERB.new(DATA.read)
     erb.run(binding)
diff --git a/spec/support/matchers/access_matchers_for_controller.rb b/spec/support/matchers/access_matchers_for_controller.rb
index cdb62a5deee..42a9ed9ff34 100644
--- a/spec/support/matchers/access_matchers_for_controller.rb
+++ b/spec/support/matchers/access_matchers_for_controller.rb
@@ -43,6 +43,7 @@ module AccessMatchersForController
       user = create(:user)
       membership.public_send(:"add_#{role}", user)
     end
+
     user
   end
 
diff --git a/spec/support/select2_helper.rb b/spec/support/select2_helper.rb
index 55da961e173..90618ba5b19 100644
--- a/spec/support/select2_helper.rb
+++ b/spec/support/select2_helper.rb
@@ -17,6 +17,7 @@ module Select2Helper
     selector = options.fetch(:from)
 
     first(selector, visible: false)
+
     if options[:multiple]
       execute_script("$('#{selector}').select2('val', ['#{value}']).trigger('change');")
     else
diff --git a/spec/support/stub_env.rb b/spec/support/stub_env.rb
index f621463e621..695152e2d4e 100644
--- a/spec/support/stub_env.rb
+++ b/spec/support/stub_env.rb
@@ -4,6 +4,7 @@ module StubENV
 
   def stub_env(key_or_hash, value = nil)
     init_stub unless env_stubbed?
+
     if key_or_hash.is_a? Hash
       key_or_hash.each { |k, v| add_stubbed_value(k, v) }
     else
diff --git a/spec/support/test_env.rb b/spec/support/test_env.rb
index 664698fcbaf..25ff6094408 100644
--- a/spec/support/test_env.rb
+++ b/spec/support/test_env.rb
@@ -325,6 +325,7 @@ module TestEnv
     if component_needs_update?(install_dir, version)
       # Cleanup the component entirely to ensure we start fresh
       FileUtils.rm_rf(install_dir)
+
       unless system('rake', task)
         raise ComponentFailedToInstallError
       end
diff --git a/spec/support/wait_for_requests.rb b/spec/support/wait_for_requests.rb
index f4130d68271..fda0e29f983 100644
--- a/spec/support/wait_for_requests.rb
+++ b/spec/support/wait_for_requests.rb
@@ -53,6 +53,7 @@ module WaitForRequests
     wait_until = Time.now + max_wait_time.seconds
     loop do
       break if yield
+
       if Time.now > wait_until
         raise "Condition not met: #{condition_name}"
       else