Strict fails when variable is used in an 'if'

[JakeWharton]

{% if page.thing %}
<tag>{{ page.thing }}</tag>
{% endif %}

Even in strict mode, the use of a conditional on the raw tag value should not fail since the template is guarding against its absence.

[JakeWharton]

This behavior is documented here: https://shopify.github.io/liquid/basics/types/#nil

[JakeWharton]

Based on RenderSettingsTest.renderWithStrictVariablesInAnd1 this behavior seems intentional. Somewhat unfortunate since it basically makes the otherwise super-useful strict mode unusable the first time you introduce a conditional reference.

Is this something you'd be open to changing?

[mosabua]

Seems reasonable to change that to me.

[bkiers]

@JakeWharton sorry, missed this. Will have a look at that.

[bkiers]

@JakeWharton when running this Ruby script:

template = Liquid::Template.parse('{% if page.thing %}<tag>{{ page.thing }}</tag>{% endif %}')
rendered = template.render({}, {strict_variables: true})

puts "rendered : `#{rendered}`"
puts "errors   : #{template.errors}"

the following output is printed:

rendered : ``
errors   : [#<Liquid::UndefinedVariable: Liquid error: undefined variable page>]

So although guarded, Liquid does record the error. The difference with Liqp and Ruby's Liquid is that Liqp throws the exception. A better way would be for Liqp to do something similar: record the errors in an instance variable of the Template class and continue rendering. This is a breaking change, but in line with how Liquid handles it, so I'll put this in the 0.8.0 release.

[JakeWharton]

Oh, interesting. Thanks for digging in. This must mean I'm running it in non-strict mode through Ruby 😶. I had no idea!

[bkiers]

It need not be a breaking change: I'll introduce a boolean in the RenderSettings that will indicate when in strict mode an exception should be thrown when encountering a non-existing variable.

The unit test will look like this:

@Test
public void raiseExceptionsInStrictModeFalseTest() {
    RenderSettings renderSettings = new RenderSettings.Builder()
            .withStrictVariables(true)
            .withRaiseExceptionsInStrictMode(false)
            .build();

    Template template = Template.parse("{{a}}{{b}}{{c}}").withRenderSettings(renderSettings);

    String rendered = template.render("b", "FOO");

    // There should be 2 exceptions recorded for non-existing variables `a` and `c`
    assertThat(template.errors().size(), is(2));

    // Rendering should not terminate
    assertThat(rendered, is("FOO"));
}

the above is in line with Ruby's Liquid lib. And to keep it backwards compatible in Liqp, the raiseExceptionsInStrictMode is default true.