From 850e65a74c1721e42b868e7ee16ec4a3bef7bb9f Mon Sep 17 00:00:00 2001
From: Teemu <teemu@aikais.in>
Date: Thu, 13 Oct 2016 20:35:19 +0300
Subject: [PATCH] [Fix #3291] Improve Rails safety method detection

Only assume `raw` and `html_safe` to be a Rails helpers if calls have
one and zero arguments respectively.

Also unify related test descriptions.
---
 CHANGELOG.md                                 |  1 +
 lib/rubocop/cop/rails/output_safety.rb       | 23 ++++++++----
 spec/rubocop/cop/rails/output_safety_spec.rb | 37 +++++++++++++++-----
 3 files changed, 46 insertions(+), 15 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index cbe91003727e..35c826c29c47 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -5,6 +5,7 @@
 ### Bug fixes
 
 * [#3607](https://github.com/bbatsov/rubocop/pull/3607): Fix `Style/RedundantReturn` cop for empty if body. ([@pocke][])
+* [#3291](https://github.com/bbatsov/rubocop/issues/3291): Improve detection of `raw` and `html_safe` methods in `Rails/OutputSafety`. ([@lumeet][])
 
 ## 0.44.1 (2016-10-13)
 
diff --git a/lib/rubocop/cop/rails/output_safety.rb b/lib/rubocop/cop/rails/output_safety.rb
index 21fd75cc051b..00e8b222d116 100644
--- a/lib/rubocop/cop/rails/output_safety.rb
+++ b/lib/rubocop/cop/rails/output_safety.rb
@@ -30,13 +30,24 @@ class OutputSafety < Cop
               'prefer `safe_join` or other Rails tag helpers instead.'.freeze
 
         def on_send(node)
-          receiver, method_name, *_args = *node
+          return unless looks_like_rails_html_safe?(node) ||
+                        looks_like_rails_raw?(node)
 
-          if receiver && method_name == :html_safe
-            add_offense(node, :selector)
-          elsif receiver.nil? && method_name == :raw
-            add_offense(node, :selector)
-          end
+          add_offense(node, :selector)
+        end
+
+        private
+
+        def looks_like_rails_html_safe?(node)
+          receiver, method_name, *args = *node
+
+          receiver && method_name == :html_safe && args.empty?
+        end
+
+        def looks_like_rails_raw?(node)
+          receiver, method_name, *args = *node
+
+          receiver.nil? && method_name == :raw && args.one?
         end
       end
     end
diff --git a/spec/rubocop/cop/rails/output_safety_spec.rb b/spec/rubocop/cop/rails/output_safety_spec.rb
index 279f19d56fe7..cf9349bf3663 100644
--- a/spec/rubocop/cop/rails/output_safety_spec.rb
+++ b/spec/rubocop/cop/rails/output_safety_spec.rb
@@ -5,35 +5,54 @@
 describe RuboCop::Cop::Rails::OutputSafety do
   subject(:cop) { described_class.new }
 
-  it 'records an offense for html_safe methods with a receiver' do
+  it 'registers an offense for html_safe methods with a receiver and no ' \
+     'arguments' do
     source = ['foo.html_safe',
               '"foo".html_safe']
     inspect_source(cop, source)
     expect(cop.offenses.size).to eq(2)
   end
 
-  it 'does not record an offense for html_safe methods without a receiver' do
-    source = ['html_safe foo',
-              'html_safe "foo"']
+  it 'accepts html_safe methods without a receiver' do
+    source = 'html_safe'
     inspect_source(cop, source)
     expect(cop.offenses).to be_empty
   end
 
-  it 'records an offense for raw methods without a receiver' do
+  it 'accepts html_safe methods with arguments' do
+    source = ['foo.html_safe one',
+              '"foo".html_safe two']
+    inspect_source(cop, source)
+    expect(cop.offenses).to be_empty
+  end
+
+  it 'registers an offense for raw methods without a receiver' do
     source = ['raw(foo)',
               'raw "foo"']
     inspect_source(cop, source)
     expect(cop.offenses.size).to eq(2)
   end
 
-  it 'does not record an offense for raw methods with a receiver' do
-    source = ['foo.raw',
-              '"foo".raw']
+  it 'accepts raw methods with a receiver' do
+    source = ['foo.raw(foo)',
+              '"foo".raw "foo"']
+    inspect_source(cop, source)
+    expect(cop.offenses).to be_empty
+  end
+
+  it 'accepts raw methods without arguments' do
+    source = 'raw'
+    inspect_source(cop, source)
+    expect(cop.offenses).to be_empty
+  end
+
+  it 'accepts raw methods with more than one arguments' do
+    source = 'raw one, two'
     inspect_source(cop, source)
     expect(cop.offenses).to be_empty
   end
 
-  it 'does not record an offense for comments' do
+  it 'accepts comments' do
     source = ['# foo.html_safe',
               '# raw foo']
     inspect_source(cop, source)