Skip to content

Add references of default parameters and default values of destructuring #65

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 3 commits into from
May 18, 2015
Merged

Add references of default parameters and default values of destructuring #65

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
23bb40b
Add references of default parameters and default values of destructuring
mysticatea May 10, 2015
706f496
Add init property to Reference.
mysticatea May 16, 2015
766ce6c
Remove redundant try-finally
mysticatea May 18, 2015
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"browserify": "^9.0.3",
"chai": "^2.1.1",
"coffee-script": "^1.9.1",
"espree": "^1.11.0",
"espree": "^2.0.2",
"esprima": "^2.1.0",
"gulp": "~3.8.10",
"gulp-babel": "^4.0.0",
Expand Down
7 changes: 6 additions & 1 deletion src/reference.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ const RW = READ | WRITE;
* @class Reference
*/
export default class Reference {
constructor(ident, scope, flag, writeExpr, maybeImplicitGlobal, partial) {
constructor(ident, scope, flag, writeExpr, maybeImplicitGlobal, partial, init) {
/**
* Identifier syntax node.
* @member {esprima#Identifier} Reference#identifier
Expand Down Expand Up @@ -71,6 +71,11 @@ export default class Reference {
* @member {boolean} Reference#partial
*/
this.partial = partial;
/**
* Whether the Reference is to write of initialization.
* @member {boolean} Reference#init
*/
this.init = init;
}
this.__maybeImplicitGlobal = maybeImplicitGlobal;
}
Expand Down
192 changes: 151 additions & 41 deletions src/referencer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,32 +29,37 @@ import { ParameterDefinition, Definition } from './definition';
import assert from 'assert';

class PatternVisitor extends esrecurse.Visitor {
constructor(rootPattern, referencer, callback) {
constructor(rootPattern, callback) {
super();
this.referencer = referencer;
this.rootPattern = rootPattern;
this.callback = callback;
}

perform(pattern) {
if (pattern.type === Syntax.Identifier) {
this.callback(pattern, true);
return;
}
this.visit(pattern);
this.assignments = [];
this.rightHandNodes = [];
this.restElements = [];
}

Identifier(pattern) {
this.callback(pattern, false);
const lastRestElement = getLast(this.restElements);
this.callback(pattern, {
topLevel: pattern === this.rootPattern,
rest: lastRestElement != null && lastRestElement.argument === pattern,
assignments: this.assignments
});
}

ObjectPattern(pattern) {
var i, iz, property;
for (i = 0, iz = pattern.properties.length; i < iz; ++i) {
property = pattern.properties[i];
if (property.shorthand) {
this.visit(property.key);
continue;

// Computed property's key is a right hand node.
if (property.computed) {
this.rightHandNodes.push(property.key);
}

// If it's shorthand, its key is same as its value.
// If it's shorthand and has its default value, its key is same as its value.left (the value is AssignmentPattern).
// If it's not shorthand, the name of new variable is its value's.
this.visit(property.value);
}
}
Expand All @@ -63,27 +68,105 @@ class PatternVisitor extends esrecurse.Visitor {
var i, iz, element;
for (i = 0, iz = pattern.elements.length; i < iz; ++i) {
element = pattern.elements[i];
if (element) {
this.visit(element);
}
this.visit(element);
}
}

AssignmentPattern(pattern) {
this.visit(pattern.left);
// FIXME: Condier TDZ scope.
this.referencer.visit(pattern.right);
this.assignments.push(pattern);
try {
this.visit(pattern.left);
this.rightHandNodes.push(pattern.right);
} finally {
this.assignments.pop();
}
Copy link
Member

Choose a reason for hiding this comment

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

Why these try-finally is needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's to be exceptions safety.
I'm afraid to leak the resource, so I use finally for a couple of open/close API (or processes of reverting) from long habit.
But I agree that it's redundant here.

}

RestElement(pattern) {
this.restElements.push(pattern);
try {
this.visit(pattern.argument);
} finally {
this.restElements.pop();
}
Copy link
Member

Choose a reason for hiding this comment

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

Ditto.

}

MemberExpression(node) {
// Computed property's key is a right hand node.
if (node.computed) {
this.rightHandNodes.push(node.property);
}
// the object is only read, write to its property.
this.rightHandNodes.push(node.object);
}

//
// ForInStatement.left and AssignmentExpression.left are LeftHandSideExpression.
// By spec, LeftHandSideExpression is Pattern or MemberExpression.
// (see also: https://github.com/estree/estree/pull/20#issuecomment-74584758)
// But espree 2.0 and esprima 2.0 parse to ArrayExpression, ObjectExpression, etc...
//

SpreadElement(node) {
this.visit(node.argument);
}

ArrayExpression(node) {
node.elements.forEach(this.visit, this);
}

ObjectExpression(node) {
node.properties.forEach(property => {
// Computed property's key is a right hand node.
if (property.computed) {
this.rightHandNodes.push(property.key);
}
this.visit(property.value);
});
}

AssignmentExpression(node) {
this.assignments.push(node);
try {
this.visit(node.left);
this.rightHandNodes.push(node.right);
} finally {
Copy link
Member

Choose a reason for hiding this comment

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

Ah, let's drop this try-finally.

The other part looks very nice to me :D
Thank you for your great patch! So after fixing it, I'll merge your PR soon!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oops, sorry!

this.assignments.pop();
}
}

CallExpression(node) {
// arguments are right hand nodes.
node.arguments.forEach(a => { this.rightHandNodes.push(a); });
this.visit(node.callee);
}
}

function getLast(xs) {
return xs[xs.length - 1] || null;
}

function traverseIdentifierInPattern(rootPattern, referencer, callback) {
var visitor = new PatternVisitor(rootPattern, referencer, callback);
visitor.perform(rootPattern);
// Call the callback at left hand identifier nodes, and Collect right hand nodes.
var visitor = new PatternVisitor(rootPattern, callback);
visitor.visit(rootPattern);

// Process the right hand nodes recursively.
if (referencer != null) {
visitor.rightHandNodes.forEach(referencer.visit, referencer);
}
}

function isPattern(node) {
var nodeType = node.type;
return nodeType === Syntax.Identifier || nodeType === Syntax.ObjectPattern || nodeType === Syntax.ArrayPattern || nodeType === Syntax.SpreadElement || nodeType === Syntax.RestElement || nodeType === Syntax.AssignmentPattern;
return (
nodeType === Syntax.Identifier ||
nodeType === Syntax.ObjectPattern ||
nodeType === Syntax.ArrayPattern ||
nodeType === Syntax.SpreadElement ||
nodeType === Syntax.RestElement ||
nodeType === Syntax.AssignmentPattern
);
}

// Importing ImportDeclaration.
Expand Down Expand Up @@ -168,7 +251,7 @@ export default class Referencer extends esrecurse.Visitor {
// http://people.mozilla.org/~jorendorff/es6-draft.html#sec-runtime-semantics-forin-div-ofexpressionevaluation-abstract-operation
// TDZ scope hides the declaration's names.
this.scopeManager.__nestTDZScope(node, iterationNode);
this.visitVariableDeclaration(this.currentScope(), Variable.TDZ, iterationNode.left, 0);
this.visitVariableDeclaration(this.currentScope(), Variable.TDZ, iterationNode.left, 0, true);
}

materializeIterationScope(node) {
Expand All @@ -178,12 +261,32 @@ export default class Referencer extends esrecurse.Visitor {
letOrConstDecl = node.left;
this.visitVariableDeclaration(this.currentScope(), Variable.Variable, letOrConstDecl, 0);
this.visitPattern(letOrConstDecl.declarations[0].id, (pattern) => {
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, null, true);
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, null, true, true);
});
}

visitPattern(node, callback) {
traverseIdentifierInPattern(node, this, callback);
referencingDefaultValue(pattern, assignments, maybeImplicitGlobal, init) {
const scope = this.currentScope();
assignments.forEach(assignment => {
scope.__referencing(
pattern,
Reference.WRITE,
assignment.right,
maybeImplicitGlobal,
pattern !== assignment.left,
init);
});
}

visitPattern(node, options, callback) {
if (typeof options === 'function') {
callback = options;
options = {processRightHandNodes: false}
}
traverseIdentifierInPattern(
node,
options.processRightHandNodes ? this : null,
callback);
}

visitFunction(node) {
Expand Down Expand Up @@ -215,15 +318,18 @@ export default class Referencer extends esrecurse.Visitor {
// Consider this function is in the MethodDefinition.
this.scopeManager.__nestFunctionScope(node, this.isInnerMethodDefinition);

// Process parameter declarations.
for (i = 0, iz = node.params.length; i < iz; ++i) {
this.visitPattern(node.params[i], (pattern) => {
this.visitPattern(node.params[i], {processRightHandNodes: true}, (pattern, info) => {
this.currentScope().__define(pattern,
new ParameterDefinition(
pattern,
node,
i,
false
info.rest
));

this.referencingDefaultValue(pattern, info.assignments, null, true);
});
}

Expand Down Expand Up @@ -313,34 +419,33 @@ export default class Referencer extends esrecurse.Visitor {
if (node.left.type === Syntax.VariableDeclaration) {
this.visit(node.left);
this.visitPattern(node.left.declarations[0].id, (pattern) => {
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, null, true);
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, null, true, true);
});
} else {
if (!isPattern(node.left)) {
this.visit(node.left);
}
this.visitPattern(node.left, (pattern) => {
this.visitPattern(node.left, {processRightHandNodes: true}, (pattern, info) => {
var maybeImplicitGlobal = null;
if (!this.currentScope().isStrict) {
maybeImplicitGlobal = {
pattern: pattern,
node: node
};
}
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, maybeImplicitGlobal, true);
this.referencingDefaultValue(pattern, info.assignments, maybeImplicitGlobal, false);
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, maybeImplicitGlobal, true, false);
});
}
this.visit(node.right);
this.visit(node.body);
}
}

visitVariableDeclaration(variableTargetScope, type, node, index) {
visitVariableDeclaration(variableTargetScope, type, node, index, fromTDZ) {
// If this was called to initialize a TDZ scope, this needs to make definitions, but doesn't make references.
var decl, init;

decl = node.declarations[index];
init = decl.init;
this.visitPattern(decl.id, (pattern, toplevel) => {
this.visitPattern(decl.id, {processRightHandNodes: !fromTDZ}, (pattern, info) => {
variableTargetScope.__define(pattern,
new Definition(
type,
Expand All @@ -351,24 +456,28 @@ export default class Referencer extends esrecurse.Visitor {
node.kind
));

if (!fromTDZ) {
this.referencingDefaultValue(pattern, info.assignments, null, true);
}
if (init) {
this.currentScope().__referencing(pattern, Reference.WRITE, init, null, !toplevel);
this.currentScope().__referencing(pattern, Reference.WRITE, init, null, !info.topLevel, true);
}
});
}

AssignmentExpression(node) {
if (isPattern(node.left)) {
if (node.operator === '=') {
this.visitPattern(node.left, (pattern, toplevel) => {
this.visitPattern(node.left, {processRightHandNodes: true}, (pattern, info) => {
var maybeImplicitGlobal = null;
if (!this.currentScope().isStrict) {
maybeImplicitGlobal = {
pattern: pattern,
node: node
};
}
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, maybeImplicitGlobal, !toplevel);
this.referencingDefaultValue(pattern, info.assignments, maybeImplicitGlobal, false);
this.currentScope().__referencing(pattern, Reference.WRITE, node.right, maybeImplicitGlobal, !info.topLevel, false);
});
} else {
this.currentScope().__referencing(node.left, Reference.RW, node.right);
Expand All @@ -382,7 +491,7 @@ export default class Referencer extends esrecurse.Visitor {
CatchClause(node) {
this.scopeManager.__nestCatchScope(node);

this.visitPattern(node.param, (pattern) => {
this.visitPattern(node.param, {processRightHandNodes: true}, (pattern, info) => {
this.currentScope().__define(pattern,
new Definition(
Variable.CatchClause,
Expand All @@ -392,6 +501,7 @@ export default class Referencer extends esrecurse.Visitor {
null,
null
));
this.referencingDefaultValue(pattern, info.assignments, null, true);
});
this.visit(node.body);

Expand Down
4 changes: 2 additions & 2 deletions src/scope.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,7 +344,7 @@ export default class Scope {
}
}

__referencing(node, assign, writeExpr, maybeImplicitGlobal, partial) {
__referencing(node, assign, writeExpr, maybeImplicitGlobal, partial, init) {
// because Array element may be null
if (!node || node.type !== Syntax.Identifier) {
return;
Expand All @@ -355,7 +355,7 @@ export default class Scope {
return;
}

let ref = new Reference(node, this, assign || Reference.READ, writeExpr, maybeImplicitGlobal, !!partial);
let ref = new Reference(node, this, assign || Reference.READ, writeExpr, maybeImplicitGlobal, !!partial, !!init);
this.references.push(ref);
this.__left.push(ref);
}
Expand Down
Loading