rubocop / rubocop-rails

A RuboCop extension focused on enforcing Rails best practices and coding conventions.
https://docs.rubocop.org/rubocop-rails
MIT License
803 stars 257 forks source link

New cop that detects `redirect_to '/'` #856

Open anthony-robin opened 1 year ago

anthony-robin commented 1 year ago

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

I often see in projects code such as redirect_to '/', alert: 'Lorem ipsum' which is not the most elegant writing. IMO the Rails route helper root_path seems a better way to handle it instead of hardcoding /.

Describe the solution you'd like

I would like a cop that throw an offense on this above line and suggest a replacement to redirect_to root_path, alert: 'Lorem ispum':

# Flag an offense
def index
  redirect_to '/', alert: 'Lorem ipsum'
end

# Autocorrect it
def index
  redirect_to root_path, alert: 'Lorem ipsum'
end

What do you think about this idea ? Thank you :)

andyw8 commented 1 year ago

Is there perhaps a more general rule here of not allowing redirect_to with a string literal?

vlad-pisanov commented 1 year ago

@andyw8 sometimes it's necessary to redirect_to 'https://some-other-website.com'

andyw8 commented 1 year ago

True, so perhaps it should only allow a string if it's a full URL (i.e. beginning http).

andyw8 commented 1 year ago

This is actually kind of related to https://github.com/rubocop/rubocop-rails/issues/856 but enforcing the opposite.

koic commented 1 year ago

@andyw8 Is the link #856 wrong?

andyw8 commented 1 year ago

Yes! But I can't find the one I meant, I think it was about named routes in tests.

andyw8 commented 1 year ago

Ah, this I think: https://github.com/rubocop/rails-style-guide/issues/328