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.

Sign up for GitHub

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

Jump to bottom

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

Closed
samrjenkins opened this issue Mar 30, 2023 · 5 comments
Closed

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

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 •
edited
Loading

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
[Fix rubocop#967] Add new Rails/UnusedRenderContent cop
b80dc0e
samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Apr 2, 2023
@samrjenkins
[Fix rubocop#967] Add new Rails/UnusedRenderContent cop
6518a87
@samrjenkins samrjenkins mentioned this issue Apr 2, 2023
Merged
9 tasks
samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Apr 7, 2023
@samrjenkins
[Fix rubocop#967] Add new Rails/UnusedRenderContent cop
86fe4df
samrjenkins added a commit to samrjenkins/rubocop-rails that referenced this issue Apr 7, 2023
@samrjenkins
[Fix rubocop#967] Add new Rails/UnusedRenderContent cop
fac79f3
@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
@samrjenkins
[Fix rubocop#967] Add new Rails/UnusedRenderContent cop
e12b63a
@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
Assignees
No one assigned
Labels
feature request Request for new functionality
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@koic @samrjenkins @vlad-pisanov