Skip to content

Commit

Permalink
Add reportUsedIgnorePattern option to no-unused-vars rule
Browse files Browse the repository at this point in the history 
Address comments

Fix tests

Report on all variables except for initializers

Report only on declarations

Match message formatting with other rule messages

Move variable.eslintUsed into isUsedVariable function

Refactor report messages for consistency

Fix tests
  • Loading branch information
@Pearce-Ropion
Pearce-Ropion committed Mar 27, 2024
1 parent 778082d commit a364f2f
Show file tree
Hide file tree
Showing 3 changed files with 277 additions and 40 deletions.
43 changes: 40 additions & 3 deletions docs/src/rules/no-unused-vars.md
View file
Expand Up @@ -137,7 +137,13 @@ By default this rule is enabled with `all` option for caught errors and variable
```json
{
"rules": {
"no-unused-vars": ["error", { "vars": "all", "args": "after-used", "caughtErrors": "all", "ignoreRestSiblings": false }]
"no-unused-vars": ["error", {
"vars": "all",
"args": "after-used",
"caughtErrors": "all",
"ignoreRestSiblings": false,
"reportUsedIgnorePattern": false
}]
}
}
```
Expand Down Expand Up @@ -435,8 +441,6 @@ class Bar {
}
```

:::

Examples of **correct** code for the `{ "ignoreClassWithStaticInitBlock": true }` option

::: correct
Expand All @@ -453,6 +457,39 @@ class Foo {
}
```

### reportUsedIgnorePattern

The `reportUsedIgnorePattern` option is a boolean (default: `false`).
Using this option will report variables that match any of the valid ignore
pattern options (`varsIgnorePattern`, `argsIgnorePattern`, `caughtErrorsIgnorePattern`, or
`destructuredArrayIgnorePattern`) if they have been used.

Examples of **incorrect** code for the `{ "reportUsedIgnorePattern": true }` option:

::: incorrect

```js
/*eslint no-unused-vars: ["error", { "reportUsedIgnorePattern": true, "varsIgnorePattern": "[iI]gnored" }]*/

var firstVarIgnored = 1;
var secondVar = 2;
console.log(firstVarIgnored, secondVar);
```

:::

Examples of **correct** code for the `{ "reportUsedIgnorePattern": true }` option:

::: correct

```js
/*eslint no-unused-vars: ["error", { "reportUsedIgnorePattern": true, "varsIgnorePattern": "[iI]gnored" }]*/

var firstVar = 1;
var secondVar = 2;
console.log(firstVar, secondVar);
```

:::

## When Not To Use It
Expand Down
200 changes: 167 additions & 33 deletions lib/rules/no-unused-vars.js
View file
Expand Up @@ -23,6 +23,13 @@ const astUtils = require("./utils/ast-utils");
* @property {string} additional Any additional info to be appended at the end.
*/

/**
* Bag of data used for formatting the `usedIgnoredVar` lint message.
* @typedef {Object} UsedIgnoredVarMessageData
* @property {string} varName The name of the unused var.
* @property {string} additional Any additional info to be appended at the end.
*/

//------------------------------------------------------------------------------
// Rule Definition
//------------------------------------------------------------------------------
Expand Down Expand Up @@ -73,6 +80,9 @@ module.exports = {
},
ignoreClassWithStaticInitBlock: {
type: "boolean"
},
reportUsedIgnorePattern: {
type: "boolean"
}
},
additionalProperties: false
Expand All @@ -82,7 +92,8 @@ module.exports = {
],

messages: {
unusedVar: "'{{varName}}' is {{action}} but never used{{additional}}."
unusedVar: "'{{varName}}' is {{action}} but never used{{additional}}.",
usedIgnoredVar: "'{{varName}}' is marked as ignored but is used{{additional}}."
}
},

Expand All @@ -96,7 +107,8 @@ module.exports = {
args: "after-used",
ignoreRestSiblings: false,
caughtErrors: "all",
ignoreClassWithStaticInitBlock: false
ignoreClassWithStaticInitBlock: false,
reportUsedIgnorePattern: false
};

