Merge pull request #970 from samrjenkins/add-unused-render-content-cop

Add new Rails/UnusedRenderContent cop

changelog/new_add_unused_render_content_cop.md

@@ -0,0 +1 @@
+* [#967](https://github.com/rubocop/rubocop-rails/issues/967): Add new `Rails/UnusedRenderContent` cop. ([@samrjenkins][])

config/default.yml

@@ -1155,6 +1155,12 @@ Rails/UnusedIgnoredColumns:
   Include:
     - app/models/**/*.rb
 
+Rails/UnusedRenderContent:
+  Description: 'Do not specify body content for a response with a non-content status code.'
+  Enabled: pending
+  Severity: warning
+  VersionAdded: '<<next>>'
+
 Rails/Validation:
   Description: 'Use validates :attribute, hash of validations.'
   Enabled: true

lib/rubocop/cop/rails/unused_render_content.rb

@@ -0,0 +1,68 @@
+# frozen_string_literal: true
+
+module RuboCop
+  module Cop
+    module Rails
+      # If you try to render content along with a non-content status code (100-199, 204, 205, or 304),
+      # it will be dropped from the response.
+      #
+      # This cop checks for uses of `render` which specify both body content and a non-content status.
+      #
+      # @example
+      #   # bad
+      #   render 'foo', status: :continue
+      #   render status: 100, plain: 'Ruby!'
+      #
+      #   # good
+      #   render status: :continue
+      #   render status: 100
+      class UnusedRenderContent < Base
+        extend AutoCorrector
+        include RangeHelp
+
+        MSG = 'Do not specify body content for a response with a non-content status code'
+        RESTRICT_ON_SEND = %i[render].freeze
+        NON_CONTENT_STATUS_CODES = Set[*100..199, 204, 205, 304] & ::Rack::Utils::SYMBOL_TO_STATUS_CODE.values
+        NON_CONTENT_STATUSES = Set[
+          *::Rack::Utils::SYMBOL_TO_STATUS_CODE.invert.fetch_values(*NON_CONTENT_STATUS_CODES)
+        ]
+        BODY_OPTIONS = Set[
+          :action,
+          :body,
+          :content_type,
+          :file,
+          :html,
+          :inline,
+          :json,
+          :js,
+          :layout,
+          :plain,
+          :raw,
+          :template,
+          :text,
+          :xml
+        ]
+
+        def_node_matcher :non_content_status?, <<~PATTERN
+          (pair
+            (sym :status)
+            {(sym NON_CONTENT_STATUSES) (int NON_CONTENT_STATUS_CODES)}
+          )
+        PATTERN
+
+        def_node_matcher :unused_render_content?, <<~PATTERN
+          (send nil? :render {
+            (hash <#non_content_status? $(pair (sym BODY_OPTIONS) _) ...>) |
+            $({str sym} _) (hash <#non_content_status? ...>)
+          })
+        PATTERN
+
+        def on_send(node)
+          unused_render_content?(node) do |unused_content_node|
+            add_offense(unused_content_node)
+          end
+        end
+      end
+    end
+  end
+end

lib/rubocop/cop/rails_cops.rb

@@ -129,6 +129,7 @@
 require_relative 'rails/unique_validation_without_index'
 require_relative 'rails/unknown_env'
 require_relative 'rails/unused_ignored_columns'
+require_relative 'rails/unused_render_content'
 require_relative 'rails/validation'
 require_relative 'rails/where_equals'
 require_relative 'rails/where_exists'

spec/rubocop/cop/rails/unused_render_content_spec.rb

@@ -0,0 +1,131 @@
+# frozen_string_literal: true
+
+RSpec.describe RuboCop::Cop::Rails::UnusedRenderContent, :config do
+  it 'does not register an offense when specifying body content with a status that takes a body' do
+    expect_no_offenses(<<~RUBY)
+      render status: :ok, plain: 'Ruby!'
+    RUBY
+  end
+
+  it 'does not register an offense when no body content is specified with a status that does not take a body' do
+    expect_no_offenses(<<~RUBY)
+      render status: :no_content
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: :continue and a positional string argument' do
+    expect_offense(<<~RUBY)
+      render 'foo', status: :continue
+             ^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: :switching_protocols and a positional symbol argument across ' \
+     'multiple lines' do
+    expect_offense(<<~RUBY)
+      render(
+        :foo,
+        ^^^^ Do not specify body content for a response with a non-content status code
+        status: :switching_protocols
+      )
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: :processing and an :action option as the last argument' do
+    expect_offense(<<~RUBY)
+      render status: :processing, action: :foo
+                                  ^^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: :early_hints and a :body option as the first argument' do
+    expect_offense(<<~RUBY)
+      render body: 'foo', status: :early_hints
+             ^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: :no_content and a :content_type option between other options' do
+    expect_offense(<<~RUBY)
+      render status: :no_content, content_type: 'foo', another: 'option'
+                                  ^^^^^^^^^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: :reset_content and a :file option' do
+    expect_offense(<<~RUBY)
+      render status: :reset_content, file: 'foo'
+                                     ^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: :not_modified and a :html option' do
+    expect_offense(<<~RUBY)
+      render status: :not_modified, html: 'foo'
+                                    ^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 100 and a :inline option' do
+    expect_offense(<<~RUBY)
+      render status: 100, inline: 'foo'
+                          ^^^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 101 and a :json option' do
+    expect_offense(<<~RUBY)
+      render status: 101, json: 'foo'
+                          ^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 102 and a :js option' do
+    expect_offense(<<~RUBY)
+      render status: 102, js: 'foo'
+                          ^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 103 and a :layout option' do
+    expect_offense(<<~RUBY)
+      render status: 103, layout: 'foo'
+                          ^^^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 204 and a :plain option' do
+    expect_offense(<<~RUBY)
+      render status: 204, plain: 'foo'
+                          ^^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 205 and a :raw option' do
+    expect_offense(<<~RUBY)
+      render status: 205, raw: 'foo'
+                          ^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 304 and a :template option' do
+    expect_offense(<<~RUBY)
+      render status: 304, template: 'foo'
+                          ^^^^^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 304 and a :text option' do
+    expect_offense(<<~RUBY)
+      render status: 304, text: 'foo'
+                          ^^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+
+  it 'registers an offense when specifying status: 304 and a :xml option' do
+    expect_offense(<<~RUBY)
+      render status: 304, xml: 'foo'
+                          ^^^^^^^^^^ Do not specify body content for a response with a non-content status code
+    RUBY
+  end
+end