Rubocop: Style/ArrayCoercion auto-correction not equal for nil values

[andrewngo]

If a value can be nil, then ArrayCoercion auto-corrected change does not produce an equivalent change. It's a good suggestion but not always valid.

Expected behavior

Don't auto-correct unless certain

Actual behavior

Changes code

Steps to reproduce the problem

test_value = nil

original_result = [test_value] unless test_value.is_a?(Array)
new_result = Array(test_value)

original_result == new_result # false

# original_result [nil]
# new_result []

RuboCop version

Include the output of rubocop -V or bundle exec rubocop -V if using Bundler. Here's an example:

$ [bundle exec] rubocop -V
0.88.0 (using Parser 2.7.1.4, rubocop-ast 0.2.0, running on ruby 2.6.5 x86_64-darwin16)

[marcandre]

Agreed, this cop is, at the very least, not safe. Thanks for raising this issue.

Any object that responds_to? :to_a is an issue:

Array({a: 1}) # => [[:a, 1]]  != [{a: 1}]
Array(Set[1, 2, 3]) # => [1, 2, 3]  != [Set[1, 2, 3]]
Array(1..) # => RangeError ! = [1..]

I'll make it as unsafe but the issue should remain open: does this cop deserve its badge?

[0x0badc0de]

What about using Array.new(1, value)?

irb(main):001:0> Array.new(1, {a: 1})
=> [{:a=>1}]
irb(main):002:0> Array.new(1, Set[1, 2, 3])
=> [#<Set: {1, 2, 3}>]
irb(main):003:0> Array.new(1, 1..)
=> [1..]

[0x0badc0de]

Ah, it won't work for arrays args as expected :(

[0x0badc0de]

This one seems to work https://stackoverflow.com/a/29431497

irb(main):001:0> [{a: 1}].flatten(1)
=> [{:a=>1}]
irb(main):002:0> [Set[1, 2, 3]].flatten(1)
=> [#<Set: {1, 2, 3}>]
irb(main):003:0> [1..].flatten(1)
=> [1..]
irb(main):004:0> [Time.now].flatten(1)
=> [2020-07-27 17:07:10.792690033 +0300]
irb(main):005:0> [[1, 2]].flatten(1)
=> [1, 2]
irb(main):006:0> [[{a:1}, Time.now]].flatten(1)
=> [{:a=>1}, 2020-07-27 17:07:32.01765441 +0300]

[marcandre]

There just isn't a reliable builtin way to do this.

Maybe this cop could enforce the use of proper duck-typing:

# bad?
x = [x] unless x.is_a?(Array)
# good
x = [x] unless x.respond_to? :to_ary

Note: I did mean to use to_ary (implicit conversion to Array), and to_a (explicit conversion) is not good.

Given the fact that to_ary (and how it differs from to_a) is pretty obscure, I think that the best solution is that this cop should be entirely retired. There's nothing inherently wrong with x = [x] unless x.is_a?(Array) and there is not any better and reliable alternative.

[sos4nt]

Just checking whether x responds to to_ary might not be enough, you probably also have to invoke it:

x = x.respond_to?(:to_ary) ? x.to_ary : [x]

IMO, x = [x] unless x.is_a?(Array) is _much_ easier to understand. It almost literally says _"put x in an array unless x is an array"_. I don't think that's "bad" at all.

+1 for retiring this cop

[bbatsov]

There's also the option of simply disabling the cop by default, which is a bit less extreme (at least people can run it on demand then). The fact that relatively few people complained about running into conversion issues leads me to believe that's not a very common problem, but that' definitely a cop that can't be made safe.

[marcandre]

Now disabled by default #8792