Skip to content

Commit

Permalink
Refactor compiled_source
Browse files Browse the repository at this point in the history
This is a continuation of #46706 to make sure we don't need to set an
instance variable to `@original_source` for the `compile` method to use.

We can't call `strict_locals!` after encode so we need to set it to a
local variable in `complile`. We changed the `strict_locals!` method to
check `NONE` instead of lazily defining instance variables which let us
simplify `strict_locals?` to return the value of `strict_locals!`. This
simplifies and clarifies the code.

Co-authored-by: Aaron Patterson tenderlove@ruby-lang.org
  • Loading branch information
eileencodes committed Dec 14, 2022
1 parent 06a872f commit 334fa12
Showing 1 changed file with 20 additions and 18 deletions.
38 changes: 20 additions & 18 deletions actionview/lib/action_view/template.rb
Expand Up @@ -122,6 +122,8 @@ class Template
attr_reader :identifier, :handler
attr_reader :variable, :format, :variant, :virtual_path

NONE = Object.new

def initialize(source, identifier, handler, locals:, format: nil, variant: nil, virtual_path: nil)
@source = source.dup
@identifier = identifier
Expand All @@ -139,6 +141,7 @@ def initialize(source, identifier, handler, locals:, format: nil, variant: nil,
@format = format
@variant = variant
@compile_mutex = Mutex.new
@strict_locals = NONE
end

# The locals this template has been or will be compiled for, or nil if this
Expand Down Expand Up @@ -177,10 +180,10 @@ def render(view, locals, buffer = nil, add_to_stack: true, &block)
instrument_render_template do
compile!(view)
if buffer
view._run(method_name, self, locals, buffer, add_to_stack: add_to_stack, has_strict_locals: @strict_locals, &block)
view._run(method_name, self, locals, buffer, add_to_stack: add_to_stack, has_strict_locals: strict_locals?, &block)
nil
else
view._run(method_name, self, locals, OutputBuffer.new, add_to_stack: add_to_stack, has_strict_locals: @strict_locals, &block)&.to_s
view._run(method_name, self, locals, OutputBuffer.new, add_to_stack: add_to_stack, has_strict_locals: strict_locals?, &block)&.to_s
end
end
rescue => e
Expand Down Expand Up @@ -256,20 +259,20 @@ def encode!
# and extracting any arguments declared in the format
# locals: (message:, label: "My Message")
def strict_locals!
self.source.sub!(STRICT_LOCALS_REGEX, "")
@strict_locals = $1
if @strict_locals == NONE
self.source.sub!(STRICT_LOCALS_REGEX, "")
@strict_locals = $1

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

return if @strict_locals.nil? # Magic comment not found
@strict_locals = "**nil" if @strict_locals.blank?
end

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

def strict_locals?
if defined?(@strict_locals)
@strict_locals
else
STRICT_LOCALS_REGEX === self.source
end
strict_locals!
end

# Exceptions are marshalled when using the parallel test runner with DRb, so we need
Expand Down Expand Up @@ -321,20 +324,19 @@ def compile!(view)
# involves setting strict_locals! if applicable, encoding the template, and setting
# frozen string literal.
def compiled_source
strict_locals!
set_strict_locals = strict_locals!
source = encode!
code = @handler.call(self, source)

method_arguments =
if @strict_locals
"output_buffer, #{@strict_locals}"
if set_strict_locals
"output_buffer, #{set_strict_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}(#{method_arguments})
@virtual_path = #{@virtual_path.inspect};#{locals_code};#{code}
Expand Down Expand Up @@ -381,10 +383,10 @@ def compile(mod)
# Account for when code in the template is not syntactically valid; e.g. if we're using
# ERB and the user writes <%= foo( %>, attempting to call a helper `foo` and interpolate
# the result into the template, but missing an end parenthesis.
raise SyntaxErrorInTemplate.new(self, @original_source)
raise SyntaxErrorInTemplate.new(self, encode!)
end

return unless @strict_locals
return unless strict_locals?

# Check compiled method parameters to ensure that only kwargs
# were provided as strict locals, preventing `locals: (foo, *foo)` etc
Expand Down Expand Up @@ -423,7 +425,7 @@ def handle_render_error(view, e)
end

def locals_code
return "" if @strict_locals
return "" if strict_locals?

# Only locals with valid variable names get set directly. Others will
# still be available in local_assigns.
Expand Down

0 comments on commit 334fa12

Please sign in to comment.