Make constantize look down the ancestor chain (excluding Object)

[marcandre]

constantize is meant to avoid looking up in Object, e.g. so that "String::Fixnum".constantize raises an error.

Unfortunately, the ancestor chain is not looked up at all.

class A
  Foo = :bar
end
class B < A
end
"B::Foo".constantize # => raises an error, should return B::Foo (which is == A::Foo)

I ran into this when registering an observer (say "some_class/observer"), where SomeClass::Observer is defined in a base class of SomeClass. That currently fails, as Observer has to be defined directly in SomeClass.

One solution is for me to write:

class SomeClass < SomeBaseClass
  Observer = Observer # to satisfy Rails constantize lookup!!
end

I hope you appreciate the irony :-)

I feel that there is no reason for constantize to be so strict, though. The attached patch fixes this.

Thanks.

[jeremy]

👍

/cc @wycats @tenderlove

[pixeltrix]

I'm 👎 on this because the ancestors chain can be quite large so could have a negative performance impact, e.g:

>> class Foo < ActiveRecord::Base; end
>> Foo.ancestors.size
=> 67

Also as implemented it wouldn't detect constants in the ancestors chain that appear after Object - the implementation would need to be something like this:

def constantize(camel_cased_word) #:nodoc:
  names = camel_cased_word.split('::')
  names.shift if names.empty? || names.first.empty?

  names.inject(Object) do |constant, name|
    constant = constant.ancestors.detect(constant) do |ancestor|
      ancestor.const_defined?(name) unless ancestor == Object
    end
    constant.const_get(name, false)
  end
end

One possible performance optimisation would be to let ruby search the ancestor chain by passing true for the second parameter and then rejecting any constants returned that have Object as their parent - though I'm not sure whether there are some edge cases around that idea.

[marcandre]

@pixeltrix Thanks for your comment! I pushed a newer, shinier and better version that I dedicate to you :-)

On performance: the patch has basically no impact in cases where the constant seeked is directly owned by class/module, which are the cases handled correctly currently. For cases that are currently not handled properly, then yes, there will be a small performance difference, but slower is better than buggy!

In case the constant does not exist at all (even at the Object level or lower), the newer version will raise immediately, instead of looking down the chain for nothing.

As for the after Object bit, you're absolutely right for the top level search, and there was actually another bug in constantize. Currently, for example, "XML".constantize doesn't work although XML is globally accessible as LibXML is included in Object and has a constant XML. The new patch fixes this.

On the other hand, when not looking up from Object, but from another module, we don't want to lookup past Object. These are globally accessible constants and that's exactly what we don't want to return. E.g. "String::XML".constantize must raise an error.

So thanks to your comments, I've fixed the top level search and also improved the performance of my patch in cases (top level or when constant doesn't exist at all).

[marcandre]

@pixeltrix Oh, I've also included the optimization you suggested, so ancestors are looked up only if the name is globally accessible. If it isn't, then you're right, we know it's somewhere in the ancestry chain before Object.

[pixeltrix]

The return returns the candidate constant from the enclosing method which might not be the end of the chain, e.g:

class Alpha
  class Beta
    class Omega
    end
  end
end

class Delta < Alpha; end
class Beta; end

>> puts "Delta::Beta::Omega".constantize
=> Alpha::Beta

The following implementation handles that - it's basically the same as yours:

def constantize(camel_cased_word)
  names = camel_cased_word.split('::')
  names.shift if names.empty? || names.first.empty?

  names.inject(Object) do |constant, name|
    if constant == Object
      constant.const_get(name)
    else
      candidate = constant.const_get(name)
      next candidate if constant.const_defined?(name, false)
      next candidate unless Object.const_defined?(name)

      constant = constant.ancestors.each do |ancestor|
        break constant if ancestor == Object
        break ancestor if ancestor.const_defined?(name, false)
      end

      constant.const_get(name, false)
    end
  end
end

The two next candidate lines could be written as:

next candidate if constant.const_defined?(name, false) || !Object.const_defined?(name)

I prefer the two lines as it makes the intention easier to see using the unless but I'm not particularly fussed either way.