Deprecate direct usage of @params. Update ActionView::Base for instance var deprecation.

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@4715 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

14 files changed, 66 insertions, 32 deletions

diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG
index d8104951c7..d8e390c9d4 100644
--- a/actionpack/CHANGELOG
+++ b/actionpack/CHANGELOG
@@ -2,7 +2,7 @@
 
 * Add support for the param_name parameter to the auto_complete_field helper. #5026 [david.a.williams@gmail.com]
 
-* Deprecation! @session and @flash will be removed after 1.2. Use the session and flash methods instead. You'll get printed warnings during tests and logged warnings in dev mode when you access either instance variable directly. [Jeremy Kemper]
+* Deprecation! @params, @session, @flash will be removed after 1.2. Use the corresponding instance methods instead. You'll get printed warnings during tests and logged warnings in dev mode when you access either instance variable directly. [Jeremy Kemper]
 
 * Make Routing noisy when an anchor regexp is assigned to a segment. #5674 [francois.beausoleil@gmail.com]
 
diff --git a/actionpack/examples/address_book_controller.rb b/actionpack/examples/address_book_controller.rb
index 01d498e1bc..9ca6612512 100644
--- a/actionpack/examples/address_book_controller.rb
+++ b/actionpack/examples/address_book_controller.rb
@@ -28,11 +28,11 @@ class AddressBookController < ActionController::Base
   end
   
   def person
-    @person = @address_book.find_person(@params["id"])
+    @person = @address_book.find_person(params[:id])
   end
   
   def create_person
-    @address_book.create_person(@params["person"])
+    @address_book.create_person(params[:person])
     redirect_to :action => "index"
   end
     
@@ -49,4 +49,4 @@ begin
   AddressBookController.process_cgi(CGI.new) if $0 == __FILE__
 rescue => e
   CGI.new.out { "#{e.class}: #{e.message}" }
-end
\ No newline at end of file
+end
diff --git a/actionpack/examples/blog_controller.cgi b/actionpack/examples/blog_controller.cgi
index b0c14033a2..3868c306df 100755
--- a/actionpack/examples/blog_controller.cgi
+++ b/actionpack/examples/blog_controller.cgi
@@ -32,7 +32,7 @@ class BlogController < ActionController::Base
   end
   
   def create
-    @session["posts"].unshift(Post.new(@params["post"]["title"], @params["post"]["body"]))
+    @session["posts"].unshift(Post.new(params[:post][:title], params[:post][:body]))
     flash["alert"] = "New post added!"
     redirect_to :action => "index"
   end
diff --git a/actionpack/examples/debate_controller.cgi b/actionpack/examples/debate_controller.cgi
index b82ac6259d..dbe022568b 100755
--- a/actionpack/examples/debate_controller.cgi
+++ b/actionpack/examples/debate_controller.cgi
@@ -25,19 +25,19 @@ class DebateController < ActionController::Base
   end
   
   def topic
-    @topic = @debate.find_topic(@params["id"])
+    @topic = @debate.find_topic(params[:id])
   end
   
   # def new_topic() end <-- This is not needed as the template doesn't require any assigns
 
   def create_topic
-    @debate.create_topic(@params["topic"])
+    @debate.create_topic(params[:topic])
     redirect_to :action => "index"
   end
 
   def create_reply
-    @debate.create_reply(@params["reply"])
-    redirect_to :action => "topic", :path_params => { "id" => @params["reply"]["topic_id"] }
+    @debate.create_reply(params[:reply])
+    redirect_to :action => "topic", :path_params => { "id" => params[:reply][:topic_id] }
   end
     
   private
@@ -54,4 +54,4 @@ begin
   DebateController.process_cgi(CGI.new) if $0 == __FILE__
 rescue => e
   CGI.new.out { "#{e.class}: #{e.message}" }
-end
\ No newline at end of file
+end
diff --git a/actionpack/lib/action_controller/base.rb b/actionpack/lib/action_controller/base.rb
index 03b89826ce..8823076fcc 100755
--- a/actionpack/lib/action_controller/base.rb
+++ b/actionpack/lib/action_controller/base.rb
@@ -287,17 +287,17 @@ module ActionController #:nodoc:
     # Holds the request object that's primarily used to get environment variables through access like
     # <tt>request.env["REQUEST_URI"]</tt>.
     attr_accessor :request
-    
+
     # Holds a hash of all the GET, POST, and Url parameters passed to the action. Accessed like <tt>params["post_id"]</tt>
     # to get the post_id. No type casts are made, so all values are returned as strings.
-    attr_accessor :params
-    
+    attr_internal :params
+
     # Holds the response object that's primarily used to set additional HTTP headers through access like 
     # <tt>response.headers["Cache-Control"] = "no-cache"</tt>. Can also be used to access the final body HTML after a template
     # has been rendered through response.body -- useful for <tt>after_filter</tt>s that wants to manipulate the output,
     # such as a OutputCompressionFilter.
     attr_accessor :response
