Skip to content

Commit

Permalink
Support hoistable / non-hoistable variables
Browse files Browse the repository at this point in the history
In this commit, we fix the let/const declaration behavior.

var i = 20;  // (1)
{
    i;  // (1)
    let i = 20;  // (2)
    i;  // (2)
}

ref #33
  • Loading branch information
Constellation committed Nov 16, 2014
1 parent 255da0f commit b8623c2
Show file tree
Hide file tree
Showing 3 changed files with 200 additions and 21 deletions.
83 changes: 62 additions & 21 deletions escope.js
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,10 @@
* @member {Map} Scope#set
*/
this.set = new Map();

this.__hoistable = new Map();
this.__nonHoistable = new Map();

/**
* The tainted variables of this scope, as <code>{ Variable.name :
* boolean }</code>.
Expand Down Expand Up @@ -483,7 +487,7 @@
this.__left = [];

if (opt.naming) {
this.__define(block.id, {
this.__define(block.id, false, {
type: Variable.FunctionName,
name: block.id,
node: block
Expand All @@ -495,6 +499,7 @@
this.taints.set('arguments', true);
this.set.set('arguments', variable);
this.variables.push(variable);
this.__defineHoistable(variable, false);
}

if (block.type === Syntax.FunctionExpression && block.id) {
Expand Down Expand Up @@ -589,29 +594,37 @@
}

this.__left = null;
this.__hoistable = null;
this.__nonHoistable = null;
currentScope = this.upper;
};

Scope.prototype.__resolve = function __resolve(ref) {
var variable, name;
name = ref.identifier.name;
if (this.set.has(name)) {
variable = this.set.get(name);
variable.references.push(ref);
variable.stack = variable.stack && ref.from.variableScope === this.variableScope;
if (ref.tainted) {
variable.tainted = true;
this.taints.set(variable.name, true);
}
ref.resolved = variable;
return true;
if ((variable = this.__hoistable.get(name))) {
return this.__resolveVariable(ref, variable);
}
return false;
};

Scope.prototype.__resolveVariable = function (ref, variable) {
variable.references.push(ref);
// FIXME: It is broken in ES6.
if (ref.from.variableScope === this.variableScope) {
variable.stack = false;
}
if (ref.tainted) {
variable.tainted = true;
this.taints.set(variable.name, true);
}
ref.resolved = variable;
return true;
};

Scope.prototype.__delegateToUpperScope = function __delegateToUpperScope(ref) {
if (this.upper) {
this.upper.__left.push(ref);
this.upper.__referencingEagerly(ref);
}
this.through.push(ref);
};
Expand All @@ -634,7 +647,20 @@
}
};

Scope.prototype.__define = function __define(node, info) {
Scope.prototype.__defineHoistable = function (variable, hoistable) {
var name;
name = variable.name;
if (hoistable) {
this.__nonHoistable['delete'](name);
this.__hoistable.set(name, variable);
} else {
if (!this.__hoistable.has(name)) {
this.__nonHoistable.set(name, variable);
}
}
};

Scope.prototype.__define = function __define(node, hoistable, info) {
var name, variable;
if (node && node.type === Syntax.Identifier) {
name = node.name;
Expand All @@ -649,6 +675,20 @@
variable.identifiers.push(node);
variable.defs.push(info);
}
this.__defineHoistable(variable, hoistable);
}
};

// Resolve if the current scope has non hoistable variable.
Scope.prototype.__referencingEagerly = function (ref) {
var name, variable;

name = ref.identifier.name;
variable = this.__nonHoistable.get(name);
if (variable) {
this.__resolveVariable(ref, variable);
} else {
this.__left.push(ref);
}
};

Expand All @@ -658,7 +698,7 @@
if (node && node.type === Syntax.Identifier) {
ref = new Reference(node, this, assign || Reference.READ, writeExpr, maybeImplicitGlobal);
this.references.push(ref);
this.__left.push(ref);
this.__referencingEagerly(ref);
}
};

Expand Down Expand Up @@ -876,7 +916,7 @@
* @return {ScopeManager}
*/
function analyze(tree, providedOptions) {
var resultScopes, scopeManager, variableTargetScope;
var resultScopes, scopeManager, variableTargetScope, hoistable;

options = updateDeeply(defaultOptions(), providedOptions);
resultScopes = scopes = [];
Expand Down Expand Up @@ -933,7 +973,7 @@
break;

case Syntax.CatchClause:
currentScope.__define(node.param, {
currentScope.__define(node.param, false, {
type: Variable.CatchClause,
name: node.param,
node: node
Expand Down Expand Up @@ -987,13 +1027,13 @@
// Since
// in ES5, FunctionDeclaration should be in FunctionBody.
// in ES6, FunctionDeclaration should be block scoped.
currentScope.upper.__define(node.id, {
currentScope.upper.__define(node.id, true, {
type: Variable.FunctionName,
name: node.id,
node: node
});
for (i = 0, iz = node.params.length; i < iz; ++i) {
currentScope.__define(node.params[i], {
currentScope.__define(node.params[i], false, {
type: Variable.Parameter,
name: node.params[i],
node: node,
Expand All @@ -1006,7 +1046,7 @@
case Syntax.ArrowFunctionExpression:
// id is defined in upper scope
for (i = 0, iz = node.params.length; i < iz; ++i) {
currentScope.__define(node.params[i], {
currentScope.__define(node.params[i], false, {
type: Variable.Parameter,
name: node.params[i],
node: node,
Expand Down Expand Up @@ -1099,10 +1139,11 @@
break;

case Syntax.VariableDeclaration:
variableTargetScope = (node.kind === 'var') ? currentScope.variableScope : currentScope;
hoistable = node.kind === 'var';
variableTargetScope = hoistable ? currentScope.variableScope : currentScope;
for (i = 0, iz = node.declarations.length; i < iz; ++i) {
decl = node.declarations[i];
variableTargetScope.__define(decl.id, {
variableTargetScope.__define(decl.id, hoistable, {
type: Variable.Variable,
name: decl.id,
node: decl,
Expand Down
52 changes: 52 additions & 0 deletions test/arguments.coffee
Original file line number Diff line number Diff line change
@@ -0,0 +1,52 @@
# -*- coding: utf-8 -*-
# Copyright (C) 2014 Yusuke Suzuki <utatane.tea@gmail.com>
#
# Redistribution and use in source and binary forms, with or without
# modification, are permitted provided that the following conditions are met:
#
# * Redistributions of source code must retain the above copyright
# notice, this list of conditions and the following disclaimer.
# * Redistributions in binary form must reproduce the above copyright
# notice, this list of conditions and the following disclaimer in the
# documentation and/or other materials provided with the distribution.
#
# THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
# AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
# ARE DISCLAIMED. IN NO EVENT SHALL <COPYRIGHT HOLDER> BE LIABLE FOR ANY
# DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
# (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
# LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
# ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
# (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
# THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.

expect = require('chai').expect
esprima = require 'esprima'
harmony = require '../third_party/esprima'
escope = require '..'

describe 'arguments', ->
it 'arguments are correctly materialized', ->
ast = esprima.parse """
(function () {
arguments;
}());
"""

scopeManager = escope.analyze ast
expect(scopeManager.scopes).to.have.length 2
globalScope = scopeManager.scopes[0]
expect(globalScope.type).to.be.equal 'global'
expect(globalScope.variables).to.have.length 0
expect(globalScope.references).to.have.length 0

scope = scopeManager.scopes[1]
expect(scope.type).to.be.equal 'function'
expect(scope.variables).to.have.length 1
expect(scope.variables[0].name).to.be.equal 'arguments'
expect(scope.isArgumentsMaterialized()).to.be.true
expect(scope.references).to.have.length 1
expect(scope.references[0].resolved).to.be.equal scope.variables[0]

# vim: set sw=4 ts=4 et tw=80 :
86 changes: 86 additions & 0 deletions test/es6-block-scope.coffee
Original file line number Diff line number Diff line change
Expand Up @@ -104,4 +104,90 @@ describe 'ES6 block scope', ->
expect(scope.variables[0].name).to.be.equal 'arguments'
expect(scope.references).to.have.length 0

it 'let is not hoistable#1', ->
ast = harmony.parse """
var i = 42;
{
i; // points global's i
let i = 20;
i; // points block scoped i
}
"""

scopeManager = escope.analyze ast, ecmaVersion: 6
expect(scopeManager.scopes).to.have.length 2

globalScope = scopeManager.scopes[0]
expect(globalScope.type).to.be.equal 'global'
expect(globalScope.variables).to.have.length 1
expect(globalScope.variables[0].name).to.be.equal 'i'
expect(globalScope.references).to.have.length 1

scope = scopeManager.scopes[1]
expect(scope.type).to.be.equal 'block'
expect(scope.variables).to.have.length 1
expect(scope.variables[0].name).to.be.equal 'i'
expect(scope.references).to.have.length 3
expect(scope.references[0].resolved).to.be.null # Since global scope
expect(scope.references[1].resolved).to.be.equal scope.variables[0]
expect(scope.references[2].resolved).to.be.equal scope.variables[0]

it 'let is not hoistable#2', ->
ast = harmony.parse """
(function () {
var i = 42; // (1)
i; // (1)
{
i; // (1)
{
i; // (1)
let i = 20; // (2)
i; // (2)
}
let i = 30; // (3)
i; // (3)
}
i; // (1)
}());
"""

scopeManager = escope.analyze ast, ecmaVersion: 6
expect(scopeManager.scopes).to.have.length 4

globalScope = scopeManager.scopes[0]
expect(globalScope.type).to.be.equal 'global'
expect(globalScope.variables).to.have.length 0
expect(globalScope.references).to.have.length 0

scope = scopeManager.scopes[1]
expect(scope.type).to.be.equal 'function'
expect(scope.variables).to.have.length 2
expect(scope.variables[0].name).to.be.equal 'arguments'
expect(scope.variables[1].name).to.be.equal 'i'
v1 = scope.variables[1]
expect(scope.references).to.have.length 3
expect(scope.references[0].resolved).to.be.equal v1
expect(scope.references[1].resolved).to.be.equal v1
expect(scope.references[2].resolved).to.be.equal v1

scope = scopeManager.scopes[2]
expect(scope.type).to.be.equal 'block'
expect(scope.variables).to.have.length 1
expect(scope.variables[0].name).to.be.equal 'i'
v3 = scope.variables[0]
expect(scope.references).to.have.length 3
expect(scope.references[0].resolved).to.be.equal v1
expect(scope.references[1].resolved).to.be.equal v3
expect(scope.references[2].resolved).to.be.equal v3

scope = scopeManager.scopes[3]
expect(scope.type).to.be.equal 'block'
expect(scope.variables).to.have.length 1
expect(scope.variables[0].name).to.be.equal 'i'
v2 = scope.variables[0]
expect(scope.references).to.have.length 3
expect(scope.references[0].resolved).to.be.equal v1
expect(scope.references[1].resolved).to.be.equal v2
expect(scope.references[2].resolved).to.be.equal v2

# vim: set sw=4 ts=4 et tw=80 :

2 comments on commit b8623c2

@Constellation
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hm, it's correct?
Need to investigate more.

@Constellation
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe it's not correct. I'll fix it.

Please sign in to comment.