Merge branch '18040-rubocop-line-break-after-guard-clause' into 'master'

Adds Rubocop rule for line break after guard clause

Closes #18040

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

11 files changed, 173 insertions, 0 deletions

diff --git a/spec/factories/notes.rb b/spec/factories/notes.rb
index f0d05504b7e..ab4ae123429 100644
--- a/spec/factories/notes.rb
+++ b/spec/factories/notes.rb
@@ -130,6 +130,7 @@ FactoryGirl.define do
     before(:create) do |note, evaluator|
       discussion = evaluator.in_reply_to
       next unless discussion
+
       discussion = discussion.to_discussion if discussion.is_a?(Note)
       next unless discussion
 
diff --git a/spec/lib/gitlab/git/diff_collection_spec.rb b/spec/lib/gitlab/git/diff_collection_spec.rb
index ee657101f4c..65edc750f39 100644
--- a/spec/lib/gitlab/git/diff_collection_spec.rb
+++ b/spec/lib/gitlab/git/diff_collection_spec.rb
@@ -487,6 +487,7 @@ describe Gitlab::Git::DiffCollection, seed_helper: true do
 
       loop do
         break if @count.zero?
+
         # It is critical to decrement before yielding. We may never reach the lines after 'yield'.
         @count -= 1
         yield @value
diff --git a/spec/models/project_services/flowdock_service_spec.rb b/spec/models/project_services/flowdock_service_spec.rb
index 5e8e880985e..fabcb142858 100644
--- a/spec/models/project_services/flowdock_service_spec.rb
+++ b/spec/models/project_services/flowdock_service_spec.rb
@@ -46,6 +46,7 @@ describe FlowdockService do
       @sample_data[:commits].each do |commit|
         # One request to Flowdock per new commit
         next if commit[:id] == @sample_data[:before]
+
         expect(WebMock).to have_requested(:post, @api_url).with(
           body: /#{commit[:id]}.*#{project.path}/
         ).once
diff --git a/spec/requests/api/projects_spec.rb b/spec/requests/api/projects_spec.rb
index 50f6c8b7d64..a41345da05b 100644
--- a/spec/requests/api/projects_spec.rb
+++ b/spec/requests/api/projects_spec.rb
@@ -437,6 +437,7 @@ describe API::Projects do
 
       project.each_pair do |k, v|
         next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k)
+
         expect(json_response[k.to_s]).to eq(v)
       end
 
@@ -643,6 +644,7 @@ describe API::Projects do
       expect(response).to have_gitlab_http_status(201)
       project.each_pair do |k, v|
         next if %i[has_external_issue_tracker path].include?(k)
+
         expect(json_response[k.to_s]).to eq(v)
       end
     end
diff --git a/spec/requests/api/v3/projects_spec.rb b/spec/requests/api/v3/projects_spec.rb
index f62ad747c73..27288b98d1c 100644
--- a/spec/requests/api/v3/projects_spec.rb
+++ b/spec/requests/api/v3/projects_spec.rb
@@ -404,6 +404,7 @@ describe API::V3::Projects do
 
       project.each_pair do |k, v|
         next if %i[has_external_issue_tracker issues_enabled merge_requests_enabled wiki_enabled].include?(k)
+
         expect(json_response[k.to_s]).to eq(v)
       end
 
@@ -547,6 +548,7 @@ describe API::V3::Projects do
       expect(response).to have_gitlab_http_status(201)
       project.each_pair do |k, v|
         next if %i[has_external_issue_tracker path].include?(k)
+
         expect(json_response[k.to_s]).to eq(v)
       end
     end
