Skip to content

Commit

Permalink
[Fix rubocop#3291] Improve Rails safety method detection
Browse files Browse the repository at this point in the history
Only assume `raw` and `html_safe` to be a Rails helpers if calls have
one and zero arguments respectively.

Also unify related test descriptions.
  • Loading branch information
lumeet authored and Neodelf committed Oct 15, 2016
1 parent d6d45f7 commit 850e65a
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 15 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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)

Expand Down
23 changes: 17 additions & 6 deletions lib/rubocop/cop/rails/output_safety.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
37 changes: 28 additions & 9 deletions spec/rubocop/cop/rails/output_safety_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down

0 comments on commit 850e65a

Please sign in to comment.