Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add new Rails/ContentTag cop #242

Merged
merged 1 commit into from
May 4, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@

* [#51](https://github.com/rubocop-hq/rubocop-rails/issues/51): Add allowed receiver class names option for `Rails/DynamicFindBy`. ([@tejasbubane][])
* [#211](https://github.com/rubocop-hq/rubocop-rails/issues/211): Add autocorrect to `Rails/RakeEnvironment` cop. ([@tejasbubane][])
* [#242](https://github.com/rubocop-hq/rubocop-rails/pull/242): Add `Rails/ContentTag` cop. ([@tabuchi0919][])

### Bug fixes

Expand Down Expand Up @@ -188,3 +189,4 @@
[@hoshinotsuyoshi]: https://github.com/hoshinotsuyoshi
[@tejasbubane]: https://github.com/tejasbubane
[@diogoosorio]: https://github.com/diogoosorio
[@tabuchi0919]: https://github.com/tabuchi0919
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -119,6 +119,14 @@ Rails/BulkChangeTable:
Include:
- db/migrate/*.rb

Rails/ContentTag:
Description: 'Use `tag` instead of `content_tag`.'
Reference:
- 'https://github.com/rails/rails/issues/25195'
- 'https://api.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-content_tag'
Enabled: true
VersionAdded: '2.6'

Rails/CreateTableWithTimestamps:
Description: >-
Checks the migration for which timestamps are not included
Expand Down
73 changes: 73 additions & 0 deletions lib/rubocop/cop/rails/content_tag.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,73 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# This cop checks that `tag` is used instead of `content_tag`
# because `content_tag` is legacy syntax.
#
# @example
# # bad
# content_tag(:p, 'Hello world!')
# content_tag(:br)
#
# # good
# tag.p('Hello world!')
# tag.br
class ContentTag < Cop
include RangeHelp
extend TargetRailsVersion

minimum_target_rails_version 5.1

MSG = 'Use `tag` instead of `content_tag`.'

def on_send(node)
return unless node.method?(:content_tag)

add_offense(node)
end

def autocorrect(node)
lambda do |corrector|
if method_name?(node.first_argument)
replace_method_with_tag_method(corrector, node)
remove_first_argument(corrector, node)
else
corrector.replace(node.loc.selector, 'tag')
end
end
end

private

def method_name?(node)
return false unless node.str_type? || node.sym_type?

/^[a-zA-Z_][a-zA-Z_0-9]*$/.match?(node.value)
end

def replace_method_with_tag_method(corrector, node)
corrector.replace(
node.loc.selector,
"tag.#{node.first_argument.value}"
)
end

def remove_first_argument(corrector, node)
if node.arguments.length > 1
corrector.remove(
range_between(child_node_beg(node, 0), child_node_beg(node, 1))
)
elsif node.arguments.length == 1
corrector.remove(node.arguments[0].loc.expression)
end
end

def child_node_beg(node, index)
node.arguments[index].loc.expression.begin_pos
end
end
end
end
end
1 change: 1 addition & 0 deletions lib/rubocop/cop/rails_cops.rb
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
require_relative 'rails/belongs_to'
require_relative 'rails/blank'
require_relative 'rails/bulk_change_table'
require_relative 'rails/content_tag'
require_relative 'rails/create_table_with_timestamps'
require_relative 'rails/date'
require_relative 'rails/delegate'
Expand Down
1 change: 1 addition & 0 deletions manual/cops.md
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* [Rails/BelongsTo](cops_rails.md#railsbelongsto)
* [Rails/Blank](cops_rails.md#railsblank)
* [Rails/BulkChangeTable](cops_rails.md#railsbulkchangetable)
* [Rails/ContentTag](cops_rails.md#railscontenttag)
* [Rails/CreateTableWithTimestamps](cops_rails.md#railscreatetablewithtimestamps)
* [Rails/Date](cops_rails.md#railsdate)
* [Rails/Delegate](cops_rails.md#railsdelegate)
Expand Down
26 changes: 26 additions & 0 deletions manual/cops_rails.md
Original file line number Diff line number Diff line change
Expand Up @@ -443,6 +443,32 @@ Name | Default value | Configurable values
Database | `<none>` | `mysql`, `postgresql`
Include | `db/migrate/*.rb` | Array

## Rails/ContentTag

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
--- | --- | --- | --- | ---
Enabled | Yes | Yes | 2.6 | -

This cop checks that `tag` is used instead of `content_tag`
because `content_tag` is legacy syntax.

### Examples

```ruby
# bad
content_tag(:p, 'Hello world!')
content_tag(:br)

# good
tag.p('Hello world!')
tag.br
```

### References

* [https://github.com/rails/rails/issues/25195](https://github.com/rails/rails/issues/25195)
* [https://api.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-content_tag](https://api.rubyonrails.org/classes/ActionView/Helpers/TagHelper.html#method-i-content_tag)

## Rails/CreateTableWithTimestamps

Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged
Expand Down
118 changes: 118 additions & 0 deletions spec/rubocop/cop/rails/content_tag_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
# frozen_string_literal: true

RSpec.describe RuboCop::Cop::Rails::ContentTag, :config do
subject(:cop) { described_class.new(config) }

context 'Rails 5.0', :rails50 do
it 'does not register an offense' do
expect_no_offenses(<<~RUBY)
content_tag(:p, 'Hello world!')
RUBY
end

it 'does not register an offense with empty tag' do
expect_no_offenses(<<~RUBY)
content_tag(:br)
RUBY
end

it 'does not register an offense with array of classnames' do
expect_no_offenses(<<~RUBY)
content_tag(:div, "Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'does not register an offense with nested content_tag' do
expect_no_offenses(<<~RUBY)
content_tag(:div) { content_tag(:strong, 'Hi') }
RUBY
end
end

context 'Rails 5.1', :rails51 do
it 'corrects an offence' do
expect_offense(<<~RUBY)
content_tag(:p, 'Hello world!')
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `tag` instead of `content_tag`.
RUBY
expect_correction(<<~RUBY)
tag.p('Hello world!')
RUBY
end

it 'corrects an offence with empty tag' do
expect_offense(<<~RUBY)
content_tag(:br)
^^^^^^^^^^^^^^^^ Use `tag` instead of `content_tag`.
RUBY
expect_correction(<<~RUBY)
tag.br()
RUBY
end

it 'corrects an offence with array of classnames' do
expect_offense(<<~RUBY)
content_tag(:div, "Hello world!", class: ["strong", "highlight"])
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `tag` instead of `content_tag`.
RUBY
expect_correction(<<~RUBY)
tag.div("Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'corrects an offence with nested content_tag' do
expect_offense(<<~RUBY)
content_tag(:div) { content_tag(:strong, 'Hi') }
^^^^^^^^^^^^^^^^^^^^^^^^^^ Use `tag` instead of `content_tag`.
^^^^^^^^^^^^^^^^^ Use `tag` instead of `content_tag`.
RUBY
expect_correction(<<~RUBY)
tag.div() { tag.strong('Hi') }
RUBY
end

it 'corrects an offence when first argument is hash' do
expect_offense(<<~RUBY)
content_tag({foo: 1})
^^^^^^^^^^^^^^^^^^^^^ Use `tag` instead of `content_tag`.
RUBY
expect_correction(<<~RUBY)
tag({foo: 1})
RUBY
end

it 'corrects an offence when first argument is non-identifier string' do
expect_offense(<<~RUBY)
content_tag('foo-bar')
^^^^^^^^^^^^^^^^^^^^^^ Use `tag` instead of `content_tag`.
RUBY
expect_correction(<<~RUBY)
tag('foo-bar')
RUBY
end

it 'does not register an offence when `tag` is used with an argument' do
expect_no_offenses(<<~RUBY)
tag.p('Hello world!')
RUBY
end

it 'does not register an offence when `tag` is used without arguments' do
expect_no_offenses(<<~RUBY)
tag.br
RUBY
end

it 'does not register an offence when `tag` is used with arguments' do
expect_no_offenses(<<~RUBY)
tag.div("Hello world!", class: ["strong", "highlight"])
RUBY
end

it 'does not register an offence when `tag` is nested' do
expect_no_offenses(<<~RUBY)
tag.div() { tag.strong('Hi') }
RUBY
end
end
end