Skip to content
This repository was archived by the owner on Jun 10, 2024. It is now read-only.

Commit 5ed9c2c

Browse files
nzakasmdjermanovic
andauthored
fix: Config with just ignores should not always be applied (#89)
* fix: Config with just ignores should check ignores When a config has `ignores` and not `files`, plus at least one other key, it should match all files that don't match `ignores` and the other keys should be merged into the resulting config. Prior to this, such a config was always merged in regardless if the filename matched `ignores`; after this change, it correctly merges in only when `ignores` matches. Refs eslint/eslint#17103 * Update ignores logic * Update tests/config-array.test.js Co-authored-by: Milos Djermanovic <[email protected]> --------- Co-authored-by: Milos Djermanovic <[email protected]>
1 parent ffdeba2 commit 5ed9c2c

File tree

2 files changed

+126
-3
lines changed

2 files changed

+126
-3
lines changed

src/config-array.js

Lines changed: 38 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -225,12 +225,36 @@ function shouldIgnorePath(ignores, filePath, relativeFilePath) {
225225

226226
}
227227

228+
/**
229+
* Determines if a given file path is matched by a config based on
230+
* `ignores` only.
231+
* @param {string} filePath The absolute file path to check.
232+
* @param {string} basePath The base path for the config.
233+
* @param {Object} config The config object to check.
234+
* @returns {boolean} True if the file path is matched by the config,
235+
* false if not.
236+
*/
237+
function pathMatchesIgnores(filePath, basePath, config) {
238+
239+
/*
240+
* For both files and ignores, functions are passed the absolute
241+
* file path while strings are compared against the relative
242+
* file path.
243+
*/
244+
const relativeFilePath = path.relative(basePath, filePath);
245+
246+
return Object.keys(config).length > 1 &&
247+
!shouldIgnorePath(config.ignores, filePath, relativeFilePath);
248+
}
249+
250+
228251
/**
229252
* Determines if a given file path is matched by a config. If the config
230253
* has no `files` field, then it matches; otherwise, if a `files` field
231254
* is present then we match the globs in `files` and exclude any globs in
232255
* `ignores`.
233256
* @param {string} filePath The absolute file path to check.
257+
* @param {string} basePath The base path for the config.
234258
* @param {Object} config The config object to check.
235259
* @returns {boolean} True if the file path is matched by the config,
236260
* false if not.
@@ -705,8 +729,20 @@ export class ConfigArray extends Array {
705729
this.forEach((config, index) => {
706730

707731
if (!config.files) {
708-
debug(`Anonymous universal config found for ${filePath}`);
709-
matchingConfigIndices.push(index);
732+
733+
if (!config.ignores) {
734+
debug(`Anonymous universal config found for ${filePath}`);
735+
matchingConfigIndices.push(index);
736+
return;
737+
}
738+
739+
if (pathMatchesIgnores(filePath, this.basePath, config)) {
740+
debug(`Matching config found for ${filePath} (based on ignores: ${config.ignores})`);
741+
matchingConfigIndices.push(index);
742+
return;
743+
}
744+
745+
debug(`Skipped config found for ${filePath} (based on ignores: ${config.ignores})`);
710746
return;
711747
}
712748

tests/config-array.test.js

Lines changed: 88 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -497,7 +497,6 @@ describe('ConfigArray', () => {
497497

498498
it('should calculate correct config when passed JS filename', () => {
499499
const filename = path.resolve(basePath, 'foo.js');
500-
501500
const config = configs.getConfig(filename);
502501

503502
expect(config.language).to.equal(JSLanguage);
@@ -690,6 +689,94 @@ describe('ConfigArray', () => {
690689
expect(config).to.be.undefined;
691690
});
692691

692+
// https://github.com/eslint/eslint/issues/17103
693+
describe('ignores patterns should be properly applied', () => {
694+
695+
it('should return undefined when a filename matches an ignores pattern but not a files pattern', () => {
696+
const matchingFilename = path.resolve(basePath, 'foo.js');
697+
const notMatchingFilename = path.resolve(basePath, 'foo.md');
698+
configs = new ConfigArray([
699+
{
700+
defs: {
701+
severity: 'error'
702+
}
703+
},
704+
{
705+
ignores: ['**/*.md'],
706+
defs: {
707+
severity: 'warn'
708+
}
709+
}
710+
], { basePath, schema });
711+
712+
configs.normalizeSync();
713+
714+
const config1 = configs.getConfig(matchingFilename);
715+
expect(config1).to.be.undefined;
716+
717+
const config2 = configs.getConfig(notMatchingFilename);
718+
expect(config2).to.be.undefined;
719+
});
720+
721+
it('should apply config with only ignores when a filename matches a files pattern', () => {
722+
const matchingFilename = path.resolve(basePath, 'foo.js');
723+
const notMatchingFilename = path.resolve(basePath, 'foo.md');
724+
configs = new ConfigArray([
725+
{
726+
files: ['**/*.js'],
727+
defs: {
728+
severity: 'error'
729+
}
730+
},
731+
{
732+
ignores: ['**/*.md'],
733+
defs: {
734+
severity: 'warn'
735+
}
736+
}
737+
], { basePath, schema });
738+
739+
configs.normalizeSync();
740+
741+
const config1 = configs.getConfig(matchingFilename);
742+
expect(config1).to.be.an('object');
743+
expect(config1.defs.severity).to.equal('warn');
744+
745+
const config2 = configs.getConfig(notMatchingFilename);
746+
expect(config2).to.be.undefined;
747+
});
748+
749+
it('should not apply config with only ignores when a filename does not match it', () => {
750+
const matchingFilename = path.resolve(basePath, 'foo.js');
751+
const notMatchingFilename = path.resolve(basePath, 'bar.js');
752+
configs = new ConfigArray([
753+
{
754+
files: ['**/*.js'],
755+
defs: {
756+
severity: 'error'
757+
}
758+
},
759+
{
760+
ignores: ['**/bar.js'],
761+
defs: {
762+
severity: 'warn'
763+
}
764+
}
765+
], { basePath, schema });
766+
767+
configs.normalizeSync();
768+
769+
const config1 = configs.getConfig(matchingFilename);
770+
expect(config1).to.be.an('object');
771+
expect(config1.defs.severity).to.equal('warn');
772+
773+
const config2 = configs.getConfig(notMatchingFilename);
774+
expect(config2).to.be.an('object');
775+
expect(config2.defs.severity).to.equal('error');
776+
});
777+
778+
});
779+
693780
});
694781

695782
describe('isIgnored()', () => {

0 commit comments

Comments
 (0)