Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow templates to define which locals they accept #45602

Merged
merged 1 commit into from Aug 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
24 changes: 24 additions & 0 deletions actionview/CHANGELOG.md
@@ -1,3 +1,27 @@
* Allow templates to set explicit locals.

By default, templates will accept any `locals` as keyword arguments. To define what `locals` a template accepts, add a `locals` magic comment:

```erb
<%# locals: (message:) -%>
<%= message %>
```

Default values can also be provided:

```erb
<%# locals: (message: "Hello, world!") -%>
<%= message %>
```

Or `locals` can be disabled entirely:

```erb
<%# locals: () %>
```

*Joel Hawksley*

* Add `include_seconds` option for `datetime_local_field`

This allows to omit seconds part in the input field, by passing `include_seconds: false`
Expand Down
20 changes: 18 additions & 2 deletions actionview/lib/action_view/base.rb
Expand Up @@ -237,11 +237,27 @@ def initialize(lookup_context, assigns, controller) # :nodoc:
_prepare_context
end

def _run(method, template, locals, buffer, add_to_stack: true, &block)
def _run(method, template, locals, buffer, add_to_stack: true, has_explicit_locals: false, &block)
_old_output_buffer, _old_virtual_path, _old_template = @output_buffer, @virtual_path, @current_template
@current_template = template if add_to_stack
@output_buffer = buffer
public_send(method, locals, buffer, &block)

if has_explicit_locals
begin
public_send(method, buffer, **locals, &block)
rescue ArgumentError => argument_error
raise(
ArgumentError,
argument_error.
message.
gsub("unknown keyword:", "unknown local:").
gsub("missing keyword:", "missing local:").
gsub("no keywords accepted", "no locals accepted")
)
end
else
public_send(method, locals, buffer, &block)
end
ensure
@output_buffer, @virtual_path, @current_template = _old_output_buffer, _old_virtual_path, _old_template
end
Expand Down
50 changes: 47 additions & 3 deletions actionview/lib/action_view/template.rb
Expand Up @@ -8,6 +8,8 @@ module ActionView
class Template
extend ActiveSupport::Autoload

EXPLICIT_LOCALS_REGEX = /\#\s+locals:\s+\((.*)\)/

# === Encodings in ActionView::Template
#
# ActionView::Template is one of a few sources of potential
Expand Down Expand Up @@ -121,12 +123,13 @@ class Template
attr_reader :variable, :format, :variant, :locals, :virtual_path

def initialize(source, identifier, handler, locals:, format: nil, variant: nil, virtual_path: nil)
@source = source
@source = source.dup
@identifier = identifier
@handler = handler
@compiled = false
@locals = locals
@virtual_path = virtual_path
@explicit_locals = nil

@variable = if @virtual_path
base = @virtual_path.end_with?("/") ? "" : ::File.basename(@virtual_path)
Expand Down Expand Up @@ -154,7 +157,7 @@ def supports_streaming?
def render(view, locals, buffer = ActionView::OutputBuffer.new, add_to_stack: true, &block)
instrument_render_template do
compile!(view)
view._run(method_name, self, locals, buffer, add_to_stack: add_to_stack, &block)
view._run(method_name, self, locals, buffer, add_to_stack: add_to_stack, has_explicit_locals: @explicit_locals.present?, &block)
end
rescue => e
handle_render_error(view, e)
Expand Down Expand Up @@ -222,6 +225,17 @@ def encode!
end
end

# This method is responsible for marking a template as having explicit locals
# and extracting any arguments declared in the format
# locals: (message:, label: "My Message")
def explicit_locals!
self.source.sub!(EXPLICIT_LOCALS_REGEX, "")
@explicit_locals = $1

return if @explicit_locals.nil? # Magic comment not found

@explicit_locals = "**nil" if @explicit_locals.blank?
end

# Exceptions are marshalled when using the parallel test runner with DRb, so we need
# to ensure that references to the template object can be marshalled as well. This means forgoing
Expand Down Expand Up @@ -273,14 +287,22 @@ def compile!(view)
# In general, this means that templates will be UTF-8 inside of Rails,
# regardless of the original source encoding.
def compile(mod)
explicit_locals!
source = encode!
code = @handler.call(self, source)

method_arguments =
if @explicit_locals.present?
"output_buffer, #{@explicit_locals}"
else
"local_assigns, output_buffer"
end

# Make sure that the resulting String to be eval'd is in the
# encoding of the code
original_source = source
source = +<<-end_src
def #{method_name}(local_assigns, output_buffer)
def #{method_name}(#{method_arguments})
@virtual_path = #{@virtual_path.inspect};#{locals_code};#{code}
end
end_src
Expand Down Expand Up @@ -311,6 +333,26 @@ def #{method_name}(local_assigns, output_buffer)
# the result into the template, but missing an end parenthesis.
raise SyntaxErrorInTemplate.new(self, original_source)
end

return unless @explicit_locals.present?

