dblock / ruby-enum

A handy way to define enums in Ruby.
MIT License
178 stars 23 forks source link

Remove const_missing method in favor of const_set #11

Closed dmoss18 closed 7 years ago

dblock commented 8 years ago

Interesting. Could you provide a longer explanation please?

This changes behavior and API. Needs tests, CHANGELOG, UPGRADING, a fixed build, etc.

dmoss18 commented 8 years ago

Longer explanation is method_missing can hijack constants at a higher level. For example:

class MyEnum
  include Ruby::Enum
  define MY_GOOD_CONST, 'good'
end

MY_HIJACKED_CONST = 'hijacked'.freeze
$> MyEnum::MY_GOOD_CONST // good
$> MyEnum::MY_HIJACKED_CONST // hijacked <-- this is bad

Now it appears that my enum contains the const that is defined outside the MyEnum class. This is unexpected and erroneous behavior.

Also, using const_set will raise a NameError instead of the custom error you defined, which is more ideal.

Tests and changelog to follow

dblock commented 8 years ago

How interesting, thanks!

dmoss18 commented 8 years ago

The real risk is having one of your enums get re-defined somewhere else:

class MyEnum
  include Ruby::Enum
  define MY_GOOD_CONST, 'good'
end

MY_GOOD_CONST = 'bad'.freeze
$> MyEnum::MY_GOOD_CONST //bad
dblock commented 7 years ago

@dmoss18 Can you see ^^^ please, would like to get this merged.

dblock commented 7 years ago

Bump @dmoss18

laertispappas commented 7 years ago

@dmoss18 Will you fix this PR? I'm about to implement this if it's OK from you.

dblock commented 7 years ago

@laertispappas you should finish it, haven't heard from @dmoss18 for a while

laertispappas commented 7 years ago

I'm gonna close this one since it's being addressed in #16