diff --git a/lib/eslint.js b/lib/eslint.js index 2774da450d57..6509b31dd1a3 100755 --- a/lib/eslint.js +++ b/lib/eslint.js @@ -46,9 +46,10 @@ estraverse.VisitorKeys.Property.push("argument"); * Parses a list of "name:boolean_value" or/and "name" options divided by comma or * whitespace. * @param {string} string The string to parse. + * @param {Comment} comment The comment node which has the string. * @returns {Object} Result map object of names and boolean values */ -function parseBooleanConfig(string) { +function parseBooleanConfig(string, comment) { var items = {}; // Collapse whitespace around : to make parsing easier string = string.replace(/\s*:\s*/g, ":"); @@ -65,7 +66,10 @@ function parseBooleanConfig(string) { name = name.substring(0, pos); } - items[name] = (value === "true"); + items[name] = { + value: (value === "true"), + comment: comment + }; }); return items; @@ -181,9 +185,10 @@ function addDeclaredGlobals(program, globalScope, config) { if (!variable) { variable = new escope.Variable(name, globalScope); variable.eslintExplicitGlobal = true; + variable.eslintExplicitGlobalComment = explicitGlobals[name].comment; globalScope.variables.push(variable); } - variable.writeable = explicitGlobals[name]; + variable.writeable = explicitGlobals[name].value; }); // mark all exported variables as such @@ -290,12 +295,12 @@ function modifyConfigsFromComments(filename, ast, config, reportingConfig, messa if (comment.type === "Block") { switch (match[1]) { case "exported": - assign(commentConfig.exported, parseBooleanConfig(value)); + assign(commentConfig.exported, parseBooleanConfig(value, comment)); break; case "globals": case "global": - assign(commentConfig.astGlobals, parseBooleanConfig(value)); + assign(commentConfig.astGlobals, parseBooleanConfig(value, comment)); break; case "eslint-env": diff --git a/lib/rules/no-unused-vars.js b/lib/rules/no-unused-vars.js index 173c28b9c716..06863ed0ad8d 100644 --- a/lib/rules/no-unused-vars.js +++ b/lib/rules/no-unused-vars.js @@ -5,6 +5,12 @@ "use strict"; +//------------------------------------------------------------------------------ +// Requirements +//------------------------------------------------------------------------------ + +var escape = require("escape-string-regexp"); + //------------------------------------------------------------------------------ // Rule Definition //------------------------------------------------------------------------------ @@ -227,6 +233,35 @@ module.exports = function(context) { return unusedVars; } + /** + * Creates the correct location of a given variables. + * The location is at its name string in a `/*global` comment. + * + * @param {escope.Variable} variable - A variable to get its location. + * @returns {{line: number, column: number}} The location object for the variable. + */ + function getLocation(variable) { + var comment = variable.eslintExplicitGlobalComment; + var content = comment.value; + var baseLoc = comment.loc.start; + var namePattern = new RegExp("\\b" + escape(variable.name) + "\\b"); + var column = namePattern.exec(content).index; + var prefix = content.slice(0, column); + var lineInComment = (prefix.match(/\n/g) || []).length; + + if (lineInComment > 0) { + column -= 1 + prefix.lastIndexOf("\n"); + } else { + // 2 is for `/*` + column += baseLoc.column + 2; + } + + return { + line: baseLoc.line + lineInComment, + column: column + }; + } + //-------------------------------------------------------------------------- // Public //-------------------------------------------------------------------------- @@ -243,7 +278,7 @@ module.exports = function(context) { if (unusedVar.eslintUsed) { continue; // explicitly exported variables } else if (unusedVar.eslintExplicitGlobal) { - context.report(programNode, MESSAGE, unusedVar); + context.report(programNode, getLocation(unusedVar), MESSAGE, unusedVar); } else if (unusedVar.defs.length > 0) { context.report(unusedVar.identifiers[0], MESSAGE, unusedVar); } diff --git a/tests/lib/eslint.js b/tests/lib/eslint.js index 74fe2b6a2253..e82a56a0fd5b 100644 --- a/tests/lib/eslint.js +++ b/tests/lib/eslint.js @@ -2252,6 +2252,38 @@ describe("eslint", function() { var messages = eslint.verify("/* eslint-env node */ return;", null, "eslint-env node"); assert.equal(messages.length, 0); }); + + it("should attach a \"/*global\" comment node to declared variables", function() { + var code = "/* global foo */\n/* global bar, baz */"; + var ok = false; + + eslint.defineRules({test: function(context) { + return { + "Program": function() { + var scope = context.getScope(); + var comments = context.getAllComments(); + assert.equal(2, comments.length); + + var foo = getVariable(scope, "foo"); + assert.equal(true, foo.eslintExplicitGlobal); + assert.equal(comments[0], foo.eslintExplicitGlobalComment); + + var bar = getVariable(scope, "bar"); + assert.equal(true, bar.eslintExplicitGlobal); + assert.equal(comments[1], bar.eslintExplicitGlobalComment); + + var baz = getVariable(scope, "baz"); + assert.equal(true, baz.eslintExplicitGlobal); + assert.equal(comments[1], baz.eslintExplicitGlobalComment); + + ok = true; + } + }; + }}); + + eslint.verify(code, {rules: {test: 2}}); + assert(ok); + }); }); describe("getDeclaredVariables(node)", function() { diff --git a/tests/lib/rules/no-unused-vars.js b/tests/lib/rules/no-unused-vars.js index 64f6a21fee77..d5ba9b89bbd7 100644 --- a/tests/lib/rules/no-unused-vars.js +++ b/tests/lib/rules/no-unused-vars.js @@ -168,6 +168,22 @@ ruleTester.run("no-unused-vars", rule, { { code: "var a; function foo() { var _b; var c_; } foo();", options: [ { vars: "local", varsIgnorePattern: "^_" } ], errors: [{ message: "\"c_\" is defined but never used", line: 1, column: 37 }] }, { code: "function foo(a, _b) { } foo();", options: [ { args: "all", argsIgnorePattern: "^_" } ], errors: [{ message: "\"a\" is defined but never used", line: 1, column: 14 }] }, { code: "function foo(a, _b, c) { return a; } foo();", options: [ { args: "after-used", argsIgnorePattern: "^_" } ], errors: [{ message: "\"c\" is defined but never used", line: 1, column: 21 }] }, - { code: "var [ firstItemIgnored, secondItem ] = items;", ecmaFeatures: {destructuring: true}, options: [ { vars: "all", varsIgnorePattern: "[iI]gnored" } ], errors: [{ message: "\"secondItem\" is defined but never used", line: 1, column: 25 }] } + { code: "var [ firstItemIgnored, secondItem ] = items;", ecmaFeatures: {destructuring: true}, options: [ { vars: "all", varsIgnorePattern: "[iI]gnored" } ], errors: [{ message: "\"secondItem\" is defined but never used", line: 1, column: 25 }] }, + + // https://github.com/eslint/eslint/issues/3617 + { + code: "\n/* global foobar, foo, bar */\nfoobar;", + errors: [ + {line: 2, column: 19, message: "\"foo\" is defined but never used" }, + {line: 2, column: 24, message: "\"bar\" is defined but never used" } + ] + }, + { + code: "\n/* global foobar,\n foo,\n bar\n */\nfoobar;", + errors: [ + {line: 3, column: 4, message: "\"foo\" is defined but never used" }, + {line: 4, column: 4, message: "\"bar\" is defined but never used" } + ] + } ] });