Regression: Enabled strict_variables throw unnecessary errors

[das-peter]

The changes from #629 seem to throw unnecessary errors.
Following template should always run even if lie isn't defined and strict_variables is enabled - just the output should change:
The {{ baked_good }} is a{% if lie is defined and lie %} lie {% else %} reality {% endif %}!

However, with strict_variables enabled it fails with TwigException: Variable "lie" does not exist.
Running Example: https://jsfiddle.net/jsz9uy3h/6/

If you use https://cdn.jsdelivr.net/npm/twig@1.13.3/twig.min.js in above fiddle it works.

For the moment I can only think of introducing context awareness into the parse function of Twig.expression.type.variable. However this seems like a good way to clutter things :|

[toptalo]

Sorry, I don't know how to fix it

[das-peter]

I tried to fiddle with the code a bit but didn't come far.
While I was able to disable the strict check based on the fact that it was running in a "is defined" combination exemption was "bleeding" into other contexts so the exemption was in effect in the global scope.
Here's how it should function according to my understanding:
Following should pass without error: {{ lie is defined and lie }}
However following examples should still throw an error:

• {{ lie is defined and lie }} {{ lie }} - the "is defined", which causes the exemption, only applies to the first block.
• {{ (lie is defined and lie) or lie }} - the "is defined", which causes the exemption, only applies to the part in the parenthesis

This means the exemption of the strict_variable check has to be stored in an isolated scope.
So for {{ lie is defined and lie }} {{ lie }} the exception should apply to following part lie is defined and lie while for {{ (lie is defined and lie) or lie }} it's scoped to (lie is defined and lie).

As I'm very unfamiliar with the code my attempts to access the scope failed.
Only thing I was able to do was to detect if an exemption would apply - here's the snipped I used for that:

        {
            type: Twig.expression.type.variable,
            // Match any letter or _, then any number of letters, numbers, _ or -
            regex: /^[a-zA-Z_]\w*/,
            next: Twig.expression.set.operationsExtended.concat([
                Twig.expression.type.parameter.start
            ]),
            compile: Twig.expression.fn.compile.push,
            validate(match) {
                return (Twig.expression.reservedWords.indexOf(match[0]) < 0);
            },
            parse(token, stack, context, nextToken) {
                const state = this;

                var preprocess = function(token, state, nextToken) {
                    // If this is a check to see if the variable exists disable
                    // the strictVariable behavior.
                    if (nextToken.type === 'Twig.expression.type.test' && nextToken.filter === 'defined') {
                        state.template.options.ignoreStrictCheck = true;
                    }
                    return context[token.value];
                }

                // Get the variable from the context
                return Twig.expression.resolveAsync.call(state, preprocess, context, [token, state, nextToken])
                    .then(value => {
                        if (state.template.options.strictVariables && !state.template.options.ignoreStrictCheck && value === undefined) {
                            throw new Twig.Error('Variable "' + token.value + '" does not exist.');
                        }
                        // If this was an overruled handling register the
                        // variable in the context.
                        if (state.template.options.ignoreStrictCheck && value === undefined) {
                            // Create dummy variable and
                            context[token.value] = '';
                            state.template.options.ignoreStrictCheck = false;
                        }

                        stack.push(value);
                    });
                ;
            }
        },

I've added a preprocess function to Twig.expression.type.variable which allows to inspect the nextToken and set the exemption if appropriate.
However, the exemption was set on template level instead on the next logic parent.
Further I think we'd need to collect what variables are exempt in the current context to ensure following template also throws an error because of the undefined & unchecked foo:
{{ lie is defined and lie and foo }}

I leave it at that for the moment but we'll see if I can continue at some point where I've more free time to pursue this.

[BrianWalters]

Ran into the same problem. I use |default all the time to set a variable to null if it could be undefined in the context. This allows for good strict variable checking for local development, which is standard for PHP Twig projects. Most of my templates now throw exceptions after I updated to twig.js to 1.14. The version I used previously was 1.12 so the regression entered sometime between then.

[dorian-marchal]

Is there any news on this?

{{ foo is defined }} still throws when foo is undefined which make strict_variables unusable in some cases.

Could the commit that introduced the regression be reverted?

---

EDIT: I looked into it a bit and simply reverting the PR wouldn't help (the strict mode is not very useful without it). It doesn't look like there is an easy fix.

I'm going to disable strict_variables for now.

[axten]

same problem here

[toptalo]

I found a solution (crutch) how to not throw an error if next token after variable is test to defined.

In case when after undefined variable goes filter is defined this variable get false value in global context.
But this solution changes context.

src/twig.expression.js#L849

// Get the variable from the context
return Twig.expression.resolveAsync.call(state, context[token.value], context)
    .then(value => {
        const nextFilterDefined = token.next && token.next.filter === 'defined';
        if (state.template.options.strictVariables && value === undefined) {
            if(!nextFilterDefined) {
                throw new Twig.Error('Variable "' + token.value + '" does not exist.');
            } else {
                context[token.value] = false;
            }
        }

        stack.push(value);
    });

Here I add token.next:
src/twig.expression.js#L1257

token = tokens.shift();
token.next = tokens[0];
tokenTemplate = Twig.expression.handler[token.type];

[andreasschiestl]

Any idea when this can be merged and released? This bug makes strict_variables kind of useless :(

[willrowe]

This is just a bug report, not an actual fix for it.