estools / escope

Escope: ECMAScript scope analyzer
BSD 2-Clause "Simplified" License
546 stars 77 forks source link

Can't locate variable within named function's scope when function assigned to other variable #125

Open lainverse opened 7 years ago

lainverse commented 7 years ago

Here is an example code:

let escope = require('escope');
let esprima = require('esprima');
let estraverse = require('estraverse');

let ast = esprima.parse(`
    var a = function a() {
        var b = 1;
    }, c = function () {
        var d = 2
    };
    function x() {
        var z = 3;
    }
`);

let scopeman = escope.analyze(ast);
let scope = scopeman.acquire(ast);
let fname = void 0;

estraverse.traverse(ast, {
    enter: (n, p) => {
        if (n.type === 'VariableDeclarator') {
            console.log(fname, n.id.name, !!scope.set.get(n.id.name));
            if (fname && scope.childScopes.length === 1)
                console.log('!!!subscope:', !!scope.childScopes[0].set.get(n.id.name));
        }
        if (/Function/.test(n.type)) {
            fname = n.id ? n.id.name : void 0;
            scope = scopeman.acquire(n);
        }
    },
    leave: (n, p) => {
        if (/Function/.test(n.type)) {
            fname = void 0;
            scope = scope.upper
        }
    }
});

Expected result: undefined 'a' true a b true undefined 'c' true undefined 'd' true x z true

Actual result (notice second and third rows): undefined 'a' true a b false !!!subscope: true undefined 'c' true undefined 'd' true x z true

Why is it like that?

Also, the only variable available in the scope of function 'a' is 'a' which points to function itself.

michaelficarra commented 7 years ago

Named function expressions create an extra scope object which only contain the function name binding. It should have a child scope which contains all of the bindings in the function body and parameter list.

lainverse commented 7 years ago

To be honest such behaviour doesn't makes much sense from my point of view, but if this is what actually happening when it runs then ok. However, why does 'acquire' points to this particular scope? It's rather inconvenient and inconsistent in combination with traverse. It makes more sense to point to actual function's scope. That one would remain available through .upper anyway in case anyone ever needs it.

michaelficarra commented 7 years ago

I disagree. The nearest scope object to the function expression's AST node is the one containing the function name. I would be okay with associating the inner scope object with the FunctionBody node, if that's not how it's already done today.

lainverse commented 7 years ago

Ah, I see there is scope.functionExpressionScope marker there to know when to go one step further.