Advising to use each_key if the block modifies the hash

[owst]

Let's say we're creating something like Rails' indifferent access for hashes:

> cat foo.rb
def my_with_indifferent_access(hash)
  hash.keys.each do |k|
    hash[k.to_sym] = hash[k] if k.is_a?(String)
  end

  hash
end

puts my_with_indifferent_access({ 'a' => 1 })[:a]

> ruby foo.rb
1

fasterer 0.3.2 says

Hash#keys.each is slower than Hash#each_key. Occurred at lines: 2.

but making that change gives a runtime error:

> ruby foo.rb
foo.rb:3:in `block in my_with_indifferent_access': can't add a new key into hash during iteration (RuntimeError)
        from foo.rb:2:in `each_key'
        from foo.rb:2:in `my_with_indifferent_access'
        from foo.rb:9:in `<main>'

While it would be great if fasterer could inspect the block of the each and attempt to determine if the hash is modified during the iteration (not outputting the warning if so), this isn't possible in general. Furthermore, looking at the current implementation, it doesn't look like any of the necessary context is available where the warning is raised.

A better solution might be to explicitly warn that blindly replacing keys.each with each_key is not safe in general, perhaps something like:

Hash#keys.each is slower than Hash#each_key. N.B. 'Hash#each_key' cannot be used if the hash is modified during theeachblock. Occurred at lines: 2.

[DamirSvrtan]

Hey @owst, thanks for reporting this issue. I have updated the error message and it's in the new version of the gem: 0.4.0.