Simplify helper use of ActiveSupport::Dependencies, and use super better for in #helpers

3 files changed, 53 insertions, 62 deletions

diff --git a/actionmailer/lib/action_mailer/helpers.rb b/actionmailer/lib/action_mailer/helpers.rb
index 31f7de8d60..1bb8682315 100644
--- a/actionmailer/lib/action_mailer/helpers.rb
+++ b/actionmailer/lib/action_mailer/helpers.rb
@@ -48,13 +48,14 @@ module ActionMailer
               file_name  = arg.to_s.underscore + '_helper'
               class_name = file_name.camelize
 
-              begin
-                require_dependency(file_name)
-              rescue LoadError => load_error
-                requiree = / -- (.*?)(\.rb)?$/.match(load_error.message).to_a[1]
-                msg = (requiree == file_name) ? "Missing helper file helpers/#{file_name}.rb" : "Can't load file: #{requiree}"
-                raise LoadError.new(msg).copy_blame!(load_error)
-              end
+              require_dependency(file_name, "Missing helper file helpers/%s.rb")
+              # begin
+              #   require_dependency(file_name)
+              # rescue LoadError => load_error
+              #   requiree = / -- (.*?)(\.rb)?$/.match(load_error.message).to_a[1]
+              #   msg = (requiree == file_name) ? "Missing helper file helpers/#{file_name}.rb" : "Can't load file: #{requiree}"
+              #   raise LoadError.new(msg).copy_blame!(load_error)
+              # end
 
               add_template_helper(class_name.constantize)
             else
@@ -97,7 +98,7 @@ module ActionMailer
             child.master_helper_module.__send__(:include, master_helper_module)
             child.helper child.name.to_s.underscore
           rescue MissingSourceFile => e
-            raise unless e.is_missing?("helpers/#{child.name.to_s.underscore}_helper")
+            raise unless e.is_missing?("#{child.name.to_s.underscore}_helper")
           end
         end
     end
diff --git a/actionpack/lib/action_controller/new_base/helpers.rb b/actionpack/lib/action_controller/new_base/helpers.rb
index 8925dd3969..60d1a6f775 100644
--- a/actionpack/lib/action_controller/new_base/helpers.rb
+++ b/actionpack/lib/action_controller/new_base/helpers.rb
@@ -56,34 +56,7 @@ module ActionController
       #   helper(:three, BlindHelper) { def mice() 'mice' end }
       #
       def helper(*args, &block)
-        args.flatten.each do |arg|
-          case arg
-          when :all
-            helper all_application_helpers
-          when String, Symbol
-            file_name  = arg.to_s.underscore + '_helper'
-            class_name = file_name.camelize
-
-            begin
-              require_dependency(file_name)
-            rescue LoadError => load_error
-              requiree = / -- (.*?)(\.rb)?$/.match(load_error.message).to_a[1]
-              if requiree == file_name
-                msg = "Missing helper file helpers/#{file_name}.rb"
-                raise LoadError.new(msg).copy_blame!(load_error)
-              else
-                raise
-              end
-            end
-
-            super class_name.constantize
-          else
-            super args
-          end
-        end
-
-        # Evaluate block in template class if given.
-        _helpers.module_eval(&block) if block_given?
+        super(*_modules_for_helpers(args), &block)
       end
 
       # Declares helper accessors for controller attributes. For example, the
@@ -97,33 +70,45 @@ module ActionController
 
       # Provides a proxy to access helpers methods from outside the view.
       def helpers
-        unless @helper_proxy
-          @helper_proxy = ActionView::Base.new
-          @helper_proxy.extend _helpers
-        else
-          @helper_proxy
-        end
+        @helper_proxy ||= ActionView::Base.new.extend(_helpers)
       end
 
-      private
-        def default_helper_module!
-          unless name.blank?
-            module_name = name.sub(/Controller$|$/, 'Helper')
-            module_path = module_name.split('::').map { |m| m.underscore }.join('/')
-            require_dependency module_path
-            helper module_name.constantize
+    private
+      def _modules_for_helpers(args)
+        args.flatten.map! do |arg|
+          case arg
+          when :all
+            _modules_for_helpers all_application_helpers
+          when String, Symbol
+            file_name = "#{arg.to_s.underscore}_helper"
+            require_dependency(file_name, "Missing helper file helpers/%s.rb")
+            file_name.camelize.constantize
+          when Module
+            arg
+          else
+            raise ArgumentError, "helper must be a String, Symbol, or Module"
           end
-        rescue MissingSourceFile => e
-          raise unless e.is_missing? module_path
-        rescue NameError => e
-          raise unless e.missing_name? module_name
         end
+      end
 
-        # Extract helper names from files in app/helpers/**/*.rb
-        def all_application_helpers
-          extract = /^#{Regexp.quote(helpers_dir)}\/?(.*)_helper.rb$/
-          Dir["#{helpers_dir}/**/*_helper.rb"].map { |file| file.sub extract, '\1' }
+      def default_helper_module!
+        unless name.blank?
+          module_name = name.sub(/Controller$|$/, 'Helper')
+          module_path = module_name.split('::').map { |m| m.underscore }.join('/')
+          require_dependency module_path
+          helper module_name.constantize
         end
+      rescue MissingSourceFile => e
+        raise e unless e.is_missing? module_path
+      rescue NameError => e
+        raise e unless e.missing_name? module_name
+      end
+
+      # Extract helper names from files in app/helpers/**/*.rb
+      def all_application_helpers
+        extract = /^#{Regexp.quote(helpers_dir)}\/?(.*)_helper.rb$/
+        Dir["#{helpers_dir}/**/*_helper.rb"].map { |file| file.sub extract, '\1' }
+      end
     end
   end
 end
diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb
index 855b720ef1..7f6f012721 100644
--- a/activesupport/lib/active_support/dependencies.rb
+++ b/activesupport/lib/active_support/dependencies.rb
@@ -143,8 +143,8 @@ module ActiveSupport #:nodoc:
         Dependencies.require_or_load(file_name)
       end
 
-      def require_dependency(file_name)
-        Dependencies.depend_on(file_name)
+      def require_dependency(file_name, message = "No such file to load -- %s")
+        Dependencies.depend_on(file_name, false, message)
       end
 
       def require_association(file_name)
@@ -230,11 +230,16 @@ module ActiveSupport #:nodoc:
       mechanism == :load
     end
 
-    def depend_on(file_name, swallow_load_errors = false)
+    def depend_on(file_name, swallow_load_errors = false, message = "No such file to load -- %s.rb")
       path = search_for_file(file_name)
       require_or_load(path || file_name)
-    rescue LoadError
-      raise unless swallow_load_errors
+    rescue LoadError => load_error
+      unless swallow_load_errors
+        if file_name = load_error.message[/ -- (.*?)(\.rb)?$/, 1]
+          raise MissingSourceFile.new(message % file_name, load_error.path).copy_blame!(load_error)
+        end
+        raise
+      end
     end
 
     def associate_with(file_name)