Merge pull request #39497 from jhawthorn/render_with_invalid_options

Raise when calling render with invalid options

Author: jhawthorn

actionpack/lib/action_controller/metal/rendering.rb

@@ -115,7 +115,9 @@ def _normalize_text(options)
 
       # Process controller specific options, as status, content-type and location.
       def _process_options(options)
-        status, content_type, location = options.values_at(:status, :content_type, :location)
+        status = options.delete(:status)
+        content_type = options.delete(:content_type)
+        location = options.delete(:location)
 
         self.status = status if status
         self.content_type = content_type if content_type

actionpack/test/controller/render_test.rb

@@ -371,15 +371,6 @@ def test_dynamic_render
     end
   end
 
-  def test_permitted_dynamic_render_file_hash
-    assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__))
-    assert_deprecated do
-      assert_raises ActionView::MissingTemplate do
-        get :dynamic_render_permit, params: { id: { file: '../\\../test/abstract_unit.rb' } }
-      end
-    end
-  end
-
   def test_dynamic_render_file_hash
     assert_raises ArgumentError do
       get :dynamic_render, params: { id: { file: '../\\../test/abstract_unit.rb' } }

actionpack/test/controller/renderers_test.rb

@@ -69,8 +69,8 @@ def test_using_custom_render_option
     ActionController.remove_renderer :simon
   end
 
-  def test_raises_missing_template_no_renderer
-    assert_raise ActionView::MissingTemplate do
+  def test_raises_argument_error_no_renderer
+    assert_raise ArgumentError do
       get :respond_to_mime, format: "csv"
     end
     assert_equal Mime[:csv], @response.media_type

actionview/lib/action_view/renderer/abstract_renderer.rb

@@ -158,7 +158,7 @@ def format
 
       def extract_details(options) # :doc:
         details = nil
-        @lookup_context.registered_details.each do |key|
+        details_arguments.each do |key|
           value = options[key]
 
           if value
@@ -168,6 +168,10 @@ def extract_details(options) # :doc:
         details || NO_DETAILS
       end
 
+      def details_arguments
+        @lookup_context.registered_details
+      end
+
       def prepend_formats(formats) # :doc:
         formats = Array(formats)
         return if formats.empty? || @lookup_context.html_fallback_for_js

actionview/lib/action_view/renderer/partial_renderer.rb

@@ -256,6 +256,13 @@ class PartialRenderer < AbstractRenderer
 
     def initialize(lookup_context, options)
       super(lookup_context)
+
+      options.assert_valid_keys(
+        :partial, :template, :renderable, :layout,
+        :locals, :collection, :object, :as, :cached, :spacer_template,
+        *details_arguments
+      )
+
       @options = options
       @locals  = @options[:locals] || {}
       @details = extract_details(@options)

actionview/lib/action_view/renderer/template_renderer.rb

@@ -3,6 +3,13 @@
 module ActionView
   class TemplateRenderer < AbstractRenderer #:nodoc:
     def render(context, options)
+      options.assert_valid_keys(
+        :body, :plain, :html, :file, :inline, :template, :renderable,
+        :layout, :locals, :prefixes,
+        :type,
+        *details_arguments
+      )
+
       @details = extract_details(options)
       template = determine_template(options)
 

actionview/lib/action_view/rendering.rb

@@ -165,7 +165,7 @@ def _normalize_options(options)
           options[:prefixes] ||= _prefixes
         end
 
-        options[:template] ||= (options[:action] || action_name).to_s
+        options[:template] ||= (options.delete(:action) || action_name).to_s
         options
       end
   end

actionview/test/actionpack/abstract/abstract_controller_test.rb

@@ -39,7 +39,7 @@ def _prefixes
 
       def render(options = {})
         if options.is_a?(String)
-          options = { _template_name: options }
+          options = { action: options }
         end
         super
       end
@@ -49,7 +49,7 @@ def render(options = {})
 
     class Me2 < RenderingController
       def index
-        render "index.erb"
+        render "index"
       end
 
       def index_to_string
@@ -58,7 +58,7 @@ def index_to_string
 
       def action_with_ivars
         @my_ivar = "Hello"
-        render "action_with_ivars.erb"
+        render "action_with_ivars"
       end
 
       def naked_render
@@ -200,7 +200,7 @@ def self.layout(formats)
         end
 
         def render_to_body(options = {})
-          options[:_layout] = options[:layout] || _default_layout({})
+          options[:layout] = options[:layout] || _default_layout({})
           super
         end
     end

actionview/test/template/render_test.rb

@@ -68,6 +68,16 @@ def test_render_without_options
     assert_match(/You invoked render but did not give any of (.+) option\./, e.message)
   end
 
+  def test_render_throws_exception_when_given_partial_and_invalid_options
+    e = assert_raises(ArgumentError) { @view.render(template: "test/hello_world", invalid_option: true) }
+    assert_includes e.message, "Unknown key: :invalid_option"
+  end
+
+  def test_render_throws_exception_when_given_template_and_invalid_options
+    e = assert_raises(ArgumentError) { @view.render(partial: "test/partial", invalid_option: true) }
+    assert_includes e.message, "Unknown key: :invalid_option"
+  end
+
   def test_render_template
     assert_equal "Hello world!", @view.render(template: "test/hello_world")
   end
@@ -739,30 +749,30 @@ def setup
 
   def test_render_utf8_template_with_magic_comment
     with_external_encoding Encoding::ASCII_8BIT do
-      result = @view.render(template: "test/utf8_magic", formats: [:html], layouts: "layouts/yield")
+      result = @view.render(template: "test/utf8_magic", formats: [:html])
       assert_equal Encoding::UTF_8, result.encoding
       assert_equal "\nРусский \nтекст\n\nUTF-8\nUTF-8\nUTF-8\n", result
     end
   end
 
   def test_render_utf8_template_with_default_external_encoding
     with_external_encoding Encoding::UTF_8 do
-      result = @view.render(template: "test/utf8", formats: [:html], layouts: "layouts/yield")
+      result = @view.render(template: "test/utf8", formats: [:html])
       assert_equal Encoding::UTF_8, result.encoding
       assert_equal "Русский текст\n\nUTF-8\nUTF-8\nUTF-8\n", result
     end
   end
 
   def test_render_utf8_template_with_incompatible_external_encoding
     with_external_encoding Encoding::SHIFT_JIS do
-      e = assert_raises(ActionView::Template::Error) { @view.render(template: "test/utf8", formats: [:html], layouts: "layouts/yield") }
+      e = assert_raises(ActionView::Template::Error) { @view.render(template: "test/utf8", formats: [:html], layout: "layouts/yield") }
       assert_match "Your template was not saved as valid Shift_JIS", e.cause.message
     end
   end
 
   def test_render_utf8_template_with_partial_with_incompatible_encoding
     with_external_encoding Encoding::SHIFT_JIS do
-      e = assert_raises(ActionView::Template::Error) { @view.render(template: "test/utf8_magic_with_bare_partial", formats: [:html], layouts: "layouts/yield") }
+      e = assert_raises(ActionView::Template::Error) { @view.render(template: "test/utf8_magic_with_bare_partial", formats: [:html], layout: "layouts/yield") }
       assert_match "Your template was not saved as valid Shift_JIS", e.cause.message
     end
   end