Merge pull request #7129 from kennyj/fix_7079

Improve error handling when using partial name with hyphen. #7079

2 files changed, 19 insertions, 7 deletions

diff --git a/actionpack/lib/action_view/renderer/partial_renderer.rb b/actionpack/lib/action_view/renderer/partial_renderer.rb
index a08a566b35..128cade1b2 100644
--- a/actionpack/lib/action_view/renderer/partial_renderer.rb
+++ b/actionpack/lib/action_view/renderer/partial_renderer.rb
@@ -344,12 +344,6 @@ module ActionView
         paths.map! { |path| retrieve_variable(path).unshift(path) }
       end
 
-      if String === partial && @variable.to_s !~ /^[a-z_][a-zA-Z_0-9]*$/
-        raise ArgumentError.new("The partial name (#{partial}) is not a valid Ruby identifier; " +
-                                "make sure your partial name starts with a lowercase letter or underscore, " +
-                                "and is followed by any combination of letters, numbers and underscores.")
-      end
-
       self
     end
 
@@ -456,8 +450,19 @@ module ActionView
       keys
     end
 
+    IDENTIFIER_ERROR_MESSAGE = "The partial name (%s) is not a valid Ruby identifier; " +
+                               "make sure your partial name starts with a lowercase letter or underscore, " +
+                               "and is followed by any combination of letters, numbers and underscores."
+
     def retrieve_variable(path)
-      variable = @options.fetch(:as) { path[%r'_?(\w+)(\.\w+)*$', 1] }.try(:to_sym)
+      variable = if as = @options[:as]
+        raise ArgumentError.new(IDENTIFIER_ERROR_MESSAGE % (path)) unless as.to_s =~ /\A[a-z_]\w*\z/
+        as.to_sym
+      else
+        base = path[-1] == "/" ? "" : File.basename(path)
+        raise ArgumentError.new(IDENTIFIER_ERROR_MESSAGE % (path)) unless base =~ /\A_?([a-z]\w*)(\.\w+)*\z/
+        $1.to_sym
+      end
       variable_counter = :"#{variable}_counter" if @collection
       [variable, variable_counter]
     end
diff --git a/actionpack/test/template/render_test.rb b/actionpack/test/template/render_test.rb
index 3ce1d20bd9..164b8b9fa1 100644
--- a/actionpack/test/template/render_test.rb
+++ b/actionpack/test/template/render_test.rb
@@ -187,6 +187,13 @@ module RenderTestCases
     assert_equal "'#{nil.inspect}' is not an ActiveModel-compatible object. It must implement :to_partial_path.", e.message
   end
 
+  def test_render_partial_with_hyphen
+    e = assert_raises(ArgumentError) { @view.render(:partial => "test/a-in") }
+    assert_equal "The partial name (test/a-in) is not a valid Ruby identifier; " +
+      "make sure your partial name starts with a lowercase letter or underscore, " +
+      "and is followed by any combination of letters, numbers and underscores.", e.message
+  end
+
   def test_render_partial_with_errors
     e = assert_raises(ActionView::Template::Error) { @view.render(:partial => "test/raise") }
     assert_match %r!method.*doesnt_exist!, e.message