Skip to content

Commit

Permalink
[[FIX]] Account for hoisting of importing bindings
Browse files Browse the repository at this point in the history 
Bindings created via ES2015 `import` declarations are "hoisted"
during module instantiation. This enables reference prior to
declaration, though JSHint previously reported an error for such cases.

Remove the restriction by introducing a new binding type, "import", that
is interpreted as hoisted and (unlike the existing "function" type)
immutable.

Extend the enforcement of the "latedef" linting option (which previously
only concerned function declarations) to warn about imported bindings
referenced prior to declaration. Although this a new warning, it is
backwards-compatible because it only occurs in situations that were
previously reported as errors.
  • Loading branch information
@jugglinmike
jugglinmike committed Apr 23, 2017
1 parent 1854e68 commit bd36953
Show file tree
Hide file tree
Showing 6 changed files with 29 additions and 27 deletions.
6 changes: 3 additions & 3 deletions src/jshint.js
View file
Expand Up @@ -4587,7 +4587,7 @@ var JSHINT = (function() {
this.name = identifier();
// Import bindings are immutable (see ES6 8.1.1.5.5)
state.funct["(scope)"].addlabel(this.name, {
type: "const",
type: "import",
initialized: true,
token: state.tokens.curr });

Expand All @@ -4614,7 +4614,7 @@ var JSHINT = (function() {
this.name = identifier();
// Import bindings are immutable (see ES6 8.1.1.5.5)
state.funct["(scope)"].addlabel(this.name, {
type: "const",
type: "import",
initialized: true,
token: state.tokens.curr });
}
Expand All @@ -4640,7 +4640,7 @@ var JSHINT = (function() {

// Import bindings are immutable (see ES6 8.1.1.5.5)
state.funct["(scope)"].addlabel(importName, {
type: "const",
type: "import",
initialized: true,
token: state.tokens.curr });

Expand Down
14 changes: 8 additions & 6 deletions src/scope-manager.js
View file
Expand Up @@ -278,6 +278,7 @@ var scopeManager = function(state, predefined, exported, declared) {
var usedLabel = currentLabels[usedLabelName];
if (usedLabel) {
var usedLabelType = usedLabel["(type)"];
var isImmutable = usedLabelType === "const" || usedLabelType === "import";

if (usedLabel["(useOutsideOfScope)"] && !state.option.funcscope) {
var usedTokens = usage["(tokens)"];
Expand All @@ -295,7 +296,7 @@ var scopeManager = function(state, predefined, exported, declared) {
_current["(labels)"][usedLabelName]["(unused)"] = false;

// check for modifying a const
if (usedLabelType === "const" && usage["(modified)"]) {
if (isImmutable && usage["(modified)"]) {
for (j = 0; j < usage["(modified)"].length; j++) {
error("E013", usage["(modified)"][j], usedLabelName);
}
Expand Down Expand Up @@ -621,22 +622,23 @@ var scopeManager = function(state, predefined, exported, declared) {
* adds an indentifier to the relevant current scope and creates warnings/errors as necessary
* @param {string} labelName
* @param {Object} opts
* @param {String} opts.type - the type of the label e.g. "param", "var", "let, "const", "function"
* @param {String} opts.type - the type of the label e.g. "param", "var", "let, "const", "import", "function"
* @param {Token} opts.token - the token pointing at the declaration
* @param {boolean} opts.initialized - whether the binding should be created in an "initialized" state.
*/
addlabel: function(labelName, opts) {

var type = opts.type;
var token = opts.token;
var isblockscoped = type === "let" || type === "const" || type === "class";
var isblockscoped = type === "let" || type === "const" ||
type === "class" || type === "import";
var ishoisted = type === "function" || type === "import";
var isexported = (isblockscoped ? _current : _currentFunctBody)["(type)"] === "global" &&
_.has(exported, labelName);

// outer shadow check (inner is only on non-block scoped)
_checkOuterShadow(labelName, token, type);

// if is block scoped (let or const)
if (isblockscoped) {

var declaredInCurrentScope = _current["(labels)"][labelName];
Expand All @@ -651,9 +653,9 @@ var scopeManager = function(state, predefined, exported, declared) {
if (!declaredInCurrentScope && _current["(usages)"][labelName]) {
var usage = _current["(usages)"][labelName];
// if its in a sub function it is not necessarily an error, just latedef
if (usage["(onlyUsedSubFunction)"]) {
if (usage["(onlyUsedSubFunction)"] || ishoisted) {
_latedefWarning(type, labelName, token);
} else {
} else if (!ishoisted) {
// this is a clear illegal usage for block scoped variables
warning("E056", token, labelName, type);
}
Expand Down
14 changes: 0 additions & 14 deletions tests/test262/expectations.txt
View file
Expand Up @@ -491,9 +491,6 @@ language/module-code/eval-rqstd-abrupt.js
language/module-code/eval-rqstd-once.js
language/module-code/eval-rqstd-order.js
language/module-code/eval-self-once.js
language/module-code/instn-iee-bndng-fun.js
language/module-code/instn-iee-bndng-gen.js
language/module-code/instn-iee-bndng-var.js
language/module-code/instn-iee-err-ambiguous-as.js
language/module-code/instn-iee-err-ambiguous.js
language/module-code/instn-iee-err-circular-as.js
Expand All @@ -504,16 +501,6 @@ language/module-code/instn-iee-err-not-found-as.js
language/module-code/instn-iee-err-not-found.js
language/module-code/instn-iee-star-cycle.js
language/module-code/instn-iee-trlng-comma.js
language/module-code/instn-named-bndng-dflt-fun-anon.js
language/module-code/instn-named-bndng-dflt-gen-anon.js
language/module-code/instn-named-bndng-dflt-fun-named.js
language/module-code/instn-named-bndng-dflt-gen-named.js
language/module-code/instn-named-bndng-dflt-star.js
language/module-code/instn-named-bndng-dflt-named.js
language/module-code/instn-named-bndng-fun.js
language/module-code/instn-named-bndng-gen.js
language/module-code/instn-named-bndng-trlng-comma.js
language/module-code/instn-named-bndng-var.js
language/module-code/instn-named-err-ambiguous-as.js
language/module-code/instn-named-err-ambiguous.js
language/module-code/instn-named-err-dflt-thru-star-as.js
Expand All @@ -529,7 +516,6 @@ language/module-code/instn-resolve-err-reference.js
language/module-code/instn-resolve-err-syntax.js
language/module-code/instn-resolve-order-depth.js
language/module-code/instn-resolve-order-src.js
language/module-code/instn-star-binding.js
language/module-code/instn-star-err-not-found.js
language/module-code/instn-star-star-cycle.js
language/module-code/parse-err-decl-pos-export-switch-case-dflt.js
Expand Down
3 changes: 1 addition & 2 deletions tests/unit/core.js
View file
Expand Up @@ -814,8 +814,7 @@ exports.testES6Modules = function (test) {
[58, "'emGet' has already been declared."],
[58, "'set' has already been declared."],
[59, "'_' has already been declared."],
[60, "'ember2' has already been declared."],
[65, "'newImport' was used before it was declared, which is illegal for 'const' variables."]
[60, "'ember2' has already been declared."]
];

var testRun = TestRun(test);
Expand Down
8 changes: 8 additions & 0 deletions tests/unit/fixtures/latedef-esnext.js
View file
Expand Up @@ -67,3 +67,11 @@ export class df {}
export var dg;
export let dh;
export const di = {};

void importedName;
void importedModule;
void importedNamespace;

import { importedName } from './m.js';
import importedModule from './m.js';
import * as importedNamespace from './m.js';
11 changes: 9 additions & 2 deletions tests/unit/options.js
View file
Expand Up @@ -250,7 +250,7 @@ exports.latedef = function (test) {
.addError(18, "Inner functions should be listed at the top of the outer function.")
.test(src, { es3: true, latedef: true });

TestRun(test)
var es2015Errors = TestRun(test)
.addError(4, "'c' was used before it was defined.")
.addError(6, "'e' was used before it was defined.")
.addError(8, "'h' was used before it was defined.")
Expand All @@ -259,7 +259,14 @@ exports.latedef = function (test) {
.addError(20, "'ai' was used before it was defined.")
.addError(31, "'bi' was used before it was defined.")
.addError(48, "'ci' was used before it was defined.")
.test(esnextSrc, {esnext: true, latedef: true});
.addError(75, "'importedName' was used before it was defined.")
.addError(76, "'importedModule' was used before it was defined.")
.addError(77, "'importedNamespace' was used before it was defined.");

es2015Errors
.test(esnextSrc, {esversion: 2015, latedef: true});
es2015Errors
.test(esnextSrc, {esversion: 2015, latedef: "nofunc"});

TestRun(test, "shouldn't warn when marking a var as exported")
.test("var a;", { exported: ["a"], latedef: true });
Expand Down

0 comments on commit bd36953

Please sign in to comment.