From 92c1943ba73ea01e87086236e8736539b0eed558 Mon Sep 17 00:00:00 2001 From: Nitin Kumar Date: Tue, 28 Feb 2023 20:38:44 +0530 Subject: [PATCH] fix: correctly iterate files matched by glob patterns (#16831) * fix: correctly iterate files matched by glob patterns * test: add a test case for #14742 * test: improve test case * chore: remove .only * chore: revert unwanted changes * test: fix root path * chore: fix lint * fix: correctly iterate files matched by glob patterns for new config system * test: add case for flat config * chore: remove .only --- lib/cli-engine/file-enumerator.js | 10 +-- lib/eslint/eslint-helpers.js | 4 +- .../{curly-path}/client/eslint.config.js | 1 + tests/fixtures/{curly-path}/client/src/one.js | 1 + .../{curly-path}/server/eslint.config.js | 1 + tests/fixtures/{curly-path}/server/src/two.js | 1 + tests/lib/cli-engine/file-enumerator.js | 68 +++++++++++++++++++ tests/lib/eslint/flat-eslint.js | 17 +++++ 8 files changed, 97 insertions(+), 6 deletions(-) create mode 100644 tests/fixtures/{curly-path}/client/eslint.config.js create mode 100644 tests/fixtures/{curly-path}/client/src/one.js create mode 100644 tests/fixtures/{curly-path}/server/eslint.config.js create mode 100644 tests/fixtures/{curly-path}/server/src/two.js diff --git a/lib/cli-engine/file-enumerator.js b/lib/cli-engine/file-enumerator.js index 9625e4aa701..5dff8d09ccd 100644 --- a/lib/cli-engine/file-enumerator.js +++ b/lib/cli-engine/file-enumerator.js @@ -349,7 +349,7 @@ class FileEnumerator { return this._iterateFilesWithFile(absolutePath); } if (globInputPaths && isGlobPattern(pattern)) { - return this._iterateFilesWithGlob(absolutePath, isDot); + return this._iterateFilesWithGlob(pattern, isDot); } return []; @@ -398,15 +398,17 @@ class FileEnumerator { _iterateFilesWithGlob(pattern, dotfiles) { debug(`Glob: ${pattern}`); - const directoryPath = path.resolve(getGlobParent(pattern)); - const globPart = pattern.slice(directoryPath.length + 1); + const { cwd } = internalSlotsMap.get(this); + const directoryPath = path.resolve(cwd, getGlobParent(pattern)); + const absolutePath = path.resolve(cwd, pattern); + const globPart = absolutePath.slice(directoryPath.length + 1); /* * recursive if there are `**` or path separators in the glob part. * Otherwise, patterns such as `src/*.js`, it doesn't need recursive. */ const recursive = /\*\*|\/|\\/u.test(globPart); - const selector = new Minimatch(pattern, minimatchOpts); + const selector = new Minimatch(absolutePath, minimatchOpts); debug(`recursive? ${recursive}`); diff --git a/lib/eslint/eslint-helpers.js b/lib/eslint/eslint-helpers.js index 72cf9114e73..54b0c69d7c8 100644 --- a/lib/eslint/eslint-helpers.js +++ b/lib/eslint/eslint-helpers.js @@ -526,9 +526,9 @@ async function findFiles({ } // save patterns for later use based on whether globs are enabled - if (globInputPaths && isGlobPattern(filePath)) { + if (globInputPaths && isGlobPattern(pattern)) { - const basePath = globParent(filePath); + const basePath = path.resolve(cwd, globParent(pattern)); // group in cwd if possible and split out others if (isPathInside(basePath, cwd)) { diff --git a/tests/fixtures/{curly-path}/client/eslint.config.js b/tests/fixtures/{curly-path}/client/eslint.config.js new file mode 100644 index 00000000000..cab59b0a888 --- /dev/null +++ b/tests/fixtures/{curly-path}/client/eslint.config.js @@ -0,0 +1 @@ +module.exports = { rules: { "no-console": "off" } }; diff --git a/tests/fixtures/{curly-path}/client/src/one.js b/tests/fixtures/{curly-path}/client/src/one.js new file mode 100644 index 00000000000..0dab8b6bcb0 --- /dev/null +++ b/tests/fixtures/{curly-path}/client/src/one.js @@ -0,0 +1 @@ +console.log("one"); diff --git a/tests/fixtures/{curly-path}/server/eslint.config.js b/tests/fixtures/{curly-path}/server/eslint.config.js new file mode 100644 index 00000000000..7e8b0562ad8 --- /dev/null +++ b/tests/fixtures/{curly-path}/server/eslint.config.js @@ -0,0 +1 @@ +module.exports = { rules: { "no-console": "warn" } }; diff --git a/tests/fixtures/{curly-path}/server/src/two.js b/tests/fixtures/{curly-path}/server/src/two.js new file mode 100644 index 00000000000..ef0e38f5b55 --- /dev/null +++ b/tests/fixtures/{curly-path}/server/src/two.js @@ -0,0 +1 @@ +console.log("two"); diff --git a/tests/lib/cli-engine/file-enumerator.js b/tests/lib/cli-engine/file-enumerator.js index 2ccf15e73d1..3a96cf5eb84 100644 --- a/tests/lib/cli-engine/file-enumerator.js +++ b/tests/lib/cli-engine/file-enumerator.js @@ -13,6 +13,7 @@ const path = require("path"); const os = require("os"); const { assert } = require("chai"); const sh = require("shelljs"); +const sinon = require("sinon"); const { Legacy: { CascadingConfigArrayFactory @@ -182,6 +183,73 @@ describe("FileEnumerator", () => { }); }); + // https://github.com/eslint/eslint/issues/14742 + describe("with 5 directories ('{lib}', '{lib}/client', '{lib}/client/src', '{lib}/server', '{lib}/server/src') that contains two files '{lib}/client/src/one.js' and '{lib}/server/src/two.js'", () => { + const root = path.join(os.tmpdir(), "eslint/file-enumerator"); + const files = { + "{lib}/client/src/one.js": "console.log('one.js');", + "{lib}/server/src/two.js": "console.log('two.js');", + "{lib}/client/.eslintrc.json": JSON.stringify({ + rules: { + "no-console": "error" + }, + env: { + mocha: true + } + }), + "{lib}/server/.eslintrc.json": JSON.stringify({ + rules: { + "no-console": "off" + }, + env: { + mocha: true + } + }) + }; + const { prepare, cleanup, getPath } = createCustomTeardown({ + cwd: root, + files + }); + + /** @type {FileEnumerator} */ + let enumerator; + + beforeEach(async () => { + await prepare(); + enumerator = new FileEnumerator({ + cwd: path.resolve(getPath("{lib}/server")) + }); + }); + + afterEach(cleanup); + + describe("when running eslint in the server directory", () => { + it("should use the config '{lib}/server/.eslintrc.json' for '{lib}/server/src/two.js'.", () => { + const spy = sinon.spy(fs, "readdirSync"); + + const list = [ + ...enumerator.iterateFiles(["src/**/*.{js,json}"]) + ]; + + // should enter the directory '{lib}/server/src' directly + assert.strictEqual(spy.getCall(0).firstArg, path.join(root, "{lib}/server/src")); + assert.strictEqual(list.length, 1); + assert.strictEqual(list[0].config.length, 2); + assert.strictEqual(list[0].config[0].name, "DefaultIgnorePattern"); + assert.strictEqual(list[0].config[1].filePath, getPath("{lib}/server/.eslintrc.json")); + assert.deepStrictEqual( + list.map(entry => entry.filePath), + [ + path.join(root, "{lib}/server/src/two.js") + ] + ); + + // destroy the spy + sinon.restore(); + }); + }); + }); + // This group moved from 'tests/lib/util/glob-utils.js' when refactoring to keep the cumulated test cases. describe("with 'tests/fixtures/glob-utils' files", () => { let fixtureDir; diff --git a/tests/lib/eslint/flat-eslint.js b/tests/lib/eslint/flat-eslint.js index acb61e9e3e0..187b09d7baa 100644 --- a/tests/lib/eslint/flat-eslint.js +++ b/tests/lib/eslint/flat-eslint.js @@ -850,6 +850,23 @@ describe("FlatESLint", () => { assert.strictEqual(results[0].suppressedMessages.length, 0); }); + // https://github.com/eslint/eslint/issues/14742 + it("should run", async () => { + eslint = new FlatESLint({ + cwd: getFixturePath("{curly-path}", "server") + }); + const results = await eslint.lintFiles(["src/**/*.{js,json}"]); + + assert.strictEqual(results.length, 1); + assert.strictEqual(results[0].messages.length, 1); + assert.strictEqual(results[0].messages[0].ruleId, "no-console"); + assert.strictEqual( + results[0].filePath, + getFixturePath("{curly-path}/server/src/two.js") + ); + assert.strictEqual(results[0].suppressedMessages.length, 0); + }); + // https://github.com/eslint/eslint/issues/16265 describe("Dot files in searches", () => {