Detect useless assignment and return

[tjwallace]

Given this code, I'd like RuboCop to detect that the assignment is useless and autocorrect the code to remove the assignment.

# bad
def foo(input)
  output = input.sort
  output
end

# good
def foo(input)
  input.sort
end

I'm not sure if this is a new cop or an addition to the existing Link/UselessAssignment cop.

[frostmark]

I will try to make it

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution and understanding!

[stale]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

[tjwallace]

This issues been automatically closed due to lack of activity. Feel free to re-open it if you ever come back to it.

😞

[marcandre]

@tjwallace I don't particularly like stale bot either. You are free to reopen this.

Are you sure this is a pattern that happens enough to warrant a check?

Note that there can not be any code between output = ... and output (otherwise there could be side effects), so I think it's an easy node pattern, like (... (lvasgn _name _expression) (lvar _name)), call that on_begin (and alias on_kwbegin) and you have your cop...

[fatkodima]

Can confirm this is a popular pattern - I have seen those many times. Also I have found 3 offenses in rails codebase and 1 in rubocops own.

[gurgeous]

Sorry, upgrading late here... This cop might be a bit aggressive. We have tons of data manipulation code like this:

def normalize(input)
  string = input
  string = string.gsub(...)
  string = string.split.first
  string = string.upcase
  string
end

Stylistically there are many ways to approach this, but removing the final assignment is probably not helpful. Thanks for the new rule, we would be happy to enable if it was slightly less aggressive.

[marcandre]

Maybe I'm taking your example too literally, but isn't it all much easier to understand without the string?

def normalize(input)
  input
    .gsub(...)
    .split.first
    .upcase
end

[gurgeous]

Sometimes we use method chaining, but often we avoid it. I probably should've been more literal with my example. This is still made up, but more realistic. This pattern makes it easy to document, reorder, etc. Quite common with data processing IMO.

def normalize(input)
  string = input

  # clean up whitespace
  string = string.gsub(...)

  # grab the first word
  string = string.split.first

  # normalize case to use as hash key
  string = string.upcase

  string
end

[marcandre]

Inline comments work well too, reordering is not an issue either... 😅

def normalize(input)
  input
    .string.gsub(...) # clean up whitespace
    .split.first      # grab the first word
    .upcase           # normalize case to use as hash key
end