Skip to content
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.

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

Implement Export{Default,Namespace}Specifier parsing ourselves. #146

Merged
merged 6 commits into from
Apr 25, 2017
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
198 changes: 198 additions & 0 deletions lib/parsers/acorn-extensions.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,198 @@
"use strict";

const tt = require("acorn").tokTypes;
const hasOwn = Object.prototype.hasOwnProperty;

exports.enableAll = function (parser) {
exports.enableTolerance(parser);
exports.enableExportExtensions(parser);
};

exports.enableTolerance = function (parser) {
// It's not Reify's job to enforce strictness.
parser.strict = false;

// Tolerate recoverable parse errors.
parser.raiseRecoverable = noopRaiseRecoverable;
};

function noopRaiseRecoverable() {}

exports.enableExportExtensions = function (parser) {
// Lookahead method.
parser.lookAhead = lookAhead;

// Export-related modifications.
parser.parseExport = parseExport;
parser.isExportDefaultSpecifier = isExportDefaultSpecifier;
parser.parseExportSpecifiersMaybe = parseExportSpecifiersMaybe;
parser.parseExportFromWithCheck = parseExportFromWithCheck;
parser.parseExportFrom = parseExportFrom;
parser.checkExport = checkExport;
parser.shouldParseExportDeclaration = shouldParseExportDeclaration;
};

function parseExport(node, exports) {
this.next();
if (this.type === tt.star) {
const specifier = this.startNode();
this.next();
if (this.eatContextual("as")) {
// export * as ns from '...'
specifier.exported = this.parseIdent(true);
node.specifiers = [
this.finishNode(specifier, "ExportNamespaceSpecifier")
];
this.parseExportSpecifiersMaybe(node);
this.parseExportFromWithCheck(node, exports, true);
} else {
// export * from '...'
this.parseExportFromWithCheck(node, exports, true);
return this.finishNode(node, "ExportAllDeclaration");
}
} else if (this.isExportDefaultSpecifier()) {
// export def from '...'
const specifier = this.startNode();
specifier.exported = this.parseIdent(true);
node.specifiers = [
this.finishNode(specifier, "ExportDefaultSpecifier")
];
if (this.type === tt.comma &&
this.lookAhead(1).type === tt.star) {
// export def, * as ns from '...'
this.expect(tt.comma);
const specifier = this.startNode();
this.expect(tt.star);
this.expectContextual("as");
specifier.exported = this.parseIdent(true);
node.specifiers.push(
this.finishNode(specifier, "ExportNamespaceSpecifier")
);
Copy link
Contributor

@jdalton jdalton Apr 25, 2017

Choose a reason for hiding this comment

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

Just curious does the execution of the this.expect stuff just above matter?
I noticed one was before the const specifier =.....

Copy link
Owner Author

Choose a reason for hiding this comment

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

Yes, this.expect(...) modifies this.start (by calling this.eat(...) which calls this.next()), which is used by this.startNode to set the .start location of the new node, so the relative ordering of this.expect(...) and this.startNode() is important.

Copy link
Contributor

Choose a reason for hiding this comment

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

😵

} else {
// export def, { x, y as z } from '...'
this.parseExportSpecifiersMaybe(node);
}
this.parseExportFromWithCheck(node, exports, true);
} else if (this.eat(tt._default)) {
// export default ...
this.checkExport(exports, "default", this.lastTokStart);
let isAsync;
if (this.type === tt._function || (isAsync = this.isAsyncFunction())) {
let fNode = this.startNode();
this.next();
if (isAsync) this.next();
node.declaration = this.parseFunction(fNode, "nullableID", false, isAsync);
} else if (this.type === tt._class) {
let cNode = this.startNode();
node.declaration = this.parseClass(cNode, "nullableID");
} else {
node.declaration = this.parseMaybeAssign();
this.semicolon();
}
return this.finishNode(node, "ExportDefaultDeclaration");
} else if (this.shouldParseExportDeclaration()) {
// export var|const|let|function|class ...
node.declaration = this.parseStatement(true);
if (node.declaration.type === "VariableDeclaration") {
this.checkVariableExport(exports, node.declaration.declarations);
} else {
this.checkExport(
exports,
node.declaration.id.name,
node.declaration.id.start
);
}
node.specifiers = [];
node.source = null;
} else {
// export { x, y as z } [from '...']
node.declaration = null;
node.specifiers = this.parseExportSpecifiers(exports);
this.parseExportFrom(node, false);
}
return this.finishNode(node, "ExportNamedDeclaration");
}

function lookAhead(n) {
const old = Object.assign(Object.create(null), this);
while (n-- > 0) this.nextToken();
const copy = Object.assign(Object.create(null), this);
Object.assign(this, old);
return copy;
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the copying about?

Copy link
Owner Author

Choose a reason for hiding this comment

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

We're taking a snapshot of the parser so that we can roll it back after advancing its state to look at later tokens.

Copy link
Contributor

Choose a reason for hiding this comment

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

It's a shallow snapshot so not too bad.

}

