Skip to content

Commit

Permalink
Fix: Pass internal config paths in FileEnumerator default (fixes #13789
Browse files Browse the repository at this point in the history
…) (#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.
  • Loading branch information
btmills committed Oct 27, 2020
1 parent 631ae8b commit aeef485
Show file tree
Hide file tree
Showing 2 changed files with 32 additions and 1 deletion.
6 changes: 5 additions & 1 deletion lib/cli-engine/file-enumerator.js
Expand Up @@ -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,
Expand Down
27 changes: 27 additions & 0 deletions tests/lib/cli-engine/file-enumerator.js
Expand Up @@ -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(["."]));
});
});
});

0 comments on commit aeef485

Please sign in to comment.