From 0c8187bcc715b50b5f6974345ef382cebeae6791 Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Sun, 26 May 2024 11:12:59 +0200 Subject: [PATCH 1/6] new method `getConfigStatus`, redefine `isFileIgnored` The methods`isFileIgnored`/`isIgnored` will only return `true` if a file is globally ignored. --- packages/config-array/src/config-array.js | 89 +- .../config-array/tests/config-array.test.js | 1016 +++++++++++++---- 2 files changed, 890 insertions(+), 215 deletions(-) diff --git a/packages/config-array/src/config-array.js b/packages/config-array/src/config-array.js index 3aadb0f..1c75b32 100644 --- a/packages/config-array/src/config-array.js +++ b/packages/config-array/src/config-array.js @@ -490,6 +490,67 @@ function assertExtraConfigTypes(extraConfigTypes) { } } +/** + * Calculates whether a file has a config or why it doesn't. + * @param {ConfigArray} configArray A normalized config array. + * @param {string} filePath The path of the file to check. + * @returns {"ignored"|"external"|"unconfigured"|"matched"} One of the constants returned by + * {@linkcode ConfigArray.getConfigStatus}. + */ +function calculateConfigStatus(configArray, filePath) { + // check if the file is outside the base path + const relativeFilePath = path.relative(configArray.basePath, filePath); + if (relativeFilePath.startsWith("..")) return "external"; + + // next check to see if the file should be ignored + if ( + configArray.isDirectoryIgnored(path.dirname(filePath)) || + shouldIgnorePath(configArray.ignores, filePath, relativeFilePath) + ) { + return "ignored"; + } + + // filePath isn't automatically ignored, so try to find a matching config + const universalPattern = /\/\*{1,2}$/; + + for (const config of configArray) { + if (!config.files) continue; + + /* + * If a config has a files pattern ending in /** or /*, and the + * filePath only matches those patterns, then the config is only + * applied if there is another config where the filePath matches + * a file with a specific extensions such as *.js. + */ + + const universalFiles = config.files.filter(pattern => + universalPattern.test(pattern), + ); + + let filteredConfig; + + // universal patterns were found so we need to filter the files + if (universalFiles.length) { + const nonUniversalFiles = config.files.filter( + pattern => !universalPattern.test(pattern), + ); + + filteredConfig = { + files: nonUniversalFiles, + ignores: config.ignores, + }; + } else { + filteredConfig = config; + } + + if (pathMatches(filePath, configArray.basePath, filteredConfig)) { + return "matched"; + } + } + + return "unconfigured"; +} + //------------------------------------------------------------------------------ // Public Interface //------------------------------------------------------------------------------ @@ -582,6 +643,7 @@ export class ConfigArray extends Array { directoryMatches: new Map(), files: undefined, ignores: undefined, + configStatus: new Map(), }); // load the configs into this array @@ -985,6 +1047,31 @@ export class ConfigArray extends Array { return finalConfig; } + /** + * Determines whether a file has a config or why it doesn't. + * @param {string} filePath The path of the file to check. + * @returns {"ignored"|"external"|"unconfigured"|"matched"} One of the following values: + * * `"ignored"`: the file is ignored + * * `"external"`: the file is outside the base path + * * `"unconfigured"`: the file is not matched by any config + * * `"matched"`: the file has a matching config + */ + getConfigStatus(filePath) { + assertNormalized(this); + + // first check the cache for a filename match to avoid duplicate work + const cache = dataCache.get(this).configStatus; + let configStatus = cache.get(filePath); + + if (!configStatus) { + // no cached value found, so calculate + configStatus = calculateConfigStatus(this, filePath); + cache.set(configStatus); + } + + return configStatus; + } + /** * Determines if the given filepath is ignored based on the configs. * @param {string} filePath The complete path of a file to check. @@ -1001,7 +1088,7 @@ export class ConfigArray extends Array { * @returns {boolean} True if the path is ignored, false if not. */ isFileIgnored(filePath) { - return this.getConfig(filePath) === undefined; + return this.getConfigStatus(filePath) === "ignored"; } /** diff --git a/packages/config-array/tests/config-array.test.js b/packages/config-array/tests/config-array.test.js index f796f56..d1dc608 100644 --- a/packages/config-array/tests/config-array.test.js +++ b/packages/config-array/tests/config-array.test.js @@ -961,6 +961,793 @@ describe("ConfigArray", () => { }); }); + describe("getConfigStatus()", () => { + it("should throw an error when not normalized", () => { + const filename = path.resolve(basePath, "foo.js"); + assert.throws(() => { + unnormalizedConfigs.getConfigStatus(filename); + }, /normalized/); + }); + + it('should return "matched" when passed JS filename', () => { + const filename = path.resolve(basePath, "foo.js"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "matched", + ); + }); + + it('should return "external" when passed JS filename in parent directory', () => { + const filename = path.resolve(basePath, "../foo.js"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "external", + ); + }); + + it('should return "matched" when passed HTML filename', () => { + const filename = path.resolve(basePath, "foo.html"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "matched", + ); + }); + + it('should return "ignored" when passed ignored .gitignore filename', () => { + const filename = path.resolve(basePath, ".gitignore"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "ignored", + ); + }); + + it('should return "matched" when passed CSS filename', () => { + const filename = path.resolve(basePath, "foo.css"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "matched", + ); + }); + + it('should return "matched" when passed docx filename', () => { + const filename = path.resolve(basePath, "sss.docx"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "matched", + ); + }); + + it('should return "ignored" when passed ignored node_modules filename', () => { + const filename = path.resolve(basePath, "node_modules/foo.js"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "ignored", + ); + }); + + it('should return "unconfigured" when passed matching both files and ignores in a config', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.xsl"], + ignores: ["fixtures/test.xsl"], + defs: { + xsl: true, + }, + }, + ], + { basePath }, + ); + + configs.normalizeSync(); + const filename = path.resolve(basePath, "fixtures/test.xsl"); + + assert.strictEqual( + configs.getConfigStatus(filename), + "unconfigured", + ); + }); + + it('should return "matched" when negated pattern comes after matching pattern', () => { + configs = new ConfigArray( + [ + { + files: ["**/foo.*"], + ignores: ["**/*.txt", "!foo.txt"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "bar.txt")), + "unconfigured", + ); + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo.txt")), + "matched", + ); + }); + + it('should return "ignored" when negated pattern comes before matching pattern', () => { + configs = new ConfigArray( + [ + { + ignores: ["!foo.txt", "**/*.txt"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "bar.txt")), + "ignored", + ); + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo.txt")), + "ignored", + ); + }); + + it('should return "matched" when matching files and ignores has a negated pattern after matching pattern', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + ignores: ["**/*.test.js", "!foo.test.js"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "bar.test.js")), + "unconfigured", + ); + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo.test.js")), + "matched", + ); + }); + + it('should return "ignored" when file is inside of ignored directory', () => { + configs = new ConfigArray( + [ + { + ignores: ["ignoreme"], + }, + { + files: ["**/*.js"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "ignoreme/foo.js"), + ), + "ignored", + ); + }); + + it('should return "matched" when file is inside of unignored directory', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["foo/*", "!foo/bar"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "foo/bar/a.js"), + ), + "matched", + ); + }); + + it('should return "ignored" when file is ignored, unignored, and then reignored', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["a.js", "!a*.js", "a.js"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "a.js")), + "ignored", + ); + }); + + it('should return "ignored" when the parent directory of a file is ignored', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["foo"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "foo/bar/a.js"), + ), + "ignored", + ); + }); + + it('should "ignored" true when an ignored directory is later negated with **', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["**/node_modules/**"], + }, + { + ignores: ["!node_modules/package/**"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "node_modules/package/a.js"), + ), + "ignored", + ); + }); + + it('should return "ignored" when an ignored directory is later negated with *', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["**/node_modules/**"], + }, + { + ignores: ["!node_modules/package/*"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "node_modules/package/a.js"), + ), + "ignored", + ); + }); + + it('should return "unconfigured" when there are only patterns ending with /*', () => { + configs = new ConfigArray( + [ + { + files: ["foo/*"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "unconfigured", + ); + }); + + it('should return "unconfigured" when there are only patterns ending with /**', () => { + configs = new ConfigArray( + [ + { + files: ["foo/**"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "unconfigured", + ); + }); + + it('should return "matched" when files pattern matches and there is a pattern ending with /**', () => { + configs = new ConfigArray( + [ + { + files: ["foo/*.js", "foo/**"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "matched", + ); + }); + + it('should return "matched" when file has the same name as a directory that is ignored by a pattern that ends with `/`', () => { + configs = new ConfigArray( + [ + { + files: ["**/foo"], + }, + { + ignores: ["foo/"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo")), + "matched", + ); + }); + + it('should return "matched" when file is in the parent directory of directories that are ignored by a pattern that ends with `/`', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["foo/*/"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "matched", + ); + }); + + it('should return "ignored" when file is in a directory that is ignored by a pattern that ends with `/`', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["foo/"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "ignored", + ); + }); + + it('should return "ignored" when file is in a directory that is ignored by a pattern that does not end with `/`', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["foo"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "ignored", + ); + }); + + it('should return "matched" when file is in a directory that is ignored and then unignored by pattern that ends with `/`', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["foo/", "!foo/"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "matched", + ); + }); + + it('should return "ignored" when file is in a directory that is ignored along with its files by a pattern that ends with `/**` and then unignored by pattern that ends with `/`', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: [ + "foo/**", + + // only the directory is unignored, files are not + "!foo/", + ], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "ignored", + ); + }); + + it('should return "ignored" when file is in a directory that is ignored along with its files by a pattern that ends with `/**` and then unignored by pattern that does not end with `/`', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: [ + "foo/**", + + // only the directory is unignored, files are not + "!foo", + ], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "ignored", + ); + }); + + it('should return "matched" when file is in a directory that is ignored along its files by pattern that ends with `/**` and then unignored along its files by pattern that ends with `/**`', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: [ + "foo/**", + + // both the directory and the files are unignored + "!foo/**", + ], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "matched", + ); + }); + + it('should return "ignored" when file is ignored by a pattern and there are unignore patterns that target files of a directory with the same name', () => { + configs = new ConfigArray( + [ + { + files: ["**/foo"], + }, + { + ignores: ["foo", "!foo/*", "!foo/**"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo")), + "ignored", + ); + }); + + it('should return "ignored" when file is in a directory that is ignored even if an unignore pattern that ends with `/*` matches the file', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: ["foo", "!foo/*"], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus(path.join(basePath, "foo/a.js")), + "ignored", + ); + }); + + // https://github.com/eslint/eslint/issues/17964#issuecomment-1879840650 + it('should return "ignored" for all files ignored in a directory tree except for explicitly unignored ones', () => { + configs = new ConfigArray( + [ + { + files: ["**/*.js"], + }, + { + ignores: [ + // ignore all files and directories + "tests/format/**/*", + + // unignore all directories + "!tests/format/**/*/", + + // unignore specific files + "!tests/format/**/jsfmt.spec.js", + ], + }, + ], + { + basePath, + }, + ); + + configs.normalizeSync(); + + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "tests/format/foo.js"), + ), + "ignored", + ); + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "tests/format/jsfmt.spec.js"), + ), + "matched", + ); + assert.strictEqual( + configs.getConfigStatus( + path.join(basePath, "tests/format/subdir/foo.js"), + ), + "ignored", + ); + assert.strictEqual( + configs.getConfigStatus( + path.join( + basePath, + "tests/format/subdir/jsfmt.spec.js", + ), + ), + "matched", + ); + }); + + // https://github.com/eslint/eslint/pull/16579/files + describe("gitignore-style unignores", () => { + it('should return "unconfigured" when a subdirectory is ignored and then we try to unignore a directory', () => { + configs = new ConfigArray( + [ + { + ignores: [ + "/**/node_modules/*", + "!/node_modules/foo", + ], + }, + ], + { basePath }, + ); + + configs.normalizeSync(); + const filename = path.resolve( + basePath, + "node_modules/foo/bar.js", + ); + + assert.strictEqual( + configs.getConfigStatus(filename), + "unconfigured", + ); + }); + + it('should return "unconfigured" when a subdirectory is ignored and then we try to unignore a file', () => { + configs = new ConfigArray( + [ + { + ignores: [ + "/**/node_modules/*", + "!/node_modules/foo/**", + ], + }, + ], + { basePath }, + ); + + configs.normalizeSync(); + const filename = path.resolve( + basePath, + "node_modules/foo/bar.js", + ); + + assert.strictEqual( + configs.getConfigStatus(filename), + "unconfigured", + ); + }); + + it("should return true when all descendant directories are ignored and then we try to unignore a file", () => { + configs = new ConfigArray( + [ + { + ignores: [ + "/**/node_modules/**", + "!/node_modules/foo/**", + ], + }, + ], + { basePath }, + ); + + configs.normalizeSync(); + const filename = path.resolve( + basePath, + "node_modules/foo/bar.js", + ); + + assert.strictEqual( + configs.getConfigStatus(filename), + "unconfigured", + ); + }); + + it('should return "ignored" when all descendant directories are ignored without leading slash and then we try to unignore a file', () => { + configs = new ConfigArray( + [ + { + ignores: [ + "**/node_modules/**", + "!/node_modules/foo/**", + ], + }, + ], + { basePath }, + ); + + configs.normalizeSync(); + const filename = path.resolve( + basePath, + "node_modules/foo/bar.js", + ); + + assert.strictEqual( + configs.getConfigStatus(filename), + "ignored", + ); + }); + }); + }); + describe("isIgnored()", () => { it("should throw an error when not normalized", () => { const filename = path.resolve(basePath, "foo.js"); @@ -973,9 +1760,9 @@ describe("ConfigArray", () => { assert.strictEqual(configs.isIgnored(filename), false); }); - it("should return true when passed JS filename in parent directory", () => { + it("should return false when passed JS filename in parent directory", () => { const filename = path.resolve(basePath, "../foo.js"); - assert.strictEqual(configs.isIgnored(filename), true); + assert.strictEqual(configs.isIgnored(filename), false); }); it("should return false when passed HTML filename", () => { @@ -994,7 +1781,7 @@ describe("ConfigArray", () => { assert.strictEqual(configs.isIgnored(filename), false); }); - it("should return true when passed docx filename", () => { + it("should return false when passed docx filename", () => { const filename = path.resolve(basePath, "sss.docx"); assert.strictEqual(configs.isIgnored(filename), false); @@ -1005,50 +1792,6 @@ describe("ConfigArray", () => { assert.strictEqual(configs.isIgnored(filename), true); }); - it("should return true when passed matching both files and ignores in a config", () => { - configs = new ConfigArray( - [ - { - files: ["**/*.xsl"], - ignores: ["fixtures/test.xsl"], - defs: { - xsl: true, - }, - }, - ], - { basePath }, - ); - - configs.normalizeSync(); - const filename = path.resolve(basePath, "fixtures/test.xsl"); - - assert.strictEqual(configs.isIgnored(filename), true); - }); - - it("should return false when negated pattern comes after matching pattern", () => { - configs = new ConfigArray( - [ - { - files: ["**/foo.*"], - ignores: ["**/*.txt", "!foo.txt"], - }, - ], - { - basePath, - }, - ); - - configs.normalizeSync(); - - assert.strictEqual( - configs.isIgnored(path.join(basePath, "bar.txt")), - true, - ); - assert.strictEqual( - configs.isIgnored(path.join(basePath, "foo.txt")), - false, - ); - }); it("should return true when negated pattern comes before matching pattern", () => { configs = new ConfigArray( @@ -1073,31 +1816,6 @@ describe("ConfigArray", () => { true, ); }); - - it("should return false when matching files and ignores has a negated pattern comes after matching pattern", () => { - configs = new ConfigArray( - [ - { - files: ["**/*.js"], - ignores: ["**/*.test.js", "!foo.test.js"], - }, - ], - { - basePath, - }, - ); - - configs.normalizeSync(); - - assert.strictEqual( - configs.isIgnored(path.join(basePath, "bar.test.js")), - true, - ); - assert.strictEqual( - configs.isIgnored(path.join(basePath, "foo.test.js")), - false, - ); - }); }); describe("isFileIgnored()", () => { @@ -1114,10 +1832,10 @@ describe("ConfigArray", () => { assert.strictEqual(configs.isFileIgnored(filename), false); }); - it("should return true when passed JS filename in parent directory", () => { + it("should return false when passed JS filename in parent directory", () => { const filename = path.resolve(basePath, "../foo.js"); - assert.strictEqual(configs.isFileIgnored(filename), true); + assert.strictEqual(configs.isFileIgnored(filename), false); }); it("should return false when passed HTML filename", () => { @@ -1138,7 +1856,7 @@ describe("ConfigArray", () => { assert.strictEqual(configs.isFileIgnored(filename), false); }); - it("should return true when passed docx filename", () => { + it("should return false when passed docx filename", () => { const filename = path.resolve(basePath, "sss.docx"); assert.strictEqual(configs.isFileIgnored(filename), false); @@ -1150,51 +1868,6 @@ describe("ConfigArray", () => { assert.strictEqual(configs.isFileIgnored(filename), true); }); - it("should return true when passed matching both files and ignores in a config", () => { - configs = new ConfigArray( - [ - { - files: ["**/*.xsl"], - ignores: ["fixtures/test.xsl"], - defs: { - xsl: true, - }, - }, - ], - { basePath }, - ); - - configs.normalizeSync(); - const filename = path.resolve(basePath, "fixtures/test.xsl"); - - assert.strictEqual(configs.isFileIgnored(filename), true); - }); - - it("should return false when negated pattern comes after matching pattern", () => { - configs = new ConfigArray( - [ - { - files: ["**/foo.*"], - ignores: ["**/*.txt", "!foo.txt"], - }, - ], - { - basePath, - }, - ); - - configs.normalizeSync(); - - assert.strictEqual( - configs.isFileIgnored(path.join(basePath, "bar.txt")), - true, - ); - assert.strictEqual( - configs.isFileIgnored(path.join(basePath, "foo.txt")), - false, - ); - }); - it("should return true when negated pattern comes before matching pattern", () => { configs = new ConfigArray( [ @@ -1219,32 +1892,7 @@ describe("ConfigArray", () => { ); }); - it("should return false when matching files and ignores has a negated pattern comes after matching pattern", () => { - configs = new ConfigArray( - [ - { - files: ["**/*.js"], - ignores: ["**/*.test.js", "!foo.test.js"], - }, - ], - { - basePath, - }, - ); - - configs.normalizeSync(); - - assert.strictEqual( - configs.isFileIgnored(path.join(basePath, "bar.test.js")), - true, - ); - assert.strictEqual( - configs.isFileIgnored(path.join(basePath, "foo.test.js")), - false, - ); - }); - - it("should return false when file is inside of ignored directory", () => { + it("should return true when file is inside of ignored directory", () => { configs = new ConfigArray( [ { @@ -1269,7 +1917,7 @@ describe("ConfigArray", () => { ); }); - it("should return false when file is inside of ignored directory", () => { + it("should return false when file is inside of unignored directory", () => { configs = new ConfigArray( [ { @@ -1394,66 +2042,6 @@ describe("ConfigArray", () => { ); }); - it("should return true when there are only patterns ending with /*", () => { - configs = new ConfigArray( - [ - { - files: ["foo/*"], - }, - ], - { - basePath, - }, - ); - - configs.normalizeSync(); - - assert.strictEqual( - configs.isFileIgnored(path.join(basePath, "foo/a.js")), - true, - ); - }); - - it("should return true when there are only patterns ending with /**", () => { - configs = new ConfigArray( - [ - { - files: ["foo/**"], - }, - ], - { - basePath, - }, - ); - - configs.normalizeSync(); - - assert.strictEqual( - configs.isFileIgnored(path.join(basePath, "foo/a.js")), - true, - ); - }); - - it("should return false when files pattern matches and there is a pattern ending with /**", () => { - configs = new ConfigArray( - [ - { - files: ["foo/*.js", "foo/**"], - }, - ], - { - basePath, - }, - ); - - configs.normalizeSync(); - - assert.strictEqual( - configs.isFileIgnored(path.join(basePath, "foo/a.js")), - false, - ); - }); - it("should return false when file has the same name as a directory that is ignored by a pattern that ends with `/`", () => { configs = new ConfigArray( [ @@ -1546,7 +2134,7 @@ describe("ConfigArray", () => { ); }); - it("should return false when file is in a directory that is ignored and then unignored by pattern that end with `/`", () => { + it("should return false when file is in a directory that is ignored and then unignored by pattern that ends with `/`", () => { configs = new ConfigArray( [ { @@ -1757,7 +2345,7 @@ describe("ConfigArray", () => { // https://github.com/eslint/eslint/pull/16579/files describe("gitignore-style unignores", () => { - it("should return true when a subdirectory is ignored and then we try to unignore a directory", () => { + it("should return false when a subdirectory is ignored and then we try to unignore a directory", () => { configs = new ConfigArray( [ { @@ -1776,10 +2364,10 @@ describe("ConfigArray", () => { "node_modules/foo/bar.js", ); - assert.strictEqual(configs.isFileIgnored(filename), true); + assert.strictEqual(configs.isFileIgnored(filename), false); }); - it("should return true when a subdirectory is ignored and then we try to unignore a file", () => { + it("should return false when a subdirectory is ignored and then we try to unignore a file", () => { configs = new ConfigArray( [ { @@ -1798,10 +2386,10 @@ describe("ConfigArray", () => { "node_modules/foo/bar.js", ); - assert.strictEqual(configs.isFileIgnored(filename), true); + assert.strictEqual(configs.isFileIgnored(filename), false); }); - it("should return true when all descendant directories are ignored and then we try to unignore a file", () => { + it("should return false when all descendant directories are ignored and then we try to unignore a file", () => { configs = new ConfigArray( [ { @@ -1820,7 +2408,7 @@ describe("ConfigArray", () => { "node_modules/foo/bar.js", ); - assert.strictEqual(configs.isFileIgnored(filename), true); + assert.strictEqual(configs.isFileIgnored(filename), false); }); it("should return true when all descendant directories are ignored without leading slash and then we try to unignore a file", () => { From 0525daa3fce77db289ba6af2745a3d9dc7891adb Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Mon, 27 May 2024 21:36:46 +0200 Subject: [PATCH 2/6] fix tests --- .../config-array/tests/config-array.test.js | 55 +++++++++---------- 1 file changed, 26 insertions(+), 29 deletions(-) diff --git a/packages/config-array/tests/config-array.test.js b/packages/config-array/tests/config-array.test.js index d1dc608..322dbde 100644 --- a/packages/config-array/tests/config-array.test.js +++ b/packages/config-array/tests/config-array.test.js @@ -1646,13 +1646,13 @@ describe("ConfigArray", () => { // https://github.com/eslint/eslint/pull/16579/files describe("gitignore-style unignores", () => { - it('should return "unconfigured" when a subdirectory is ignored and then we try to unignore a directory', () => { + it('should return "ignored" when a subdirectory is ignored and then we try to unignore a directory', () => { configs = new ConfigArray( [ { ignores: [ - "/**/node_modules/*", - "!/node_modules/foo", + "**/node_modules/*", + "!node_modules/", ], }, ], @@ -1667,17 +1667,17 @@ describe("ConfigArray", () => { assert.strictEqual( configs.getConfigStatus(filename), - "unconfigured", + "ignored", ); }); - it('should return "unconfigured" when a subdirectory is ignored and then we try to unignore a file', () => { + it('should return "ignored" when a subdirectory is ignored and then we try to unignore a file', () => { configs = new ConfigArray( [ { ignores: [ - "/**/node_modules/*", - "!/node_modules/foo/**", + "**/node_modules/*", + "!node_modules/foo/*.js", ], }, ], @@ -1692,17 +1692,17 @@ describe("ConfigArray", () => { assert.strictEqual( configs.getConfigStatus(filename), - "unconfigured", + "ignored", ); }); - it("should return true when all descendant directories are ignored and then we try to unignore a file", () => { + it('should return "ignored" when all descendant directories are ignored and then we try to unignore a file', () => { configs = new ConfigArray( [ { ignores: [ - "/**/node_modules/**", - "!/node_modules/foo/**", + "**/node_modules/**", + "!node_modules/foo/*.js", ], }, ], @@ -1717,7 +1717,7 @@ describe("ConfigArray", () => { assert.strictEqual( configs.getConfigStatus(filename), - "unconfigured", + "ignored", ); }); @@ -2345,13 +2345,13 @@ describe("ConfigArray", () => { // https://github.com/eslint/eslint/pull/16579/files describe("gitignore-style unignores", () => { - it("should return false when a subdirectory is ignored and then we try to unignore a directory", () => { + it("should return true when a subdirectory is ignored and then we try to unignore a directory", () => { configs = new ConfigArray( [ { ignores: [ - "/**/node_modules/*", - "!/node_modules/foo", + "**/node_modules/*", + "!node_modules/", ], }, ], @@ -2364,16 +2364,16 @@ describe("ConfigArray", () => { "node_modules/foo/bar.js", ); - assert.strictEqual(configs.isFileIgnored(filename), false); + assert.strictEqual(configs.isFileIgnored(filename), true); }); - it("should return false when a subdirectory is ignored and then we try to unignore a file", () => { + it("should return true when a subdirectory is ignored and then we try to unignore a file", () => { configs = new ConfigArray( [ { ignores: [ - "/**/node_modules/*", - "!/node_modules/foo/**", + "**/node_modules/*", + "!node_modules/foo/*.js", ], }, ], @@ -2386,16 +2386,16 @@ describe("ConfigArray", () => { "node_modules/foo/bar.js", ); - assert.strictEqual(configs.isFileIgnored(filename), false); + assert.strictEqual(configs.isFileIgnored(filename), true); }); - it("should return false when all descendant directories are ignored and then we try to unignore a file", () => { + it("should return true when all descendant directories are ignored and then we try to unignore a file", () => { configs = new ConfigArray( [ { ignores: [ - "/**/node_modules/**", - "!/node_modules/foo/**", + "**/node_modules/**", + "!node_modules/foo/*.js", ], }, ], @@ -2408,7 +2408,7 @@ describe("ConfigArray", () => { "node_modules/foo/bar.js", ); - assert.strictEqual(configs.isFileIgnored(filename), false); + assert.strictEqual(configs.isFileIgnored(filename), true); }); it("should return true when all descendant directories are ignored without leading slash and then we try to unignore a file", () => { @@ -2922,14 +2922,11 @@ describe("ConfigArray", () => { ); }); - it("should return false when first-level subdirectories are ignored with leading slash and then one is negated", () => { + it("should return false when attempting to ignore first-level subdirectories with leading slash", () => { configs = new ConfigArray( [ { - ignores: [ - "/**/node_modules/*", - "!**/node_modules/foo/", - ], + ignores: ["/**/node_modules/*"], }, ], { basePath }, From 5ce6b188226bdc1d569723d78175cd5e2aff4cda Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Tue, 28 May 2024 07:44:50 +0200 Subject: [PATCH 3/6] new method `getConfigWithStatus` --- packages/config-array/src/config-array.js | 156 ++++++++-------------- 1 file changed, 56 insertions(+), 100 deletions(-) diff --git a/packages/config-array/src/config-array.js b/packages/config-array/src/config-array.js index 1c75b32..2710326 100644 --- a/packages/config-array/src/config-array.js +++ b/packages/config-array/src/config-array.js @@ -490,67 +490,6 @@ function assertExtraConfigTypes(extraConfigTypes) { } } -/** - * Calculates whether a file has a config or why it doesn't. - * @param {ConfigArray} configArray A normalized config array. - * @param {string} filePath The path of the file to check. - * @returns {"ignored"|"external"|"unconfigured"|"matched"} One of the constants returned by - * {@linkcode ConfigArray.getConfigStatus}. - */ -function calculateConfigStatus(configArray, filePath) { - // check if the file is outside the base path - const relativeFilePath = path.relative(configArray.basePath, filePath); - if (relativeFilePath.startsWith("..")) return "external"; - - // next check to see if the file should be ignored - if ( - configArray.isDirectoryIgnored(path.dirname(filePath)) || - shouldIgnorePath(configArray.ignores, filePath, relativeFilePath) - ) { - return "ignored"; - } - - // filePath isn't automatically ignored, so try to find a matching config - const universalPattern = /\/\*{1,2}$/; - - for (const config of configArray) { - if (!config.files) continue; - - /* - * If a config has a files pattern ending in /** or /*, and the - * filePath only matches those patterns, then the config is only - * applied if there is another config where the filePath matches - * a file with a specific extensions such as *.js. - */ - - const universalFiles = config.files.filter(pattern => - universalPattern.test(pattern), - ); - - let filteredConfig; - - // universal patterns were found so we need to filter the files - if (universalFiles.length) { - const nonUniversalFiles = config.files.filter( - pattern => !universalPattern.test(pattern), - ); - - filteredConfig = { - files: nonUniversalFiles, - ignores: config.ignores, - }; - } else { - filteredConfig = config; - } - - if (pathMatches(filePath, configArray.basePath, filteredConfig)) { - return "matched"; - } - } - - return "unconfigured"; -} - //------------------------------------------------------------------------------ // Public Interface //------------------------------------------------------------------------------ @@ -643,7 +582,6 @@ export class ConfigArray extends Array { directoryMatches: new Map(), files: undefined, ignores: undefined, - configStatus: new Map(), }); // load the configs into this array @@ -882,11 +820,14 @@ export class ConfigArray extends Array { } /** - * Returns the config object for a given file path. + * Returns the config object for a given file path and a status that can be used to determine why a file has no config. * @param {string} filePath The complete path of a file to get a config for. - * @returns {Object} The config object for this file. + * @returns {{ config?: Object, status: "ignored"|"external"|"unconfigured"|"matched" }} + * An object with an optional property `config` and property `status`. + * `config` is the config object for the specified file as returned by {@linkcode ConfigArray.getConfig}, + * `status` a is one of the constants returned by {@linkcode ConfigArray.getConfigStatus}. */ - getConfig(filePath) { + getConfigWithStatus(filePath) { assertNormalized(this); const cache = this[ConfigArraySymbol.configCache]; @@ -896,7 +837,20 @@ export class ConfigArray extends Array { return cache.get(filePath); } - let finalConfig; + let configWithStatus; + + // check to see if the file is outside the base path + + const relativeFilePath = path.relative(this.basePath, filePath); + + if (relativeFilePath.startsWith("..")) { + debug(`No config for file ${filePath} outside of base path`); + + // cache and return result + configWithStatus = Object.freeze({ status: "external" }); + cache.set(filePath, configWithStatus); + return configWithStatus; + } // next check to see if the file should be ignored @@ -904,20 +858,19 @@ export class ConfigArray extends Array { if (this.isDirectoryIgnored(path.dirname(filePath))) { debug(`Ignoring ${filePath} based on directory pattern`); - // cache and return result - finalConfig is undefined at this point - cache.set(filePath, finalConfig); - return finalConfig; + // cache and return result + configWithStatus = Object.freeze({ status: "ignored" }); + cache.set(filePath, configWithStatus); + return configWithStatus; } - // TODO: Maybe move elsewhere? - const relativeFilePath = path.relative(this.basePath, filePath); - if (shouldIgnorePath(this.ignores, filePath, relativeFilePath)) { debug(`Ignoring ${filePath} based on file pattern`); - // cache and return result - finalConfig is undefined at this point - cache.set(filePath, finalConfig); - return finalConfig; + // cache and return result + configWithStatus = Object.freeze({ status: "ignored" }); + cache.set(filePath, configWithStatus); + return configWithStatus; } // filePath isn't automatically ignored, so try to construct config @@ -1011,24 +964,26 @@ export class ConfigArray extends Array { if (!matchFound) { debug(`No matching configs found for ${filePath}`); - // cache and return result - finalConfig is undefined at this point - cache.set(filePath, finalConfig); - return finalConfig; + // cache and return result + configWithStatus = Object.freeze({ status: "unconfigured" }); + cache.set(filePath, configWithStatus); + return configWithStatus; } // check to see if there is a config cached by indices - finalConfig = cache.get(matchingConfigIndices.toString()); + const indicesKey = matchingConfigIndices.toString(); + configWithStatus = cache.get(indicesKey); - if (finalConfig) { + if (configWithStatus) { // also store for filename for faster lookup next time - cache.set(filePath, finalConfig); + cache.set(filePath, configWithStatus); - return finalConfig; + return configWithStatus; } // otherwise construct the config - finalConfig = matchingConfigIndices.reduce((result, index) => { + let finalConfig = matchingConfigIndices.reduce((result, index) => { try { return this[ConfigArraySymbol.schema].merge( result, @@ -1041,15 +996,28 @@ export class ConfigArray extends Array { finalConfig = this[ConfigArraySymbol.finalizeConfig](finalConfig); - cache.set(filePath, finalConfig); - cache.set(matchingConfigIndices.toString(), finalConfig); + configWithStatus = Object.freeze({ + config: finalConfig, + status: "matched", + }); + cache.set(filePath, configWithStatus); + cache.set(indicesKey, configWithStatus); - return finalConfig; + return configWithStatus; + } + + /** + * Returns the config object for a given file path. + * @param {string} filePath The complete path of a file to get a config for. + * @returns {Object|undefined} The config object for this file or `undefined`. + */ + getConfig(filePath) { + return this.getConfigWithStatus(filePath).config; } /** * Determines whether a file has a config or why it doesn't. - * @param {string} filePath The path of the file to check. + * @param {string} filePath The complete path of the file to check. * @returns {"ignored"|"external"|"unconfigured"|"matched"} One of the following values: * * `"ignored"`: the file is ignored * * `"external"`: the file is outside the base path @@ -1057,19 +1025,7 @@ export class ConfigArray extends Array { * * `"matched"`: the file has a matching config */ getConfigStatus(filePath) { - assertNormalized(this); - - // first check the cache for a filename match to avoid duplicate work - const cache = dataCache.get(this).configStatus; - let configStatus = cache.get(filePath); - - if (!configStatus) { - // no cached value found, so calculate - configStatus = calculateConfigStatus(this, filePath); - cache.set(configStatus); - } - - return configStatus; + return this.getConfigWithStatus(filePath).status; } /** From f2549dd28345b3bac51ad32b56a5a41c1bee5a8d Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Tue, 28 May 2024 08:53:07 +0200 Subject: [PATCH 4/6] unit tests for `getConfigWithStatus` --- .../config-array/tests/config-array.test.js | 75 +++++++++++++++++++ 1 file changed, 75 insertions(+) diff --git a/packages/config-array/tests/config-array.test.js b/packages/config-array/tests/config-array.test.js index 322dbde..f9c18fa 100644 --- a/packages/config-array/tests/config-array.test.js +++ b/packages/config-array/tests/config-array.test.js @@ -635,6 +635,81 @@ describe("ConfigArray", () => { }); }); + describe("getConfigWithStatus()", () => { + it("should throw an error when not normalized", () => { + const filename = path.resolve(basePath, "foo.js"); + + assert.throws(() => { + unnormalizedConfigs.getConfigWithStatus(filename); + }, /normalized/); + }); + + describe("should return expected results", () => { + it("for a file outside the base path", () => { + const filename = path.resolve(basePath, "../foo.js"); + const configWithStatus = + configs.getConfigWithStatus(filename); + + assert(Object.isFrozen(configWithStatus)); + assert.strictEqual(configWithStatus.config, undefined); + assert.strictEqual(configWithStatus.status, "external"); + }); + + it("for a file ignored based on directory pattern", () => { + const filename = path.resolve( + basePath, + "node_modules/foo.js", + ); + const configWithStatus = + configs.getConfigWithStatus(filename); + + assert(Object.isFrozen(configWithStatus)); + assert.strictEqual(configWithStatus.config, undefined); + assert.strictEqual(configWithStatus.status, "ignored"); + }); + + it("for a file ignored based on file pattern", () => { + const filename = path.resolve(basePath, ".gitignore"); + const configWithStatus = + configs.getConfigWithStatus(filename); + + assert(Object.isFrozen(configWithStatus)); + assert.strictEqual(configWithStatus.config, undefined); + assert.strictEqual(configWithStatus.status, "ignored"); + }); + + it("for a file without a matching config object", () => { + configs = new ConfigArray( + [ + { + files: ["**/*"], + }, + ], + { basePath }, + ); + + configs.normalizeSync(); + const filename = path.resolve(basePath, "foo.bar"); + const configWithStatus = + configs.getConfigWithStatus(filename); + + assert(Object.isFrozen(configWithStatus)); + assert.strictEqual(configWithStatus.config, undefined); + assert.strictEqual(configWithStatus.status, "unconfigured"); + }); + + it("for a file with a config", () => { + const filename = path.resolve(basePath, "foo.js"); + const configWithStatus = + configs.getConfigWithStatus(filename); + + assert(Object.isFrozen(configWithStatus)); + assert.notEqual(configWithStatus.config, undefined); + assert.strictEqual(configWithStatus.status, "matched"); + }); + }); + }); + describe("getConfig()", () => { it("should throw an error when not normalized", () => { const filename = path.resolve(basePath, "foo.js"); From b194ef83574a2fd60b848cf860b3b0c94483dfba Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Wed, 29 May 2024 08:00:34 +0200 Subject: [PATCH 5/6] return prepared objects in `getConfigWithStatus` --- packages/config-array/src/config-array.js | 32 ++++++++++++----------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/packages/config-array/src/config-array.js b/packages/config-array/src/config-array.js index 2710326..da587a6 100644 --- a/packages/config-array/src/config-array.js +++ b/packages/config-array/src/config-array.js @@ -80,6 +80,14 @@ const META_FIELDS = new Set(["name"]); */ const FILES_AND_IGNORES_SCHEMA = new ObjectSchema(filesAndIgnoresSchema); +// Precomputed constant objects returned by `ConfigArray.getConfigWithStatus`. + +const CONFIG_WITH_STATUS_EXTERNAL = Object.freeze({ status: "external" }); +const CONFIG_WITH_STATUS_IGNORED = Object.freeze({ status: "ignored" }); +const CONFIG_WITH_STATUS_UNCONFIGURED = Object.freeze({ + status: "unconfigured", +}); + /** * Wrapper error for config validation errors that adds a name to the front of the * error message. @@ -837,8 +845,6 @@ export class ConfigArray extends Array { return cache.get(filePath); } - let configWithStatus; - // check to see if the file is outside the base path const relativeFilePath = path.relative(this.basePath, filePath); @@ -847,9 +853,8 @@ export class ConfigArray extends Array { debug(`No config for file ${filePath} outside of base path`); // cache and return result - configWithStatus = Object.freeze({ status: "external" }); - cache.set(filePath, configWithStatus); - return configWithStatus; + cache.set(filePath, CONFIG_WITH_STATUS_EXTERNAL); + return CONFIG_WITH_STATUS_EXTERNAL; } // next check to see if the file should be ignored @@ -859,18 +864,16 @@ export class ConfigArray extends Array { debug(`Ignoring ${filePath} based on directory pattern`); // cache and return result - configWithStatus = Object.freeze({ status: "ignored" }); - cache.set(filePath, configWithStatus); - return configWithStatus; + cache.set(filePath, CONFIG_WITH_STATUS_IGNORED); + return CONFIG_WITH_STATUS_IGNORED; } if (shouldIgnorePath(this.ignores, filePath, relativeFilePath)) { debug(`Ignoring ${filePath} based on file pattern`); // cache and return result - configWithStatus = Object.freeze({ status: "ignored" }); - cache.set(filePath, configWithStatus); - return configWithStatus; + cache.set(filePath, CONFIG_WITH_STATUS_IGNORED); + return CONFIG_WITH_STATUS_IGNORED; } // filePath isn't automatically ignored, so try to construct config @@ -965,14 +968,13 @@ export class ConfigArray extends Array { debug(`No matching configs found for ${filePath}`); // cache and return result - configWithStatus = Object.freeze({ status: "unconfigured" }); - cache.set(filePath, configWithStatus); - return configWithStatus; + cache.set(filePath, CONFIG_WITH_STATUS_UNCONFIGURED); + return CONFIG_WITH_STATUS_UNCONFIGURED; } // check to see if there is a config cached by indices const indicesKey = matchingConfigIndices.toString(); - configWithStatus = cache.get(indicesKey); + let configWithStatus = cache.get(indicesKey); if (configWithStatus) { // also store for filename for faster lookup next time From 651b2999f269f20c6ab07ab0d33a4a65569aa70a Mon Sep 17 00:00:00 2001 From: Francesco Trotta Date: Wed, 29 May 2024 08:13:35 +0200 Subject: [PATCH 6/6] test that returned objects are reused --- .../config-array/tests/config-array.test.js | 34 +++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/packages/config-array/tests/config-array.test.js b/packages/config-array/tests/config-array.test.js index f9c18fa..0e3aa19 100644 --- a/packages/config-array/tests/config-array.test.js +++ b/packages/config-array/tests/config-array.test.js @@ -653,6 +653,13 @@ describe("ConfigArray", () => { assert(Object.isFrozen(configWithStatus)); assert.strictEqual(configWithStatus.config, undefined); assert.strictEqual(configWithStatus.status, "external"); + + const newFilename = path.resolve(basePath, "../bar.js"); + const newConfigWithStatus = + configs.getConfigWithStatus(newFilename); + + // check that returned objects are reused + assert.strictEqual(newConfigWithStatus, configWithStatus); }); it("for a file ignored based on directory pattern", () => { @@ -666,6 +673,16 @@ describe("ConfigArray", () => { assert(Object.isFrozen(configWithStatus)); assert.strictEqual(configWithStatus.config, undefined); assert.strictEqual(configWithStatus.status, "ignored"); + + const newFilename = path.resolve( + basePath, + "node_modules/bar.js", + ); + const newConfigWithStatus = + configs.getConfigWithStatus(newFilename); + + // check that returned objects are reused + assert.strictEqual(newConfigWithStatus, configWithStatus); }); it("for a file ignored based on file pattern", () => { @@ -676,6 +693,16 @@ describe("ConfigArray", () => { assert(Object.isFrozen(configWithStatus)); assert.strictEqual(configWithStatus.config, undefined); assert.strictEqual(configWithStatus.status, "ignored"); + + const newFilename = path.resolve( + basePath, + "dir/.gitignore", + ); + const newConfigWithStatus = + configs.getConfigWithStatus(newFilename); + + // check that returned objects are reused + assert.strictEqual(newConfigWithStatus, configWithStatus); }); it("for a file without a matching config object", () => { @@ -696,6 +723,13 @@ describe("ConfigArray", () => { assert(Object.isFrozen(configWithStatus)); assert.strictEqual(configWithStatus.config, undefined); assert.strictEqual(configWithStatus.status, "unconfigured"); + + const newFilename = path.resolve(basePath, "foo.baz"); + const newConfigWithStatus = + configs.getConfigWithStatus(newFilename); + + // check that returned objects are reused + assert.strictEqual(newConfigWithStatus, configWithStatus); }); it("for a file with a config", () => {