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/RenderPlain cop #276

Merged
merged 1 commit into from
Jul 3, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### New features

* [#283](https://github.com/rubocop-hq/rubocop-rails/pull/283): Add new `Rails/FindById` cop. ([@fatkodima][])
* [#276](https://github.com/rubocop-hq/rubocop-rails/pull/276): Add new `Rails/RenderPlainText` cop. ([@fatkodima][])
* [#275](https://github.com/rubocop-hq/rubocop-rails/pull/275): Add new `Rails/MatchRoute` cop. ([@fatkodima][])
* [#271](https://github.com/rubocop-hq/rubocop-rails/pull/271): Add new `Rails/RenderInline` cop. ([@fatkodima][])
* [#281](https://github.com/rubocop-hq/rubocop-rails/pull/281): Add new `Rails/MailerName` cop. ([@fatkodima][])
Expand Down
8 changes: 8 additions & 0 deletions config/default.yml
Original file line number Diff line number Diff line change
Expand Up @@ -490,6 +490,14 @@ Rails/RenderInline:
Enabled: 'pending'
VersionAdded: '2.7'

Rails/RenderPlainText:
Description: 'Prefer `render plain:` over `render text:`.'
StyleGuide: 'https://rails.rubystyle.guide/#plain-text-rendering'
Enabled: 'pending'
VersionAdded: '2.7'
# Convert only when `content_type` is explicitly set to `text/plain`.
ContentTypeCompatibility: true
fatkodima marked this conversation as resolved.
Show resolved Hide resolved

Rails/RequestReferer:
Description: 'Use consistent syntax for request.referer.'
Enabled: true
Expand Down
1 change: 1 addition & 0 deletions docs/modules/ROOT/pages/cops.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
* xref:cops_rails.adoc#railsrefutemethods[Rails/RefuteMethods]
* xref:cops_rails.adoc#railsrelativedateconstant[Rails/RelativeDateConstant]
* xref:cops_rails.adoc#railsrenderinline[Rails/RenderInline]
* xref:cops_rails.adoc#railsrenderplaintext[Rails/RenderPlainText]
* xref:cops_rails.adoc#railsrequestreferer[Rails/RequestReferer]
* xref:cops_rails.adoc#railsreversiblemigration[Rails/ReversibleMigration]
* xref:cops_rails.adoc#railssafenavigation[Rails/SafeNavigation]
Expand Down
59 changes: 59 additions & 0 deletions docs/modules/ROOT/pages/cops_rails.adoc
Original file line number Diff line number Diff line change
Expand Up @@ -2900,6 +2900,65 @@ end

* https://rails.rubystyle.guide/#inline-rendering

== Rails/RenderPlainText

|===
| Enabled by default | Safe | Supports autocorrection | VersionAdded | VersionChanged

| Pending
| Yes
| Yes
| 2.7
| -
|===

This cop identifies places where `render text:` can be
replaced with `render plain:`.

=== Examples

[source,ruby]
----
# bad - explicit MIME type to `text/plain`
render text: 'Ruby!', content_type: 'text/plain'

# good - short and precise
render plain: 'Ruby!'

# good - explicit MIME type not to `text/plain`
render text: 'Ruby!', content_type: 'text/html'
----

==== ContentTypeCompatibility: true (default)

[source,ruby]
----
# good - sets MIME type to `text/html`
render text: 'Ruby!'
----

==== ContentTypeCompatibility: false

[source,ruby]
----
# bad - sets MIME type to `text/html`
render text: 'Ruby!'
----

=== Configurable attributes

|===
| Name | Default value | Configurable values

| ContentTypeCompatibility
| `true`
| Boolean
|===

=== References

* https://rails.rubystyle.guide/#plain-text-rendering

== Rails/RequestReferer

|===
Expand Down
76 changes: 76 additions & 0 deletions lib/rubocop/cop/rails/render_plain_text.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,76 @@
# frozen_string_literal: true

module RuboCop
module Cop
module Rails
# This cop identifies places where `render text:` can be
# replaced with `render plain:`.
#
# @example
# # bad - explicit MIME type to `text/plain`
# render text: 'Ruby!', content_type: 'text/plain'
#
# # good - short and precise
# render plain: 'Ruby!'
#
# # good - explicit MIME type not to `text/plain`
# render text: 'Ruby!', content_type: 'text/html'
#
# @example ContentTypeCompatibility: true (default)
# # good - sets MIME type to `text/html`
# render text: 'Ruby!'
#
# @example ContentTypeCompatibility: false
# # bad - sets MIME type to `text/html`
# render text: 'Ruby!'
#
class RenderPlainText < Cop
MSG = 'Prefer `render plain:` over `render text:`.'

def_node_matcher :render_plain_text?, <<~PATTERN
(send nil? :render $(hash <$(pair (sym :text) $_) ...>))
PATTERN

def on_send(node)
render_plain_text?(node) do |options_node, _option_node, _option_value|
content_type_node = find_content_type(options_node)
add_offense(node) if compatible_content_type?(content_type_node)
end
end

def autocorrect(node)
render_plain_text?(node) do |options_node, option_node, option_value|
content_type_node = find_content_type(options_node)
rest_options = options_node.pairs - [option_node, content_type_node].compact

lambda do |corrector|
corrector.replace(
node,
replacement(rest_options, option_value)
)
end
end
end

private

def find_content_type(node)
node.pairs.find { |p| p.key.value.to_sym == :content_type }
end

def compatible_content_type?(node)
(node && node.value.value == 'text/plain') ||
(!node && !cop_config['ContentTypeCompatibility'])
end

def replacement(rest_options, option_value)
if rest_options.any?
"render plain: #{option_value.source}, #{rest_options.map(&:source).join(', ')}"
else
"render plain: #{option_value.source}"
end
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 @@ -62,6 +62,7 @@
require_relative 'rails/refute_methods'
require_relative 'rails/relative_date_constant'
require_relative 'rails/render_inline'
require_relative 'rails/render_plain_text'
require_relative 'rails/request_referer'
require_relative 'rails/reversible_migration'
require_relative 'rails/safe_navigation'
Expand Down
63 changes: 63 additions & 0 deletions spec/rubocop/cop/rails/render_plain_text_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
# frozen_string_literal: true

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

shared_examples 'checks_common_offense' do
it 'registers an offense and corrects when using `render text:` with `content_type: "text/plain"`' do
expect_offense(<<~RUBY)
render text: 'Ruby!', content_type: 'text/plain'
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Prefer `render plain:` over `render text:`.
RUBY

expect_correction(<<~RUBY)
render plain: 'Ruby!'
RUBY
end

it 'does not register an offense when using `render text:` with `content_type: "text/html"`' do
expect_no_offenses(<<~RUBY)
render text: 'Ruby!', content_type: 'text/html'
RUBY
end

it 'does not register an offense when using `render plain:`' do
expect_no_offenses(<<~RUBY)
render plain: 'Ruby!'
RUBY
end
end

context 'when ContentTypeCompatibility set to true' do
fatkodima marked this conversation as resolved.
Show resolved Hide resolved
let(:cop_config) do
{ 'ContentTypeCompatibility' => true }
end

it 'does not register an offense when using `render text:`' do
expect_no_offenses(<<~RUBY)
render text: 'Ruby!'
RUBY
end

it_behaves_like('checks_common_offense')
end

context 'when ContentTypeCompatibility set to false' do
let(:cop_config) do
{ 'ContentTypeCompatibility' => false }
end

it 'registers an offense and corrects when using `render text:`' do
expect_offense(<<~RUBY)
render text: 'Ruby!'
^^^^^^^^^^^^^^^^^^^^ Prefer `render plain:` over `render text:`.
RUBY

expect_correction(<<~RUBY)
render plain: 'Ruby!'
RUBY
end

it_behaves_like('checks_common_offense')
end
end