1
0
Fork 0
mirror of https://github.com/rails/rails.git synced 2022-11-09 12:12:34 -05:00

Deprecate rendering templates with . in the name

Allowing templates with "." introduces some ambiguity. Is index.html.erb
a template named "index" with format "html", or is it a template named
"index.html" without a format? We know it's probably the former, but if
we asked ActionView to render "index.html" we would currently get some
combination of the two: a Template with index.html as the name and
virtual path, but with html as the format.

This deprecates having "." anywhere in the template's name, we should
reserve this character for specifying formats. I think in 99% of cases
this will be people specifying `index.html` instead of simply `index`.

This was actually once deprecated in the 3.x series (removed in
6c57177f2c) but I don't think we can rely
on nobody having introduced this in the past 8 years.
This commit is contained in:
John Hawthorn 2020-03-17 18:36:41 -07:00
parent d56d2e7406
commit f84773a587
6 changed files with 27 additions and 8 deletions

View file

@ -365,7 +365,9 @@ class ExpiresInRenderTest < ActionController::TestCase
def test_dynamic_render def test_dynamic_render
assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__)) assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__))
assert_raises ActionView::MissingTemplate do assert_raises ActionView::MissingTemplate do
get :dynamic_render, params: { id: '../\\../test/abstract_unit.rb' } assert_deprecated do
get :dynamic_render, params: { id: '../\\../test/abstract_unit.rb' }
end
end end
end end

View file

@ -109,7 +109,7 @@ class RendererTest < ActiveSupport::TestCase
xml = "<p>Hello world!</p>\n" xml = "<p>Hello world!</p>\n"
assert_equal html, render["respond_to/using_defaults"] assert_equal html, render["respond_to/using_defaults"]
assert_equal xml, render["respond_to/using_defaults.xml.builder"] assert_equal xml, assert_deprecated { render["respond_to/using_defaults.xml.builder"] }
assert_equal xml, render["respond_to/using_defaults", formats: :xml] assert_equal xml, render["respond_to/using_defaults", formats: :xml]
end end

View file

@ -227,6 +227,10 @@ module ActionView
end end
def find_template_paths_from_details(path, details) def find_template_paths_from_details(path, details)
if path.name.include?(".")
ActiveSupport::Deprecation.warn("Rendering actions with '.' in the name is deprecated: #{path}")
end
query = build_query(path, details) query = build_query(path, details)
find_template_paths(query) find_template_paths(query)
end end
@ -331,6 +335,11 @@ module ActionView
end end
def find_template_paths_from_details(path, details) def find_template_paths_from_details(path, details)
if path.name.include?(".")
# Fall back to the unoptimized resolver, which will warn
return super
end
candidates = find_candidate_template_paths(path) candidates = find_candidate_template_paths(path)
regex = build_regex(path, details) regex = build_regex(path, details)

View file

@ -8,7 +8,7 @@ class FallbackFileSystemResolverTest < ActiveSupport::TestCase
end end
def test_should_have_no_virtual_path def test_should_have_no_virtual_path
templates = @root_resolver.find_all("hello_world.erb", "#{FIXTURE_LOAD_PATH}/test", false, locale: [], formats: [:html], variants: [], handlers: [:erb]) templates = @root_resolver.find_all("hello_world", "#{FIXTURE_LOAD_PATH}/test", false, locale: [], formats: [:html], variants: [], handlers: [:erb])
assert_equal 1, templates.size assert_equal 1, templates.size
assert_equal "Hello world!", templates[0].source assert_equal "Hello world!", templates[0].source
assert_nil templates[0].virtual_path assert_nil templates[0].virtual_path

View file

@ -200,7 +200,9 @@ module RenderTestCases
def test_render_outside_path def test_render_outside_path
assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__)) assert File.exist?(File.expand_path("../../test/abstract_unit.rb", __dir__))
assert_raises ActionView::MissingTemplate do assert_raises ActionView::MissingTemplate do
@view.render(template: "../\\../test/abstract_unit.rb") assert_deprecated do
@view.render(template: "../\\../test/abstract_unit.rb")
end
end end
end end
@ -338,8 +340,10 @@ module RenderTestCases
end end
def test_render_partial_collection_with_partial_name_containing_dot def test_render_partial_collection_with_partial_name_containing_dot
assert_equal "Hello: davidHello: mary", assert_deprecated do
@view.render(partial: "test/customer.mobile", collection: [ Customer.new("david"), Customer.new("mary") ]) assert_equal "Hello: davidHello: mary",
@view.render(partial: "test/customer.mobile", collection: [ Customer.new("david"), Customer.new("mary") ])
end
end end
def test_render_partial_collection_as_by_string def test_render_partial_collection_as_by_string
@ -579,7 +583,11 @@ module RenderTestCases
def test_render_ignores_templates_with_malformed_template_handlers def test_render_ignores_templates_with_malformed_template_handlers
%w(malformed malformed.erb malformed.html.erb malformed.en.html.erb).each do |name| %w(malformed malformed.erb malformed.html.erb malformed.en.html.erb).each do |name|
assert File.exist?(File.expand_path("#{FIXTURE_LOAD_PATH}/test/malformed/#{name}~")), "Malformed file (#{name}~) which should be ignored does not exists" assert File.exist?(File.expand_path("#{FIXTURE_LOAD_PATH}/test/malformed/#{name}~")), "Malformed file (#{name}~) which should be ignored does not exists"
assert_raises(ActionView::MissingTemplate) { @view.render(template: "test/malformed/#{name}") } assert_raises(ActionView::MissingTemplate) do
ActiveSupport::Deprecation.silence do
@view.render(template: "test/malformed/#{name}")
end
end
end end
end end

View file

@ -169,7 +169,7 @@ module ResolverSharedTests
def test_virtual_path_is_preserved_with_dot def test_virtual_path_is_preserved_with_dot
with_file "test/hello_world.html.erb", "Hello html!" with_file "test/hello_world.html.erb", "Hello html!"
template = context.find("hello_world.html", "test", false, [], {}) template = assert_deprecated { context.find("hello_world.html", "test", false, [], {}) }
assert_equal "test/hello_world.html", template.virtual_path assert_equal "test/hello_world.html", template.virtual_path
template = context.find("hello_world", "test", false, [], {}) template = context.find("hello_world", "test", false, [], {})