Permalink
Browse files

[[Fix]] Correct `unused` for function-scoped vars

In the absence of the `funcscope` option, JSHint validates
function-scoped variable references as though they were block scoped.
JSHint does not apply block-scoping semantics to reference counting,
however: re-declarations within nested blocks are *not* considered
distinct when determining whether a given binding is used.

Ensure that when a variable is re-declared within the same function
scope, usage tracking is "shared" across all identifiers.

This change required correcting two existing tests. Both alterations
removed warnings, meaning they do not reflect a backwards-incompatible
change in behavior.

1. In the following source code, the variable `ok` is used, so JSHint
   should not report otherwise:

        function(match) {
          var ch = this.string.charAt(this.pos);
          if (typeof match == "string") var ok = ch == match;
          else var ok = ch && (match.test ? match.test(ch) :
          if (ok) {++this.pos; return ch;}
        }

2. In the following source code, the `var` declaration for `d` is not
   technically used, but it is also invalid due to ES2015 block scoping
   semantics. Because JSHint continues to correctly emit an error for
   this code, the removal of the "unused" warning has little practical
   effect.

        function sup(a) {
          var b = 4;
          let c = 5;
          let d = 6;
          if (false) {
            var d = 7;
          }
          return b + c + a + d;
        }
  • Loading branch information...
jugglinmike committed Dec 5, 2015
1 parent 46db923 commit 91fa9fc4f923aa1619e6cf98ced28ab86555e7dd
@@ -386,24 +386,43 @@ var scopeManager = function(state, predefined, exported, declared) {
});
}

