Skip to content

Commit

Permalink
Fix an incorrect autocorrect for Rails/ContentTag cop
Browse files Browse the repository at this point in the history
Follow up to rubocop#242.

This PR allows `content_tag` when the first argument is a variable because
`content_tag(name)` is simpler rather than `tag.public_send(name)`.

When `public_send` is used, it becomes complicated as follows.

First, `content_tag(name)` cannot be corrected to `tag.name`
when the first argument is a variable.

So use `public_send` and prevent the following errors in that case:

## Case 1: Using `public_send` to prevent the following `NoMethodError`

Original code:

```ruby
content_tag(name, 'foo', class: 'bar')
```

Auto-corrected code (before):

```ruby
tag(name, 'foo', class: 'bar')
#=> NoMethodError (undefined method `each_pair' for "foo":String)
```

Auto-corrected code (after):

```ruby
tag.public_send(name, 'foo', class: 'bar')
```

## Case 2: Using `symbolize_keys` to prevent the following `ArgumentError`

Original code:

```ruby
content_tag(name, 'foo', {'class' => 'bar'})
```

Auto-corrected code (before):

```ruby
tag.public_send(name, 'foo', {'class' => 'bar'})
#=> `ArgumentError (wrong number of arguments (given 3, expected 1..2))`
```

Auto-corrected code (after):

```ruby
tag.public_send(name, 'foo', {'class' => 'bar'}.symbolize_keys)
```

The `symbolize_keys` may not be needed, but for safe auto-correction
it will be added if optional argument keys are not all symbols.

## Case 3: Using `o ? o.symbolize_keys : {}` to prevent the following `ArgumentError`

Original code:

```ruby
content_tag(name, 'foo', options)
```

Auto-corrected code (before):

When the third argument is `nil`.

```ruby
options = nil
tag.public_send(name, 'foo', options)
#=> `ArgumentError (wrong number of arguments (given 3, expected 1..2))`
```

Auto-corrected code (after):

```ruby
tag.public_send(name, 'foo', options ? options.symbolize_keys : {})
```

Guard with the empty hash in case the third argument is `nil`.
  • Loading branch information
koic committed May 16, 2020
1 parent b8e2510 commit e5fd936
Show file tree
Hide file tree
Showing 3 changed files with 80 additions and 0 deletions.
9 changes: 9 additions & 0 deletions lib/rubocop/cop/rails/content_tag.rb
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,11 @@ module Rails
# This cop checks that `tag` is used instead of `content_tag`
# because `content_tag` is legacy syntax.
#
# !!! Note
#
# Allow `content_tag` when the first argument is a variable because
# `content_tag(name)` is simpler rather than `tag.public_send(name)`.
#
# @example
# # bad
# content_tag(:p, 'Hello world!')
Expand All @@ -14,6 +19,7 @@ module Rails
# # good
# tag.p('Hello world!')
# tag.br
# content_tag(name, 'Hello world!')
class ContentTag < Cop
include RangeHelp
extend TargetRailsVersion
Expand All @@ -25,6 +31,9 @@ class ContentTag < Cop
def on_send(node)
return unless node.method?(:content_tag)

first_argument = node.first_argument
return if first_argument.variable? || first_argument.send_type? || first_argument.const_type?

add_offense(node)
end

Expand Down
6 changes: 6 additions & 0 deletions manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -452,6 +452,11 @@ Enabled | Yes | Yes | 2.6 | -
This cop checks that `tag` is used instead of `content_tag`
because `content_tag` is legacy syntax.

!!! Note

Allow `content_tag` when the first argument is a variable because
`content_tag(name)` is simpler rather than `tag.public_send(name)`.

### Examples

```ruby
Expand All @@ -462,6 +467,7 @@ content_tag(:br)
# good
tag.p('Hello world!')
tag.br
content_tag(name, 'Hello world!')
```

### References
Expand Down
65 changes: 65 additions & 0 deletions spec/rubocop/cop/rails/content_tag_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,71 @@
RUBY
end

context 'when the first argument is a method or a variable' do
it 'does not register an offence when the first argument is a method' do
expect_no_offenses(<<~RUBY)
content_tag(name, "Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'does not register an offence when the first argument is a lvar' do
expect_no_offenses(<<~RUBY)
name = do_something
content_tag(name, "Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'does not register an offence when the first argument is an ivar' do
expect_no_offenses(<<~RUBY)
content_tag(@name, "Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'does not register an offence when the first argument is a cvar' do
expect_no_offenses(<<~RUBY)
content_tag(@@name, "Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'does not register an offence when the first argument is a gvar' do
expect_no_offenses(<<~RUBY)
content_tag($name, "Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'does not register an offence when the first argument is a constant' do
expect_no_offenses(<<~RUBY)
content_tag(CONST, "Hello world!", class: ["strong", "highlight"])
RUBY
end

context 'when options argument is a hash or a variable' do
it 'does not register an offence when the last argument is a variable' do
expect_no_offenses(<<~RUBY)
content_tag(name, "Hello world!", options)
RUBY
end

it 'does not register an offence when the last argument is a hash of string keys with brace' do
expect_no_offenses(<<~RUBY)
content_tag(name, "Hello world!", {'class' => 'foo'})
RUBY
end

it 'does not register an offence when the last argument is a hash of string keys without brace' do
expect_no_offenses(<<~RUBY)
content_tag(name, "Hello world!", 'class' => 'foo')
RUBY
end

it 'does not register an offence when the last argument is a hash of string and symbol keys without brace' do
expect_no_offenses(<<~RUBY)
content_tag(name, "Hello world!", 'class' => 'foo', id: 'baz')
RUBY
end
end
end

it 'corrects an offence with nested content_tag' do
expect_offense(<<~RUBY)
content_tag(:div) { content_tag(:strong, 'Hi') }
Expand Down

0 comments on commit e5fd936

Please sign in to comment.