Consider preventing setting instance variables in actions followed by redirects

Everyone can contribute. Help move this issue forward while earning points, leveling up and collecting rewards.

gitlab-org/release/retrospectives#2 (closed) describes an incident where a feature was broken due to the use of redirect_to instead of render_X. The feature was broken because an instance variable was set and had to be present, but a redirect results in a new request, resulting in the instance variable not being present.

To prevent this from happening again, we should consider adding a RuboCop cop that prevents an action from:

  1. Assigning an instance variable
  2. Calling redirect_to

In other words, patterns like this would be blacklisted:

def foo
  @number = 10

  redirect_to :something
end

If we want to get really fancy, we could extend the cop to only flag cases where the target method (:something here) actually reads one of the defined instance variables. This however might be difficult to implement.

There might be false positives where we set a variable and never need it after the redirect, but in those cases we should refactor the code to not use instance variables, as there is no point in doing so anyway.

Edited by 🤖 GitLab Bot 🤖