// if we have a sub scope we can copy too and we are still within the function boundary
// If this is not a function boundary, transfer function-scoped labels to
// the parent block (a rough simulation of variable hoisting). Previously
// existing labels in the parent block should take precedence so that things and stuff.
if (subScope && !isUnstackingFunctionBody &&
!isUnstackingFunctionParams && !isUnstackingFunctionOuter) {
var labelNames = Object.keys(currentLabels);
for (i = 0; i < labelNames.length; i++) {

var defLabelName = labelNames[i];

// if its function scoped and
// not already defined (caught with shadow, shouldn't also trigger out of scope)
if (!currentLabels[defLabelName]["(blockscoped)"] &&
currentLabels[defLabelName]["(type)"] !== "exception" &&
!this.funct.has(defLabelName, { excludeCurrent: true })) {
subScope["(labels)"][defLabelName] = currentLabels[defLabelName];
// we do not warn about out of scope usages in the global scope
if (_currentFunctBody["(type)"] !== "global") {
subScope["(labels)"][defLabelName]["(useOutsideOfScope)"] = true;
var defLabel = currentLabels[defLabelName];

if (!defLabel["(blockscoped)"] && defLabel["(type)"] !== "exception") {
var shadowed = subScope["(labels)"][defLabelName];

// Do not overwrite a label if it exists in the parent scope
// because it is shared by adjacent blocks. Copy the `unused`
// property so that any references found within the current block
// are counted toward that higher-level declaration.
if (shadowed) {
shadowed["(unused)"] &= defLabel["(unused)"];

// "Hoist" the variable to the parent block, decorating the label
// so that future references, though technically valid, can be
// reported as "out-of-scope" in the absence of the `funcscope`
// option.
} else {
defLabel["(useOutsideOfScope)"] =
// Do not warn about out-of-scope usages in the global scope
_currentFunctBody["(type)"] !== "global" &&
// When a higher scope contains a binding for the label, the
// label is a re-declaration and should not prompt "used
// out-of-scope" warnings.
!this.funct.has(defLabelName, { excludeCurrent: true });

subScope["(labels)"][defLabelName] = defLabel;
}

delete currentLabels[defLabelName];
}
}
@@ -214,7 +214,6 @@ exports.codemirror3 = function (test) {
.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(1533, "Value of 'e' may be overwritten in IE 8 and earlier.")
.addError(3168, "'ok' is defined but never used.")
.addError(4093, "Unnecessary semicolon.")
.test(src, opt, { CodeMirror: true });

@@ -0,0 +1,35 @@
(function() {
void funcBefore;
void topBlockBefore;
void nestedBlockBefore;

var funcBefore, funcAfter;

{
void funcBefore;
void topBlockBefore;
void nestedBlockBefore;

var topBlockBefore, topBlockAfter;

{
void funcBefore;
void topBlockBefore;
void nestedBlockBefore;

var nestedBlockBefore, nestedBlockAfter;

void nestedBlockAfter;
void topBlockAfter;
void funcAfter;
}

void nestedBlockAfter;
void topBlockAfter;
void funcAfter;
}

void nestedBlockAfter;
void topBlockAfter;
void funcAfter;
}());
@@ -0,0 +1,63 @@
(function() {
// Usage in same scope as declaration (function)
var func1;
void func1;
void func2;
var func2;

var func3, func4, func5, func6;
var unusedFunc;

{
void func3;

// Redeclaration (direct descendent of function)
var func4;
void func4;
void func5;
var func5;

// Usage in same scope as declaration (block)
var topBlock1;
void topBlock1;
void topBlock2;
var topBlock2;

var topBlock3, topBlock4, topBlock5, topBlock6, topBlock7;
var unusedTopBlock;

{
void func6;

// Redeclaration (indirect descendent of function)
var func7;
void func7;
void func8;
var func8;

void topBlock5;

// Redeclaration (direct descendent of block)
var topBlock6;
void topBlock6;
void topBlock7;
var topBlock7;

// Usage in same scope as declaration (nested block)
var nestedBlock1;
void nestedBlock1;
void nestedBlock2;
var nestedBlock2;

var unusedNestedBlock;

{
// Redeclaration (indirect descendent of block)
var topBlock3;
void topBlock3;
void topBlock4;
var topBlock4;
}
}
}
})();
@@ -809,7 +809,8 @@ exports.undefDeleteStrict = function (test) {
test.done();
};

exports.unused = function (test) {
exports.unused = {};
exports.unused.basic = function (test) {
var src = fs.readFileSync(__dirname + '/fixtures/unused.js', 'utf8');

var allErrors = [
@@ -894,6 +895,93 @@ exports.unused = function (test) {
test.done();
};

// Regression test for gh-2784
exports.unused.usedThroughShadowedDeclaration = function (test) {
var code = [
"(function() {",
" var x;",
" {",
" var x;",
" void x;",
" }",
"}());"
];

TestRun(test)
.addError(4, "'x' is already defined.")
.test(code, { unused: true });

test.done();
};

exports.unused.unusedThroughShadowedDeclaration = function (test) {
var code = [
"(function() {",
" {",
" var x;",
" void x;",
" }",
" {",
" var x;",
" }",
"})();"
];

TestRun(test)
.addError(7, "'x' is already defined.")
.test(code, { unused: true });

test.done();
};

exports.unused.hoisted = function (test) {
var code = [
"(function() {",
" {",
" var x;",
" }",
" {",
" var x;",
" }",
" void x;",
"}());"
];

TestRun(test)
.addError(6, "'x' is already defined.")
.addError(8, "'x' used out of scope.")
.test(code, { unused: true });

test.done();
};

exports.unused.crossBlocks = function (test) {
var code = fs.readFileSync(__dirname + '/fixtures/unused-cross-blocks.js', 'utf8');

TestRun(test)
.addError(15, "'func4' is already defined.")
.addError(18, "'func5' is already defined.")
.addError(41, "'topBlock6' is already defined.")
.addError(44, "'topBlock7' is already defined.")
.addError(56, "'topBlock3' is already defined.")
.addError(59, "'topBlock4' is already defined.")
.addError(9, "'unusedFunc' is defined but never used.")
.addError(27, "'unusedTopBlock' is defined but never used.")
.addError(52, "'unusedNestedBlock' is defined but never used.")
.test(code, { unused: true });

TestRun(test)
.addError(15, "'func4' is already defined.")
.addError(18, "'func5' is already defined.")
.addError(41, "'topBlock6' is already defined.")
.addError(44, "'topBlock7' is already defined.")
.addError(56, "'topBlock3' is already defined.")
.addError(59, "'topBlock4' is already defined.")
.test(code);

test.done();
};

exports['param overrides function name expression'] = function (test) {
TestRun(test)
.test([
@@ -1878,7 +1966,8 @@ exports.quotesAndTemplateLiterals = function (test) {
test.done();
};

exports.scope = function (test) {
exports.scope = {};
exports.scope.basic = function (test) {
var src = fs.readFileSync(__dirname + '/fixtures/scope.js', 'utf8');

TestRun(test, 1)
@@ -1900,6 +1989,25 @@ exports.scope = function (test) {
test.done();
};

exports.scope.crossBlocks = function (test) {
var code = fs.readFileSync(__dirname + '/fixtures/scope-cross-blocks.js', 'utf8');

TestRun(test)
.addError(3, "'topBlockBefore' used out of scope.")
.addError(4, "'nestedBlockBefore' used out of scope.")
.addError(11, "'nestedBlockBefore' used out of scope.")
.addError(27, "'nestedBlockAfter' used out of scope.")
.addError(32, "'nestedBlockAfter' used out of scope.")
.addError(33, "'topBlockAfter' used out of scope.")
.test(code);

TestRun(test)
.test(code, { funcscope: true });


test.done();
};

/*
* Tests `esnext` and `moz` options.
*
@@ -2969,7 +2969,6 @@ exports["make sure var variables can shadow let variables"] = function (test) {
.addError(1, "'a' is defined but never used.")
.addError(2, "'b' is defined but never used.")
.addError(3, "'c' is defined but never used.")
.addError(9, "'d' is defined but never used.")
.addError(9, "'d' has already been declared.")
.test(code, { esnext: true, unused: true, undef: true, funcscope: true });

0 comments on commit 91fa9fc

Please sign in to comment.