[Pkg-puppet-devel] [SCM] Puppet packaging for Debian branch, experimental, updated. debian/2.6.8-1-844-g7ec39d5

[Daniel Pittman]

The following commit has been merged in the experimental branch:
commit 5d7ef5caf30a0c5b3253340c5f2722e51c56c75e
Author: Daniel Pittman <daniel at puppetlabs.com>
Date:   Tue Apr 19 20:23:27 2011 -0700

    (#7062) better argument handling in the action wrapper methods
    
    We previously used *args to collect all arguments to the action when_invoked
    block, then tried vaguely to massage some little bits of them into the right
    shape.
    
    Methods defined with blocks, in Ruby 1.8, also have some fun behaviours.  The
    most special is that if you pass more than one argument to a block defined
    with only one Ruby will automatically coerce the arguments into an array – and
    this is preserved when it is bound to a method.
    
    This led to routine situations where we would pass the wrong number of
    arguments to the block because, say, the user gave an extra argument on the
    command line.
    
    Instead of failing this would transmogrify the arguments in counterintuitive
    ways, and end up with horrible stack traces when that interacted badly with
    the code as written.
    
    Now, instead, we work out the right argument format based on the arguments
    that the when_invoked block takes.  This gives much better (albeit perhaps not
    so user friendly) behaviour at the interface level.  Which is, at least,
    consistent with other Ruby API.
    
    Reviewed-By: Max Martin <max at puppetlabs.com>

diff --git a/lib/puppet/interface/action.rb b/lib/puppet/interface/action.rb
index e644d89..08bc0a3 100644
--- a/lib/puppet/interface/action.rb
+++ b/lib/puppet/interface/action.rb
@@ -151,15 +151,37 @@ class Puppet::Interface::Action
   def when_invoked=(block)
 
     internal_name = "#{@name} implementation, required on Ruby 1.8".to_sym
-    file    = __FILE__ + "+eval"
-    line    = __LINE__ + 1
+
+    arity = block.arity
+    if arity == 0 then
+      # This will never fire on 1.8.7, which treats no arguments as "*args",
+      # but will on 1.9.2, which treats it as "no arguments".  Which bites,
+      # because this just begs for us to wind up in the horrible situation
+      # where a 1.8 vs 1.9 error bites our end users. --daniel 2011-04-19
+      raise ArgumentError, "action when_invoked requires at least one argument (options)"
+    elsif arity > 0 then
+      range = Range.new(1, arity - 1)
+      decl = range.map { |x| "arg#{x}" } << "options = {}"
+      optn = ""
+      args = "[" + (range.map { |x| "arg#{x}" } << "options").join(", ") + "]"
+    else
+      range = Range.new(1, arity.abs - 1)
+      decl = range.map { |x| "arg#{x}" } << "*rest"
+      optn = "rest << {} unless rest.last.is_a?(Hash)"
+      if arity == -1 then
+        args = "rest"
+      else
+        args = "[" + range.map { |x| "arg#{x}" }.join(", ") + "] + rest"
+      end
+    end
+
+    file    = __FILE__ + "+eval[wrapper]"
+    line    = __LINE__ + 2 # <== points to the same line as 'def' in the wrapper.
     wrapper = <<WRAPPER
-def #{@name}(*args)
-  if args.last.is_a? Hash then
-    options = args.last
-  else
-    args << (options = {})
-  end
+def #{@name}(#{decl.join(", ")})
+  #{optn}
+  args = #{args}
+  options = args.last
 
   action = get_action(#{name.inspect})
   action.validate_args(args)
diff --git a/spec/unit/interface/action_spec.rb b/spec/unit/interface/action_spec.rb
index 0eb450e..24826a6 100755
--- a/spec/unit/interface/action_spec.rb
+++ b/spec/unit/interface/action_spec.rb
@@ -12,6 +12,62 @@ describe Puppet::Interface::Action do
     end
   end
 
+  describe "#when_invoked=" do
+    it "should fail if the block has arity 0" do
+      pending "Ruby 1.8 (painfully) treats argument-free blocks as arity -1" if
+        RUBY_VERSION =~ /^1\.8/
+
+      expect {
+        Puppet::Interface.new(:action_when_invoked, '1.0.0') do
+          action :foo do
+            when_invoked do
+            end
+          end
+        end
+      }.to raise_error ArgumentError, /foobra/
+    end
+
+    it "should work with arity 1 blocks" do
+      face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do
+        action :foo do
+          when_invoked {|one| }
+        end
+      end
+      # -1, because we use option defaulting. :(
+      face.method(:foo).arity.should == -1
+    end
+
+    it "should work with arity 2 blocks" do
+      face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do
+        action :foo do
+          when_invoked {|one, two| }
+        end
+      end
+      # -2, because we use option defaulting. :(
+      face.method(:foo).arity.should == -2
+    end
+
+    it "should work with arity 1 blocks that collect arguments" do
+      face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do
+        action :foo do
+          when_invoked {|*one| }
+        end
+      end
+      # -1, because we use only varargs
+      face.method(:foo).arity.should == -1
+    end
+
+    it "should work with arity 2 blocks that collect arguments" do
+      face = Puppet::Interface.new(:action_when_invoked, '1.0.0') do
+        action :foo do
+          when_invoked {|one, *two| }
+        end
+      end
+      # -2, because we take one mandatory argument, and one varargs
+      face.method(:foo).arity.should == -2
+    end
+  end
+
   describe "when invoking" do
     it "should be able to call other actions on the same object" do
       face = Puppet::Interface.new(:my_face, '0.0.1') do

-- 
Puppet packaging for Debian