Skip to content

Set Context#initialize instance variables before squashing assigns#1307

Merged
dylanahsmith merged 1 commit into
masterfrom
context-set-ivs-before-squash
Oct 7, 2020
Merged

Set Context#initialize instance variables before squashing assigns#1307
dylanahsmith merged 1 commit into
masterfrom
context-set-ivs-before-squash

Conversation

@dylanahsmith
Copy link
Copy Markdown
Contributor

Problem

While rebasing a liquid-c PR (Shopify/liquid-c#60), I noticed in this test test failure

ContextTest#test_context_initialization_with_a_proc_in_environment:
NoMethodError: undefined method `each' for nil:NilClass
    /Users/dylants/src/liquid/lib/liquid/strainer_factory.rb:27:in `strainer_from_cache'
    /Users/dylants/src/liquid/lib/liquid/strainer_factory.rb:14:in `create'
    /Users/dylants/src/liquid/lib/liquid/context.rb:56:in `strainer'
    /Users/dylants/src/liquid/lib/liquid/context.rb:170:in `c_evaluate'
    /Users/dylants/src/liquid/lib/liquid/context.rb:170:in `[]'
    ../liquid/test/integration/context_test.rb:470:in `block in test_context_initialization_with_a_proc_in_environment'
    /Users/dylants/src/liquid/lib/liquid/context.rb:207:in `lookup_and_evaluate'
    /Users/dylants/src/liquid/lib/liquid/context.rb:271:in `block (2 levels) in squash_instance_assigns_with_environments'
    /Users/dylants/src/liquid/lib/liquid/context.rb:269:in `each'
    /Users/dylants/src/liquid/lib/liquid/context.rb:269:in `block in squash_instance_assigns_with_environments'
    /Users/dylants/src/liquid/lib/liquid/context.rb:268:in `each_key'
    /Users/dylants/src/liquid/lib/liquid/context.rb:268:in `squash_instance_assigns_with_environments'
    /Users/dylants/src/liquid/lib/liquid/context.rb:37:in `initialize'
    ../liquid/test/integration/context_test.rb:470:in `new'
    ../liquid/test/integration/context_test.rb:470:in `test_context_initialization_with_a_proc_in_environment'

that in Liquid::Context#initialize we call squash_instance_assigns_with_environments, which can evaluate Procs in the environment, which means that we are passing in a partially initialized Context object into those Proc objects.

Solution

Move the call to squash_instance_assigns_with_environments to the end of Liquid::Context#initialize so that we aren't exposing it in a partially initialized state.

@dylanahsmith dylanahsmith merged commit d250a7f into master Oct 7, 2020
@dylanahsmith dylanahsmith deleted the context-set-ivs-before-squash branch October 7, 2020 01:00
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants