Skip to content

Commit

Permalink
[[Fix]] Scope issues by refactoring
Browse files Browse the repository at this point in the history
Removes implied, scope and the keeping of variables on funct, by
refactoring all scope management into an expanded block scope manager
which keeps track of variable scope. As part of this it now tracks usages
and resolves them at the end of a scope so that a usage can be resolved to
a hoisted variable. This fixes jshint#2439, fixes jshint#2426, fixes jshint#2410, fixes
 jshint#2409, fixes jshint#2374, fixes jshint#2430, fixes jshint#2368, fixes jshint#2342, fixes jshint#2363,
 fixes jshint#2292, fixes jshint#2199, fixes jshint#2191, fixes jshint#2090, fixes jshint#1996, fixes
 jshint#1775, fixes jshint#1682, fixes jshint#1462, fixes jshint#1061.
  • Loading branch information
lukeapage committed Jun 14, 2015
1 parent 3be8d36 commit fc2592d
Show file tree
Hide file tree
Showing 17 changed files with 1,938 additions and 669 deletions.
1,388 changes: 785 additions & 603 deletions src/jshint.js

Large diffs are not rendered by default.

7 changes: 4 additions & 3 deletions src/messages.js
Expand Up @@ -20,7 +20,7 @@ var errors = {
E010: "'with' is not allowed in strict mode.",

// Constants
E011: "const '{a}' has already been declared.",
E011: "'{a}' has already been declared.",
E012: "const '{a}' is initialized to 'undefined'.",
E013: "Attempting to override '{a}' which is a constant.",

Expand Down Expand Up @@ -70,7 +70,8 @@ var errors = {
E052: "Unclosed template literal.",
E053: "Export declaration must be in global scope.",
E054: "Class properties must be methods. Expected '(' but instead saw '{a}'.",
E055: "The '{a}' option cannot be set after any executable code."
E055: "The '{a}' option cannot be set after any executable code.",
E056: "'{a}' was used before it was declared, which is illegal for '{b}' variables."
};

var warnings = {
Expand Down Expand Up @@ -168,7 +169,7 @@ var warnings = {
W089: "The body of a for in should be wrapped in an if statement to filter " +
"unwanted properties from the prototype.",
W090: "'{a}' is not a statement label.",
W091: "'{a}' is out of scope.",
W091: null,
W093: "Did you mean to return a conditional instead of an assignment?",
W094: "Unexpected comma.",
W095: "Expected a string and instead saw {a}.",
Expand Down
15 changes: 6 additions & 9 deletions tests/regression/thirdparty.js
Expand Up @@ -49,6 +49,8 @@ exports.jQuery_1_7 = function (test) {
.addError(5691, "'i' is defined but never used.")
.addError(7141, "'i' is defined but never used.")
.addError(6061, "'cur' is defined but never used.")
.addError(7099, "'selector' used out of scope.")
.addError(7107, "'selector' used out of scope.")
.test(src, { undef: true, unused: true }, globals);

test.done();
Expand Down Expand Up @@ -88,6 +90,7 @@ exports.prototype_1_7 = function (test) {
.addError(2989, "'tagName' used out of scope.")
.addError(2989, "'tagName' used out of scope.")
.addError(2990, "'tagName' used out of scope.")
.addError(3827, "'getOffsetParent' is a function.")
.addError(3844, "'positionedOffset' is a function.")
.addError(3860, "'cumulativeOffset' is a function.")
.addError(4036, "'ret' is already defined.")
Expand Down Expand Up @@ -156,21 +159,22 @@ exports.lodash_0_6_1 = function (test) {
.addError(632, "Missing semicolon.")
.addError(920, "'isArguments' is a function.")
.addError(963, "'isFunction' is a function.")
.addError(988, "'noNodeClass' used out of scope.")
.addError(1122, "'isArr' used out of scope.")
.addError(1127, "'className' used out of scope.")
.addError(1129, "Use '===' to compare with 'true'.")
.addError(1153, "'isArr' used out of scope.")
.addError(1159, "'isArr' used out of scope.")
.addError(1490, "Use '===' to compare with '0'.")
.addError(1504, "'noNodeClass' used out of scope.")
.addError(1670, "Missing semicolon.")
.addError(2731, "'array' is already defined.")
.addError(2732, "'array' is a statement label.")
.addError(3374, "Possible strict violation.")
.addError(3377, "Missing '()' invoking a constructor.")
.addError(3384, "Missing semicolon.")
.addError(3677, "Missing '()' invoking a constructor.")
.addError(3683, "Missing '()' invoking a constructor.")
.addError(3825, "Possible strict violation.")
.addError(4063, "'useSourceURL' used out of scope.")
.addError(4225, "Possible strict violation.")
.addError(4226, "Possible strict violation.")
.addError(4242, "Possible strict violation.")
Expand Down Expand Up @@ -212,18 +216,11 @@ exports.codemirror3 = function (test) {
};

TestRun(test)
.addError(1157, "'result' is defined but never used.")
.addError(1342, "Value of 'e' may be overwritten in IE 8 and earlier.")
.addError(1526, "Value of 'e' may be overwritten in IE 8 and earlier.")
.addError(1532, "'mX' is defined but never used.")
.addError(1532, "'mY' is defined but never used.")
.addError(1533, "Value of 'e' may be overwritten in IE 8 and earlier.")
.addError(2218, "'state' is defined but never used.")
.addError(2427, "'style' is defined but never used.")
.addError(2696, "'target' is defined but never used.")
.addError(3168, "'ok' is defined but never used.")
.addError(4093, "Unnecessary semicolon.")
.addError(4277, "'range' is defined but never used.")
.test(src, opt, { CodeMirror: true });

test.done();
Expand Down
164 changes: 156 additions & 8 deletions tests/unit/core.js
Expand Up @@ -22,7 +22,8 @@ exports.testCustomGlobals = function (test) {
for (var i = 0, g; g = report.globals[i]; i += 1)
dict[g] = true;

for (i = 0, g = null; g = custom[i]; i += 1)
var customKeys = Object.keys(custom);
for (i = 0, g = null; g = customKeys[i]; i += 1)
test.ok(g in dict);

// Regression test (GH-665)
Expand Down Expand Up @@ -50,6 +51,31 @@ exports.testUnusedDefinedGlobals = function (test) {
test.done();
};

exports.testExportedDefinedGlobals = function (test) {
var src = ["/*global foo, bar */",
"export { bar, foo };"];

// Test should pass
TestRun(test).test(src, { esnext: true, unused: true }, {});

var report = JSHINT.data();
test.deepEqual(report.globals, ['bar', 'foo']);

test.done();
};

exports.testGlobalVarDeclarations = function (test) {
var src = "var a;";

// Test should pass
TestRun(test).test(src, { es3: true, node: true }, {});

var report = JSHINT.data();
test.deepEqual(report.globals, ['a']);

test.done();
};

exports.globalDeclarations = function (test) {
var src = "exports = module.exports = function (test) {};";

Expand All @@ -71,7 +97,9 @@ exports.globalDeclarations = function (test) {
exports.multilineGlobalDeclarations = function (test) {
var src = fs.readFileSync(__dirname + "/fixtures/multiline-global-declarations.js", "utf8");

TestRun(test).test(src);
TestRun(test)
.addError(12, "'pi' is defined but never used.")
.test(src, { unused: true });

test.done();
};
Expand Down Expand Up @@ -312,7 +340,7 @@ exports.argsInCatchReused = function (test) {
TestRun(test)
.addError(6, "'e' is already defined.")
.addError(12, "Do not assign to the exception parameter.")
.addError(23, "'e' is not defined.")
.addError(23, "'e' used out of scope.")
.test(src, { es3: true, undef: true });

test.done();
Expand Down Expand Up @@ -579,13 +607,16 @@ exports.testCatchBlocks = function (test) {
var src = fs.readFileSync(__dirname + '/fixtures/gh247.js', 'utf8');

TestRun(test)
.addError(11, "'w' is not defined.")
.addError(19, "'w' is already defined.")
.addError(35, "'u2' used out of scope.")
.addError(36, "'w2' used out of scope.")
.test(src, { es3: true, undef: true, devel: true });

src = fs.readFileSync(__dirname + '/fixtures/gh618.js', 'utf8');

TestRun(test)
.addError(5, "Value of 'x' may be overwritten in IE 8 and earlier.")
.addError(15, "Value of 'y' may be overwritten in IE 8 and earlier.")
.test(src, { es3: true, undef: true, devel: true });

TestRun(test)
Expand Down Expand Up @@ -750,6 +781,9 @@ exports.testES6ModulesNamedExportsAffectUnused = function (test) {
"};",
"var x = 23;",
"var z = 42;",
"let c = 2;",
"const d = 7;",
"export { c, d };",
"export { a, x };",
"export var b = { baz: 'baz' };",
"export function boo() { return z; }",
Expand All @@ -763,8 +797,8 @@ exports.testES6ModulesNamedExportsAffectUnused = function (test) {
];

TestRun(test)
.addError(16, "const 'c1u' is initialized to 'undefined'.")
.addError(16, "const 'c2u' is initialized to 'undefined'.")
.addError(19, "const 'c1u' is initialized to 'undefined'.")
.addError(19, "const 'c2u' is initialized to 'undefined'.")
.test(src1, {
esnext: true,
unused: true
Expand Down Expand Up @@ -793,8 +827,9 @@ exports.testConstRedeclaration = function (test) {
];

TestRun(test)
.addError(2, "const 'a' has already been declared.")
.addError(9, "const 'a' has already been declared.")
.addError(2, "'a' has already been declared.")
.addError(9, "'a' has already been declared.")
.addError(13, "'b' has already been declared.")
.test(src, {
esnext: true
});
Expand Down Expand Up @@ -860,6 +895,11 @@ exports.testConstModification = function (test) {
"delete a[0];",
"new a();",
"new a;",
"function e() {",
" f++;",
"}",
"const f = 1;",
"e();"
];

TestRun(test)
Expand All @@ -871,6 +911,7 @@ exports.testConstModification = function (test) {
.addError(8, "Attempting to override 'a' which is a constant.")
.addError(8, "You might be leaking a variable (a) here.")
.addError(53, "Missing '()' invoking a constructor.")
.addError(55, "Attempting to override 'f' which is a constant.")
.test(src, {
esnext: true
});
Expand Down Expand Up @@ -1443,3 +1484,110 @@ exports.beginningArraysAreNotJSON = function (test) {
test.done();

};

exports.labelsOutOfScope = function (test) {
var src = [
"function a() {",
" if (true) {",
" bar: switch(2) {",
" }",
" foo: switch(1) {",
" case 1:",
" (function () {",
" baz: switch(3) {",
" case 3:",
" break foo;",
" case 2:",
" break bar;",
" case 3:",
" break doesnotexist;",
" }",
" })();",
" if (true) {",
" break foo;",
" }",
" break foo;",
" case 2:",
" break bar;",
" case 3:",
" break baz;",
" }",
" }",
"}"
];

TestRun(test)
.addError(10, "'foo' is not a statement label.")
.addError(12, "'bar' is not a statement label.")
.addError(14, "'doesnotexist' is not a statement label.")
.addError(22, "'bar' is not a statement label.")
.addError(24, "'baz' is not a statement label.")
.test(src);

test.done();
}

exports.labelDoesNotExistInGlobalScope = function (test) {
var src = [
"switch(1) {",
" case 1:",
" break nonExistent;",
"}"
];

TestRun(test)
.addError(3, "'nonExistent' is not a statement label.")
.test(src);

test.done();
};

exports.labelsContinue = function (test) {
var src = [
"exists: while(true) {",
" if (false) {",
" continue exists;",
" }",
" continue nonExistent;",
"}"
];

TestRun(test)
.addError(5, "'nonExistent' is not a statement label.")
.test(src);

test.done();
};

exports.catchWithNoParam = function (test) {
var src = [
"try{}catch(){}"
];

TestRun(test)
.addError(1, "Expected an identifier and instead saw ')'.")
.test(src);

test.done();
};

exports.catchWithNoParam = function (test) {
var src = [
"try{}",
"if (true) { console.log(); }"
];

TestRun(test)
.addError(2, "Expected 'catch' and instead saw 'if'.")
.test(src);

var src = [
"try{}"
];

TestRun(test)
.addError(1, "Expected 'catch' and instead saw ''.")
.test(src);

test.done();
};
2 changes: 1 addition & 1 deletion tests/unit/envs.js
Expand Up @@ -30,7 +30,7 @@ function globalsImplied(test, globals, options) {
JSHINT(wrap(globals), options || {});
var report = JSHINT.data();

test.ok(report.implieds !== null);
test.ok(report.implieds != null);
test.ok(report.globals === undefined);

var implieds = [];
Expand Down
6 changes: 3 additions & 3 deletions tests/unit/fixtures/es6-import-export.js
Expand Up @@ -6,9 +6,9 @@ import { one } from "foo";
import { default as _ } from "underscore";
import {} from "ember";
import "ember";
import * as ember from "ember";
import _, * as ember from "ember";
import _, { default as ember } from "ember";
import * as ember2 from "ember";
import _2, * as ember3 from "ember";
import _3, { default as ember } from "ember";

$.ajax();
emGet("foo");
Expand Down
14 changes: 14 additions & 0 deletions tests/unit/fixtures/gh247.js
Expand Up @@ -21,3 +21,17 @@ function test() {

alert("w:" + w);
}

function infunc() {
var i2 = 5;

try {
var u2 = "I'm trying here!";
} catch (e) {
var w2 = "Let's play catch.";
}

alert("i2:" + i2);
alert("u2:" + u2);
alert("w2:" + w2);
}
8 changes: 8 additions & 0 deletions tests/unit/fixtures/gh618.js
Expand Up @@ -5,3 +5,11 @@ try {
} catch (x) {}

console.log(x);

if (true) {
var y;
}

try {
throw "boom";
} catch (y) {}

0 comments on commit fc2592d

Please sign in to comment.