dblock / ruby-enum

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

NameError: wrong constant name default on 0.7.1 #17

Closed gjtorikian closed 7 years ago

gjtorikian commented 7 years ago

Given a Gemfile like:

source 'https://rubygems.org/'

gem 'ruby-enum', '0.7.1'
gem 'rake'

And a Rakefile like:

task :go do
  require 'ruby-enum'

  class Parse
    include Ruby::Enum

    define :default, 0
    define :normalize, (1 << 8)
    define :validate_utf8, (1 << 9)
    define :smart, (1 << 10)
  end

  puts Parse.to_h[:smart]
end

Running rake go bombs on 0.7.1:

rake aborted!
NameError: wrong constant name default
/Users/gjtorikian/Desktop/foo/Rakefile:7:in `<class:Parse>'
/Users/gjtorikian/Desktop/foo/Rakefile:4:in `block in <top (required)>'
/Users/gjtorikian/.rbenv/versions/2.2.3/bin/bundle:23:in `load'
/Users/gjtorikian/.rbenv/versions/2.2.3/bin/bundle:23:in `<main>'
Tasks: TOP => go
(See full trace by running task with --trace)

Versions 0.7.0 and lower worked as expected, though.

dblock commented 7 years ago

@laertispappas suggestions?

laertispappas commented 7 years ago

@gjtorikian Thanks for the report.

default or any literal that starts with lower case is not a valid constant in Ruby since it needs to start with an upper case letter. After replacing const_missing with const_set we no longer support lower case enum names.

We can address the problem by rescuing the NameError exception and defining an Enum method like this:

def define (key, value)
  # ...
  const_set key, value
rescue NameError
  define_singleton_method(key) { value }
end   

@dblock With the above Parse::default or Parse.default can be accessed. I don't know how clear this solution is though since our current implementation use constants as it's internal structure. I think we should force users to use constants when defining enums and raise a meaningful error message if he tries to define an enum with the first char in lowercase.

dblock commented 7 years ago

Didn't realize that this would become a limitation. Maybe we can support both with a Ruby::Enum::Const and Ruby::Enum::Set, then alias the latter to Ruby::Enum?

laertispappas commented 7 years ago

@dblock I don't understand what you are proposing. Can you give an example?

dblock commented 7 years ago

Right now we have a single implementation, Ruby::Enum, but we could very well have two that define in different ways, one with const_set and another with the old way. So someone who wants to use lowercase symbols could still use the old style module.

laertispappas commented 7 years ago

Agree with you it's the implementation tha concerns me. If no one is working on this I will fork a branch to prepare a fix.

laertispappas commented 7 years ago

Fixes in #18 . Released on v0.7.2