Skip to content

Commit

Permalink
[[FEAT]] List outer scoped variables of W083
Browse files Browse the repository at this point in the history 
Debugging W083 is kind of difficult without knowing which outer
scoped variables it is referring to. This commit shows the variable
names inside parenthesis at the end of the warning message.

References:

    Closes GH-3211
  • Loading branch information
@flaviojs @jugglinmike
flaviojs authored and jugglinmike committed Nov 26, 2017
1 parent 2f6ac13 commit d03662c
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 24 deletions.
4 changes: 2 additions & 2 deletions src/jshint.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3059,8 +3059,8 @@ var JSHINT = (function() {
// If the function we just parsed accesses any non-local variables
// trigger a warning. Otherwise, the function is safe even within
// a loop.
if (f["(isCapturing)"]) {
warning("W083", token);
if (f["(outerMutables)"]) {
warning("W083", token, f["(outerMutables)"].join(", "));
}
}

Expand Down
2 changes: 1 addition & 1 deletion src/messages.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ var warnings = {
"Use a function expression or move the statement to the top of " +
"the outer function.",
W083: "Functions declared within loops referencing an outer scoped " +
"variable may lead to confusing semantics.",
"variable may lead to confusing semantics. ({a})",
W084: "Expected a conditional expression and instead saw an assignment.",
W085: "Don't use 'with'.",
W086: "Expected a 'break' statement before '{a}'.",
Expand Down
5 changes: 4 additions & 1 deletion src/scope-manager.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -315,7 +315,10 @@ var scopeManager = function(state, predefined, exported, declared) {
isImmutable = labelType === "const" ||
(labelType === null && _scopeStack[0]["(predefined)"][usedLabelName] === false);
if (isUnstackingFunctionOuter && !isImmutable) {
state.funct["(isCapturing)"] = true;
if (!state.funct["(outerMutables)"]) {
state.funct["(outerMutables)"] = [];
}
state.funct["(outerMutables)"].push(usedLabelName);
}

// not exiting the global scope, so copy the usage down in case its an out of scope usage
Expand Down
51 changes: 31 additions & 20 deletions tests/unit/options.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1390,13 +1390,13 @@ exports.loopfunc = function (test) {

// By default, not functions are allowed inside loops
TestRun(test)
.addError(4, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(8, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(20, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(25, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(4, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (v)")
.addError(8, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (v)")
.addError(20, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (nonExistent)")
.addError(25, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (p)")
.addError(12, 5, "Function declarations should not be placed in blocks. Use a function " +
"expression or move the statement to the top of the outer function.")
.addError(42, 7, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(42, 7, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
.test(src, {es3: true});

// When loopfunc is true, only function declaration should fail.
Expand Down Expand Up @@ -1427,11 +1427,11 @@ exports.loopfunc = function (test) {
"}"
];
TestRun(test)
.addError(2, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(5, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(11, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(14, 15, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(17, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(2, 13, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
.addError(5, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
.addError(11, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
.addError(14, 15, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
.addError(17, 9, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
.test(es6LoopFuncSrc, {esnext: true});

// functions declared in the expressions that loop should warn
Expand All @@ -1442,13 +1442,13 @@ exports.loopfunc = function (test) {
"for(var c = function(){return j;};;){c();}"];

TestRun(test)
.addError(1, 25, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(3, 16, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(1, 25, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (i)")
.addError(3, 16, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (j)")
.test(src2, { es3: true, loopfunc: false, boss: true });

TestRun(test, "Allows closing over immutable bindings (ES5)")
.addError(6, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(7, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(6, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (outerVar)")
.addError(7, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (innerVar)")
.test([
"var outerVar;",
"",
Expand All @@ -1463,8 +1463,8 @@ exports.loopfunc = function (test) {
]);

TestRun(test, "Allows closing over immutable bindings (globals)")
.addError(8, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(15, 10, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(8, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (mutableGlobal)")
.addError(15, 10, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (immutableGlobal)")
.test([
"/* globals immutableGlobal: false, mutableGlobal: true */",
"while (false) {",
Expand All @@ -1486,10 +1486,10 @@ exports.loopfunc = function (test) {
]);

TestRun(test, "Allows closing over immutable bindings (ES2015)")
.addError(10, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(11, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(18, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(19, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics.")
.addError(10, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (outerLet)")
.addError(11, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (innerLet)")
.addError(18, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (OuterClass)")
.addError(19, 8, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (InnerClass)")
.test([
"let outerLet;",
"const outerConst = 0;",
Expand All @@ -1514,6 +1514,17 @@ exports.loopfunc = function (test) {
"}"
], { esversion: 2015 });

TestRun(test, "W083 lists multiple outer scope variables")
.addError(3, 11, "Functions declared within loops referencing an outer scoped variable may lead to confusing semantics. (a, b)")
.test([
"var a, b;",
"for (;;) {",
" var f = function() {",
" return a + b;",
" };",
"}"
]);

test.done();
};

Expand Down

0 comments on commit d03662c

Please sign in to comment.