const firstOption = context.options[0];
Expand All @@ -110,6 +122,7 @@ module.exports = {
config.ignoreRestSiblings = firstOption.ignoreRestSiblings || config.ignoreRestSiblings;
config.caughtErrors = firstOption.caughtErrors || config.caughtErrors;
config.ignoreClassWithStaticInitBlock = firstOption.ignoreClassWithStaticInitBlock || config.ignoreClassWithStaticInitBlock;
config.reportUsedIgnorePattern = firstOption.reportUsedIgnorePattern || config.reportUsedIgnorePattern;

if (firstOption.varsIgnorePattern) {
config.varsIgnorePattern = new RegExp(firstOption.varsIgnorePattern, "u");
Expand All @@ -136,22 +149,40 @@ module.exports = {
* @returns {UnusedVarMessageData} The message data to be used with this unused variable.
*/
function getDefinedMessageData(unusedVar) {
const defType = unusedVar.defs && unusedVar.defs[0] && unusedVar.defs[0].type;
let type;
let pattern;

if (defType === "CatchClause" && config.caughtErrorsIgnorePattern) {
type = "args";
pattern = config.caughtErrorsIgnorePattern.toString();
} else if (defType === "Parameter" && config.argsIgnorePattern) {
type = "args";
pattern = config.argsIgnorePattern.toString();
} else if (defType !== "Parameter" && defType !== "CatchClause" && config.varsIgnorePattern) {
type = "vars";
pattern = config.varsIgnorePattern.toString();
}
const def = unusedVar.defs && unusedVar.defs[0];
let additional = "";

if (def) {
let type;
let pattern;

switch (def.type) {
case "CatchClause":
if (config.caughtErrorsIgnorePattern) {
type = "args";
pattern = config.caughtErrorsIgnorePattern;
}
break;

case "Parameter":
if (config.argsIgnorePattern) {
type = "args";
pattern = config.argsIgnorePattern;
}
break;

default:
if (config.varsIgnorePattern) {
type = "vars";
pattern = config.varsIgnorePattern;
}
break;
}

const additional = type ? `. Allowed unused ${type} must match ${pattern}` : "";
if (type) {
additional = `. Allowed unused ${type} must match ${pattern.toString()}`;
}
}

return {
varName: unusedVar.name,
Expand All @@ -167,13 +198,24 @@ module.exports = {
* @returns {UnusedVarMessageData} The message data to be used with this unused variable.
*/
function getAssignedMessageData(unusedVar) {
const def = unusedVar.defs[0];
const def = unusedVar.defs && unusedVar.defs[0];
let additional = "";

if (config.destructuredArrayIgnorePattern && def && def.name.parent.type === "ArrayPattern") {
additional = `. Allowed unused elements of array destructuring patterns must match ${config.destructuredArrayIgnorePattern.toString()}`;
} else if (config.varsIgnorePattern) {
additional = `. Allowed unused vars must match ${config.varsIgnorePattern.toString()}`;
if (def) {
let type;
let pattern;

if (def.name.parent.type === "ArrayPattern" && config.destructuredArrayIgnorePattern) {
type = "elements of array destructuring";
pattern = config.destructuredArrayIgnorePattern;
} else if (config.varsIgnorePattern) {
type = "vars";
pattern = config.varsIgnorePattern;
}

if (type) {
additional = `. Allowed unused ${type} must match ${pattern.toString()}`;
}
}

return {
Expand All @@ -183,6 +225,58 @@ module.exports = {
};
}

/**
* Generate the warning message about a variable being used even though
* it is marked as being ignored.
* @param {Variable} usedIgnoredVar eslint-scope variable object
* @returns {UsedIgnoredVarMessageData} The message data to be used with
* this used ignored variable.
*/
function getUsedIgnoredMessageData(usedIgnoredVar) {
const def = usedIgnoredVar.defs && usedIgnoredVar.defs[0];
let additional = "";

if (def) {
let type;
let pattern;

switch (def.type) {
case "CatchClause":
if (config.caughtErrorsIgnorePattern) {
type = "args";
pattern = config.caughtErrorsIgnorePattern;
}
break;

case "Parameter":
if (config.argsIgnorePattern) {
type = "args";
pattern = config.argsIgnorePattern;
}
break;

default:
if (def.name.parent.type === "ArrayPattern" && config.destructuredArrayIgnorePattern) {
type = "elements of array destructuring";
pattern = config.destructuredArrayIgnorePattern;
} else if (config.varsIgnorePattern) {
type = "vars";
pattern = config.varsIgnorePattern;
}
break;
}

if (type) {
additional = `. Used ${type} must not match ${pattern.toString()}`;
}
}

return {
varName: usedIgnoredVar.name,
additional
};
}

//--------------------------------------------------------------------------
// Helpers
//--------------------------------------------------------------------------
Expand Down Expand Up @@ -532,8 +626,13 @@ module.exports = {
* @private
*/
function isUsedVariable(variable) {
const functionNodes = getFunctionDefinitions(variable),
isFunctionDefinition = functionNodes.length > 0;
if (variable.eslintUsed) {
return true;
}

const functionNodes = getFunctionDefinitions(variable);
const isFunctionDefinition = functionNodes.length > 0;

let rhsNode = null;

return variable.references.some(ref => {
Expand Down Expand Up @@ -571,11 +670,14 @@ module.exports = {
/**
* Gets an array of variables without read references.
* @param {Scope} scope an eslint-scope Scope object.
* @param {Variable[]} unusedVars an array that saving result.
* @returns {Variable[]} unused variables of the scope and descendant scopes.
* @param {Variable[]} unusedVars an array containing unused variables that should be reported
* @param {Variable[]} usedIgnoredVars an array containing used variables that match an ignore pattern
* @returns {[Variable[], Variable[]]} an array containing
* 1. unused variables of the scope and descendant scopes.
* 2. used variables that match an ignore pattern of the scope and descendant scope
* @private
*/
function collectUnusedVariables(scope, unusedVars) {
function collectUnusedVariables(scope, unusedVars, usedIgnoredVars) {
const variables = scope.variables;
const childScopes = scope.childScopes;
let i, l;
Expand All @@ -589,8 +691,13 @@ module.exports = {
continue;
}

// skip function expression names and variables marked with markVariableAsUsed()
if (scope.functionExpressionScope || variable.eslintUsed) {
// skip function expression names
if (scope.functionExpressionScope) {
continue;
}

// skip variables marked with markVariableAsUsed()
if (!config.reportUsedIgnorePattern && variable.eslintUsed) {
continue;
}

Expand All @@ -615,6 +722,10 @@ module.exports = {
config.destructuredArrayIgnorePattern &&
config.destructuredArrayIgnorePattern.test(def.name.name)
) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
usedIgnoredVars.push(variable);
}

continue;
}

Expand All @@ -634,6 +745,10 @@ module.exports = {

// skip ignored parameters
if (config.caughtErrorsIgnorePattern && config.caughtErrorsIgnorePattern.test(def.name.name)) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
usedIgnoredVars.push(variable);
}

continue;
}
} else if (type === "Parameter") {
Expand All @@ -650,6 +765,10 @@ module.exports = {

// skip ignored parameters
if (config.argsIgnorePattern && config.argsIgnorePattern.test(def.name.name)) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
usedIgnoredVars.push(variable);
}

continue;
}

Expand All @@ -661,6 +780,10 @@ module.exports = {

// skip ignored variables
if (config.varsIgnorePattern && config.varsIgnorePattern.test(def.name.name)) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
usedIgnoredVars.push(variable);
}

continue;
}
}
Expand All @@ -673,10 +796,10 @@ module.exports = {
}

for (i = 0, l = childScopes.length; i < l; ++i) {
collectUnusedVariables(childScopes[i], unusedVars);
collectUnusedVariables(childScopes[i], unusedVars, usedIgnoredVars);
}

return unusedVars;
return [unusedVars, usedIgnoredVars];
}

//--------------------------------------------------------------------------
Expand All @@ -685,7 +808,7 @@ module.exports = {

return {
"Program:exit"(programNode) {
const unusedVars = collectUnusedVariables(sourceCode.getScope(programNode), []);
const [unusedVars, usedIgnoredVars] = collectUnusedVariables(sourceCode.getScope(programNode), [], []);

for (let i = 0, l = unusedVars.length; i < l; ++i) {
const unusedVar = unusedVars[i];
Expand Down Expand Up @@ -722,8 +845,19 @@ module.exports = {
});
}
}

if (config.reportUsedIgnorePattern) {
for (const usedIgnoredVar of usedIgnoredVars) {
for (const definition of usedIgnoredVar.defs) {
context.report({
node: definition.name,
messageId: "usedIgnoredVar",
data: getUsedIgnoredMessageData(usedIgnoredVar)
});
}
}
}
}
};

}
};

0 comments on commit a364f2f

Please sign in to comment.