feat: add no-useless-assignment rule (#17625)

* feat: add `no-useless-assignment` rule * fix: add test cases and fix * chore: remove wrong comment * test: add test and update * feat: report if it is not used between assignments * fix: add test case and fix * Apply suggestions from code review Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com> * docs: update * fix: false negatives for same segments & refactor * fix: rule-types.json * fix: add update assignment test case & fix * fix: add test for global vars & fix * Update docs/src/rules/no-useless-assignment.md Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com> * docs: update description * docs: add further_reading * chore: improve jsdoc and rename function * docs: update doc * fix: false positives for try catch * test: fix tests/conf/eslint-all.js * feat: add no-useless-assignment to eslint-config-eslint * docs: fix document * Update docs/src/rules/no-useless-assignment.md Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * fix: false negatives * docs: add example code * test: move test case * fix: false positives for assignment pattern * test: use flat-rule-tester * Update lib/rules/no-useless-assignment.js Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com> * fix: false positives for `finally` block * fix: false positives for markVariableAsUsed() * chore: rename function * fix: false positives for unreachable segments * Update lib/rules/no-useless-assignment.js Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com> * test: revert tests/conf/eslint-all.js * test: use rule-tester --------- Co-authored-by: Nicholas C. Zakas <nicholas@humanwhocodes.com> Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
eslint · Jan 9, 2024 · b4e0503 · b4e0503
1 parent 806f708
commit b4e0503
Show file tree

Hide file tree

Showing 25 changed files with 1,800 additions and 31 deletions.
diff --git a/docs/src/_data/further_reading_links.json b/docs/src/_data/further_reading_links.json
@@ -747,5 +747,40 @@
         "logo": "https://codepoints.net/favicon.ico",
         "title": "U+1680 OGHAM SPACE MARK:   – Unicode – Codepoints",
         "description": " , codepoint U+1680 OGHAM SPACE MARK in Unicode, is located in the block “Ogham”. It belongs to the Ogham script and is a Space Separator."
+    },
+    "https://en.wikipedia.org/wiki/Dead_store": {
+        "domain": "en.wikipedia.org",
+        "url": "https://en.wikipedia.org/wiki/Dead_store",
+        "logo": "https://en.wikipedia.org/static/apple-touch/wikipedia.png",
+        "title": "Dead store - Wikipedia",
+        "description": null
+    },
+    "https://rules.sonarsource.com/javascript/RSPEC-1854/": {
+        "domain": "rules.sonarsource.com",
+        "url": "https://rules.sonarsource.com/javascript/RSPEC-1854/",
+        "logo": "https://rules.sonarsource.com/favicon.ico",
+        "title": "JavaScript static code analysis: Unused assignments should be removed",
+        "description": "Dead stores refer to assignments made to local variables that are subsequently never used or immediately overwritten. Such assignments are\nunnecessary and don’t contribute to the functionality or clarity of the code. They may even negatively impact performance. Removing them enhances code\ncleanlines…"
+    },
+    "https://cwe.mitre.org/data/definitions/563.html": {
+        "domain": "cwe.mitre.org",
+        "url": "https://cwe.mitre.org/data/definitions/563.html",
+        "logo": "https://cwe.mitre.org/favicon.ico",
+        "title": "CWE - CWE-563: Assignment to Variable without Use (4.13)",
+        "description": "Common Weakness Enumeration (CWE) is a list of software weaknesses."
+    },
+    "https://wiki.sei.cmu.edu/confluence/display/c/MSC13-C.+Detect+and+remove+unused+values": {
+        "domain": "wiki.sei.cmu.edu",
+        "url": "https://wiki.sei.cmu.edu/confluence/display/c/MSC13-C.+Detect+and+remove+unused+values",
+        "logo": "https://wiki.sei.cmu.edu/confluence/s/-ctumb3/9012/tu5x00/7/_/favicon.ico",
+        "title": "MSC13-C. Detect and remove unused values - SEI CERT C Coding Standard - Confluence",
+        "description": null
+    },
+    "https://wiki.sei.cmu.edu/confluence/display/java/MSC56-J.+Detect+and+remove+superfluous+code+and+values": {
+        "domain": "wiki.sei.cmu.edu",
+        "url": "https://wiki.sei.cmu.edu/confluence/display/java/MSC56-J.+Detect+and+remove+superfluous+code+and+values",
+        "logo": "https://wiki.sei.cmu.edu/confluence/s/-ctumb3/9012/tu5x00/7/_/favicon.ico",
+        "title": "MSC56-J. Detect and remove superfluous code and values - SEI CERT Oracle Coding Standard for Java - Confluence",
+        "description": null
     }
 }
diff --git a/docs/src/rules/no-unused-vars.md b/docs/src/rules/no-unused-vars.md
@@ -1,6 +1,8 @@
 ---
 title: no-unused-vars
 rule_type: problem
+related_rules:
+- no-useless-assignment
 ---
 
 

