Hello.

I'm quite uncertain about the deprecation of the content_tag helper method targeted by the new Rails/ContentTag cop in #242 as it seems to me, reading Rails source code, that only the legacy tag method is deprecated.

The difference between tag and content_tag helper methods

When reading the Rails documentation I see that the legacy tag syntax use with arguments is deprecated and will be removed. It is called like so :

tag(:img, src: "/flower.jpg")

<img src="/flower.jpg" />

It returns a tag that is by default not open, and XHTML compliant because it ends with a /> at the end.

When you look at the source of the method, you see why tag is deprecated and will be replaced:

def tag(name = nil, options = nil, open = false, escape = true)
  if name.nil?
    tag_builder
  else
    "<#{name}#{tag_builder.tag_options(options, escape) if options}#{open ? ">" : " />"}".html_safe
  end
end

This method builds its HTML string by itself and checks for name nullity to piggy back on tag_builder

The content_tag helper method on the other hand generates an HTML 5 compliant output with a closing tag at the end.

The source code shows it utilises tag_builder all the time and outputs good HTML code:

# File actionview/lib/action_view/helpers/tag_helper.rb, line 270
def content_tag(name, content_or_options_with_block = nil, options = nil, escape = true, &block)
  if block_given?
    options = content_or_options_with_block if content_or_options_with_block.is_a?(Hash)
    tag_builder.content_tag_string(name, capture(&block), options, escape)
  else
    tag_builder.content_tag_string(name, content_or_options_with_block, options, escape)
  end
end

Solution

I'm proposing changing this cop to only warn of usage of the the tag helper method with arguments like :

tag :br

And not when used with the argument-less syntax.

tag.br

Why I think it matters

Usage of tag.something is way slower than using content_tag(:something) as you can see with this simple benchmark :

50_000.times { tag.span("hey") } }
# 132.25800002692267

50_000.times { content_tag(:span, "hey") } }
# 69.53899998916313

50_000.times { tag.span { "hey" } } }
# 199.94600000791252

50_000.times { content_tag(:span) { "hey" } } }
# 161.7930000065826

You can reproduce it by dropping this code inside any view :

Rails.logger.debug %(50_000.times { tag.span("hey") } })
Rails.logger.debug(Benchmark.ms { 50_000.times { tag.span("hey") } })

Rails.logger.debug %(50_000.times { tag.span { "hey" } } })
Rails.logger.debug(Benchmark.ms { 50_000.times { tag.span { "hey" } } })

Rails.logger.debug %(50_000.times { content_tag(:span, "hey") } })
Rails.logger.debug(Benchmark.ms { 50_000.times { content_tag(:span, "hey") } })

Rails.logger.debug %(50_000.times { content_tag(:span) { "hey" } } })
Rails.logger.debug(Benchmark.ms { 50_000.times { content_tag(:span) { "hey" } } })

Avoiding suggesting to switch to an almost 2 times slower method would save a ton of CPU time in thousands of Rails applications. The framework is not particularly focused on pure speed, so every little bit of performance improvements helps, I think.

Why tag.something is slow

The new tag helper relies method_missing on tag_builder which is very slow. compared to passing a string.

https://github.com/rails/rails/blob/a5cda0407fffe4214db3c40487d9d6394b391bc1/actionview/lib/action_view/helpers/tag_helper.rb#L119