-    
+
     # Holds a hash of objects in the session. Accessed like <tt>session[:person]</tt> to get the object tied to the "person"
     # key. The session will hold any type of object as values, but the key should be a string or symbol.
     attr_internal :session
@@ -932,7 +932,7 @@ module ActionController #:nodoc:
       end
 
       def assign_shortcuts(request, response)
-        @request, @params, @cookies = request, request.parameters, request.cookies
+        @request, @_params, @cookies = request, request.parameters, request.cookies
 
         @response         = response
         @response.session = request.session
@@ -946,7 +946,7 @@ module ActionController #:nodoc:
 
 
       # TODO: assigns cookies headers params request response template
-      DEPRECATED_INSTANCE_VARIABLES = %w(flash session)
+      DEPRECATED_INSTANCE_VARIABLES = %w(flash params session)
 
       # Gone after 1.2.
       def assign_deprecated_shortcuts(request, response)
@@ -1019,16 +1019,18 @@ module ActionController #:nodoc:
       end
 
       def add_class_variables_to_assigns
-        %w( template_root logger template_class ignore_missing_templates ).each do |cvar|
+        %w(template_root logger template_class ignore_missing_templates).each do |cvar|
           @assigns[cvar] = self.send(cvar)
         end
       end
 
       def protected_instance_variables
         if view_controller_internals
-          [ "@assigns", "@performed_redirect", "@performed_render" ]
+          %w(@assigns @performed_redirect @performed_render)
         else
-          [ "@assigns", "@performed_redirect", "@performed_render", "@request", "@response", "@session", "@cookies", "@template", "@request_origin", "@parent_controller" ]
+          %w(@assigns @performed_redirect @performed_render
+             @request @response @_params @_session @session
+             @cookies @template @request_origin @parent_controller)
         end
       end
 
diff --git a/actionpack/lib/action_controller/pagination.rb b/actionpack/lib/action_controller/pagination.rb
index f7cd74120e..54084c0361 100644
--- a/actionpack/lib/action_controller/pagination.rb
+++ b/actionpack/lib/action_controller/pagination.rb
@@ -191,7 +191,7 @@ module ActionController
 
     def paginator_and_collection_for(collection_id, options) #:nodoc:
       klass = options[:class_name].constantize
-      page  = @params[options[:parameter]]
+      page  = params[options[:parameter]]
       count = count_collection_for_pagination(klass, options)
       paginator = Paginator.new(self, count, options[:per_page], page)
       collection = find_collection_for_pagination(klass, options, paginator)
diff --git a/actionpack/lib/action_controller/rescue.rb b/actionpack/lib/action_controller/rescue.rb
index 9b57d35587..7cd05eab32 100644
--- a/actionpack/lib/action_controller/rescue.rb
+++ b/actionpack/lib/action_controller/rescue.rb
@@ -79,7 +79,7 @@ module ActionController #:nodoc:
         begin
           perform_action_without_rescue
         rescue Object => exception
