Skip to content

Commit

Permalink
Merge pull request rails#44611 from Shopify/no-enumerable-constants
Browse files Browse the repository at this point in the history
Avoid adding constants to Enumerable
  • Loading branch information
byroot committed Mar 4, 2022
2 parents bd4d97c + 25396b1 commit b783e8a
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 9 deletions.
35 changes: 26 additions & 9 deletions activesupport/lib/active_support/core_ext/enumerable.rb
@@ -1,13 +1,30 @@
# frozen_string_literal: true

module Enumerable
INDEX_WITH_DEFAULT = Object.new
private_constant :INDEX_WITH_DEFAULT
module ActiveSupport
module EnumerableCoreExt # :nodoc:
module Constants
private
def const_missing(name)
if name == :SoleItemExpectedError
::ActiveSupport::EnumerableCoreExt::SoleItemExpectedError
else
super
end
end
end
end
end

module Enumerable
# Error generated by +sole+ when called on an enumerable that doesn't have
# exactly one item.
class SoleItemExpectedError < StandardError; end

# HACK: For performance reasons, Enumerable shouldn't have any constants of its own.
# So we move SoleItemExpectedError into ActiveSupport::EnumerableCoreExt.
ActiveSupport::EnumerableCoreExt::SoleItemExpectedError = remove_const(:SoleItemExpectedError)
singleton_class.prepend(ActiveSupport::EnumerableCoreExt::Constants)

# Enumerable#sum was added in Ruby 2.4, but it only works with Numeric elements
# when we omit an identity.

Expand Down Expand Up @@ -106,17 +123,17 @@ def index_by
#
# %i( created_at updated_at ).index_with(Time.now)
# # => { created_at: 2020-03-09 22:31:47, updated_at: 2020-03-09 22:31:47 }
def index_with(default = INDEX_WITH_DEFAULT)
def index_with(default = (no_default = true))
if block_given?
result = {}
each { |elem| result[elem] = yield(elem) }
result
elsif default != INDEX_WITH_DEFAULT
elsif no_default
to_enum(:index_with) { size if respond_to?(:size) }
else
result = {}
each { |elem| result[elem] = default }
result
else
to_enum(:index_with) { size if respond_to?(:size) }
end
end

Expand Down Expand Up @@ -240,8 +257,8 @@ def in_order_of(key, series)
def sole
case count
when 1 then return first # rubocop:disable Style/RedundantReturn
when 0 then raise SoleItemExpectedError, "no item found"
when 2.. then raise SoleItemExpectedError, "multiple items found"
when 0 then raise ActiveSupport::EnumerableCoreExt::SoleItemExpectedError, "no item found"
when 2.. then raise ActiveSupport::EnumerableCoreExt::SoleItemExpectedError, "multiple items found"
end
end
end
Expand Down
9 changes: 9 additions & 0 deletions activesupport/test/core_ext/enumerable_test.rb
Expand Up @@ -360,4 +360,13 @@ def test_sole
assert_raise(expected_raise) { GenericEnumerable.new([1, 2]).sole }
assert_raise(expected_raise) { GenericEnumerable.new([1, nil]).sole }
end

def test_doesnt_bust_constant_cache
skip "Only applies to MRI" unless defined?(RubyVM.stat) && RubyVM.stat(:global_constant_state)

object = Object.new
assert_no_difference -> { RubyVM.stat(:global_constant_state) } do
object.extend(Enumerable)
end
end
end

0 comments on commit b783e8a

Please sign in to comment.