# Check compiled method parameters to ensure that only kwargs
# were provided as explicit locals, preventing `locals: (foo, *foo)` etc
# and allowing `locals: (foo:)`.

non_kwarg_parameters =
(mod.instance_method(method_name).parameters - [[:req, :output_buffer]]).
select { |parameter| ![:keyreq, :key, :keyrest, :nokey].include?(parameter[0]) }

return unless non_kwarg_parameters.any?

mod.undef_method(method_name)

raise ArgumentError.new(
Copy link
Member

Choose a reason for hiding this comment

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

Hum, if we raise we should probably remove the method before, no?

"#{non_kwarg_parameters.map { |_, name| "`#{name}`" }.to_sentence} set as non-keyword " \
"#{'argument'.pluralize(non_kwarg_parameters.length)} for #{short_identifier}. " \
"Locals can only be set as keyword arguments."
)
end

def handle_render_error(view, e)
Expand All @@ -323,6 +365,8 @@ def handle_render_error(view, e)
end

def locals_code
return "" if @explicit_locals.present?

# Only locals with valid variable names get set directly. Others will
# still be available in local_assigns.
locals = @locals - Module::RUBY_RESERVED_KEYWORDS
Expand Down
59 changes: 59 additions & 0 deletions actionview/test/template/template_test.rb
Expand Up @@ -149,6 +149,57 @@ def test_encoding_can_be_specified_with_magic_comment
assert_equal "\nhello \u{fc}mlat", render
end

def test_locals_can_be_disabled
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: () -%>")
render(foo: "bar")
end

assert_match(/no locals accepted/, error.message)
end

def test_locals_can_not_be_specified_with_positional_arguments
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: (foo) -%>")
render(foo: "bar")
end

assert_match(/`foo` set as non-keyword argument/, error.message)
end

def test_locals_can_be_specified_with_splat_arguments
@template = new_template("<%# locals: (**etc) -%><%= etc[:foo] %>")
assert_equal "bar", render(foo: "bar")
end

def test_locals_can_be_specified
@template = new_template("<%# locals: (message:) -%>\n<%= message %>")
assert_equal "Hello", render(message: "Hello")
end

def test_default_locals_can_be_specified
@template = new_template("<%# locals: (message: 'Hello') -%>\n<%= message %>")
assert_equal "Hello", render
end

def test_required_locals_can_be_specified
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: (message:) -%>")
render
end

assert_match(/missing local: :message/, error.message)
end

def test_extra_locals_raises_error
error = assert_raises(ActionView::Template::Error) do
@template = new_template("<%# locals: (message:) -%>")
render(message: "Hi", foo: "bar")
end

assert_match(/unknown local: :foo/, error.message)
end

# TODO: This is currently handled inside ERB. The case of explicitly
# lying about encodings via the normal Rails API should be handled
# inside Rails.
Expand All @@ -167,6 +218,14 @@ def test_encoding_can_be_specified_with_magic_comment_in_erb
end
end

def test_encoding_and_arguments_can_be_specified_with_magic_comment_in_erb
with_external_encoding Encoding::UTF_8 do
@template = new_template("<%# encoding: ISO-8859-1 %>\n<%# locals: (message: 'Hi!') %>\nhello \xFCmlat\n<%= message %>", virtual_path: nil)
assert_equal Encoding::UTF_8, render.encoding
assert_match(/hello \u{fc}mlat\nHi!/, render)
end
end

def test_error_when_template_isnt_valid_utf8
e = assert_raises ActionView::Template::Error do
@template = new_template("hello \xFCmlat", virtual_path: nil)
Expand Down
22 changes: 22 additions & 0 deletions guides/source/action_view_overview.md
Expand Up @@ -318,6 +318,28 @@ You can also specify a second partial to be rendered between instances of the ma

Rails will render the `_product_ruler` partial (with no data passed to it) between each pair of `_product` partials.

#### Explicit Locals
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@byroot thinking about this some more, what do you think about calling this Strict Locals instead of Explicit Locals? Strict feels better to me.

Copy link
Member

Choose a reason for hiding this comment

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

👍

Copy link
Contributor Author

Choose a reason for hiding this comment

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


By default, templates will accept any `locals` as keyword arguments. To define what `locals` a template accepts, add a `locals` magic comment:

```erb
<%# locals: (message:) -%>
<%= message %>
```

Default values can also be provided:

```erb
<%# locals: (message: "Hello, world!") -%>
<%= message %>
```

Or `locals` can be disabled entirely:

```erb
<%# locals: () %>
```

### Layouts

Layouts can be used to render a common view template around the results of Rails controller actions. Typically, a Rails application will have a couple of layouts that pages will be rendered within. For example, a site might have one layout for a logged in user and another for the marketing or sales side of the site. The logged in user layout might include top-level navigation that should be present across many controller actions. The sales layout for a SaaS app might include top-level navigation for things like "Pricing" and "Contact Us" pages. You would expect each layout to have a different look and feel. You can read about layouts in more detail in the [Layouts and Rendering in Rails](layouts_and_rendering.html) guide.
Expand Down