diff --git a/spec/rubocop/cop/line_break_after_guard_clauses_spec.rb b/spec/rubocop/cop/line_break_after_guard_clauses_spec.rb
new file mode 100644
index 00000000000..8899dc85384
--- /dev/null
+++ b/spec/rubocop/cop/line_break_after_guard_clauses_spec.rb
@@ -0,0 +1,160 @@
+require 'spec_helper'
+require 'rubocop'
+require 'rubocop/rspec/support'
+require_relative '../../../rubocop/cop/line_break_after_guard_clauses'
+
+describe RuboCop::Cop::LineBreakAfterGuardClauses do
+  include CopHelper
+
+  subject(:cop) { described_class.new }
+
+  shared_examples 'examples with guard clause' do |title|
+    %w[if unless].each do |conditional|
+      it "flags violation for #{title} #{conditional} without line breaks" do
+        source = <<~RUBY
+          #{title} #{conditional} condition
+          do_stuff
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses.size).to eq(1)
+        offense = cop.offenses.first
+
+        expect(offense.line).to eq(1)
+        expect(cop.highlights).to eq(["#{title} #{conditional} condition"])
+        expect(offense.message).to eq('Add a line break after guard clauses')
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} with line break" do
+        source = <<~RUBY
+          #{title} #{conditional} condition
+
+          do_stuff
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} on multiple lines without line break" do
+        source = <<~RUBY
+          #{conditional} condition
+            #{title}
+          end
+          do_stuff
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} without line breaks when followed by end keyword" do
+        source = <<~RUBY
+          def test
+            #{title} #{conditional} condition
+          end
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} without line breaks when followed by elsif keyword" do
+        source = <<~RUBY
+          if model
+            #{title} #{conditional} condition
+          elsif
+            do_something
+          end
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} without line breaks when followed by else keyword" do
+        source = <<~RUBY
+          if model
+            #{title} #{conditional} condition
+          else
+            do_something
+          end
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} without line breaks when followed by when keyword" do
+        source = <<~RUBY
+          case model
+            when condition_a
+              #{title} #{conditional} condition
+            when condition_b
+              do_something
+            end
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} without line breaks when followed by rescue keyword" do
+        source = <<~RUBY
+          begin
+            #{title} #{conditional} condition
+          rescue StandardError
+            do_something
+          end
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} without line breaks when followed by ensure keyword" do
+        source = <<~RUBY
+          def foo
+            #{title} #{conditional} condition
+          ensure
+            do_something
+          end
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "doesn't flag violation for #{title} #{conditional} without line breaks when followed by another guard clause" do
+        source = <<~RUBY
+          #{title} #{conditional} condition
+          #{title} #{conditional} condition
+
+          do_stuff
+        RUBY
+        inspect_source(cop, source)
+
+        expect(cop.offenses).to be_empty
+      end
+
+      it "autocorrects #{title} #{conditional} guard clauses without line break" do
+        source = <<~RUBY
+          #{title} #{conditional} condition
+          do_stuff
+        RUBY
+        autocorrected = autocorrect_source(cop, source)
+
+        expected_source = <<~RUBY
+          #{title} #{conditional} condition
+
+          do_stuff
+        RUBY
+        expect(autocorrected).to eql(expected_source)
+      end
+    end
+  end
+
+  %w[return fail raise next break throw].each do |example|
+    it_behaves_like 'examples with guard clause', example
+  end
+end
diff --git a/spec/services/notification_service_spec.rb b/spec/services/notification_service_spec.rb
index b13e12e7c94..db5de572b6d 100644
--- a/spec/services/notification_service_spec.rb
+++ b/spec/services/notification_service_spec.rb
@@ -280,6 +280,7 @@ describe NotificationService, :mailer do
             next if member.id == @u_disabled.id
             # Author should not be notified
             next if member.id == note.author.id
+
             should_email(member)
           end
 
@@ -327,6 +328,7 @@ describe NotificationService, :mailer do
             next if member.id == @u_disabled.id
             # Author should not be notified
             next if member.id == note.author.id
+
             should_email(member)
           end
 
diff --git a/spec/support/fixture_helpers.rb b/spec/support/fixture_helpers.rb
index 5515c355cea..128aaaf25fe 100644
--- a/spec/support/fixture_helpers.rb
+++ b/spec/support/fixture_helpers.rb
@@ -1,6 +1,7 @@
 module FixtureHelpers
   def fixture_file(filename)
     return '' if filename.blank?
+
     File.read(expand_fixture_path(filename))
   end
 
diff --git a/spec/support/generate-seed-repo-rb b/spec/support/generate-seed-repo-rb
index ef3c8e7087f..4ee33f9725b 100755
--- a/spec/support/generate-seed-repo-rb
+++ b/spec/support/generate-seed-repo-rb
@@ -33,6 +33,7 @@ end
 def capture!(cmd, dir)
   output = IO.popen(cmd, 'r', chdir: dir) { |io| io.read }
   raise "command failed with #{$?}: #{cmd.join(' ')}" unless $?.success?
+
   output.chomp
 end
 
diff --git a/spec/support/gitaly.rb b/spec/support/gitaly.rb
index 1512b3e0620..c7e8a39a617 100644
--- a/spec/support/gitaly.rb
+++ b/spec/support/gitaly.rb
@@ -4,6 +4,7 @@ RSpec.configure do |config|
       allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(false)
     else
       next if example.metadata[:skip_gitaly_mock]
+
       allow(Gitlab::GitalyClient).to receive(:feature_enabled?).and_return(true)
     end
   end
diff --git a/spec/unicorn/unicorn_spec.rb b/spec/unicorn/unicorn_spec.rb
index 41de94d35c2..79a566975df 100644
--- a/spec/unicorn/unicorn_spec.rb
+++ b/spec/unicorn/unicorn_spec.rb
@@ -71,6 +71,7 @@ describe 'Unicorn' do
     timeout = 5 * 60
     timeout.times do
       return if File.exist?(ready_file)
+
       pid = Process.waitpid(master_pid, Process::WNOHANG)
       raise "unicorn failed to boot: #{$?}" unless pid.nil?