Skip to content

Commit

Permalink
Check for uninitialized instance variables
Browse files Browse the repository at this point in the history
  • Loading branch information
jeremy committed Sep 9, 2008
1 parent 8b4461c commit dc0411f
Show file tree
Hide file tree
Showing 4 changed files with 24 additions and 20 deletions.
2 changes: 1 addition & 1 deletion actionpack/lib/action_controller/base.rb
Expand Up @@ -453,7 +453,7 @@ def view_paths=(value)
# ArticleController.prepend_view_path(["views/default", "views/custom"])
#
def prepend_view_path(path)
@view_paths = superclass.view_paths.dup if @view_paths.nil?
@view_paths = superclass.view_paths.dup if !defined?(@view_paths) || @view_paths.nil?
@view_paths.unshift(*path)
end

Expand Down
10 changes: 6 additions & 4 deletions actionpack/lib/action_view/base.rb
Expand Up @@ -278,9 +278,9 @@ def render(options = {}, local_assigns = {}, &block) #:nodoc:
# the same name but differing formats. See +Request#template_format+
# for more details.
def template_format
return @template_format if @template_format

if controller && controller.respond_to?(:request)
if defined? @template_format
@template_format
elsif controller && controller.respond_to?(:request)
@template_format = controller.request.template_format
else
@template_format = :html
Expand Down Expand Up @@ -366,7 +366,9 @@ def _render_with_layout(options, local_assigns, &block) #:nodoc:
end
else
begin
original_content_for_layout, @content_for_layout = @content_for_layout, render(options)
original_content_for_layout = @content_for_layout if defined?(@content_for_layout)
@content_for_layout = render(options)

if (options[:inline] || options[:file] || options[:text])
@cached_content_for_layout = @content_for_layout
render(:file => partial_layout, :locals => local_assigns)
Expand Down
14 changes: 7 additions & 7 deletions actionpack/lib/action_view/renderable.rb
@@ -1,8 +1,7 @@
module ActionView
module Renderable
# NOTE: The template that this mixin is beening include into is frozen
# So you can not set or modify any instance variables

# NOTE: The template that this mixin is being included into is frozen
# so you cannot set or modify any instance variables
module Renderable #:nodoc:
extend ActiveSupport::Memoizable

def self.included(base)
Expand Down Expand Up @@ -33,10 +32,11 @@ def render(view, local_assigns = {})
view.send(:_set_controller_content_type, mime_type) if respond_to?(:mime_type)

view.send(method_name(local_assigns), local_assigns) do |*names|
if proc = view.instance_variable_get("@_proc_for_layout")
ivar = :@_proc_for_layout
if view.instance_variable_defined?(ivar) and proc = view.instance_variable_get(ivar)
view.capture(*names, &proc)
else
view.instance_variable_get("@content_for_#{names.first || 'layout'}")
elsif view.instance_variable_defined?(ivar = :"@content_for_#{names.first || :layout}")
view.instance_variable_get(ivar)
end
end
end
Expand Down
18 changes: 10 additions & 8 deletions actionpack/lib/action_view/renderable_partial.rb
@@ -1,8 +1,7 @@
module ActionView
module RenderablePartial
# NOTE: The template that this mixin is beening include into is frozen
# So you can not set or modify any instance variables

# NOTE: The template that this mixin is being included into is frozen
# so you cannot set or modify any instance variables
module RenderablePartial #:nodoc:
extend ActiveSupport::Memoizable

def variable_name
Expand Down Expand Up @@ -30,10 +29,13 @@ def render_partial(view, object = nil, local_assigns = {}, as = nil)
local_assigns[variable_name]

if view.respond_to?(:controller)
object ||= ActiveSupport::Deprecation::DeprecatedObjectProxy.new(
view.controller.instance_variable_get("@#{variable_name}"),
"@#{variable_name} will no longer be implicitly assigned to #{variable_name}"
)
ivar = :"@#{variable_name}"
object ||=
if view.controller.instance_variable_defined?(ivar)
ActiveSupport::Deprecation::DeprecatedObjectProxy.new(
view.controller.instance_variable_get(ivar),
"#{ivar} will no longer be implicitly assigned to #{variable_name}")
end
end

# Ensure correct object is reassigned to other accessors
Expand Down

2 comments on commit dc0411f

@ryanlowe
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’m updating to Rails 2.2 and I got a lot of deprecation messages like:

DEPRECATION WARNING: @change will no longer be implicitly assigned to change.

This deprecation check seems to be a little overzealous. If I set an ivar @change in the action and then pass it to a partial with:

:locals => { :change => @change }

then I get this deprecation message even if I use change and not @change in the partial. I wasn’t taking advantage of an implicit assignment. If I don’t use @change in the partial because I only use the local change, should I still get this deprecation message?

@NZKoz
Copy link
Member

@NZKoz NZKoz commented on dc0411f Nov 23, 2008

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you open a ticket in lighthouse and then email the core list about this?

It’s entirely possible that this warning is a little over-eager and would be good to catch it.

Please sign in to comment.