Rails/UnusedRenderContent suggests code that does not work in Rails 6.1

[shepmaster]

Expected behavior

The lint should not suggest code that is invalid.

Actual behavior

The lint suggests code that is invalid in Rails 6.1.

Steps to reproduce the problem

Create a fresh Rails 6.1 application:

mkdir repro
cd repro
rvm use ruby-3.2.2
rvm gemset create repro
gem update --system
gem install rails --version 6.1.7.6
rails new repro
cd repro
rails s

Create a basic controller and remove the views for the index:

rails g scaffold animals
rm app/views/animals/index.*

Change the index to return status: :no_content with JSON:

def index
  render status: :no_content, json: {}
end

Request the index page:

curl 127.0.0.1:3000/animals

Note the success:

Started GET "/animals" for 127.0.0.1 at 2023-09-11 12:39:16 -0400
Processing by AnimalsController#index as */*
Completed 204 No Content in 0ms (Views: 0.1ms | Allocations: 363)

Run RuboCop-Rails

% rubocop --require rubocop-rails --only Rails/UnusedRenderContent

Offenses:

app/controllers/animals_controller.rb:6:33: W: Rails/UnusedRenderContent: Do not specify body content for a response with a non-content status code
    render status: :no_content, json: {}
                                ^^^^^^^^

Make the change, request the page, and note the error:

Started GET "/animals" for 127.0.0.1 at 2023-09-11 12:44:15 -0400
Processing by AnimalsController#index as */*
Completed 500 Internal Server Error in 4ms (ActiveRecord: 0.0ms | Allocations: 3482)

ActionView::MissingTemplate (Missing template animals/index, application/index with {:locale=>[:en], :formats=>[:html, :text, :js, :css, :ics, :csv, :vcf, :vtt, :png, :jpeg, :gif, :bmp, :tiff, :svg, :mpeg, :mp3, :ogg, :m4a, :webm, :mp4, :otf, :ttf, :woff, :woff2, :xml, :rss, :atom, :yaml, :multipart_form, :url_encoded_form, :json, :pdf, :zip, :gzip], :variants=>[], :handlers=>[:raw, :erb, :html, :builder, :ruby, :jbuilder]}.

RuboCop version

1.56.3 (using Parser 3.2.2.3, rubocop-ast 1.29.0, running on ruby 3.2.2) [arm64-darwin22]

RuboCop-Rails version

2.21.0

[shepmaster]

/cc @samrjenkins

[shepmaster]

If my analysis is correct, perhaps the simplest thing is to restrict this lint to Rails >= 7?

[samrjenkins]

@shepmaster thanks for tagging me. I will get back to you shortly

[samrjenkins]

@shepmaster I apologise for the delay. It is the evening in the UK and I am starting to wind down 😴

I actually developed this cop mainly against Rails 6.1.7. I have also had a play around with the example code you provided.

I think that the check this cop performs is still valid. However, the examples I provided in the documentation are not as helpful as they could be. When no content is provided to render as an argument, it appears Rails will always try to find a corresponding view template. Hence the ActionView::MissingTemplate error.

I think the guidance in the documentation should probably promote the use of head instead of render in order to serve a response with headers only and to avoid Rails attempting to find a matching view template.

In your particular example, I believe that the way to address the linting offence without seeing the ActionView::MissingTemplate error is to change your controller action from

def index
  render status: :no_content, json: {}
end

to

def index
  head :no_content
end

Please can you try this and let me know your thoughts?

[shepmaster]

I can certainly try that and get back to you. Out of curiosity, does your app have templates for controllers you use :no_content for? If so, why?

[samrjenkins]

The app which prompted me to write this cop does not have templates for endpoints which respond with no_content. As I think you suspect, writing a template for an endpoint which issues a no_content response would be redundant and probably quite misleading for other engineers who might assume the template is being rendered in the response body.

However, we did have one legacy endpoint which was of the form:

def update
  render status: :no_content, json: { error_message: 'example error message'  }
end

which was obviously misleading because it gives the impression that the response will contain a JSON body when, in reality, Rails will drop the body content from the response.

We changed this endpoint:

def show
  render status: :unprocessable_entity, json: { error_message: 'example error message'  }
end

[shepmaster]

I can certainly try that and get back to you

Switching to head seemed to work in our case, so I'd 👍 making that the cop's suggestion.

does not have templates for endpoints which respond with no_content

I'm not following when the current solution render status: :no_content would work — is it only when there is a template on disk? That seems like an even stranger case to be in... a template that is completely unused.

[samrjenkins]

Right, it's the morning! I have a clearer brain 🤓

You are correct. The current suggested solution, for example:

class UsersController < ActionController::Base
  def index
    render status: :no_content
  end
end

will only work when there is a matching template file such as views/users/index.html.erb. If a matching template file is not found, it would appear that you will see the ActionView::MissingTemplate error.

It seems like, in reality, there is no scenario when you would want to use render status: :no_content.

I think I feel the same way as you about having an unused template. This seems like an anti pattern which is misleading and should be avoided.

I'll try to take a look at the documentation and examples today. Thanks for raising this 😊

[samrjenkins]

@shepmaster I have revised the documentation on my fork. I will build a pull request later today.

[jaredbeck]

Personally, I prefer render json: '', status: 204 over head :no_content, because I (and my non-Ruby colleagues) have memorized what 204 means, but we have not memorized the symbols (Rack::Utils::SYMBOL_TO_STATUS_CODE).

Actually, I'd really like to use render nothing: true, status: 204. That used to be my favorite, before the nothing option was removed.

If they added support for head(204) I'd use it. :)

[samrjenkins]

@jaredbeck i believe that head accepts either a status code or a symbol as an argument. I believe your example (head(204)) should work.