Skip to content

Commit

Permalink
feat: Add reportUsedIgnorePattern option to no-unused-vars rule (#…
Browse files Browse the repository at this point in the history 
…17662)

* Add `reportUsedIgnorePattern` option to `no-unused-vars` rule

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

* Report when ignored variable use used.

* Fix docs and jsdocs

* Full revert jsdoc update

* Refactor message data handlers

* Fix english

* english is hard

* Fix auto prettier
  • Loading branch information
@Pearce-Ropion
Pearce-Ropion committed Apr 2, 2024
1 parent 651ec91 commit 97ce45b
Show file tree
Hide file tree
Showing 3 changed files with 305 additions and 34 deletions.
43 changes: 42 additions & 1 deletion 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 @@ -455,6 +461,41 @@ 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

If you don't want to be notified about unused variables or function arguments, you can safely turn this rule off.
208 changes: 179 additions & 29 deletions lib/rules/no-unused-vars.js
View file
Expand Up @@ -15,6 +15,11 @@ const astUtils = require("./utils/ast-utils");
// Typedefs
//------------------------------------------------------------------------------

/**
* A simple name for the types of variables that this rule supports
* @typedef {'array-destructure'|'catch-clause'|'parameter'|'variable'} VariableType
*/

/**
* Bag of data used for formatting the `unusedVar` lint message.
* @typedef {Object} UnusedVarMessageData
Expand All @@ -23,6 +28,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 +85,9 @@ module.exports = {
},
ignoreClassWithStaticInitBlock: {
type: "boolean"
},
reportUsedIgnorePattern: {
type: "boolean"
}
},
additionalProperties: false
Expand All @@ -82,7 +97,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 +112,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 +127,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 @@ -129,34 +147,93 @@ module.exports = {
}
}

/**
* Gets a given variable's description and configured ignore pattern
* based on the provided variableType
* @param {VariableType} variableType a simple name for the types of variables that this rule supports
* @throws {Error} (Unreachable)
* @returns {[string | undefined, string | undefined]} the given variable's description and
* ignore pattern
*/
function getVariableDescription(variableType) {
let pattern;
let variableDescription;

switch (variableType) {
case "array-destructure":
pattern = config.destructuredArrayIgnorePattern;
variableDescription = "elements of array destructuring";
break;

case "catch-clause":
pattern = config.caughtErrorsIgnorePattern;
variableDescription = "args";
break;

case "parameter":
pattern = config.argsIgnorePattern;
variableDescription = "args";
break;

case "variable":
pattern = config.varsIgnorePattern;
variableDescription = "vars";
break;

default:
throw new Error(`Unexpected variable type: ${variableType}`);
}

if (pattern) {
pattern = pattern.toString();
}

return [variableDescription, pattern];
}

/**
* Generates the message data about the variable being defined and unused,
* including the ignore pattern if configured.
* @param {Variable} unusedVar eslint-scope variable object.
* @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;
const def = unusedVar.defs && unusedVar.defs[0];
let additionalMessageData = "";

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();
}
if (def) {
let pattern;
let variableDescription;

switch (def.type) {
case "CatchClause":
if (config.caughtErrorsIgnorePattern) {
[variableDescription, pattern] = getVariableDescription("catch-clause");
}
break;

const additional = type ? `. Allowed unused ${type} must match ${pattern}` : "";
case "Parameter":
if (config.argsIgnorePattern) {
[variableDescription, pattern] = getVariableDescription("parameter");
}
break;

default:
if (config.varsIgnorePattern) {
[variableDescription, pattern] = getVariableDescription("variable");
}
break;
}

if (pattern && variableDescription) {
additionalMessageData = `. Allowed unused ${variableDescription} must match ${pattern}`;
}
}

return {
varName: unusedVar.name,
action: "defined",
additional
additional: additionalMessageData
};
}

Expand All @@ -167,19 +244,51 @@ module.exports = {
* @returns {UnusedVarMessageData} The message data to be used with this unused variable.
*/
function getAssignedMessageData(unusedVar) {
const def = unusedVar.defs[0];
let additional = "";
const def = unusedVar.defs && unusedVar.defs[0];
let additionalMessageData = "";

if (def) {
let pattern;
let variableDescription;

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.name.parent.type === "ArrayPattern" && config.destructuredArrayIgnorePattern) {
[variableDescription, pattern] = getVariableDescription("array-destructure");
} else if (config.varsIgnorePattern) {
[variableDescription, pattern] = getVariableDescription("variable");
}

if (pattern && variableDescription) {
additionalMessageData = `. Allowed unused ${variableDescription} must match ${pattern}`;
}
}

return {
varName: unusedVar.name,
action: "assigned a value",
additional
additional: additionalMessageData
};
}

/**
* Generate the warning message about a variable being used even though
* it is marked as being ignored.
* @param {Variable} variable eslint-scope variable object
* @param {VariableType} variableType a simple name for the types of variables that this rule supports
* @returns {UsedIgnoredVarMessageData} The message data to be used with
* this used ignored variable.
*/
function getUsedIgnoredMessageData(variable, variableType) {
const [variableDescription, pattern] = getVariableDescription(variableType);

let additionalMessageData = "";

if (pattern && variableDescription) {
additionalMessageData = `. Used ${variableDescription} must not match ${pattern}`;
}

return {
varName: variable.name,
additional: additionalMessageData
};
}

Expand Down Expand Up @@ -532,8 +641,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 @@ -589,8 +703,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 +734,14 @@ module.exports = {
config.destructuredArrayIgnorePattern &&
config.destructuredArrayIgnorePattern.test(def.name.name)
) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
context.report({
node: def.name,
messageId: "usedIgnoredVar",
data: getUsedIgnoredMessageData(variable, "array-destructure")
});
}

continue;
}

Expand All @@ -634,6 +761,14 @@ module.exports = {

// skip ignored parameters
if (config.caughtErrorsIgnorePattern && config.caughtErrorsIgnorePattern.test(def.name.name)) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
context.report({
node: def.name,
messageId: "usedIgnoredVar",
data: getUsedIgnoredMessageData(variable, "catch-clause")
});
}

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

// skip ignored parameters
if (config.argsIgnorePattern && config.argsIgnorePattern.test(def.name.name)) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
context.report({
node: def.name,
messageId: "usedIgnoredVar",
data: getUsedIgnoredMessageData(variable, "parameter")
});
}

continue;
}

Expand All @@ -661,6 +804,14 @@ module.exports = {

// skip ignored variables
if (config.varsIgnorePattern && config.varsIgnorePattern.test(def.name.name)) {
if (config.reportUsedIgnorePattern && isUsedVariable(variable)) {
context.report({
node: def.name,
messageId: "usedIgnoredVar",
data: getUsedIgnoredMessageData(variable, "variable")
});
}

continue;
}
}
Expand Down Expand Up @@ -724,6 +875,5 @@ module.exports = {
}
}
};

}
};

0 comments on commit 97ce45b

Please sign in to comment.