Rubocop: Remove unnecessary arguments for builtin methods.

[Lykos]

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

I found something like array.join('') in my code recently. This can of course be replaced by array.join. There is also string.split("\n") or string.gsub(a, ''). I will try to collect more cases like this in this issue (help is welcome).

Describe the solution you'd like

For a list of important builtin functions (e.g. String#split and Array#join), if an argument is passed explicitly that leads to the same behavior as leaving the argument away, suggest to remove that argument.

Additional context

Note that I might do this myself eventually. Depends on how much time I have and how eager someone else is to take it.

[tejasbubane]

With static analysis, rubocop cannot determine if the receiver is array or string. So this will only work for literals like [1, 2, 3].join('') which is rather rare IMO. Does it still make sense to add such a cop?

PS: I am willing to work on this if maintainers agree on the functionality.

[Lykos]

I think we should just assume that it's an array (or similar) type in case of join. It's true that you will only get 100% correctness if you know the types. (And furthermore, you need to know the state of those types, someone might have monkey patched String#join for example) However, we have similar issues for most of the cops:

• All the suggestions for numeric types, e.g. replacing a == 0 with a.zero?, assume that the type is some sort of numeric type.
• All the suggestions for enumerable types, e.g. replacing a.select { |i| !p(i) } with a.reject { |i| p(i) }, assume that the type has the same meaning for select and reject as arrays.

This cop would be in line with that. In most cases, for a.join, a would either be some sort of Enumerable or at least have a definition of join that is compatible. So it would work fine for those cases. It would get some corner cases incorrect where you define a custom join method somewhere with a different semantic. But that can happen to most cops and that's what # rubocop:disable is for. If someones codebase has a lot of these, they should disable the cop. That's in line with existing cops.

Only restricting this cop on literals or cases where we are 100% sure about the receiver would make this cop almost useless.

So I would suggest just making the assumption that join, split (and whatever others come into our mind) are called on builtin types (or at least defined in line with the builtin ones).

[Drenmi]

I think the request is fair. We should just make sure to mark it as unsafe. 👍

[koic]

The default argument for Array#join is $, instead of blank string.
https://ruby-doc.org/core-2.7.0/Array.html#method-i-join

This cop seems undesirable, as it is not redundant that blank string is specified as an explicit argument.

[pocke]

This cop seems undesirable, as it is not redundant that blank string is specified as an explicit argument.

I don't think so.
Anyone no longer uses$, and it has been deprecated in Ruby 2.7.
https://bugs.ruby-lang.org/issues/14240

So actually we can ignore $,, it means we can be aware of Array#join as def join(delm = '').

[koic]

Ah, I didn't know this deprecation warning.

% rbenv local 2.6.5 && ruby -ve '$, = ","'
ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-darwin17]
% rbenv local 2.7.0 && ruby -ve '$, = ","'
ruby 2.7.0p0 (2019-12-25 revision 647ee6f091) [x86_64-darwin17]
-e:1: warning: `$,' is deprecated

Surely, it seems that blank string is fine for default argument.

[stale[bot]]

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[bot]]

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