Change our caching strategy for ESLint instances…

…which fixes bugs and allows us not to care about `process.chdir`.

Author: savetheclocktower

lib/worker.js

@@ -5,16 +5,17 @@
 
 require('util').inspect.defaultOptions.depth = null;
 
-const { join, normalize, sep } = require('path');
+const { join, dirname } = require('path');
 const { createRequire } = require('module');
 const compareVersions = require('compare-versions');
 const ndjson = require('ndjson');
 
-const PATHS_CACHE = {};
-const ESLINT_CACHE = {};
-
 const MINIMUM_ESLINT_VERSION = '7.0.0';
 
+const PATHS_CACHE = new Map();
+const ESLINT_CACHE = new Map();
+
+
 class IncompatibleVersionError extends Error {
   constructor (version) {
     // eslint-disable-next-line max-len
@@ -56,39 +57,28 @@ function log (message) {
   emit({ log: message });
 }
 
-function getPathRoot (filePath, eslintPath) {
-  log(`getPathRoot ${filePath} // ${eslintPath}`);
-  filePath = normalize(filePath).split(sep);
-  eslintPath = normalize(eslintPath).split(sep);
-
-  for (let index in filePath) {
-    if (eslintPath[index] !== filePath[index]) {
-      let ret = filePath.slice(0, index).join(sep);
-      return ret + sep;
-    }
-  }
-
-  throw new Error('linter-eslint-node: Cannot determine root');
-}
-
-function buildCommonConstructorOptions (config) {
+function buildCommonConstructorOptions (config, cwd) {
   let {
     advanced: { disableEslintIgnore },
     autofix: { rulesToDisableWhileFixing }
   } = config;
 
   return {
+    cwd,
     ignore: !disableEslintIgnore,
+    // `fix` can be a function, so we'll use it to ignore any rules that the
+    // user has told us to ignore. This isn't a "common" option, but it's easy
+    // to overwrite with `fix: false` for the lint-only instance.
     fix: ({ ruleId }) => !rulesToDisableWhileFixing.includes(ruleId)
   };
 }
 
 function clearESLintCache () {
-  for (let key in PATHS_CACHE) {
-    delete PATHS_CACHE[key];
+  for (let key of PATHS_CACHE) {
+    PATHS_CACHE.delete(key);
   }
-  for (let key in ESLINT_CACHE) {
-    delete ESLINT_CACHE[key];
+  for (let key of ESLINT_CACHE) {
+    ESLINT_CACHE.delete(key);
   }
 }
 
@@ -100,52 +90,72 @@ function resolveESLint (filePath) {
   }
 }
 
-function getESLint (filePath, projectPath, config, { legacyPackagePresent }) {
-  if (!PATHS_CACHE[filePath]) {
-    PATHS_CACHE[filePath] = resolveESLint(filePath);
+let builtInEslintPath;
+function resolveBuiltInESLint () {
+  if (!builtInEslintPath) {
+    builtInEslintPath = createRequire(__dirname).resolve('eslint');
   }
+  return builtInEslintPath;
+}
 
-  let eslintPath = PATHS_CACHE[filePath];
+function getESLint (filePath, config, { legacyPackagePresent }) {
+  let resolveDir = dirname(filePath);
+  if (!PATHS_CACHE.has(resolveDir)) {
+    PATHS_CACHE.set(resolveDir, resolveESLint(filePath));
+  }
 
-  if (!ESLINT_CACHE[eslintPath]) {
+  let eslintPath = PATHS_CACHE.get(resolveDir);
+
+  // We need to manage an arbitrary number of different ESLint versions without
+  // having them clash. On top of that, a declaration of `ESLint` accepts a
+  // `cwd` option on instantiation and infers its configuration based on the
+  // `.eslintrc`s and `.eslintignore`s that it sees from that particular
+  // directory.
+  //
+  // Thus an `ESLint` instance is good for the directory it was created in and
+  // nothing more. If two files are siblings in a project's file tree, linting
+  // the first one will create an instance that can be reused when we lint the
+  // second, but can't be reused with any file in the parent directory or in
+  // any child directories.
+  //
+  // TODO: Add an option to disable this method of caching. Could be useful as
+  // a diagnostic tool and as a workaround when users report bugs.
+  if (!ESLINT_CACHE.has(resolveDir)) {
     const eslintRootPath = eslintPath.replace(/eslint([/\\]).*?$/, 'eslint$1');
     const packageMeta = require(join(eslintRootPath, 'package.json'));
 
-    let { ESLint } = createRequire(eslintPath)('eslint');
-    let commonOptions = buildCommonConstructorOptions(config);
+    const { ESLint } = createRequire(eslintPath)('eslint');
+    let commonOptions = buildCommonConstructorOptions(config, resolveDir);
+
+    const eslintLint = new ESLint({ ...commonOptions, fix: false });
+    const eslintFix = new ESLint({ ...commonOptions });
 
-    // `fix` is a predicate in `commonOptions` and represents the "yes,
-    // except..." outcome. For the linter version, we overwrite it with `fix:
-    // false`.
-    ESLINT_CACHE[eslintPath] = {
+    ESLINT_CACHE.set(resolveDir, {
       ESLint,
-      eslintLint: new ESLint({ ...commonOptions, fix: false }),
-      eslintFix: new ESLint({ ...commonOptions }),
-      workingDir: getPathRoot(filePath, eslintPath),
+      eslintLint,
+      eslintFix,
       eslintPath: eslintRootPath,
       eslintVersion: packageMeta.version,
-      isBuiltIn: eslintPath === PATHS_CACHE[__dirname]
-    };
+      isBuiltIn: eslintPath === resolveBuiltInESLint()
+    });
   }
 
-  let cache = ESLINT_CACHE[eslintPath];
+  let cached = ESLINT_CACHE.get(resolveDir);
 
-  if (compareVersions(cache.eslintVersion, MINIMUM_ESLINT_VERSION) < 1) {
+  if (compareVersions(cached.eslintVersion, MINIMUM_ESLINT_VERSION) < 1) {
     // Unsupported version.
-    throw new IncompatibleVersionError(cache.eslintVersion);
-  } else if ((compareVersions(cache.eslintVersion, '8.0.0') < 1) && legacyPackagePresent) {
+    throw new IncompatibleVersionError(cached.eslintVersion);
+  } else if ((compareVersions(cached.eslintVersion, '8.0.0') < 1) && legacyPackagePresent) {
     // We're dealing with version 7 of ESLint. The legacy `linter-eslint`
     // package is present and capable of linting with this version, so we
     // should halt instead of trying to lint everything twice.
-    throw new VersionOverlapError(cache.eslintVersion);
+    throw new VersionOverlapError(cached.eslintVersion);
   }
 
-  return ESLINT_CACHE[eslintPath];
+  return cached;
 }
 
 async function lint (eslint, workingDir, filePath, fileContent) {
-  process.chdir(workingDir);
-
   if (typeof fileContent === 'string') {
     return eslint.lintText(fileContent, { filePath });
   } else {
@@ -235,7 +245,6 @@ async function processMessage (bundle) {
     isModified,
     key,
     legacyPackagePresent,
-    projectPath,
     type
   } = bundle;
 
@@ -258,18 +267,9 @@ async function processMessage (bundle) {
     return;
   }
 
-  // if (!projectPath) {
-  //   emit({
-  //     key,
-  //     error: `Must provide projectPath`,
-  //     type: 'no-project'
-  //   });
-  //   return;
-  // }
-
   let eslint;
   try {
-    eslint = getESLint(filePath, projectPath, config, { legacyPackagePresent });
+    eslint = getESLint(filePath, config, { legacyPackagePresent });
   } catch (err) {
     if (err instanceof IncompatibleVersionError) {
       emit({