Merge pull request #242 from tabuchi0919/tag-instead-of-content-tag

Add new Rails/ContentTag cop

CHANGELOG.md

@@ -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
 
@@ -188,3 +189,4 @@
 [@hoshinotsuyoshi]: https://github.com/hoshinotsuyoshi
 [@tejasbubane]: https://github.com/tejasbubane
 [@diogoosorio]: https://github.com/diogoosorio
+[@tabuchi0919]: https://github.com/tabuchi0919

config/default.yml

@@ -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

lib/rubocop/cop/rails/content_tag.rb

@@ -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

lib/rubocop/cop/rails_cops.rb

@@ -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'

manual/cops.md

@@ -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)

manual/cops_rails.md

@@ -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

spec/rubocop/cop/rails/content_tag_spec.rb

@@ -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