Rubocop: Style/MutableConstant should detect "late" freeze

[sos4nt]

I have a hash with a default value assigned to a constant like this:

ERROR_CODES = {
  100 => :FooError,
  200 => :BarError,
  300 => :BazError
}
ERROR_CODES.default = :GenericError
ERROR_CODES.freeze

Although the hash is frozen, RuboCop objects that is is not. This is because it is not frozen upon assignment. IMO, RuboCop should also detect a "late" freeze, as shown in the example.

[bbatsov]

That's relatively hard to do, so I don't think we should bother with it. Obviously you can just move the freeze in your code.

[sos4nt]

Obviously you can just move the freeze in your code.

Sorry, I don't see how I could move that freezesomewhere else. 😕

[bbatsov]

default doesn't work for frozen hashes? I thought this was some Hash API, but I guess it's something from ActiveSupport...

[sos4nt]

No it doesn't. Calling Hash#default= is considered a modification of the hash.

The only workaround I can think of is:

ERROR_CODES = {
  100 => :FooError,
  200 => :BarError,
  300 => :BazError
}.tap { |hash| hash.default = :GenericError }.freeze

but I'd rather avoid that hack.

[marcandre]

In case you haven't considered, I like the following:

ERROR_CODES = Hash.new(:GenericError).merge!(
  100 => :FooError,
  200 => :BarError,
  300 => :BazError
).freeze

[sos4nt]

@marcandre nice one, that would work. Unfortunately Rubycop doesn't enforce freeze in that case, it only does so for literals.

[znz]

I use following style to avoid such offense.

error_codes = {
  100 => :FooError,
  200 => :BarError,
  300 => :BazError
}
error_codes.default = :GenericError
ERROR_CODES = error_codes.freeze

[vitormd]

Though I don't believe it is a good idea to kinda "Monkey patch" the hash default value.
I would live it as nil
You could still do:

ERROR_CODES = {
  100 => :FooError,
  200 => :BarError,
  300 => :BazError
}
DEFAULT_ERROR = :GenericError

or

ERROR_CODES = {
  default: :GenericError
  100 => :FooError,
  200 => :BarError,
  300 => :BazError
}

or

ERROR_CODES = {
  100 => :FooError,
  200 => :BarError,
  300 => :BazError
}

def some_single_source_of_truth_method(code)
  ERROR_CODES = {
    100 => :FooError,
    200 => :BarError,
    300 => :BazError
  }.fetch(code) { :GenericError }
end

[bbatsov]

There hasn't been much activity on this ticket and our Core Team is spread too thin on so many tasks, so I'll just close it for the sake of having a cleaner lists of tasks to focus on. We'd gladly review a PR, but it's unlikely that'd we're going to tackle this ourselves in the foreseeable future.

The problem seems to be pretty rare in practice, so probably just some exclusion would be fine for the people affected by it.