From 63264980c0b2de6bb4f5d86b56919bb66912a339 Mon Sep 17 00:00:00 2001 From: Anton Kovalyov Date: Mon, 13 Feb 2012 12:12:18 -0800 Subject: [PATCH] Unused and implied collections now respect variable hoisting as well. Fixes GH-431. --- jshint.js | 38 ++++++++++++++++++++++++++++++++---- tests/unit/fixtures/gh431.js | 15 ++++++++++++++ tests/unit/options.js | 21 +++++++++++++++++++- 3 files changed, 69 insertions(+), 5 deletions(-) create mode 100644 tests/unit/fixtures/gh431.js diff --git a/jshint.js b/jshint.js index bc080222fe..5bf937c4b8 100644 --- a/jshint.js +++ b/jshint.js @@ -2601,6 +2601,7 @@ loop: for (;;) { if (typeof a === 'function') { a = false; } + if (!a) { a = [line]; implied[name] = a; @@ -4104,10 +4105,22 @@ loop: for (;;) { } advance('(end)'); - var isDefined = function (name, context) { + var markDefined = function (name, context) { do { - if (typeof context[name] === 'string') + if (typeof context[name] === 'string') { + // JSHINT marks unused variables as 'unused' and + // unused function declaration as 'unction'. This + // code changes such instances back 'var' and + // 'closure' so that the code in JSHINT.data() + // doesn't think they're unused. + + if (context[name] === 'unused') + context[name] = 'var'; + else if (context[name] === 'unction') + context[name] = 'closure'; + return true; + } context = context['(context)']; } while (context); @@ -4115,11 +4128,29 @@ loop: for (;;) { return false; }; + var clearImplied = function (name, line) { + if (!implied[name]) + return; + + var newImplied = []; + for (var i = 0; i < implied[name].length; i += 1) { + if (implied[name][i] !== line) + newImplied.push(implied[name][i]); + } + + if (newImplied.length === 0) + delete implied[name]; + else + implied[name] = newImplied; + }; + // Check queued 'x is not defined' instances to see if they're still undefined. for (i = 0; i < JSHINT.undefs.length; i += 1) { k = JSHINT.undefs[i].slice(0); - if (!isDefined(k[2].value, k[0])) { + if (markDefined(k[2].value, k[0])) { + clearImplied(k[2].value, k[2].line); + } else { warning.apply(warning, k.slice(1)); } } @@ -4171,7 +4202,6 @@ loop: for (;;) { if (globals.length > 0) { data.globals = globals; } - for (i = 1; i < functions.length; i += 1) { f = functions[i]; fu = {}; diff --git a/tests/unit/fixtures/gh431.js b/tests/unit/fixtures/gh431.js new file mode 100644 index 0000000000..bea92c2d89 --- /dev/null +++ b/tests/unit/fixtures/gh431.js @@ -0,0 +1,15 @@ +var test = function() { + var fun1 = function (){ + fun2(); + }; + + function fun2() {} + + var fun3 = function() { + }; + + function fun5() {} + + fun1(); + fun4(); +}; diff --git a/tests/unit/options.js b/tests/unit/options.js index 1b8ea08f33..062a45bb8a 100644 --- a/tests/unit/options.js +++ b/tests/unit/options.js @@ -9,7 +9,8 @@ var JSHINT = require('../../jshint.js').JSHINT, fs = require('fs'), TestRun = require('../helpers/testhelper').setup.testRun, - fixture = require('../helpers/fixture').fixture; + fixture = require('../helpers/fixture').fixture, + assert = require('assert'); /** * Option `shadow` allows you to re-define variables later in code. @@ -120,6 +121,24 @@ exports.undefwstrict = function () { TestRun().test(src, { undef: false }); }; +// Regression test for GH-431 +exports["implied and unused should respect hoisting"] = function () { + var src = fs.readFileSync(__dirname + '/fixtures/gh431.js', 'utf8'); + TestRun() + .addError(14, "'fun4' is not defined.") + .test(src, { undef: true }); + + JSHINT.flag = true; + JSHINT(src, { undef: true }); + var report = JSHINT.data(); + + assert.eql(report.implieds.length, 1); + assert.eql(report.implieds[0].name, 'fun4'); + assert.eql(report.implieds[0].line, [14]); + + assert.eql(report.unused.length, 2); +}; + /** * The `proto` and `iterator` options allow you to prohibit the use of the * special `__proto__` and `__iterator__` properties, respectively.