Skip to content

Commit

Permalink
No longer make define_method public, use send instead.
Browse files Browse the repository at this point in the history
MemoizedHelpers::ClassMethods module was making define_method
public for ease of use within the module. No longer do this and
use send instead. This fixes rspec#873.
  • Loading branch information
Thomas Holmes committed Apr 18, 2013
1 parent 5200ad6 commit 5280e95
Showing 1 changed file with 2 additions and 4 deletions.
6 changes: 2 additions & 4 deletions lib/rspec/core/memoized_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,7 +192,7 @@ def let(name, &block)
# We have to pass the block directly to `define_method` to
# allow it to use method constructs like `super` and `return`.
raise "#let or #subject called without a block" if block.nil?
MemoizedHelpers.module_for(self).define_method(name, &block)
MemoizedHelpers.module_for(self).send(:define_method, name, &block)

# Apply the memoization. The method has been defined in an ancestor
# module so we can use `super` here to get the value.
Expand Down Expand Up @@ -293,7 +293,7 @@ def subject(name=nil, &block)
let(name, &block)
alias_method :subject, name

self::NamedSubjectPreventSuper.define_method(name) do
self::NamedSubjectPreventSuper.send(:define_method, name) do
raise NotImplementedError, "`super` in named subjects is not supported"
end
else
Expand Down Expand Up @@ -466,13 +466,11 @@ def self.module_for(example_group)
get_constant_or_yield(example_group, :LetDefinitions) do
mod = Module.new do
include Module.new {
public_class_method :define_method
example_group.const_set(:NamedSubjectPreventSuper, self)
}

# Expose `define_method` as a public method, so we can
# easily use it below.
public_class_method :define_method
end

example_group.__send__(:include, mod)
Expand Down

2 comments on commit 5280e95

@JonRowe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Heh, I just worked on this myself to, but I took the alternative approach and fixed the leak.

@thomas-holmes
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! I tried to get that sorted out for a bit, but I didn't have much luck. I think I don't completely understand the cause of the problem.

Please sign in to comment.