Merge pull request #4612 from rafaelfranca/av-refactor

Some improvements on ActionView::Helpers::Tags

6 files changed, 53 insertions, 28 deletions

diff --git a/actionpack/lib/action_view/helpers/active_model_helper.rb b/actionpack/lib/action_view/helpers/active_model_helper.rb
index 973135e2ea..1187956081 100644
--- a/actionpack/lib/action_view/helpers/active_model_helper.rb
+++ b/actionpack/lib/action_view/helpers/active_model_helper.rb
@@ -16,7 +16,9 @@ module ActionView
         end
       end
 
-      module_eval "def content_tag(*) error_wrapping(super) end", __FILE__, __LINE__
+      def content_tag(*)
+        error_wrapping(super)
+      end
 
       def tag(type, options, *)
         tag_generate_errors?(options) ? error_wrapping(super) : super
diff --git a/actionpack/lib/action_view/helpers/tags.rb b/actionpack/lib/action_view/helpers/tags.rb
index 89b3efda5f..e874d4ca42 100644
--- a/actionpack/lib/action_view/helpers/tags.rb
+++ b/actionpack/lib/action_view/helpers/tags.rb
@@ -1,28 +1,30 @@
 module ActionView
   module Helpers
-    module Tags
-      autoload :Base,                    'action_view/helpers/tags/base'
-      autoload :Label,                   'action_view/helpers/tags/label'
-      autoload :TextField,               'action_view/helpers/tags/text_field'
-      autoload :PasswordField,           'action_view/helpers/tags/password_field'
-      autoload :HiddenField,             'action_view/helpers/tags/hidden_field'
-      autoload :FileField,               'action_view/helpers/tags/file_field'
-      autoload :SearchField,             'action_view/helpers/tags/search_field'
-      autoload :TelField,                'action_view/helpers/tags/tel_field'
-      autoload :UrlField,                'action_view/helpers/tags/url_field'
-      autoload :EmailField,              'action_view/helpers/tags/email_field'
-      autoload :NumberField,             'action_view/helpers/tags/number_field'
-      autoload :RangeField,              'action_view/helpers/tags/range_field'
-      autoload :TextArea,                'action_view/helpers/tags/text_area'
-      autoload :CheckBox,                'action_view/helpers/tags/check_box'
-      autoload :RadioButton,             'action_view/helpers/tags/radio_button'
-      autoload :Select,                  'action_view/helpers/tags/select'
-      autoload :CollectionSelect,        'action_view/helpers/tags/collection_select'
-      autoload :GroupedCollectionSelect, 'action_view/helpers/tags/grouped_collection_select'
-      autoload :TimeZoneSelect,          'action_view/helpers/tags/time_zone_select'
-      autoload :DateSelect,              'action_view/helpers/tags/date_select'
-      autoload :TimeSelect,              'action_view/helpers/tags/time_select'
-      autoload :DatetimeSelect,          'action_view/helpers/tags/datetime_select'
+    module Tags #:nodoc:
+      extend ActiveSupport::Autoload
+
+      autoload :Base
+      autoload :Label
+      autoload :TextField
+      autoload :PasswordField
+      autoload :HiddenField
+      autoload :FileField
+      autoload :SearchField
+      autoload :TelField
+      autoload :UrlField
+      autoload :EmailField
+      autoload :NumberField
+      autoload :RangeField
+      autoload :TextArea
+      autoload :CheckBox
+      autoload :RadioButton
+      autoload :Select
+      autoload :CollectionSelect
+      autoload :GroupedCollectionSelect
+      autoload :TimeZoneSelect
+      autoload :DateSelect
+      autoload :TimeSelect
+      autoload :DatetimeSelect
     end
   end
 end
diff --git a/actionpack/lib/action_view/helpers/tags/base.rb b/actionpack/lib/action_view/helpers/tags/base.rb
index 24956beb9c..449f94d347 100644
--- a/actionpack/lib/action_view/helpers/tags/base.rb
+++ b/actionpack/lib/action_view/helpers/tags/base.rb
@@ -19,8 +19,9 @@ module ActionView
           @auto_index = retrieve_autoindex(Regexp.last_match.pre_match) if Regexp.last_match
         end
 
-        def render(&block)
-          raise "Abstract Method called"
+        # This is what child classes implement.
+        def render
+          raise NotImplementedError, "Subclasses must implement a render method"
         end
 
         private
diff --git a/actionpack/lib/action_view/helpers/tags/date_select.rb b/actionpack/lib/action_view/helpers/tags/date_select.rb
index 5912598ca1..5d706087b0 100644
--- a/actionpack/lib/action_view/helpers/tags/date_select.rb
+++ b/actionpack/lib/action_view/helpers/tags/date_select.rb
@@ -12,10 +12,16 @@ module ActionView
           error_wrapping(datetime_selector(@options, @html_options).send("select_#{select_type}").html_safe)
         end
 
+        class << self
+          def select_type
+            @select_type ||= self.name.split("::").last.sub("Select", "").downcase
+          end
+        end
+
         private
 
         def select_type
-          self.class.name.split("::").last.sub("Select", "").downcase
+          self.class.select_type
         end
 
         def datetime_selector(options, html_options)
diff --git a/actionpack/lib/action_view/helpers/tags/text_field.rb b/actionpack/lib/action_view/helpers/tags/text_field.rb
index 0f81726eb4..ce5182d20f 100644
--- a/actionpack/lib/action_view/helpers/tags/text_field.rb
+++ b/actionpack/lib/action_view/helpers/tags/text_field.rb
@@ -13,10 +13,16 @@ module ActionView
           tag("input", options)
         end
 
+        class << self
+          def field_type
+            @field_type ||= self.name.split("::").last.sub("Field", "").downcase
+          end
+        end
+
         private
 
         def field_type
-          @field_type ||= self.class.name.split("::").last.sub("Field", "").downcase
+          self.class.field_type
         end
       end
     end
diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb
index 5be6a2e4e5..39d4768861 100644
--- a/actionpack/test/template/form_helper_test.rb
+++ b/actionpack/test/template/form_helper_test.rb
@@ -115,6 +115,14 @@ class FormHelperTest < ActionView::TestCase
     super
   end
 
+  class FooTag < ActionView::Helpers::Tags::Base
+    def initialize; end
+  end
+
+  def test_tags_base_child_without_render_method
+    assert_raise(NotImplementedError) { FooTag.new.render }
+  end
+
   def test_label
     assert_dom_equal('<label for="post_title">Title</label>', label("post", "title"))
     assert_dom_equal('<label for="post_title">The title goes here</label>', label("post", "title", "The title goes here"))