Update: support separate requires in one-var. (fixes #6175) (#9441)

* Update: support separate requires in one-var. (fixes #6175) * wip. * fix edge case. * Docs: add JSCS link
eslint · Dec 23, 2017 · be2f57e · be2f57e
1 parent 370d614
commit be2f57e
Show file tree

Hide file tree

Showing 3 changed files with 167 additions and 91 deletions.
diff --git a/docs/rules/one-var.md b/docs/rules/one-var.md
@@ -45,6 +45,7 @@ Object option:
 * `"let": "never"` requires multiple `let` declarations per block
 * `"const": "always"` requires one `const` declaration per block
 * `"const": "never"` requires multiple `const` declarations per block
+* `"separateRequires": true` enforces `requires` to be separate from declarations
 
 Alternate object option:
 
@@ -252,6 +253,28 @@ function foo() {
 }
 ```
 
+Examples of **incorrect** code for this rule with the `{ separateRequires: true }` option:
+
+```js
+/*eslint one-var: ["error", { separateRequires: true, var: "always" }]*/
+/*eslint-env node*/
+
+var foo = require("foo"),
+    bar = "bar";
+```
+
+Examples of **correct** code for this rule with the `{ separateRequires: true }` option:
+
+```js
+/*eslint one-var: ["error", { separateRequires: true, var: "always" }]*/
+/*eslint-env node*/
+
+var foo = require("foo");
+var bar = "bar";
+
+var foo = require("foo"),
+    bar = require("bar");
+```
 
 ### initialized and uninitialized
 
@@ -318,4 +341,5 @@ function foo() {
 ## Compatibility
 
 * **JSHint**: This rule maps to the `onevar` JSHint rule, but allows `let` and `const` to be configured separately.
-* **JSCS**: This rule roughly maps to [disallowMultipleVarDecl](http://jscs.info/rule/disallowMultipleVarDecl)
+* **JSCS**: This rule roughly maps to [disallowMultipleVarDecl](http://jscs.info/rule/disallowMultipleVarDecl).
+* **JSCS**: This rule option `separateRequires` roughly maps to [requireMultipleVarDecl](http://jscs.info/rule/requireMultipleVarDecl).
diff --git a/lib/rules/one-var.js b/lib/rules/one-var.js
@@ -26,6 +26,9 @@ module.exports = {
                     {
                         type: "object",
                         properties: {
+                            separateRequires: {
+                                type: "boolean"
+                            },
                             var: {
                                 enum: ["always", "never"]
                             },
@@ -62,21 +65,23 @@ module.exports = {
 
         const mode = context.options[0] || MODE_ALWAYS;
 
-        const options = {
-        };
+        const options = {};
 
         if (typeof mode === "string") { // simple options configuration with just a string
             options.var = { uninitialized: mode, initialized: mode };
             options.let = { uninitialized: mode, initialized: mode };
             options.const = { uninitialized: mode, initialized: mode };
         } else if (typeof mode === "object") { // options configuration is an object
-            if (mode.hasOwnProperty("var") && typeof mode.var === "string") {
+            if (mode.hasOwnProperty("separateRequires")) {
+                options.separateRequires = !!mode.separateRequires;
+            }
+            if (mode.hasOwnProperty("var")) {
                 options.var = { uninitialized: mode.var, initialized: mode.var };
             }
-            if (mode.hasOwnProperty("let") && typeof mode.let === "string") {
+            if (mode.hasOwnProperty("let")) {
                 options.let = { uninitialized: mode.let, initialized: mode.let };
             }
-            if (mode.hasOwnProperty("const") && typeof mode.const === "string") {
+            if (mode.hasOwnProperty("const")) {
                 options.const = { uninitialized: mode.const, initialized: mode.const };
             }
             if (mode.hasOwnProperty("uninitialized")) {
@@ -158,7 +163,17 @@ module.exports = {
         }
 
         /**
-         * Records whether initialized or uninitialized variables are defined in current scope.
+         * Check if a variable declaration is a require.
+         * @param {ASTNode} decl variable declaration Node
+         * @returns {bool} if decl is a require, return true; else return false.
+         * @private
+         */
+        function isRequire(decl) {
+            return decl.init && decl.init.type === "CallExpression" && decl.init.callee.name === "require";
+        }
+
+        /**
+         * Records whether initialized/uninitialized/required variables are defined in current scope.
          * @param {string} statementType node.kind, one of: "var", "let", or "const"
          * @param {ASTNode[]} declarations List of declarations
          * @param {Object} currentScope The scope being investigated
@@ -173,7 +188,11 @@ module.exports = {
                     }
                 } else {
                     if (options[statementType] && options[statementType].initialized === MODE_ALWAYS) {
-                        currentScope.initialized = true;
+                        if (options.separateRequires && isRequire(declarations[i])) {
+                            currentScope.required = true;
+                        } else {
+                            currentScope.initialized = true;
+                        }
                     }
                 }
             }
@@ -228,6 +247,7 @@ module.exports = {
             const declarationCounts = countDeclarations(declarations);
             const currentOptions = options[statementType] || {};
             const currentScope = getCurrentScope(statementType);
+            const hasRequires = declarations.some(isRequire);
 
             if (currentOptions.uninitialized === MODE_ALWAYS && currentOptions.initialized === MODE_ALWAYS) {
                 if (currentScope.uninitialized || currentScope.initialized) {
@@ -245,6 +265,9 @@ module.exports = {
                     return false;
                 }
             }
+            if (currentScope.required && hasRequires) {
+                return false;
+            }
             recordTypes(statementType, declarations, currentScope);
             return true;
         }
@@ -275,6 +298,16 @@ module.exports = {
 
                 const declarations = node.declarations;
                 const declarationCounts = countDeclarations(declarations);
+                const mixedRequires = declarations.some(isRequire) && !declarations.every(isRequire);
+
+                if (options[type].initialized === MODE_ALWAYS) {
+                    if (options.separateRequires && mixedRequires) {
+                        context.report({
+                            node,
+                            message: "Split requires to be separated into a single block."
+                        });
+                    }
+                }
 
                 // always
                 if (!hasOnlyOneStatement(type, declarations)) {