From aeef485dc790571b1a82ac09904329e0226b66a9 Mon Sep 17 00:00:00 2001 From: Brandon Mills Date: Mon, 26 Oct 2020 23:07:33 -0400 Subject: [PATCH] Fix: Pass internal config paths in FileEnumerator default (fixes #13789) (#13792) * Chore: Repro FileEnumerator exception with default args (refs #13789) When `CLIEngine` instantiates `FileEnumerator`, it explicitly passes its own `CascadingConfigArrayFactory` instance, which now includes values for `builtInRules`, `loadRules`, `eslintRecommendedPath`, and `eslintAllPath`. This is the only place that ESLint core instantiates `FileEnumerator`, so core does not rely on the constructor's default value for `configArrayFactory`. After `CascadingConfigArrayFactory` was extracted into `@eslint/eslintrc`, it no longer assumed values for `eslintRecommendedPath` and `eslintAllPath`, which `CLIEngine` now provides. If those values are not passed, as is the case with the default `CascadingConfigArrayFactory` for `configArrayFactory`, file enumeration will encounter the exception that was reported in #13789. From the perspective of ESLint core, the default value for `configArrayFactory` is dead code. However, even though `FileEnumerator` is an undocumented API, it's called by `eslint-plugin-import`'s `no-unused-modules` rule, which hits the exception reproduced by this test. * Fix: Pass internal config paths in FileEnumerator default (fixes #13789) When `CLIEngine` instantiates `FileEnumerator`, it explicitly passes its own `CascadingConfigArrayFactory` instance, which now includes values for `builtInRules`, `loadRules`, `eslintRecommendedPath`, and `eslintAllPath`. This is the only place that ESLint core instantiates `FileEnumerator`, so core does not rely on the constructor's default value for `configArrayFactory`. After `CascadingConfigArrayFactory` was extracted into `@eslint/eslintrc`, it no longer assumed values for `eslintRecommendedPath` and `eslintAllPath`, which `CLIEngine` now provides. If those values are not passed, as is the case with the default `CascadingConfigArrayFactory` for `configArrayFactory`, file enumeration will encounter the exception that was reported in #13789. From the perspective of ESLint core, the default value for `configArrayFactory` is dead code. However, even though `FileEnumerator` is an undocumented API, it's called by `eslint-plugin-import`'s `no-unused-modules` rule, which hits the exception reproduced by this test. --- lib/cli-engine/file-enumerator.js | 6 +++++- tests/lib/cli-engine/file-enumerator.js | 27 +++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) diff --git a/lib/cli-engine/file-enumerator.js b/lib/cli-engine/file-enumerator.js index 446342a1a93..c2961d71ac0 100644 --- a/lib/cli-engine/file-enumerator.js +++ b/lib/cli-engine/file-enumerator.js @@ -213,7 +213,11 @@ class FileEnumerator { */ constructor({ cwd = process.cwd(), - configArrayFactory = new CascadingConfigArrayFactory({ cwd }), + configArrayFactory = new CascadingConfigArrayFactory({ + cwd, + eslintRecommendedPath: path.resolve(__dirname, "../../conf/eslint-recommended.js"), + eslintAllPath: path.resolve(__dirname, "../../conf/eslint-all.js") + }), extensions = null, globInputPaths = true, errorOnUnmatchedPattern = true, diff --git a/tests/lib/cli-engine/file-enumerator.js b/tests/lib/cli-engine/file-enumerator.js index 1561b42e105..3ea51a408c8 100644 --- a/tests/lib/cli-engine/file-enumerator.js +++ b/tests/lib/cli-engine/file-enumerator.js @@ -488,4 +488,31 @@ describe("FileEnumerator", () => { }); }); }); + + // https://github.com/eslint/eslint/issues/13789 + describe("constructor default values when config extends eslint:recommended", () => { + const root = path.join(os.tmpdir(), "eslint/file-enumerator"); + const files = { + "file.js": "", + ".eslintrc.json": JSON.stringify({ + extends: ["eslint:recommended"] + }) + }; + const { prepare, cleanup, getPath } = createCustomTeardown({ cwd: root, files }); + + + /** @type {FileEnumerator} */ + let enumerator; + + beforeEach(async () => { + await prepare(); + enumerator = new FileEnumerator({ cwd: getPath() }); + }); + + afterEach(cleanup); + + it("should not throw an exception iterating files", () => { + Array.from(enumerator.iterateFiles(["."])); + }); + }); });