Fix: prefer-const false negative about eslintUsed (fixes #5837) (#…

…6971)
eslint · Aug 26, 2016 · ca3d448 · ca3d448
1 parent 1153955
commit ca3d448
Show file tree

Hide file tree

Showing 2 changed files with 32 additions and 4 deletions.
diff --git a/lib/rules/prefer-const.js b/lib/rules/prefer-const.js
@@ -70,9 +70,14 @@ function canBecomeVariableDeclaration(identifier) {
  *
  * If the variable should not change to const, this function returns null.
  * - If the variable is reassigned.
- * - If the variable is never initialized and assigned.
+ * - If the variable is never initialized nor assigned.
  * - If the variable is initialized in a different scope from the declaration.
  * - If the unique assignment of the variable cannot change to a declaration.
+ *   e.g. `if (a) b = 1` / `return (b = 1)`
+ * - If the variable is declared in the global scope and `eslintUsed` is `true`.
+ *   `/*exported foo` directive comment makes such variables. This rule does not
+ *   warn such variables because this rule cannot distinguish whether the
+ *   exported variables are reassigned or not.
  *
  * @param {escope.Variable} variable - A variable to get.
  * @param {boolean} ignoreReadBeforeAssign -
@@ -82,7 +87,7 @@ function canBecomeVariableDeclaration(identifier) {
  *      Otherwise, null.
  */
 function getIdentifierIfShouldBeConst(variable, ignoreReadBeforeAssign) {
-    if (variable.eslintUsed) {
+    if (variable.eslintUsed && variable.scope.type === "global") {
         return null;
     }
 

diff --git a/tests/lib/rules/prefer-const.js b/tests/lib/rules/prefer-const.js
@@ -18,6 +18,12 @@ const rule = require("../../../lib/rules/prefer-const"),
 
 const ruleTester = new RuleTester();
 
+ruleTester.defineRule("use-x", context => ({
+    VariableDeclaration() {
+        context.markVariableAsUsed("x");
+    }
+}));
+
 ruleTester.run("prefer-const", rule, {
     valid: [
         { code: "var x = 0;" },
@@ -63,6 +69,7 @@ ruleTester.run("prefer-const", rule, {
             parserOptions: { ecmaVersion: 6 }
         },
         { code: "/*exported a*/ let a; function init() { a = foo(); }", parserOptions: { ecmaVersion: 6 } },
+        { code: "/*exported a*/ let a = 1", parserOptions: { ecmaVersion: 6 } },
         { code: "let a; if (true) a = 0; foo(a);", parserOptions: { ecmaVersion: 6 } },
 
         // The assignment is located in a different scope.
@@ -89,7 +96,9 @@ ruleTester.run("prefer-const", rule, {
             code: "let x; function foo() { bar(x); } x = 0;",
             options: [{ignoreReadBeforeAssign: true}],
             parserOptions: {ecmaVersion: 6}
-        }
+        },
+
+
     ],
     invalid: [
         {
@@ -336,6 +345,20 @@ ruleTester.run("prefer-const", rule, {
             output: "let x; function foo() { bar(x); } x = 0;",
             parserOptions: {ecmaVersion: 6},
             errors: [{ message: "'x' is never reassigned. Use 'const' instead.", type: "Identifier", column: 5}]
-        }
+        },
+
+        // https://github.com/eslint/eslint/issues/5837
+        {
+            code: "/*eslint use-x:error*/ let x = 1",
+            output: "/*eslint use-x:error*/ const x = 1",
+            parserOptions: {ecmaVersion: 6, ecmaFeatures: {globalReturn: true}},
+            errors: [{ message: "'x' is never reassigned. Use 'const' instead.", type: "Identifier"}]
+        },
+        {
+            code: "/*eslint use-x:error*/ { let x = 1 }",
+            output: "/*eslint use-x:error*/ { const x = 1 }",
+            parserOptions: {ecmaVersion: 6},
+            errors: [{ message: "'x' is never reassigned. Use 'const' instead.", type: "Identifier"}]
+        },
     ]
 });