From 62f448f5f6be7cd7ccca554a87039001d9617ca4 Mon Sep 17 00:00:00 2001 From: Mike Pennisi Date: Sun, 20 Sep 2020 13:23:06 -0400 Subject: [PATCH] [[FIX]] Detect duplicate exported bindings --- src/jshint.js | 101 ++++++++------- src/messages.js | 3 +- src/scope-manager.js | 15 ++- tests/test262/expectations.txt | 8 -- tests/unit/core.js | 153 +++++++++++++++++++++-- tests/unit/fixtures/es6-import-export.js | 15 +-- 6 files changed, 220 insertions(+), 75 deletions(-) diff --git a/src/jshint.js b/src/jshint.js index 54a59e9855..d394c00ede 100644 --- a/src/jshint.js +++ b/src/jshint.js @@ -2754,7 +2754,6 @@ var JSHINT = (function() { var classDeclaration = blockstmt("class", function(context) { var className, classNameToken; - var inexport = context & prodParams.export; if (!state.inES6()) { warning("W104", state.tokens.curr, "class", "6"); @@ -2781,12 +2780,12 @@ var JSHINT = (function() { } if (classNameToken) { - this.name = className; + this.name = classNameToken; state.funct["(scope)"].initialize(className); - if (inexport) { - state.funct["(scope)"].setExported(className, classNameToken); - } + } else { + this.name = null; } + state.funct["(scope)"].stack(); classBody(this, context); return this; @@ -2823,13 +2822,15 @@ var JSHINT = (function() { state.funct["(scope)"].stack(); if (classNameToken) { - this.name = className; + this.name = classNameToken; state.funct["(scope)"].addbinding(className, { type: "class", initialized: true, token: classNameToken }); state.funct["(scope)"].block.use(className, classNameToken); + } else { + this.name = null; } classBody(this, context); @@ -4339,7 +4340,6 @@ var JSHINT = (function() { // used for both let and const statements var noin = context & prodParams.noin; - var inexport = context & prodParams.export; var isLet = type === "let"; var isConst = type === "const"; var tokens, lone, value, letblock; @@ -4429,10 +4429,6 @@ var JSHINT = (function() { if (tokens.hasOwnProperty(t)) { t = tokens[t]; state.funct["(scope)"].initialize(t.id); - - if (inexport) { - state.funct["(scope)"].setExported(t.token.value, t.token); - } } } } @@ -4534,7 +4530,6 @@ var JSHINT = (function() { var varstatement = stmt("var", function(context) { var noin = context & prodParams.noin; - var inexport = context & prodParams.export; var tokens, lone, value, id; this.first = []; @@ -4577,9 +4572,6 @@ var JSHINT = (function() { type: "var", token: t.token }); - if (inexport) { - state.funct["(scope)"].setExported(t.id, t.token); - } names.push(t.token); } } @@ -4652,25 +4644,21 @@ var JSHINT = (function() { if (inblock) { warning("W082", state.tokens.curr); } - var nameToken = optionalidentifier(context) ? state.tokens.curr : null; + this.name = optionalidentifier(context) ? state.tokens.curr : null; - if (!nameToken) { + if (!this.name) { if (!inexport) { warning("W025"); } } else { - state.funct["(scope)"].addbinding(nameToken.value, { + state.funct["(scope)"].addbinding(this.name.value, { type: labelType, token: state.tokens.curr, initialized: true }); - - if (inexport) { - state.funct["(scope)"].setExported(nameToken.value, state.tokens.prev); - } } var f = doFunction(context, { - name: nameToken && nameToken.value, + name: this.name && this.name.value, statement: this, type: generator ? "generator" : null, ignoreLoopFunc: inblock // a declaration may already have warned @@ -4683,9 +4671,9 @@ var JSHINT = (function() { // mode (the scope manager will not report an error because a declaration // does not introduce a binding into the function's environment record). var enablesStrictMode = f["(isStrict)"] && !state.isStrict(); - if (nameToken && (f["(name)"] === "arguments" || f["(name)"] === "eval") && + if (this.name && (f["(name)"] === "arguments" || f["(name)"] === "eval") && enablesStrictMode) { - error("E008", nameToken); + error("E008", this.name); } if (state.tokens.next.id === "(" && state.tokens.next.line === state.tokens.curr.line) { @@ -4712,21 +4700,21 @@ var JSHINT = (function() { // This context modification restricts the use of `await` as the optional // BindingIdentifier in async function expressions. - var nameToken = optionalidentifier(isAsync ? context | prodParams.async : context) ? + this.name = optionalidentifier(isAsync ? context | prodParams.async : context) ? state.tokens.curr : null; var f = doFunction(context, { - name: nameToken && nameToken.value, + name: this.name && this.name.value, type: generator ? "generator" : null }); - if (generator && nameToken && nameToken.value === "yield") { - error("E024", nameToken, "yield"); + if (generator && this.name && this.name.value === "yield") { + error("E024", this.name, "yield"); } - if (nameToken && (f["(name)"] === "arguments" || f["(name)"] === "eval") && + if (this.name && (f["(name)"] === "arguments" || f["(name)"] === "eval") && f["(isStrict)"]) { - error("E008", nameToken); + error("E008", this.name); } return this; @@ -5633,6 +5621,7 @@ var JSHINT = (function() { stmt("export", function(context) { var ok = true; + var token; var moduleSpecifier; context = context | prodParams.export; @@ -5665,22 +5654,27 @@ var JSHINT = (function() { state.nameStack.set(state.tokens.next); advance("default"); + var def = state.tokens.curr; var exportType = state.tokens.next.id; if (exportType === "function") { this.block = true; advance("function"); - state.syntax["function"].fud(context); + token = state.syntax["function"].fud(context); + state.funct["(scope)"].setExported(token.name, def); } else if (exportType === "async" && peek().id === "function") { this.block = true; advance("async"); advance("function"); - state.syntax["function"].fud(context | prodParams.preAsync); + token = state.syntax["function"].fud(context | prodParams.preAsync); + state.funct["(scope)"].setExported(token.name, def); } else if (exportType === "class") { this.block = true; advance("class"); - state.syntax["class"].fud(context); + token = state.syntax["class"].fud(context); + state.funct["(scope)"].setExported(token.name, def); } else { expression(context, 10); + state.funct["(scope)"].setExported(null, def); } return this; } @@ -5695,15 +5689,22 @@ var JSHINT = (function() { } advance(); - exportedTokens.push(state.tokens.curr); - if (state.tokens.next.value === "as") { advance("as"); if (!state.tokens.next.identifier) { /* istanbul ignore next */ error("E030", state.tokens.next, state.tokens.next.value); } + exportedTokens.push({ + local: state.tokens.prev, + export: state.tokens.next + }); advance(); + } else { + exportedTokens.push({ + local: state.tokens.curr, + export: state.tokens.curr + }); } if (!checkPunctuator(state.tokens.next, "}")) { @@ -5717,8 +5718,8 @@ var JSHINT = (function() { moduleSpecifier = state.tokens.next; advance("(string)"); } else if (ok) { - exportedTokens.forEach(function(token) { - state.funct["(scope)"].setExported(token.value, token); + exportedTokens.forEach(function(x) { + state.funct["(scope)"].setExported(x.local, x.export); }); } @@ -5734,31 +5735,43 @@ var JSHINT = (function() { } else if (state.tokens.next.id === "var") { // ExportDeclaration :: export VariableStatement advance("var"); - state.tokens.curr.fud(context); + token = state.tokens.curr.fud(context); + token.first.forEach(function(binding) { + state.funct["(scope)"].setExported(binding, binding); + }); } else if (state.tokens.next.id === "let") { // ExportDeclaration :: export VariableStatement advance("let"); - state.tokens.curr.fud(context); + token = state.tokens.curr.fud(context); + token.first.forEach(function(binding) { + state.funct["(scope)"].setExported(binding, binding); + }); } else if (state.tokens.next.id === "const") { // ExportDeclaration :: export VariableStatement advance("const"); - state.tokens.curr.fud(context); + token = state.tokens.curr.fud(context); + token.first.forEach(function(binding) { + state.funct["(scope)"].setExported(binding, binding); + }); } else if (state.tokens.next.id === "function") { // ExportDeclaration :: export Declaration this.block = true; advance("function"); - state.syntax["function"].fud(context); + token = state.syntax["function"].fud(context); + state.funct["(scope)"].setExported(token.name, token.name); } else if (state.tokens.next.id === "async" && peek().id === "function") { // ExportDeclaration :: export Declaration this.block = true; advance("async"); advance("function"); - state.syntax["function"].fud(context | prodParams.preAsync); + token = state.syntax["function"].fud(context | prodParams.preAsync); + state.funct["(scope)"].setExported(token.name, token.name); } else if (state.tokens.next.id === "class") { // ExportDeclaration :: export Declaration this.block = true; advance("class"); - state.syntax["class"].fud(context); + token = state.syntax["class"].fud(context); + state.funct["(scope)"].setExported(token.name, token.name); } else { /* istanbul ignore next */ error("E024", state.tokens.next, state.tokens.next.value); diff --git a/src/messages.js b/src/messages.js index 6c4bd053e2..d01ee4c5b0 100644 --- a/src/messages.js +++ b/src/messages.js @@ -84,7 +84,8 @@ var errors = { "enable strict mode.", E066: "Asynchronous iteration is only available with for-of loops.", E067: "Malformed numeric literal: '{a}'.", - E068: "Decimals with leading zeros are not allowed in strict mode." + E068: "Decimals with leading zeros are not allowed in strict mode.", + E069: "Duplicate exported binding: '{a}'." }; var warnings = { diff --git a/src/scope-manager.js b/src/scope-manager.js index ee6f8a528e..4613819024 100644 --- a/src/scope-manager.js +++ b/src/scope-manager.js @@ -64,6 +64,7 @@ var scopeManager = function(state, predefined, exported, declared) { var usedPredefinedAndGlobals = Object.create(null); var impliedGlobals = Object.create(null); var unuseds = []; + var esModuleExports = []; var emitter = new events.EventEmitter(); function warning(code, token) { @@ -694,8 +695,18 @@ var scopeManager = function(state, predefined, exported, declared) { * @param {string} bindingName - the value of the identifier * @param {object} token */ - setExported: function(bindingName, token) { - this.block.use(bindingName, token); + setExported: function(localName, exportName) { + if (exportName) { + if (esModuleExports.indexOf(exportName.value) > -1) { + error("E069", exportName, exportName.value); + } + + esModuleExports.push(exportName.value); + } + + if (localName) { + this.block.use(localName.value, localName); + } }, /** diff --git a/tests/test262/expectations.txt b/tests/test262/expectations.txt index 1a4ea5b7d9..62b7e0c583 100644 --- a/tests/test262/expectations.txt +++ b/tests/test262/expectations.txt @@ -149,14 +149,6 @@ test/language/block-scope/syntax/function-declarations/in-statement-position-if- test/language/block-scope/syntax/function-declarations/in-statement-position-if-expression-statement.js(strict mode) test/language/destructuring/binding/syntax/array-elements-with-initializer.js(default) test/language/destructuring/binding/syntax/array-elements-with-initializer.js(strict mode) -test/language/module-code/early-dup-export-decl.js(default) -test/language/module-code/early-dup-export-decl.js(strict mode) -test/language/module-code/early-dup-export-dflt-id.js(default) -test/language/module-code/early-dup-export-dflt-id.js(strict mode) -test/language/module-code/early-dup-export-id-as.js(default) -test/language/module-code/early-dup-export-id-as.js(strict mode) -test/language/module-code/early-dup-export-id.js(default) -test/language/module-code/early-dup-export-id.js(strict mode) test/language/module-code/early-export-global.js(default) test/language/module-code/early-export-global.js(strict mode) test/language/module-code/early-export-unresolvable.js(default) diff --git a/tests/unit/core.js b/tests/unit/core.js index bd1b426969..c81122a025 100644 --- a/tests/unit/core.js +++ b/tests/unit/core.js @@ -866,8 +866,8 @@ exports.testES6Modules = function (test) { ]; var testRun = TestRun(test) - .addError(74, 1, "Empty export: this is unnecessary and can be removed.") - .addError(75, 1, "Empty export: consider replacing with `import 'source';`."); + .addError(75, 1, "Empty export: this is unnecessary and can be removed.") + .addError(76, 1, "Empty export: consider replacing with `import 'source';`."); importConstErrors.forEach(function(error) { testRun.addError.apply(testRun, error); }); testRun.test(src, {esnext: true}); @@ -882,7 +882,6 @@ exports.testES6Modules = function (test) { .addError(10, 1, "'import' is only available in ES6 (use 'esversion: 6').") .addError(11, 1, "'import' is only available in ES6 (use 'esversion: 6').") .addError(22, 1, "'export' is only available in ES6 (use 'esversion: 6').") - .addError(26, 1, "'export' is only available in ES6 (use 'esversion: 6').") .addError(30, 1, "'export' is only available in ES6 (use 'esversion: 6').") .addError(31, 1, "'export' is only available in ES6 (use 'esversion: 6').") .addError(32, 1, "'export' is only available in ES6 (use 'esversion: 6').") @@ -891,8 +890,6 @@ exports.testES6Modules = function (test) { .addError(44, 1, "'export' is only available in ES6 (use 'esversion: 6').") .addError(46, 1, "'export' is only available in ES6 (use 'esversion: 6').") .addError(47, 8, "'class' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).") - .addError(48, 1, "'export' is only available in ES6 (use 'esversion: 6').") - .addError(48, 16, "'class' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).") .addError(47, 1, "'export' is only available in ES6 (use 'esversion: 6').") .addError(46, 8, "'class' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).") .addError(57, 1, "'import' is only available in ES6 (use 'esversion: 6').") @@ -903,11 +900,11 @@ exports.testES6Modules = function (test) { .addError(67, 1, "'export' is only available in ES6 (use 'esversion: 6').") .addError(67, 16, "'function*' is only available in ES6 (use 'esversion: 6').") .addError(67, 26, "'yield' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).") - .addError(70, 1, "'export' is only available in ES6 (use 'esversion: 6').") - .addError(71, 1, "'import' is only available in ES6 (use 'esversion: 6').") - .addError(74, 1, "'export' is only available in ES6 (use 'esversion: 6').") + .addError(71, 1, "'export' is only available in ES6 (use 'esversion: 6').") + .addError(72, 1, "'import' is only available in ES6 (use 'esversion: 6').") .addError(75, 1, "'export' is only available in ES6 (use 'esversion: 6').") - .addError(76, 1, "'import' is only available in ES6 (use 'esversion: 6').") + .addError(76, 1, "'export' is only available in ES6 (use 'esversion: 6').") + .addError(77, 1, "'import' is only available in ES6 (use 'esversion: 6').") .test(src); var src2 = [ @@ -920,6 +917,18 @@ exports.testES6Modules = function (test) { TestRun(test) .test(src2, {}); + TestRun(test) + .test([ + "export default function() {", + " return 'foobar';", + "}" + ], {esversion: 6}); + + TestRun(test) + .test([ + "export default class Bar {}" + ], {esversion: 6}); + // See gh-3055 "Labels Break JSHint" TestRun(test, "following labeled block") .test([ @@ -984,6 +993,105 @@ exports.testES6Modules = function (test) { test.done(); }; +exports.testES6ModuleDuplicateExport = function (test) { + TestRun(test, "Same declaration, adjacent") + .addError(2, 13, "Duplicate exported binding: 'x'.") + .test([ + "var x;", + "export { x, x };" + ], { esversion: 6, module: true }); + + TestRun(test, "Same declaration, removed") + .addError(2, 16, "Duplicate exported binding: 'y'.") + .test([ + "var y, z;", + "export { y, z, y };" + ], { esversion: 6, module: true }); + + TestRun(test, "Same declaration, renamed") + .addError(2, 18, "Duplicate exported binding: 'y'.") + .test([ + "var y, z;", + "export { y, z as y };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by reference") + .addError(3, 10, "Duplicate exported binding: 'z'.") + .test([ + "var z;", + "export { z };", + "export { z };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by reference, renamed") + .addError(3, 15, "Duplicate exported binding: 'y'.") + .test([ + "var y, z;", + "export { y };", + "export { z as y };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, default") + .addError(2, 8, "Duplicate exported binding: 'default'.") + .test([ + "export default 0;", + "export default 0;" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by reference, default/renamed") + .addError(3, 15, "Duplicate exported binding: 'default'.") + .test([ + "var x;", + "export default 0;", + "export { x as default };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by reference, renamed/default") + .addError(3, 8, "Duplicate exported binding: 'default'.") + .test([ + "var x;", + "export { x as default };", + "export default 0;" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by statement - var") + .addError(2, 10, "Duplicate exported binding: 'a'.") + .test([ + "export var a;", + "export { a };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by statement - let") + .addError(2, 10, "Duplicate exported binding: 'b'.") + .test([ + "export let b;", + "export { b };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by statement - const") + .addError(2, 10, "Duplicate exported binding: 'c'.") + .test([ + "export const c = null;", + "export { c };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by statement - function") + .addError(2, 10, "Duplicate exported binding: 'd'.") + .test([ + "export function d() {}", + "export { d };" + ], { esversion: 6, module: true }); + + TestRun(test, "Distinct declarations, by statement - class") + .addError(2, 10, "Duplicate exported binding: 'e'.") + .test([ + "export class e {}", + "export { e };" + ], { esversion: 6, module: true }); + + test.done(); +}; + exports.testES6ModulesNamedExportsAffectUnused = function (test) { // Named Exports should count as used var src1 = [ @@ -1286,10 +1394,7 @@ exports.testES6ModulesDefaultExportsAffectUnused = function (test) { " bar: 'bar'", "};", "var x = 23;", - "var z = 42;", - "export default { a: a, x: x };", - "export default function boo() { return x + z; }", - "export default class MyClass { }" + "export default { a: a, x: x };" ]; TestRun(test) @@ -1298,6 +1403,28 @@ exports.testES6ModulesDefaultExportsAffectUnused = function (test) { unused: true }); + TestRun(test) + .test([ + "var x = 23;", + "var z = 42;", + "export default function boo() { return x + z; }" + ], { + esnext: true, + unused: true + }); + + TestRun(test) + .test("export default class MyClass { }", { + esnext: true, + unused: true + }); + + TestRun(test) + .test("export default class {}", { + esnext: true, + unused: true + }); + test.done(); }; diff --git a/tests/unit/fixtures/es6-import-export.js b/tests/unit/fixtures/es6-import-export.js index fe0088a7ce..cb5a21ac90 100644 --- a/tests/unit/fixtures/es6-import-export.js +++ b/tests/unit/fixtures/es6-import-export.js @@ -21,11 +21,11 @@ function foobar() {} export default foobar; -// at some point doing a double export default should error, but for now, -// makes testing a hell of a lot easier -export default function() { - return "foobar"; -} + + + + + export { foo }; export { foo, bar } from "source"; @@ -45,7 +45,7 @@ export var c = "c"; export class Foo {} export class List extends Array {} -export default class Bar {} + // imports are const's and cannot be re-assigned $ = null; @@ -67,7 +67,8 @@ import newImport from 'newImport'; export function* gen() { yield 1; } // Trailing comma -export { a, }; +var w; +export { w, }; import { x, } from "source"; // Empty