From 8555ac5a9b272aba9c7eb8ef0ecd76baf0a0171f Mon Sep 17 00:00:00 2001 From: Sam Jenkins Date: Sun, 2 Apr 2023 14:36:42 +0100 Subject: [PATCH] [Fix #967] Add new Rails/UnusedRenderContent cop --- .../new_add_unused_render_content_cop.md | 1 + config/default.yml | 6 + .../cop/rails/unused_render_content.rb | 68 +++++++++ lib/rubocop/cop/rails_cops.rb | 1 + .../cop/rails/unused_render_content_spec.rb | 131 ++++++++++++++++++ 5 files changed, 207 insertions(+) create mode 100644 changelog/new_add_unused_render_content_cop.md create mode 100644 lib/rubocop/cop/rails/unused_render_content.rb create mode 100644 spec/rubocop/cop/rails/unused_render_content_spec.rb diff --git a/changelog/new_add_unused_render_content_cop.md b/changelog/new_add_unused_render_content_cop.md new file mode 100644 index 0000000000..58429c7de2 --- /dev/null +++ b/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][]) diff --git a/config/default.yml b/config/default.yml index 66e69da650..465b3665c7 100644 --- a/config/default.yml +++ b/config/default.yml @@ -1149,6 +1149,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: '<>' + Rails/Validation: Description: 'Use validates :attribute, hash of validations.' Enabled: true diff --git a/lib/rubocop/cop/rails/unused_render_content.rb b/lib/rubocop/cop/rails/unused_render_content.rb new file mode 100644 index 0000000000..254c354b71 --- /dev/null +++ b/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 diff --git a/lib/rubocop/cop/rails_cops.rb b/lib/rubocop/cop/rails_cops.rb index 3ffdb8f311..5231787925 100644 --- a/lib/rubocop/cop/rails_cops.rb +++ b/lib/rubocop/cop/rails_cops.rb @@ -128,6 +128,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' diff --git a/spec/rubocop/cop/rails/unused_render_content_spec.rb b/spec/rubocop/cop/rails/unused_render_content_spec.rb new file mode 100644 index 0000000000..b8355a688c --- /dev/null +++ b/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