From ab64e991f3cf0c9afbad997f392169ba6b1aaeb7 Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Fri, 27 Oct 2023 11:37:18 -0400 Subject: [PATCH 1/2] fix: Empty basePath should not be treated as cwd In Node.js, `path.relative("")` always resolves to `process.cwd()`, which causes problems when people use the `Linter` API in ESLint where the `basePath` of a `ConfigArray` is set to an empty string. This ensures that only non-empty `basePath` strings are passed to `path.relative()`. Refs eslint/eslint#17669 --- src/config-array.js | 32 ++++++++++++---- tests/config-array.test.js | 78 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 8 deletions(-) diff --git a/src/config-array.js b/src/config-array.js index ccd8de4..4f3b7be 100644 --- a/src/config-array.js +++ b/src/config-array.js @@ -62,6 +62,19 @@ function assertValidFilesAndIgnores(config) { FILES_AND_IGNORES_SCHEMA.validate(validateConfig); } +/** + * A version of `path.relative()` that does not substitute `process.cwd()` + * when it sees an empty string. This is necessary because `basePath` can + * be an empty string in `ConfigArray`. In that case, we don't want the cwd + * to be substituted. + * @param {string} from The path to calculate from. + * @param {string} to The path to calculate to. + * @returns {string} The relative path between `from` and `to`. + */ +function safePathRelative(from, to) { + return from ? path.relative(from, to) : to; +} + /** * Wrapper around minimatch that caches minimatch patterns for * faster matching speed over multiple file path evaluations. @@ -252,7 +265,7 @@ function pathMatchesIgnores(filePath, basePath, config) { * file path while strings are compared against the relative * file path. */ - const relativeFilePath = path.relative(basePath, filePath); + const relativeFilePath = safePathRelative(basePath, filePath); return Object.keys(config).length > 1 && !shouldIgnorePath(config.ignores, filePath, relativeFilePath); @@ -277,7 +290,7 @@ function pathMatches(filePath, basePath, config) { * file path while strings are compared against the relative * file path. */ - const relativeFilePath = path.relative(basePath, filePath); + const relativeFilePath = safePathRelative(basePath, filePath); // match both strings and functions const match = pattern => { @@ -661,8 +674,10 @@ export class ConfigArray extends Array { return result; } - // TODO: Maybe move elsewhere? Maybe combine with getConfig() logic? - const relativeFilePath = path.relative(this.basePath, filePath); + /* + * TODO: Maybe move elsewhere? Maybe combine with getConfig() logic? + */ + const relativeFilePath = safePathRelative(this.basePath, filePath); if (shouldIgnorePath(this.ignores, filePath, relativeFilePath)) { debug(`Ignoring ${filePath}`); @@ -719,8 +734,10 @@ export class ConfigArray extends Array { return finalConfig; } - // TODO: Maybe move elsewhere? - const relativeFilePath = path.relative(this.basePath, filePath); + /* + * TODO: Maybe move somewhere else? + */ + const relativeFilePath = safePathRelative(this.basePath, filePath); if (shouldIgnorePath(this.ignores, filePath, relativeFilePath)) { debug(`Ignoring ${filePath} based on file pattern`); @@ -885,8 +902,7 @@ export class ConfigArray extends Array { assertNormalized(this); - const relativeDirectoryPath = path.relative(this.basePath, directoryPath) - .replace(/\\/g, '/'); + const relativeDirectoryPath = safePathRelative(this.basePath, directoryPath).replace(/\\/g, '/'); if (relativeDirectoryPath.startsWith('..')) { return true; diff --git a/tests/config-array.test.js b/tests/config-array.test.js index 9cbcecb..d0044ae 100644 --- a/tests/config-array.test.js +++ b/tests/config-array.test.js @@ -884,6 +884,54 @@ describe('ConfigArray', () => { }); + // https://github.com/eslint/eslint/issues/17669 + describe('basePath is an empty string', () => { + + beforeEach(() => { + configs = new ConfigArray([ + { + files: ['**/*.js'], + defs: { + severity: 'error' + } + } + ], { basePath: '', schema }); + + configs.normalizeSync(); + }); + + it('should return a config when a filename matches a files pattern', () => { + + const config = configs.getConfig('foo.js'); + expect(config).to.deep.equal({ + defs: { + severity: 'error' + } + }); + }); + + it('should return a config when a relative path filename matches a files pattern', () => { + + const config = configs.getConfig('x/foo.js'); + expect(config).to.deep.equal({ + defs: { + severity: 'error' + } + }); + }); + + it('should return a config when an absolute path filename matches a files pattern', () => { + + const config = configs.getConfig('/x/foo.js'); + expect(config).to.deep.equal({ + defs: { + severity: 'error' + } + }); + }); + + }); + }); describe('isIgnored()', () => { @@ -1742,6 +1790,36 @@ describe('ConfigArray', () => { expect(configs.isExplicitMatch(filename)).to.be.false; }); + // https://github.com/eslint/eslint/issues/17669 + describe('basePath is an empty string', () => { + + beforeEach(() => { + configs = new ConfigArray([ + { + files: ['**/*.js'], + defs: { + severity: 'error' + } + } + ], { basePath: '', schema }); + + configs.normalizeSync(); + }); + + it('should return true when a filename matches a files pattern', () => { + expect(configs.isExplicitMatch('foo.js')).to.be.true; + }); + + it('should return true when a relative path filename matches a files pattern', () => { + expect(configs.isExplicitMatch('x/foo.js')).to.be.true; + }); + + it('should return true when an absolute path filename matches a files pattern', () => { + expect(configs.isExplicitMatch('/x/foo.js')).to.be.true; + }); + + }); + }); describe('files', () => { From d87d9c83f9a99e5f99d7ef8b0c5d5bf241a9ba5c Mon Sep 17 00:00:00 2001 From: "Nicholas C. Zakas" Date: Mon, 30 Oct 2023 11:03:54 -0400 Subject: [PATCH 2/2] Add tests --- tests/config-array.test.js | 148 +++++++++++++++++++++++++++---------- 1 file changed, 110 insertions(+), 38 deletions(-) diff --git a/tests/config-array.test.js b/tests/config-array.test.js index d0044ae..92eaf5f 100644 --- a/tests/config-array.test.js +++ b/tests/config-array.test.js @@ -887,46 +887,85 @@ describe('ConfigArray', () => { // https://github.com/eslint/eslint/issues/17669 describe('basePath is an empty string', () => { - beforeEach(() => { - configs = new ConfigArray([ - { - files: ['**/*.js'], + describe('with **/*.js', () => { + beforeEach(() => { + configs = new ConfigArray([ + { + files: ['**/*.js'], + defs: { + severity: 'error' + } + } + ], { basePath: '', schema }); + + configs.normalizeSync(); + }); + + it('should return a config when a filename matches a files pattern', () => { + + const config = configs.getConfig('foo.js'); + expect(config).to.deep.equal({ defs: { severity: 'error' } - } - ], { basePath: '', schema }); + }); + }); - configs.normalizeSync(); - }); - - it('should return a config when a filename matches a files pattern', () => { - - const config = configs.getConfig('foo.js'); - expect(config).to.deep.equal({ - defs: { - severity: 'error' - } + it('should return a config when a relative path filename matches a files pattern', () => { + + const config = configs.getConfig('x/foo.js'); + expect(config).to.deep.equal({ + defs: { + severity: 'error' + } + }); }); - }); - - it('should return a config when a relative path filename matches a files pattern', () => { - - const config = configs.getConfig('x/foo.js'); - expect(config).to.deep.equal({ - defs: { - severity: 'error' - } + + it('should return a config when an absolute path filename matches a files pattern', () => { + + const config = configs.getConfig('/x/foo.js'); + expect(config).to.deep.equal({ + defs: { + severity: 'error' + } + }); }); }); - it('should return a config when an absolute path filename matches a files pattern', () => { - - const config = configs.getConfig('/x/foo.js'); - expect(config).to.deep.equal({ - defs: { - severity: 'error' - } + describe('with x/*.js', () => { + beforeEach(() => { + configs = new ConfigArray([ + { + files: ['x/*.js'], + defs: { + severity: 'error' + } + } + ], { basePath: '', schema }); + + configs.normalizeSync(); + }); + + it('should return undefined when a filename does not match a files pattern', () => { + + const config = configs.getConfig('foo.js'); + expect(config).to.be.undefined; + }); + + it('should return a config when a relative path filename matches a files pattern', () => { + + const config = configs.getConfig('x/foo.js'); + expect(config).to.deep.equal({ + defs: { + severity: 'error' + } + }); + }); + + it('should return undefined when an absolute path filename does not match a files pattern', () => { + + const config = configs.getConfig('/x/foo.js'); + expect(config).to.be.undefined; }); }); @@ -1793,10 +1832,42 @@ describe('ConfigArray', () => { // https://github.com/eslint/eslint/issues/17669 describe('basePath is an empty string', () => { + describe('with **/*.js', () => { + + beforeEach(() => { + configs = new ConfigArray([ + { + files: ['**/*.js'], + defs: { + severity: 'error' + } + } + ], { basePath: '', schema }); + + configs.normalizeSync(); + }); + + it('should return true when a filename matches a files pattern', () => { + expect(configs.isExplicitMatch('foo.js')).to.be.true; + }); + + it('should return true when a relative path filename matches a files pattern', () => { + expect(configs.isExplicitMatch('x/foo.js')).to.be.true; + }); + + it('should return true when an absolute path filename matches a files pattern', () => { + expect(configs.isExplicitMatch('/x/foo.js')).to.be.true; + }); + }); + + }); + + describe('with x/*.js', () => { + beforeEach(() => { configs = new ConfigArray([ { - files: ['**/*.js'], + files: ['x/*.js'], defs: { severity: 'error' } @@ -1806,20 +1877,21 @@ describe('ConfigArray', () => { configs.normalizeSync(); }); - it('should return true when a filename matches a files pattern', () => { - expect(configs.isExplicitMatch('foo.js')).to.be.true; + it('should return false when a filename does not match a files pattern', () => { + expect(configs.isExplicitMatch('foo.js')).to.be.false; }); it('should return true when a relative path filename matches a files pattern', () => { expect(configs.isExplicitMatch('x/foo.js')).to.be.true; }); - it('should return true when an absolute path filename matches a files pattern', () => { - expect(configs.isExplicitMatch('/x/foo.js')).to.be.true; + it('should return false when an absolute path filename does not match a files pattern', () => { + expect(configs.isExplicitMatch('/x/foo.js')).to.be.false; }); - + }); + }); describe('files', () => {