Fix VariableNotExistException for variable knowingly set to null

[kohlschuetter]

Currently, we would get a VariableNotExistException when in strict mode, even if a page property, for example, is knowingly set to null.

This is specifically important since {% if val %} returns true even for empty strings, making it almost impossible to avoid an exception and still support properties like page.last_modified_at.

Introduce a "found" state when traversing structures, and only consider a non-existant variable if found=false

This change breaks backwards compatibility with classes implementing LookupNode.Indexable. This is considered low-risk at the moment.

[msangel]

@kohlschuetter having the variable at context level needs some more explanation. Description said about missing variable and nullable variable difference. I agree somehow about this bug.

As for me solution may be in use Map implementation that permits null values, use return type something more then value or null (some wrapper with states: not_found, found_null, found_value).

But this global for template context not clear how will solve the issue.

[kohlschuetter]

@msangel sorry, I just saw your message. I've added another change that depends on this one, which is tangentially related ("sane" strict variable checks)

Regarding your question:

{% assign var = null %}{% if var %}true{% else %}false{% endif %}  

would fail without this patch (also see "paleo"'s comment in https://github.com/Shopify/liquid/issues/1034)

I thought about adding a wrapper, but then we would have to rely on all participating classes, which is hard to enforce. Adding a "found" property looks like the simpler approach.

Do you think there's a situation we currently don't cover?

[msangel]

I dug the issue and agree that we must provide a fix. What I still think is wrong in this change is:

We have global flag for anyone for everyone for any variable. This goes against thread safety, immutability (which we are trying to achieve) and static behavior. Can this functionality be done without such flags?

@kohlschuetter, don't worry about backward compatibility, a lot of recent commits are breaking compatibility, so next version probably will have own major version set. There a lot of deprecated removals. Thanks!