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

New cop idea: specifying response content when rendering a 204 No Content response #967

Closed
samrjenkins opened this issue Mar 30, 2023 · 5 comments
Labels
feature request Request for new functionality

Comments

@samrjenkins
Copy link
Contributor

samrjenkins commented Mar 30, 2023

Is your feature request related to a problem? Please describe.

Occasionally I see Rails engineers specify a 204 No Content response within a controller action along with content for the response's body:

render json: { a_name: 'a value' }, status: :no_content

This is misleading. As specified in the Rails docs (https://guides.rubyonrails.org/layouts_and_rendering.html):

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.

and can give engineers the impression that their response body will contain the content they specify.

Describe the solution you'd like

A new cop to register an offence whenever body content is specified for a response with a no-content status code.
I would be very happy to work on such a cop.

Describe alternatives you've considered

A clear and concise description of any alternative solutions or features you've considered.

Additional context

Add any other context or screenshots about the feature request here.

@vlad-pisanov
Copy link
Contributor

Should such a cop recommend head :no_content instead?

@samrjenkins
Copy link
Contributor Author

I am unsure what is best practice in this regard. I suppose head is slightly more explicit as to the lack of body than render.

Interested to hear other opinions.

@koic
Copy link
Member

koic commented Mar 30, 2023

I have never encountered a case like this (it may be lucky 😅 ). Can you show expected bad and good case example codes in practice?

@samrjenkins
Copy link
Contributor Author

@koic of course.

# Bad
render status: :continue, plain: 'content for the response body'
render status: :no_content, html: helpers.tag.strong('content for the response body')
render status: :reset_content, json: { content_for: 'the response body' }
render status: :not_modified, xml: { content_for: 'the response body' }.to_xml

# Good
render status: :continue
render status: :no_content
render status: :reset_content
render status: :not_modified

# Also good
head :continue
head :no_content
head :reset_content
head :not_modified

samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Apr 2, 2023
samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Apr 2, 2023
samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Apr 7, 2023
samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Apr 7, 2023
@samrjenkins
Copy link
Contributor Author

@koic I have spun up a PR here: #970

samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Sep 6, 2023
@koic koic added the feature request Request for new functionality label Sep 7, 2023
@koic koic closed this as completed in 8555ac5 Sep 7, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Request for new functionality
Projects
None yet
Development

No branches or pull requests

3 participants