Performance/Caller: Clearer message

[koppen]

I have the following line of code

caller[0][/`.*'/]

which RuboCop 0.49.1 flags with

caller.rb:5:11: C: Performance/Caller: Use caller(n..n) instead of caller[n].
    caller[0][/`.*'/]

Looking at the suggested solution I'd expect to change the code to

caller(0..0)[/`.*'/]

but that isn't feature equivalent because

caller[0] #=> "caller.rb:13:in `<main>'"
caller(0..0) #=> ["caller.rb:9:in `modified'"]

caller(0..0) not only returns an Array instead of a String, but even if we use the first item in the returned Array we get a different result. In short; the default message is confusing and potentially misleading (at least to me).

Expected behavior

I'd expect RuboCop to guide me to a correct solution, or not at all. The pipe dream is

Use caller(1..1).first instead of caller[0].

but that's a bit specific to my concrete case, I reckon. Perhaps we could settle for:

Use caller(n..n).first instead of caller[n].

but that might hide the changed behavior too much. More precisely:

Use caller((n + 1)..(n + 1)).first instead of caller[n].

but that's hardly concise or easily understandable. How about some form of

Use arguments with caller instead of returning the full array

¯\_(ツ)_/¯

There is some discussion ongoing on the now closed pull request as well: rubocop/rubocop#4078

Actual behavior

RuboCop suggest

Use caller(n..n) instead of caller[n].

Steps to reproduce the problem

1. Create a file containing caller[0]
2. Run RuboCop on it

RuboCop version

$ rubocop -v
0.49.1

[pocke]

Thanks for the reporting! 👍
I'll work on it.

[sleekweasel]

Isn't this rather the tail wagging the dog? Surely Ruby should be preening its parse tree to optimise this sort of nonsense invisibly - not developers forced to waste brain cells creating increasingly unreadable code.

In any case, when is caller called in time-critical situations? This is like the time someone optimised an RPC call failure mode by microseconds, ignoring that the network call was going to take milliseconds anyway.

[ianfixes]

For those of you arriving via a google search as to what this cryptic message could possibly mean (and why this bizarre syntax is "preferable"), the answer seems to be along these lines:

# bad
caller[0] # builds a large array from which only one element will be extracted

# good
caller(0..0).first # builds a single-element array from which one element will be extracted

I hope the array size is actually something to be concerned about here, because this is a very odd edit for a style checker to suggest.

[sleekweasel]

# bad
caller[0] # extracts only one element after receiving an enormous array 

I think it's more than caller(0..0) BUILDS a tiny array, and caller() builds a huge one, most of which is thrown away. But I maintain that it should be that Ruby's optimiser should notice caller[0] and do the optimisation, not the programmer forced to jump through stupid hoops.

[ianfixes]

You're right, I'm going to update that comment