fix: correctly iterate files matched by glob patterns (#16831)

snitin315 · web-flow · commit 92c1943ba73e · 2023-02-28T16:08:44.000+01:00
* 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
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -0,0 +1 @@
+console.log("two");
diff --git 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
@@ -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", () => {
 

Original file line number	Diff line number	Diff line change
`@@ -526,9 +526,9 @@ async function findFiles({`
`526`	`526`	`}`
`527`	`527`
`528`	`528`	`// save patterns for later use based on whether globs are enabled`
`529`		`- if (globInputPaths && isGlobPattern(filePath)) {`
	`529`	`+ if (globInputPaths && isGlobPattern(pattern)) {`
`530`	`530`
`531`		`- const basePath = globParent(filePath);`
	`531`	`+ const basePath = path.resolve(cwd, globParent(pattern));`
`532`	`532`
`533`	`533`	`// group in cwd if possible and split out others`
`534`	`534`	`if (isPathInside(basePath, cwd)) {`
Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+module.exports = { rules: { "no-console": "off" } };`