From 839e3b0b951a7e1b51274d21c826f6c410250e5a Mon Sep 17 00:00:00 2001
From: Koichi ITO <koic.ito@gmail.com>
Date: Tue, 5 May 2020 10:53:02 +0900
Subject: [PATCH] [Fix #7928] Fix a false message for `Style/GuardClause`

Fixes #7928.

This PR fixes a false message for `Style/GuardClause` when using
`and` or `or` operators for guard clause in `then` or `else` branches.

The following is an example:

```console
% cat example.rb
if cond
  foo || raise('hi')
else
  bar
end
```

## Before

```console
% bundle exec rubocop --only Style/GuardClause
(snip)
Offenses:

example.rb:1:1: C: Style/GuardClause: Use a guard clause (raise('hi') if
cond) instead of wrapping the code inside a conditional expression.
if cond
^^

1 file inspected, 1 offense detected
```

## After

```console
% bundle exec rubocop --only Style/GuardClause
(snip)

Offenses:

example.rb:1:1: C: Style/GuardClause: Use a guard clause (foo ||
raise('hi') if cond) instead of wrapping the code inside a conditional
expression.
if cond
^^

1 file inspected, 1 offense detected
```
---
 CHANGELOG.md                                |  1 +
 lib/rubocop/cop/style/guard_clause.rb       | 24 +++++++++-
 manual/cops_style.md                        | 11 +++++
 spec/rubocop/cop/style/guard_clause_spec.rb | 52 +++++++++++++++++++++
 4 files changed, 87 insertions(+), 1 deletion(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 07ef2ff04e2..b647b714c77 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -28,6 +28,7 @@
 * [#7885](https://github.com/rubocop-hq/rubocop/issues/7885): Fix `Style/IfUnlessModifier` logic when tabs are used for indentation. ([@jonas054][])
 * [#7909](https://github.com/rubocop-hq/rubocop/pull/7909): Fix a false positive for `Lint/ParenthesesAsGroupedExpression` when using an intended grouped parentheses. ([@koic][])
 * [#7913](https://github.com/rubocop-hq/rubocop/pull/7913): Fix a false positive for `Lint/LiteralAsCondition` when using `true` literal in `while` and similar cases. ([@koic][])
+* [#7928](https://github.com/rubocop-hq/rubocop/issues/7928): Fix a false message for `Style/GuardClause` when using `and` or `or` operators for guard clause in `then` or `else` branches. ([@koic][])
 
 ### Changes
 
diff --git a/lib/rubocop/cop/style/guard_clause.rb b/lib/rubocop/cop/style/guard_clause.rb
index 44edc3fdc51..5c0b27ad8f0 100644
--- a/lib/rubocop/cop/style/guard_clause.rb
+++ b/lib/rubocop/cop/style/guard_clause.rb
@@ -35,6 +35,17 @@ module Style
       #   # good
       #   raise 'exception' if something
       #   ok
+      #
+      #   # bad
+      #   if something
+      #     foo || raise('exception')
+      #   else
+      #     ok
+      #   end
+      #
+      #   # good
+      #   foo || raise('exception') if something
+      #   ok
       class GuardClause < Cop
         include MinBodyLength
         include StatementModifier
@@ -69,7 +80,8 @@ def on_if(node)
                else
                  opposite_keyword(node)
                end
-          register_offense(node, guard_clause.source, kw)
+
+          register_offense(node, guard_clause_source(guard_clause), kw)
         end
 
         private
@@ -98,6 +110,16 @@ def register_offense(node, scope_exiting_keyword, conditional_keyword)
                       message: format(MSG, example: example))
         end
 
+        def guard_clause_source(guard_clause)
+          parent = guard_clause.parent
+
+          if parent.and_type? || parent.or_type?
+            guard_clause.parent.source
+          else
+            guard_clause.source
+          end
+        end
+
         def too_long_for_single_line?(node, example)
           max = max_line_length
           max && node.source_range.column + example.length > max
diff --git a/manual/cops_style.md b/manual/cops_style.md
index 5d5950cf935..2c046aa9532 100644
--- a/manual/cops_style.md
+++ b/manual/cops_style.md
@@ -2589,6 +2589,17 @@ end
 # good
 raise 'exception' if something
 ok
+
+# bad
+if something
+  foo || raise('exception')
+else
+  ok
+end
+
+# good
+foo || raise('exception') if something
+ok
 ```
 
 ### Configurable attributes
diff --git a/spec/rubocop/cop/style/guard_clause_spec.rb b/spec/rubocop/cop/style/guard_clause_spec.rb
index 2c02dc0e748..362bed3e726 100644
--- a/spec/rubocop/cop/style/guard_clause_spec.rb
+++ b/spec/rubocop/cop/style/guard_clause_spec.rb
@@ -107,6 +107,58 @@ def func
     RUBY
   end
 
+  it 'registers an offense when using `|| raise` in `then` branch' do
+    expect_offense(<<~RUBY)
+      def func
+        if something
+        ^^ Use a guard clause (`work || raise('message') if something`) instead of wrapping the code inside a conditional expression.
+          work || raise('message')
+        else
+          test
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `|| raise` in `else` branch' do
+    expect_offense(<<~RUBY)
+      def func
+        if something
+        ^^ Use a guard clause (`test || raise('message') unless something`) instead of wrapping the code inside a conditional expression.
+          work
+        else
+          test || raise('message')
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `and return` in `then` branch' do
+    expect_offense(<<~RUBY)
+      def func
+        if something
+        ^^ Use a guard clause (`work and return if something`) instead of wrapping the code inside a conditional expression.
+          work and return
+        else
+          test
+        end
+      end
+    RUBY
+  end
+
+  it 'registers an offense when using `and return` in `else` branch' do
+    expect_offense(<<~RUBY)
+      def func
+        if something
+        ^^ Use a guard clause (`test and return unless something`) instead of wrapping the code inside a conditional expression.
+          work
+        else
+          test and return
+        end
+      end
+    RUBY
+  end
+
   it 'accepts a method which body does not end with if / unless' do
     expect_no_offenses(<<~RUBY)
       def func