Permalink
Browse files

Fix delegated methods: blocks and namespace pollution.

* Fix delegated methods to support blocks sent to them.
* Fix Object instance method pollution on delegation
  • Loading branch information...
1 parent 879eb14 commit ec48320c57047d41f040d3ee4f3b1f2d089325c9 @perplexes committed Oct 28, 2010
Showing with 95 additions and 43 deletions.
  1. +50 −43 lib/concern.rb
  2. +8 −0 spec/concern_spec.rb
  3. +13 −0 spec/examples/blocks.rb
  4. +24 −0 spec/examples/pollution.rb
View
93 lib/concern.rb
@@ -6,54 +6,61 @@ class Concern
def self.classify(lib)
lib.split('/').map{|part| part.gsub(/(?:^|_)(.)/){ $1.upcase } }.join('::')
end
-end
+
+ module SingletonMethods
+ def inner_concern(lib, options={})
+ # load delegate
+ lib = lib.to_s
+ klass = Concern.classify(lib)
+ begin
+ klass = eval(klass)
+ rescue
+ require lib
+ klass = eval(klass)
+ end
-class Object
- def self.concern(lib, options={})
- # load delegate
- lib = lib.to_s
- klass = Concern.classify(lib)
- begin
- klass = eval(klass)
- rescue
- require lib
- klass = eval(klass)
- end
+ unless klass.instance_methods.map{|x|x.to_s}.include?('concerned=')
+ raise "A concern must always extend Concern"
+ end
- unless klass.instance_methods.map{|x|x.to_s}.include?('concerned=')
- raise "A concern must always extend Concern"
- end
+ # make accessor
+ accessor = lib.split('/').last.to_sym
+ class_eval <<-EOF, __FILE__, __LINE__
+ def #{accessor}
+ return @#{accessor} if @#{accessor}
+ @#{accessor} = #{klass}.new
+ @#{accessor}.concerned = self
+ @#{accessor}
+ end
+ EOF
- # make accessor
- accessor = lib.split('/').last.to_sym
- eval <<-EOF
- def #{accessor}
- return @#{accessor} if @#{accessor}
- @#{accessor} = #{klass}.new
- @#{accessor}.concerned = self
- @#{accessor}
- end
- EOF
-
- # call included
- base = eval(name) #self.class is always Class, but name is class that called concern
- klass.included(base) if klass.respond_to? :included
-
- # delegate methods
- if options[:delegate]
- methods = if options[:delegate].is_a?(Array)
- options[:delegate]
- else
- klass.public_instance_methods(false)
- end
+ # call included
+ base = eval(name) #self.class is always Class, but name is class that called concern
+ klass.included(base) if klass.respond_to? :included
+
+ # delegate methods
+ if options[:delegate]
+ methods = if options[:delegate].is_a?(Array)
+ options[:delegate]
+ else
+ klass.public_instance_methods(false)
+ end
- methods.each do |method|
- eval <<-EOF
- def #{method}(*args)
- #{accessor}.#{method}(*args)
- end
- EOF
+ methods.each do |method|
+ class_eval <<-EOF
+ def #{method}(*args, &block)
+ #{accessor}.send(:#{method}, *args, &block)
+ end
+ EOF
+ end
end
end
end
+end
+
+class Object
+ def self.concern(lib, options={})
+ extend(Concern::SingletonMethods) unless self.methods.include?("inner_concern")
+ self.inner_concern(lib, options)
+ end
end
View
8 spec/concern_spec.rb
@@ -30,4 +30,12 @@
it "warns when parent is not Concern" do
`ruby spec/examples/parent_warning.rb`.should == "SUCCESS"
end
+
+ it "shouldn't pollute the global namespace" do
+ `ruby spec/examples/pollution.rb`.should == "SUCCESS"
+ end
+
+ it "should delegate blocks" do
+ `ruby spec/examples/blocks.rb`.should == "SUCCESS"
+ end
end
View
13 spec/examples/blocks.rb
@@ -0,0 +1,13 @@
+$LOAD_PATH << 'lib'
+require 'concern'
+
+class A
+ class B < Concern
+ def test
+ yield
+ end
+ end
+ concern 'a/b', :delegate => [:test]
+end
+
+print A.new.test{ "SUCCESS" }
View
24 spec/examples/pollution.rb
@@ -0,0 +1,24 @@
+$LOAD_PATH << 'lib'
+require 'concern'
+
+class A
+ class B < Concern
+ def random_method_name
+ end
+ end
+ concern 'a/b', :delegate => [:random_method_name]
+end
+
+class C
+ class D < Concern
+ def random_method_name2
+ end
+ end
+ concern 'c/d', :delegate => true
+end
+
+if Object.instance_methods.include?("random_method_name") && Object.instance_methods.include?("random_method_name2")
+ print "FAIL"
+else
+ print "SUCCESS"
+end

1 comment on commit ec48320

@grosser

nice catch on the pollution and blocks, didnt notice that,
(if you made 2 seperate commits it would be more clear)
ill merge em soon

Please sign in to comment.