-          if defined?(Breakpoint) && @params["BP-RETRY"]
+          if defined?(Breakpoint) && params["BP-RETRY"]
             msg = exception.backtrace.first
             if md = /^(.+?):(\d+)(?::in `(.+)')?$/.match(msg) then
               origin_file, origin_line = md[1], md[2].to_i
@@ -87,7 +87,7 @@ module ActionController #:nodoc:
               set_trace_func(lambda do |type, file, line, method, context, klass|
                 if file == origin_file and line == origin_line then
                   set_trace_func(nil)
-                  @params["BP-RETRY"] = false
+                  params["BP-RETRY"] = false
 
                   callstack = caller
                   callstack.slice!(0) if callstack.first["rescue.rb"]
diff --git a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml
index 81995320c2..82007ff5e6 100644
--- a/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml
+++ b/actionpack/lib/action_controller/templates/rescues/_request_and_response.rhtml
@@ -11,7 +11,7 @@
     <%= form_tag(@request.request_uri, "method" => @request.method) %>
       <input type="hidden" name="BP-RETRY" value="1" />
   
-      <% for key, values in @params %>
+      <% for key, values in params %>
         <% next if key == "BP-RETRY" %>
         <% for value in Array(values) %>
           <input type="hidden" name="<%= key %>" value="<%= value %>" />
diff --git a/actionpack/lib/action_controller/verification.rb b/actionpack/lib/action_controller/verification.rb
index 8e0b255f26..ebf30becac 100644
--- a/actionpack/lib/action_controller/verification.rb
+++ b/actionpack/lib/action_controller/verification.rb
@@ -76,8 +76,8 @@ module ActionController #:nodoc:
 
     def verify_action(options) #:nodoc:
       prereqs_invalid =
-        [*options[:params] ].find { |v| @params[v].nil?  } ||
-        [*options[:session]].find { |v| @session[v].nil? } ||
+        [*options[:params] ].find { |v| params[v].nil?  } ||
+        [*options[:session]].find { |v| session[v].nil? } ||
         [*options[:flash]  ].find { |v| flash[v].nil?    }
       
       if !prereqs_invalid && options[:method]
diff --git a/actionpack/lib/action_view/base.rb b/actionpack/lib/action_view/base.rb
index 13b2ef4821..b38501359c 100644
--- a/actionpack/lib/action_view/base.rb
+++ b/actionpack/lib/action_view/base.rb
@@ -148,7 +148,8 @@ module ActionView #:nodoc:
     attr_accessor :base_path, :assigns, :template_extension
     attr_accessor :controller
 
-    attr_reader :logger, :params, :request, :response, :session, :headers, :flash
+    attr_reader :logger, :request, :response, :headers
+    attr_internal :flash, :params, :session
 
     # Specify trim mode for the ERB compiler. Defaults to '-'.
     # See ERB documentation for suitable values.
diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb
index 0ef31cb851..56b1752392 100644
--- a/actionpack/test/controller/action_pack_assertions_test.rb
+++ b/actionpack/test/controller/action_pack_assertions_test.rb
@@ -54,7 +54,7 @@ class ActionPackAssertionsController < ActionController::Base
   end
 
   def render_based_on_parameters
-    render_text "Mr. #{@params["name"]}"
+    render_text "Mr. #{params[:name]}"
   end
 
   def render_url
diff --git a/actionpack/test/controller/components_test.rb b/actionpack/test/controller/components_test.rb
index 075bc98a28..57e3b21acf 100644
--- a/actionpack/test/controller/components_test.rb
+++ b/actionpack/test/controller/components_test.rb
@@ -46,7 +46,7 @@ end
 
 class CalleeController < ActionController::Base
   def being_called
-    render_text "#{@params["name"] || "Lady"} of the House, speaking"
+    render_text "#{params[:name] || "Lady"} of the House, speaking"
   end
   
   def blowing_up
diff --git a/actionpack/test/controller/verification_test.rb b/actionpack/test/controller/verification_test.rb
index dc95c4b8a1..a3a913d42f 100644
--- a/actionpack/test/controller/verification_test.rb
+++ b/actionpack/test/controller/verification_test.rb
@@ -34,15 +34,15 @@ class VerificationTest < Test::Unit::TestCase
     verify :only => :must_be_post, :method => :post, :render => { :status => 405, :text => "Must be post" }, :add_headers => { "Allow" => "POST" }
 
     def guarded_one
-      render :text => "#{@params["one"]}"
+      render :text => "#{params[:one]}"
     end
 
     def guarded_with_flash
-      render :text => "#{@params["one"]}"
+      render :text => "#{params[:one]}"
     end
 
     def guarded_two
-      render :text => "#{@params["one"]}:#{@params["two"]}"
+      render :text => "#{params[:one]}:#{params[:two]}"
     end
 
     def guarded_in_session
@@ -70,7 +70,7 @@ class VerificationTest < Test::Unit::TestCase
     end
 
     def unguarded
-      render :text => "#{@params["one"]}"
+      render :text => "#{params[:one]}"
     end
 
     def two_redirects
diff --git a/actionpack/test/template/deprecated_instance_variables_test.rb b/actionpack/test/template/deprecated_instance_variables_test.rb
new file mode 100644
index 0000000000..86fc7d6139
--- /dev/null
+++ b/actionpack/test/template/deprecated_instance_variables_test.rb
@@ -0,0 +1,31 @@
+require File.dirname(__FILE__) + '/../abstract_unit'
+
+class DeprecatedInstanceVariablesTest < Test::Unit::TestCase
+  class Target < ActionController::Base
+    ActionController::Base::DEPRECATED_INSTANCE_VARIABLES.each do |var|
+      class_eval <<-end_eval
+        def old_#{var}; render :inline => '<%= @#{var}.inspect %>'  end
+        def new_#{var}; render :inline => '<%= #{var}.inspect %>'   end
+      end_eval
+    end
+
+    def rescue_action(e) raise e end
+  end
+
+  def setup
+    @request    = ActionController::TestRequest.new
+    @response   = ActionController::TestResponse.new
+    @controller = Target.new
+  end
+
+  ActionController::Base::DEPRECATED_INSTANCE_VARIABLES.each do |var|
+    class_eval <<-end_eval, __FILE__, __LINE__
+      def test_old_#{var}_is_deprecated
+        assert_deprecated('@#{var}') { get :old_#{var} }
+      end
+      def test_new_#{var}_isnt_deprecated
+        assert_not_deprecated { get :new_#{var} }
+      end
+    end_eval
+  end
+end