Move helper module compilation inside template resource

- Resource::Template compiles helper methods/module bodies into a
  collection of modules for inclusion into the template context.
- Resource::Template checks for obvious invalid input to helper
  definition methods.

3 files changed, 50 insertions, 29 deletions

diff --git a/lib/chef/mixin/template.rb b/lib/chef/mixin/template.rb
index 0e7bbeb73a..8a0cb060f6 100644
--- a/lib/chef/mixin/template.rb
+++ b/lib/chef/mixin/template.rb
@@ -94,28 +94,6 @@ class Chef
         # INTERNAL PUBLIC API
         ###
 
-        def _define_helpers(helper_methods)
-          # TODO (ruby 1.8 hack)
-          # This is most elegantly done with Object#define_singleton_method,
-          # however ruby 1.8.7 does not support that, so we create a module and
-          # include it. This should be revised when 1.8 support is not needed.
-          helper_mod = Module.new do
-            helper_methods.each do |method_name, method_body|
-              define_method(method_name, &method_body)
-            end
-          end
-          @_extension_modules << helper_mod
-          extend(helper_mod)
-        end
-
-        def _define_helpers_from_blocks(blocks)
-          blocks.each do |module_body|
-            helper_mod = Module.new(&module_body)
-            extend(helper_mod)
-            @_extension_modules << helper_mod
-          end
-        end
-
         def _extend_modules(module_names)
           module_names.each do |mod|
             extend(mod)
diff --git a/lib/chef/provider/template/content.rb b/lib/chef/provider/template/content.rb
index f26b7a5b92..9de9ad9ea2 100644
--- a/lib/chef/provider/template/content.rb
+++ b/lib/chef/provider/template/content.rb
@@ -39,8 +39,6 @@ class Chef
           context = TemplateContext.new(@new_resource.variables)
           context[:node] = @run_context.node
           context[:template_finder] = template_finder
-          context._define_helpers(@new_resource.inline_helper_blocks)
-          context._define_helpers_from_blocks(@new_resource.inline_helper_modules)
           context._extend_modules(@new_resource.helper_modules)
           file = nil
           render_template(IO.read(template_location), context) { |t| file = t }
diff --git a/lib/chef/resource/template.rb b/lib/chef/resource/template.rb
index 0e468ed401..ec62070f24 100644
--- a/lib/chef/resource/template.rb
+++ b/lib/chef/resource/template.rb
@@ -31,7 +31,6 @@ class Chef
 
       attr_reader :inline_helper_blocks
       attr_reader :inline_helper_modules
-      attr_reader :helper_modules
 
       def initialize(name, run_context=nil)
         super
@@ -80,16 +79,62 @@ class Chef
       end
 
       def helper(method_name, &block)
-        # TODO: method_name must be symbol or coerce.
-        # TODO: block is not optional.
+        unless block_given?
+          raise Exceptions::ValidationFailed,
+            "`helper(:method)` requires a block argument (e.g., `helper(:method) { code }`)"
+        end
+
+        unless method_name.kind_of?(Symbol)
+          raise Exceptions::ValidationFailed,
+            "method_name argument to `helper(method_name)` must be a symbol (e.g., `helper(:method) { code }`)"
+        end
+
         @inline_helper_blocks[method_name] = block
       end
 
       def helpers(module_name=nil,&block)
-        if block_given?
+        if block_given? and !module_name.nil?
+          raise Exceptions::ValidationFailed,
+            "Passing both a module and block to #helpers is not supported. Call #helpers multiple times instead"
+        elsif block_given?
           @inline_helper_modules << block
-        else
+        elsif module_name.kind_of?(::Module)
           @helper_modules << module_name
+        elsif module_name.nil?
+          raise Exceptions::ValidationFailed,
+            "#helpers requires either a module name or inline module code as a block.\n" +
+            "e.g.: helpers do; helper_code; end;\n" +
+            "OR: helpers(MyHelpersModule)"
+        else
+          raise Exceptions::ValidationFailed,
+            "Argument to #helpers must be a module. You gave #{module_name.inspect} (#{module_name.class})"
+        end
+      end
+
+      def helper_modules
+        compiled_helper_methods + compiled_helper_modules + @helper_modules
+      end
+
+      private
+
+      # compiles helper methods into a module that can be included in template context
+      def compiled_helper_methods
+        if inline_helper_blocks.empty?
+          []
+        else
+          resource_helper_blocks = inline_helper_blocks
+          helper_mod = Module.new do
+            resource_helper_blocks.each do |method_name, method_body|
+              define_method(method_name, &method_body)
+            end
+          end
+          [ helper_mod ]
+        end
+      end
+
+      def compiled_helper_modules
+        @inline_helper_modules.map do |module_body|
+          Module.new(&module_body)
         end
       end