function isExportDefaultSpecifier() {
if (this.type !== tt.name) {
return false;
}

const lookAhead = this.lookAhead(1);
return lookAhead.type === tt.comma ||
(lookAhead.type === tt.name &&
lookAhead.value === "from");
}

function parseExportSpecifiersMaybe(node) {
if (this.eat(tt.comma)) {
node.specifiers.push.apply(
node.specifiers,
this.parseExportSpecifiers()
);
}
}

function parseExportFromWithCheck(node, exports, expect) {
this.parseExportFrom(node, expect);

if (node.specifiers) {
for (let i = 0; i < node.specifiers.length; i++) {
const s = node.specifiers[i];
const exported = s.exported;
this.checkExport(exports, exported.name, exported.start);
}
}
}

function parseExportFrom(node, expect) {
if (this.eatContextual("from")) {
node.source = this.type === tt.string
? this.parseExprAtom()
: this.unexpected();
} else {
if (node.specifiers) {
// check for keywords used as local names
for (let i = 0; i < node.specifiers.length; i++) {
const local = node.specifiers[i].local;
if (local && (this.keywords.test(local.name) ||
Copy link
Contributor

Choose a reason for hiding this comment

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

what's the possible values of local? If it's either an object or undefined we should be explicit about the object check with typeof local === "object" && local !== null as v8 (and I guess others) favor that over a bool check.

Copy link
Owner Author

Choose a reason for hiding this comment

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

I'll do you one better: this whole loop can be eliminated (soon)!

this.reservedWords.test(local.name))) {
this.unexpected(local.start);
}
}
}

if (expect) {
this.unexpected();
} else {
Copy link
Contributor

Choose a reason for hiding this comment

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

expect the unexpected 🤣

node.source = null;
}
}

this.semicolon();
}

function checkExport(exports, name, pos) {
if (!exports) return;
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar object check for exports.

if (hasOwn.call(exports, name)) {
this.raiseRecoverable(pos, "Duplicate export '" + name + "'");
}
exports[name] = true;
}

function shouldParseExportDeclaration() {
return this.type.keyword === "var" ||
this.type.keyword === "const" ||
this.type.keyword === "class" ||
this.type.keyword === "function" ||
this.isLet() ||
this.isAsyncFunction();
}
19 changes: 11 additions & 8 deletions lib/parsers/acorn.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"use strict";

const acorn = require("acorn");
let acorn = null;
let acornExtensions = null;

exports.options = {
ecmaVersion: 8,
Expand All @@ -11,17 +12,19 @@ exports.options = {
};

function acornParse(code) {
const parser = new acorn.Parser(exports.options, code);
if (acorn === null) {
acorn = require("acorn");
}

if (acornExtensions === null) {
acornExtensions = require("./acorn-extensions.js");
}

// It's not Reify's job to enforce strictness.
parser.strict = false;
const parser = new acorn.Parser(exports.options, code);

// Tolerate recoverable parse errors.
parser.raiseRecoverable = noopRaiseRecoverable;
acornExtensions.enableAll(parser);

return parser.parse();
Copy link
Contributor

Choose a reason for hiding this comment

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

What's the lazy loading of acorn and acorn-extensions do?

Copy link
Owner Author

Choose a reason for hiding this comment

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

Hmm, probably not much if we always use the parser when we import this module (which I think we do). Are you concerned it might hurt?

}

function noopRaiseRecoverable() {}

exports.parse = acornParse;
21 changes: 12 additions & 9 deletions lib/parsers/top-level.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
"use strict";

const acorn = require("acorn");
let acorn = null;
let acornExtensions = null;

exports.options = {
ecmaVersion: 8,
Expand All @@ -25,20 +26,22 @@ function quickParseBlock() {
}

function topLevelParse(code) {
if (acorn === null) {
acorn = require("acorn");
}

if (acornExtensions === null) {
acornExtensions = require("./acorn-extensions.js");
}

const parser = new acorn.Parser(exports.options, code);

acornExtensions.enableAll(parser);

// Override the Parser's parseBlock method.
parser.parseBlock = quickParseBlock;

// It's not Reify's job to enforce strictness.
parser.strict = false;

// Tolerate recoverable parse errors.
parser.raiseRecoverable = noopRaiseRecoverable;

return parser.parse();
}

function noopRaiseRecoverable() {}

exports.parse = topLevelParse;
5 changes: 1 addition & 4 deletions test/export-tests.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
const assert = require("assert");
const parserSupportsExportFromExtensions =
process.env.REIFY_PARSER === "babylon";

describe("export declarations", () => {
it("should allow * exports", () => {
Expand Down Expand Up @@ -206,8 +204,7 @@ describe("export declarations", () => {
});
});

(parserSupportsExportFromExtensions ? it : xit
)("should support export-from extensions", () => {
it("should support export-from extensions", () => {
import {
def1, def2, def3,
ns1, ns2, ns3,
Expand Down