From 14f9904e0fc6d8a1e5627ac64c4b5b14e95177c5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jos=C3=A9=20Valim?= Date: Wed, 29 Sep 2010 11:18:31 +0200 Subject: Avoid (@_var ||= nil) pattern by using initialize methods and ensuring everyone calls super as expected. --- actionpack/lib/abstract_controller/layouts.rb | 4 +-- actionpack/lib/abstract_controller/rendering.rb | 2 +- actionpack/lib/abstract_controller/url_for.rb | 1 - actionpack/lib/action_dispatch/middleware/stack.rb | 4 +++ .../lib/action_dispatch/middleware/static.rb | 3 +-- actionpack/lib/action_dispatch/routing/url_for.rb | 6 ++++- .../lib/action_dispatch/testing/integration.rb | 1 + actionpack/lib/action_view/helpers/form_helper.rb | 31 ++++++++++++---------- actionpack/lib/action_view/test_case.rb | 22 +++++++-------- actionpack/test/abstract/layouts_test.rb | 2 +- 10 files changed, 42 insertions(+), 34 deletions(-) (limited to 'actionpack') diff --git a/actionpack/lib/abstract_controller/layouts.rb b/actionpack/lib/abstract_controller/layouts.rb index bbc9739f14..5c853c3034 100644 --- a/actionpack/lib/abstract_controller/layouts.rb +++ b/actionpack/lib/abstract_controller/layouts.rb @@ -345,11 +345,9 @@ module AbstractController # * template - The template object for the default layout (or nil) def _default_layout(require_layout = false) begin - @_layout ||= nil layout_name = _layout if action_has_layout? rescue NameError => e - raise NoMethodError, - "You specified #{@_layout.inspect} as the layout, but no such method was found" + raise e.class, "Coult not render layout: #{e.message}" end if require_layout && action_has_layout? && !layout_name diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index cc6a7f4047..5d9b35d297 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -66,7 +66,7 @@ module AbstractController attr_writer :view_context_class def view_context_class - (@view_context_class ||= nil) || self.class.view_context_class + @view_context_class || self.class.view_context_class end def initialize(*) diff --git a/actionpack/lib/abstract_controller/url_for.rb b/actionpack/lib/abstract_controller/url_for.rb index 2e9de22ecd..e5d5bef6b4 100644 --- a/actionpack/lib/abstract_controller/url_for.rb +++ b/actionpack/lib/abstract_controller/url_for.rb @@ -1,7 +1,6 @@ module AbstractController module UrlFor extend ActiveSupport::Concern - include ActionDispatch::Routing::UrlFor def _routes diff --git a/actionpack/lib/action_dispatch/middleware/stack.rb b/actionpack/lib/action_dispatch/middleware/stack.rb index 9ea188c3e2..e3cd779756 100644 --- a/actionpack/lib/action_dispatch/middleware/stack.rb +++ b/actionpack/lib/action_dispatch/middleware/stack.rb @@ -41,7 +41,11 @@ module ActionDispatch end end + # Use this instead of super to work around a warning. + alias :array_initialize :initialize + def initialize(*args) + array_initialize(*args) yield(self) if block_given? end diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 6d442b91f5..cf13938331 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -4,10 +4,9 @@ module ActionDispatch class FileHandler def initialize(at, root) @at, @root = at.chomp('/'), root.chomp('/') - @compiled_at = Regexp.compile(/^#{Regexp.escape(at)}/) unless @at.blank? + @compiled_at = (Regexp.compile(/^#{Regexp.escape(at)}/) unless @at.blank?) @compiled_root = Regexp.compile(/^#{Regexp.escape(root)}/) @file_server = ::Rack::File.new(root) - @compiled_at ||= nil end def match?(path) diff --git a/actionpack/lib/action_dispatch/routing/url_for.rb b/actionpack/lib/action_dispatch/routing/url_for.rb index 7be0f2769a..bfdea41f60 100644 --- a/actionpack/lib/action_dispatch/routing/url_for.rb +++ b/actionpack/lib/action_dispatch/routing/url_for.rb @@ -98,6 +98,11 @@ module ActionDispatch end end + def initialize(*) + @_routes = nil + super + end + def url_options default_url_options end @@ -136,7 +141,6 @@ module ActionDispatch protected def _with_routes(routes) - @_routes ||= nil old_routes, @_routes = @_routes, routes yield ensure diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 8c05462d0d..4d25bb8d7b 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -171,6 +171,7 @@ module ActionDispatch # Create and initialize a new Session instance. def initialize(app) + super @app = app # If the app is a Rails app, make url_helpers available on the session diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 7e49052c24..c47fac05ef 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -850,7 +850,7 @@ module ActionView extend ActiveSupport::Concern include Helpers::CaptureHelper, Context, Helpers::TagHelper, Helpers::FormTagHelper - attr_reader :method_name, :object_name + attr_reader :object, :method_name, :object_name DEFAULT_FIELD_OPTIONS = { "size" => 30 } DEFAULT_RADIO_OPTIONS = { } @@ -859,14 +859,9 @@ module ActionView def initialize(object_name, method_name, template_object, object = nil) @object_name, @method_name = object_name.to_s.dup, method_name.to_s.dup @template_object = template_object - @object = object - if @object_name.sub!(/\[\]$/,"") || @object_name.sub!(/\[\]\]$/,"]") - if (object ||= @template_object.instance_variable_get("@#{Regexp.last_match.pre_match}")) && object.respond_to?(:to_param) - @auto_index = object.to_param - else - raise ArgumentError, "object[] naming but object param and @object var don't exist or don't respond to to_param: #{object.inspect}" - end - end + @object_name.sub!(/\[\]$/,"") || @object_name.sub!(/\[\]\]$/,"]") + @object = retrieve_object(object) + @auto_index = retrieve_autoindex(Regexp.last_match.pre_match) if Regexp.last_match end def to_label_tag(text = nil, options = {}, &block) @@ -990,18 +985,26 @@ module ActionView content_tag(tag_name, value(object), options) end - def object - if @object - @object + def retrieve_object(object) + if object + object elsif @template_object.instance_variable_defined?("@#{@object_name}") @template_object.instance_variable_get("@#{@object_name}") end rescue NameError - # As @object_name may contain the nested syntax (item[subobject]) we - # need to fallback to nil. + # As @object_name may contain the nested syntax (item[subobject]) we need to fallback to nil. nil end + def retrieve_autoindex(pre_match) + object = self.object || @template_object.instance_variable_get("@#{pre_match}") + if object && object.respond_to?(:to_param) + object.to_param + else + raise ArgumentError, "object[] naming but object param and @object var don't exist or don't respond to to_param: #{object.inspect}" + end + end + def value(object) self.class.value(object, @method_name) end diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index de23a67cb6..915c2f90d7 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -20,12 +20,12 @@ module ActionView end def initialize + super self.class.controller_path = "" @request = ActionController::TestRequest.new @response = ActionController::TestResponse.new @request.env.delete('PATH_INFO') - @params = {} end end @@ -156,15 +156,15 @@ module ActionView # The instance of ActionView::Base that is used by +render+. def view @view ||= begin - view = ActionView::Base.new(ActionController::Base.view_paths, {}, @controller) - view.singleton_class.send :include, _helpers - view.singleton_class.send :include, @controller._routes.url_helpers - view.singleton_class.send :delegate, :alert, :notice, :to => "request.flash" - view.extend(Locals) - view.locals = self.locals - view.output_buffer = self.output_buffer - view - end + view = ActionView::Base.new(ActionController::Base.view_paths, {}, @controller) + view.singleton_class.send :include, _helpers + view.singleton_class.send :include, @controller._routes.url_helpers + view.singleton_class.send :delegate, :alert, :notice, :to => "request.flash" + view.extend(Locals) + view.locals = self.locals + view.output_buffer = self.output_buffer + view + end end alias_method :_view, :view @@ -201,7 +201,7 @@ module ActionView def method_missing(selector, *args) if @controller.respond_to?(:_routes) && - @controller._routes.named_routes.helpers.include?(selector) + @controller._routes.named_routes.helpers.include?(selector) @controller.__send__(selector, *args) else super diff --git a/actionpack/test/abstract/layouts_test.rb b/actionpack/test/abstract/layouts_test.rb index f580ad40f7..5ed6aa68b5 100644 --- a/actionpack/test/abstract/layouts_test.rb +++ b/actionpack/test/abstract/layouts_test.rb @@ -225,7 +225,7 @@ module AbstractControllerTests end test "when the layout is specified as a symbol and the method doesn't exist, raise an exception" do - assert_raises(NoMethodError) { WithSymbolAndNoMethod.new.process(:index) } + assert_raises(NameError) { WithSymbolAndNoMethod.new.process(:index) } end test "when the layout is specified as a symbol and the method returns something besides a string/false/nil, raise an exception" do -- cgit v1.2.3