diff --git a/docs/src/rules/no-useless-assignment.md b/docs/src/rules/no-useless-assignment.md
@@ -0,0 +1,156 @@
+---
+title: no-useless-assignment
+rule_type: suggestion
+related_rules:
+- no-unused-vars
+further_reading:
+- https://en.wikipedia.org/wiki/Dead_store
+- https://rules.sonarsource.com/javascript/RSPEC-1854/
+- https://cwe.mitre.org/data/definitions/563.html
+- https://wiki.sei.cmu.edu/confluence/display/c/MSC13-C.+Detect+and+remove+unused+values
+- https://wiki.sei.cmu.edu/confluence/display/java/MSC56-J.+Detect+and+remove+superfluous+code+and+values
+---
+
+
+[Wikipedia describes a "dead store"](https://en.wikipedia.org/wiki/Dead_store) as follows:
+
+> In computer programming, a local variable that is assigned a value but is not read by any subsequent instruction is referred to as a **dead store**.
+
+"Dead stores" waste processing and memory, so it is better to remove unnecessary assignments to variables.
+
+Also, if the author intended the variable to be used, there is likely a mistake around the dead store.
+For example,
+
+* you should have used a stored value but forgot to do so.
+* you made a mistake in the name of the variable to be stored.
+
+```js
+let id = "x1234";    // this is a "dead store" - this value ("x1234") is never read
+
+id = generateId();
+
+doSomethingWith(id);
+```
+
+## Rule Details
+
+This rule aims to report variable assignments when the value is not used.
+
+Examples of **incorrect** code for this rule:
+
+::: incorrect
+
+```js
+/* eslint no-useless-assignment: "error" */
+
+function fn1() {
+    let v = 'used';
+    doSomething(v);
+    v = 'unused';
+}
+
+function fn2() {
+    let v = 'used';
+    if (condition) {
+        v = 'unused';
+        return
+    }
+    doSomething(v);
+}
+
+function fn3() {
+    let v = 'used';
+    if (condition) {
+        doSomething(v);
+    } else {
+        v = 'unused';
+    }
+}
+
+function fn4() {
+    let v = 'unused';
+    if (condition) {
+        v = 'used';
+        doSomething(v);
+        return
+    }
+}
+
+function fn5() {
+    let v = 'used';
+    if (condition) {
+        let v = 'used';
+        console.log(v);
+        v = 'unused';
+    }
+    console.log(v);
+}
+```
+
+:::
+
+Examples of **correct** code for this rule:
+
+::: correct
+
+```js
+/* eslint no-useless-assignment: "error" */
+
+function fn1() {
+    let v = 'used';
+    doSomething(v);
+    v = 'used-2';
+    doSomething(v);
+}
+
+function fn2() {
+    let v = 'used';
+    if (condition) {
+        v = 'used-2';
+        doSomething(v);
+        return
+    }
+    doSomething(v);
+}
+
+function fn3() {
+    let v = 'used';
+    if (condition) {
+        doSomething(v);
+    } else {
+        v = 'used-2';
+        doSomething(v);
+    }
+}
+
+function fn4() {
+    let v = 'used';
+    for (let i = 0; i < 10; i++) {
+        doSomething(v);
+        v = 'used in next iteration';
+    }
+}
+```
+
+:::
+
+This rule will not report variables that are never read.
+Because it's clearly an unused variable. If you want it reported, please enable the [no-unused-vars](./no-unused-vars) rule.
+
+::: correct
+
+```js
+/* eslint no-useless-assignment: "error" */
+
+function fn() {
+    let v = 'unused';
+    v = 'unused-2'
+    doSomething();
+}
+```
+
+:::
+
+## When Not To Use It
+
+If you don't want to be notified about values that are never read, you can safely disable this rule.
diff --git a/lib/linter/code-path-analysis/code-path.js b/lib/linter/code-path-analysis/code-path.js
@@ -166,9 +166,9 @@ class CodePath {
         const lastSegment = resolvedOptions.last;
 
         // set up initial location information
-        let record = null;
-        let index = 0;
-        let end = 0;
+        let record;
+        let index;
+        let end;
         let segment = null;
 
         // segments that have already been visited during traversal

diff --git a/lib/linter/config-comment-parser.js b/lib/linter/config-comment-parser.js
@@ -75,11 +75,9 @@ module.exports = class ConfigCommentParser {
     parseJsonConfig(string, location) {
         debug("Parsing JSON config");
 
-        let items = {};
-
         // Parses a JSON-like comment by the same way as parsing CLI option.
         try {
-            items = levn.parse("Object", string) || {};
+            const items = levn.parse("Object", string) || {};
 
             // Some tests say that it should ignore invalid comments such as `/*eslint no-alert:abc*/`.
             // Also, commaless notations have invalid severity:
@@ -102,11 +100,15 @@ module.exports = class ConfigCommentParser {
          * Optionator cannot parse commaless notations.
          * But we are supporting that. So this is a fallback for that.
          */
-        items = {};
         const normalizedString = string.replace(/([-a-zA-Z0-9/]+):/gu, "\"$1\":").replace(/(\]|[0-9])\s+(?=")/u, "$1,");
 
         try {
-            items = JSON.parse(`{${normalizedString}}`);
+            const items = JSON.parse(`{${normalizedString}}`);
+
+            return {
+                success: true,
+                config: items
+            };
         } catch (ex) {
             debug("Manual parsing failed.");
 
@@ -124,11 +126,6 @@ module.exports = class ConfigCommentParser {
             };
 
         }
-
-        return {
-            success: true,
-            config: items
-        };
     }
 
     /**

diff --git a/lib/linter/linter.js b/lib/linter/linter.js
@@ -2139,7 +2139,7 @@ class Linter {
      *      SourceCodeFixer.
      */
     verifyAndFix(text, config, options) {
-        let messages = [],
+        let messages,
             fixedResult,
             fixed = false,
             passNumber = 0,

diff --git a/lib/rules/array-bracket-newline.js b/lib/rules/array-bracket-newline.js
@@ -74,7 +74,7 @@ module.exports = {
         function normalizeOptionValue(option) {
             let consistent = false;
             let multiline = false;
-            let minItems = 0;
+            let minItems;
 
             if (option) {
                 if (option === "consistent") {

diff --git a/lib/rules/indent-legacy.js b/lib/rules/indent-legacy.js
@@ -830,7 +830,7 @@ module.exports = {
             }
 
             let indent;
-            let nodesToCheck = [];
+            let nodesToCheck;
 
             /*
              * For this statements we should check indent from statement beginning,

diff --git a/lib/rules/indent.js b/lib/rules/indent.js
@@ -1407,7 +1407,7 @@ module.exports = {
             PropertyDefinition(node) {
                 const firstToken = sourceCode.getFirstToken(node);
                 const maybeSemicolonToken = sourceCode.getLastToken(node);
-                let keyLastToken = null;
+                let keyLastToken;
 
                 // Indent key.
                 if (node.computed) {

diff --git a/lib/rules/index.js b/lib/rules/index.js
@@ -229,6 +229,7 @@ module.exports = new LazyLoadingRuleMap(Object.entries({
     "no-unused-private-class-members": () => require("./no-unused-private-class-members"),
     "no-unused-vars": () => require("./no-unused-vars"),
     "no-use-before-define": () => require("./no-use-before-define"),
+    "no-useless-assignment": () => require("./no-useless-assignment"),
     "no-useless-backreference": () => require("./no-useless-backreference"),
     "no-useless-call": () => require("./no-useless-call"),
     "no-useless-catch": () => require("./no-useless-catch"),

diff --git a/lib/rules/key-spacing.js b/lib/rules/key-spacing.js
@@ -489,7 +489,7 @@ module.exports = {
                     }
                 }
 
-                let messageId = "";
+                let messageId;
 
                 if (isExtra) {
                     messageId = side === "key" ? "extraKey" : "extraValue";

diff --git a/lib/rules/max-len.js b/lib/rules/max-len.js
@@ -344,7 +344,7 @@ module.exports = {
                  * comments to check.
                  */
                 if (commentsIndex < comments.length) {
-                    let comment = null;
+                    let comment;
 
                     // iterate over comments until we find one past the current line
                     do {

diff --git a/lib/rules/no-dupe-class-members.js b/lib/rules/no-dupe-class-members.js
@@ -82,7 +82,7 @@ module.exports = {
                 }
 
                 const state = getState(name, node.static);
-                let isDuplicate = false;
+                let isDuplicate;
 
                 if (kind === "get") {
                     isDuplicate = (state.init || state.get);

diff --git a/lib/rules/no-empty-function.js b/lib/rules/no-empty-function.js
@@ -40,7 +40,7 @@ const ALLOW_OPTIONS = Object.freeze([
  */
 function getKind(node) {
     const parent = node.parent;
-    let kind = "";
+    let kind;
 
     if (node.type === "ArrowFunctionExpression") {
         return "arrowFunctions";
@@ -73,7 +73,7 @@ function getKind(node) {
     }
 
     // Detects prefix.
-    let prefix = "";
+    let prefix;
 
     if (node.generator) {
         prefix = "generator";

diff --git a/lib/rules/no-loss-of-precision.js b/lib/rules/no-loss-of-precision.js
@@ -64,7 +64,7 @@ module.exports = {
          */
         function notBaseTenLosesPrecision(node) {
             const rawString = getRaw(node).toUpperCase();
-            let base = 0;
+            let base;
 
             if (rawString.startsWith("0B")) {
                 base = 2;

diff --git a/lib/rules/no-trailing-spaces.js b/lib/rules/no-trailing-spaces.js
@@ -129,8 +129,7 @@ module.exports = {
                     comments = sourceCode.getAllComments(),
                     commentLineNumbers = getCommentLineNumbers(comments);
 
-                let totalLength = 0,
-                    fixRange = [];
+                let totalLength = 0;
 
                 for (let i = 0, ii = lines.length; i < ii; i++) {
                     const lineNumber = i + 1;
@@ -177,7 +176,7 @@ module.exports = {
                             continue;
                         }
 
-                        fixRange = [rangeStart, rangeEnd];
+                        const fixRange = [rangeStart, rangeEnd];
 
                         if (!ignoreComments || !commentLineNumbers.has(lineNumber)) {
                             report(node, location, fixRange);