Enable more eslint recommended rules (#11168)

* chore: enable no-constant-condition

* chore: enable no-empty rule

* chore: enable no-unreachable

* chore: enable no-cond-assign

* chore: enable no-inner-declarations

* chore: remove disabled rules that are not in eslint:recommended

* fix: oops

eslint/babel-eslint-config-internal/index.js

@@ -19,29 +19,14 @@ module.exports = {
     browser: true,
   },
   rules: {
-    camelcase: "off",
-    "consistent-return": "off",
     curly: ["error", "multi-line"],
     "linebreak-style": ["error", "unix"],
-    "new-cap": "off",
     "no-case-declarations": "error",
-    "no-cond-assign": "off",
     "no-confusing-arrow": "error",
-    "no-console": "off",
+    "no-empty": ["error", { allowEmptyCatch: true }],
-    "no-constant-condition": "off",
-    "no-empty": "off",
-    "no-inner-declarations": "off",
-    "no-labels": "off",
-    "no-loop-func": "off",
     "no-process-exit": "error",
-    "no-return-assign": "off",
-    "no-shadow": "off",
-    "no-underscore-dangle": "off",
-    "no-unreachable": "off",
-    "no-use-before-define": "off",
     "no-var": "error",
     "prefer-const": "error",
-    strict: "off",
     "flowtype/define-flow-type": "warn",
     "flowtype/use-flow-type": "warn",
   },

packages/babel-generator/src/generators/modules.js

@@ -99,7 +99,7 @@ function ExportDeclaration(node: Object) {
 
     // print "special" specifiers first
     let hasSpecial = false;
-    while (true) {
+    for (;;) {
       const first = specifiers[0];
       if (
         t.isExportDefaultSpecifier(first) ||
@@ -149,7 +149,7 @@ export function ImportDeclaration(node: Object) {
   const specifiers = node.specifiers.slice(0);
   if (specifiers && specifiers.length) {
     // print "special" specifiers first
-    while (true) {
+    for (;;) {
       const first = specifiers[0];
       if (
         t.isImportDefaultSpecifier(first) ||

packages/babel-helper-fixtures/src/index.js

@@ -78,6 +78,180 @@ function findFile(filepath: string, allowJSON: boolean) {
   return matches[0];
 }
 
+function pushTask(taskName, taskDir, suite, suiteName) {
+  const taskDirStats = fs.statSync(taskDir);
+  let actualLoc = findFile(taskDir + "/input");
+  let execLoc = findFile(taskDir + "/exec");
+
+  // If neither input nor exec is present it is not a real testcase
+  if (taskDirStats.isDirectory() && !actualLoc && !execLoc) {
+    if (fs.readdirSync(taskDir).length > 0) {
+      console.warn(`Skipped test folder with invalid layout: ${taskDir}`);
+    }
+    return;
+  } else if (!actualLoc) {
+    actualLoc = taskDir + "/input.js";
+  } else if (!execLoc) {
+    execLoc = taskDir + "/exec.js";
+  }
+
+  const expectLoc =
+    findFile(taskDir + "/output", true /* allowJSON */) ||
+    taskDir + "/output.js";
+  const stdoutLoc = taskDir + "/stdout.txt";
+  const stderrLoc = taskDir + "/stderr.txt";
+
+  const actualLocAlias =
+    suiteName + "/" + taskName + "/" + path.basename(actualLoc);
+  const expectLocAlias =
+    suiteName + "/" + taskName + "/" + path.basename(actualLoc);
+  let execLocAlias =
+    suiteName + "/" + taskName + "/" + path.basename(actualLoc);
+
+  if (taskDirStats.isFile()) {
+    const ext = path.extname(taskDir);
+    if (EXTENSIONS.indexOf(ext) === -1) return;
+
+    execLoc = taskDir;
+    execLocAlias = suiteName + "/" + taskName;
+  }
+
+  const taskOpts = cloneDeep(suite.options);
+
+  const taskOptsLoc = tryResolve(taskDir + "/options");
+  if (taskOptsLoc) extend(taskOpts, require(taskOptsLoc));
+
+  const test = {
+    optionsDir: taskOptsLoc ? path.dirname(taskOptsLoc) : null,
+    title: humanize(taskName, true),
+    disabled: taskName[0] === ".",
+    options: taskOpts,
+    validateLogs: taskOpts.validateLogs,
+    ignoreOutput: taskOpts.ignoreOutput,
+    stdout: { loc: stdoutLoc, code: readFile(stdoutLoc) },
+    stderr: { loc: stderrLoc, code: readFile(stderrLoc) },
+    exec: {
+      loc: execLoc,
+      code: readFile(execLoc),
+      filename: execLocAlias,
+    },
+    actual: {
+      loc: actualLoc,
+      code: readFile(actualLoc),
+      filename: actualLocAlias,
+    },
+    expect: {
+      loc: expectLoc,
+      code: readFile(expectLoc),
+      filename: expectLocAlias,
+    },
+  };
+
+  // If there's node requirement, check it before pushing task
+  if (taskOpts.minNodeVersion) {
+    const minimumVersion = semver.clean(taskOpts.minNodeVersion);
+
+    if (minimumVersion == null) {
+      throw new Error(
+        `'minNodeVersion' has invalid semver format: ${taskOpts.minNodeVersion}`,
+      );
+    }
+
+    if (semver.lt(nodeVersion, minimumVersion)) {
+      return;
+    }
+
+    // Delete to avoid option validation error
+    delete taskOpts.minNodeVersion;
+  }
+
+  if (taskOpts.os) {
+    let os = taskOpts.os;
+
+    if (!Array.isArray(os) && typeof os !== "string") {
+      throw new Error(
+        `'os' should be either string or string array: ${taskOpts.os}`,
+      );
+    }
+
+    if (typeof os === "string") {
+      os = [os];
+    }
+
+    if (!os.includes(process.platform)) {
+      return;
+    }
+
+    delete taskOpts.os;
+  }
+
+  // traceur checks
+
+  if (test.exec.code.indexOf("// Async.") >= 0) {
+    return;
+  }
+
+  suite.tests.push(test);
+
+  const sourceMappingsLoc = taskDir + "/source-mappings.json";
+  if (fs.existsSync(sourceMappingsLoc)) {
+    test.sourceMappings = JSON.parse(readFile(sourceMappingsLoc));
+  }
+
+  const sourceMapLoc = taskDir + "/source-map.json";
+  if (fs.existsSync(sourceMapLoc)) {
+    test.sourceMap = JSON.parse(readFile(sourceMapLoc));
+  }
+
+  const inputMapLoc = taskDir + "/input-source-map.json";
+  if (fs.existsSync(inputMapLoc)) {
+    test.inputSourceMap = JSON.parse(readFile(inputMapLoc));
+  }
+
+  if (taskOpts.throws) {
+    if (test.expect.code) {
+      throw new Error(
+        "Test cannot throw and also return output code: " + expectLoc,
+      );
+    }
+    if (test.sourceMappings) {
+      throw new Error(
+        "Test cannot throw and also return sourcemappings: " +
+          sourceMappingsLoc,
+      );
+    }
+    if (test.sourceMap) {
+      throw new Error(
+        "Test cannot throw and also return sourcemaps: " + sourceMapLoc,
+      );
+    }
+  }
+
+  if (!test.validateLogs && (test.stdout.code || test.stderr.code)) {
+    throw new Error(
+      "stdout.txt and stderr.txt are only allowed when the 'validateLogs' option is enabled: " +
+        (test.stdout.code ? stdoutLoc : stderrLoc),
+    );
+  }
+  if (test.options.ignoreOutput) {
+    if (test.expect.code) {
+      throw new Error(
+        "Test cannot ignore its output and also validate it: " + expectLoc,
+      );
+    }
+    if (!test.validateLogs) {
+      throw new Error(
+        "ignoreOutput can only be used when validateLogs is true: " +
+          taskOptsLoc,
+      );
+    }
+  }
+
+  // Delete to avoid option validation error
+  delete test.options.validateLogs;
+  delete test.options.ignoreOutput;
+}
+
 export default function get(entryLoc): Array<Suite> {
   const suites = [];
 
@@ -102,181 +276,7 @@ export default function get(entryLoc): Array<Suite> {
     if (suiteOptsLoc) suite.options = require(suiteOptsLoc);
 
     for (const taskName of fs.readdirSync(suite.filename)) {
-      push(taskName, suite.filename + "/" + taskName);
+      pushTask(taskName, suite.filename + "/" + taskName, suite, suiteName);
-    }
-
-    function push(taskName, taskDir) {
-      const taskDirStats = fs.statSync(taskDir);
-      let actualLoc = findFile(taskDir + "/input");
-      let execLoc = findFile(taskDir + "/exec");
-
-      // If neither input nor exec is present it is not a real testcase
-      if (taskDirStats.isDirectory() && !actualLoc && !execLoc) {
-        if (fs.readdirSync(taskDir).length > 0) {
-          console.warn(`Skipped test folder with invalid layout: ${taskDir}`);
-        }
-        return;
-      } else if (!actualLoc) {
-        actualLoc = taskDir + "/input.js";
-      } else if (!execLoc) {
-        execLoc = taskDir + "/exec.js";
-      }
-
-      const expectLoc =
-        findFile(taskDir + "/output", true /* allowJSON */) ||
-        taskDir + "/output.js";
-      const stdoutLoc = taskDir + "/stdout.txt";
-      const stderrLoc = taskDir + "/stderr.txt";
-
-      const actualLocAlias =
-        suiteName + "/" + taskName + "/" + path.basename(actualLoc);
-      const expectLocAlias =
-        suiteName + "/" + taskName + "/" + path.basename(actualLoc);
-      let execLocAlias =
-        suiteName + "/" + taskName + "/" + path.basename(actualLoc);
-
-      if (taskDirStats.isFile()) {
-        const ext = path.extname(taskDir);
-        if (EXTENSIONS.indexOf(ext) === -1) return;
-
-        execLoc = taskDir;
-        execLocAlias = suiteName + "/" + taskName;
-      }
-
-      const taskOpts = cloneDeep(suite.options);
-
-      const taskOptsLoc = tryResolve(taskDir + "/options");
-      if (taskOptsLoc) extend(taskOpts, require(taskOptsLoc));
-
-      const test = {
-        optionsDir: taskOptsLoc ? path.dirname(taskOptsLoc) : null,
-        title: humanize(taskName, true),
-        disabled: taskName[0] === ".",
-        options: taskOpts,
-        validateLogs: taskOpts.validateLogs,
-        ignoreOutput: taskOpts.ignoreOutput,
-        stdout: { loc: stdoutLoc, code: readFile(stdoutLoc) },
-        stderr: { loc: stderrLoc, code: readFile(stderrLoc) },
-        exec: {
-          loc: execLoc,
-          code: readFile(execLoc),
-          filename: execLocAlias,
-        },
-        actual: {
-          loc: actualLoc,
-          code: readFile(actualLoc),
-          filename: actualLocAlias,
-        },
-        expect: {
-          loc: expectLoc,
-          code: readFile(expectLoc),
-          filename: expectLocAlias,
-        },
-      };
-
-      // If there's node requirement, check it before pushing task
-      if (taskOpts.minNodeVersion) {
-        const minimumVersion = semver.clean(taskOpts.minNodeVersion);
-
-        if (minimumVersion == null) {
-          throw new Error(
-            `'minNodeVersion' has invalid semver format: ${taskOpts.minNodeVersion}`,
-          );
-        }
-
-        if (semver.lt(nodeVersion, minimumVersion)) {
-          return;
-        }
-
-        // Delete to avoid option validation error
-        delete taskOpts.minNodeVersion;
-      }
-
-      if (taskOpts.os) {
-        let os = taskOpts.os;
-
-        if (!Array.isArray(os) && typeof os !== "string") {
-          throw new Error(
-            `'os' should be either string or string array: ${taskOpts.os}`,
-          );
-        }
-
-        if (typeof os === "string") {
-          os = [os];
-        }
-
-        if (!os.includes(process.platform)) {
-          return;
-        }
-
-        delete taskOpts.os;
-      }
-
-      // traceur checks
-
-      if (test.exec.code.indexOf("// Async.") >= 0) {
-        return;
-      }
-
-      suite.tests.push(test);
-
-      const sourceMappingsLoc = taskDir + "/source-mappings.json";
-      if (fs.existsSync(sourceMappingsLoc)) {
-        test.sourceMappings = JSON.parse(readFile(sourceMappingsLoc));
-      }
-
-      const sourceMapLoc = taskDir + "/source-map.json";
-      if (fs.existsSync(sourceMapLoc)) {
-        test.sourceMap = JSON.parse(readFile(sourceMapLoc));
-      }
-
-      const inputMapLoc = taskDir + "/input-source-map.json";
-      if (fs.existsSync(inputMapLoc)) {
-        test.inputSourceMap = JSON.parse(readFile(inputMapLoc));
-      }
-
-      if (taskOpts.throws) {
-        if (test.expect.code) {
-          throw new Error(
-            "Test cannot throw and also return output code: " + expectLoc,
-          );
-        }
-        if (test.sourceMappings) {
-          throw new Error(
-            "Test cannot throw and also return sourcemappings: " +
-              sourceMappingsLoc,
-          );
-        }
-        if (test.sourceMap) {
-          throw new Error(
-            "Test cannot throw and also return sourcemaps: " + sourceMapLoc,
-          );
-        }
-      }
-
-      if (!test.validateLogs && (test.stdout.code || test.stderr.code)) {
-        throw new Error(
-          "stdout.txt and stderr.txt are only allowed when the 'validateLogs' option is enabled: " +
-            (test.stdout.code ? stdoutLoc : stderrLoc),
-        );
-      }
-      if (test.options.ignoreOutput) {
-        if (test.expect.code) {
-          throw new Error(
-            "Test cannot ignore its output and also validate it: " + expectLoc,
-          );
-        }
-        if (!test.validateLogs) {
-          throw new Error(
-            "ignoreOutput can only be used when validateLogs is true: " +
-              taskOptsLoc,
-          );
-        }
-      }
-
-      // Delete to avoid option validation error
-      delete test.options.validateLogs;
-      delete test.options.ignoreOutput;
     }
   }
 

packages/babel-parser/src/plugins/typescript/index.js

@@ -142,7 +142,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
       modified: { [key: TsModifier]: ?true },
       allowedModifiers: T[],
     ): void {
-      while (true) {
+      for (;;) {
         const startPos = this.state.start;
         const modifier: ?T = this.tsParseModifier(allowedModifiers);
 
@@ -204,7 +204,7 @@ export default (superClass: Class<Parser>): Class<Parser> =>
     ): ?(T[]) {
       const result = [];
 
-      while (true) {
+      for (;;) {
         if (this.tsIsListTerminator(kind)) {
           break;
         }

packages/babel-traverse/src/path/comments.js

@@ -20,10 +20,9 @@ export function shareCommentsWithSiblings() {
   const next = this.getSibling(this.key + 1);
   const hasPrev = Boolean(prev.node);
   const hasNext = Boolean(next.node);
-  if (hasPrev && hasNext) {
+  if (hasPrev && !hasNext) {
-  } else if (hasPrev) {
     prev.addComments("trailing", trailing);
-  } else if (hasNext) {
+  } else if (hasNext && !hasPrev) {
     next.addComments("leading", leading);
   }
 }

packages/babel-traverse/src/scope/lib/renamer.js

@@ -62,6 +62,7 @@ export default class Renamer {
 
     // retain the `name` of a class/function declaration
 
+    // eslint-disable-next-line no-unreachable
     if (!path.isFunctionDeclaration() && !path.isClassDeclaration()) return;
     if (this.binding.kind !== "hoisted") return;
 
@@ -83,6 +84,7 @@ export default class Renamer {
 
     // retain the `name` of a class/function expression
 
+    // eslint-disable-next-line no-unreachable
     if (!path.isFunctionExpression() && !path.isClassExpression()) return;
     if (this.binding.kind !== "local") return;