Skip to content

Commit

Permalink
fix: Config with just ignores should not always be applied (#89)
Browse files Browse the repository at this point in the history
* 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 <milos.djermanovic@gmail.com>

---------

Co-authored-by: Milos Djermanovic <milos.djermanovic@gmail.com>
  • Loading branch information
nzakas and mdjermanovic committed May 12, 2023
1 parent ffdeba2 commit 5ed9c2c
Show file tree
Hide file tree
Showing 2 changed files with 126 additions and 3 deletions.
40 changes: 38 additions & 2 deletions src/config-array.js
Expand Up @@ -225,12 +225,36 @@ function shouldIgnorePath(ignores, filePath, relativeFilePath) {

}

/**
* Determines if a given file path is matched by a config based on
* `ignores` only.
* @param {string} filePath The absolute file path to check.
* @param {string} basePath The base path for the config.
* @param {Object} config The config object to check.
* @returns {boolean} True if the file path is matched by the config,
* false if not.
*/
function pathMatchesIgnores(filePath, basePath, config) {

/*
* For both files and ignores, functions are passed the absolute
* file path while strings are compared against the relative
* file path.
*/
const relativeFilePath = path.relative(basePath, filePath);

return Object.keys(config).length > 1 &&
!shouldIgnorePath(config.ignores, filePath, relativeFilePath);
}


/**
* Determines if a given file path is matched by a config. If the config
* has no `files` field, then it matches; otherwise, if a `files` field
* is present then we match the globs in `files` and exclude any globs in
* `ignores`.
* @param {string} filePath The absolute file path to check.
* @param {string} basePath The base path for the config.
* @param {Object} config The config object to check.
* @returns {boolean} True if the file path is matched by the config,
* false if not.
Expand Down Expand Up @@ -705,8 +729,20 @@ export class ConfigArray extends Array {
this.forEach((config, index) => {

if (!config.files) {
debug(`Anonymous universal config found for ${filePath}`);
matchingConfigIndices.push(index);

if (!config.ignores) {
debug(`Anonymous universal config found for ${filePath}`);
matchingConfigIndices.push(index);
return;
}

if (pathMatchesIgnores(filePath, this.basePath, config)) {
debug(`Matching config found for ${filePath} (based on ignores: ${config.ignores})`);
matchingConfigIndices.push(index);
return;
}

debug(`Skipped config found for ${filePath} (based on ignores: ${config.ignores})`);
return;
}

Expand Down
89 changes: 88 additions & 1 deletion tests/config-array.test.js
Expand Up @@ -497,7 +497,6 @@ describe('ConfigArray', () => {

it('should calculate correct config when passed JS filename', () => {
const filename = path.resolve(basePath, 'foo.js');

const config = configs.getConfig(filename);

expect(config.language).to.equal(JSLanguage);
Expand Down Expand Up @@ -690,6 +689,94 @@ describe('ConfigArray', () => {
expect(config).to.be.undefined;
});

// https://github.com/eslint/eslint/issues/17103
describe('ignores patterns should be properly applied', () => {

it('should return undefined when a filename matches an ignores pattern but not a files pattern', () => {
const matchingFilename = path.resolve(basePath, 'foo.js');
const notMatchingFilename = path.resolve(basePath, 'foo.md');
configs = new ConfigArray([
{
defs: {
severity: 'error'
}
},
{
ignores: ['**/*.md'],
defs: {
severity: 'warn'
}
}
], { basePath, schema });

configs.normalizeSync();

const config1 = configs.getConfig(matchingFilename);
expect(config1).to.be.undefined;

const config2 = configs.getConfig(notMatchingFilename);
expect(config2).to.be.undefined;
});

it('should apply config with only ignores when a filename matches a files pattern', () => {
const matchingFilename = path.resolve(basePath, 'foo.js');
const notMatchingFilename = path.resolve(basePath, 'foo.md');
configs = new ConfigArray([
{
files: ['**/*.js'],
defs: {
severity: 'error'
}
},
{
ignores: ['**/*.md'],
defs: {
severity: 'warn'
}
}
], { basePath, schema });

configs.normalizeSync();

const config1 = configs.getConfig(matchingFilename);
expect(config1).to.be.an('object');
expect(config1.defs.severity).to.equal('warn');

const config2 = configs.getConfig(notMatchingFilename);
expect(config2).to.be.undefined;
});

it('should not apply config with only ignores when a filename does not match it', () => {
const matchingFilename = path.resolve(basePath, 'foo.js');
const notMatchingFilename = path.resolve(basePath, 'bar.js');
configs = new ConfigArray([
{
files: ['**/*.js'],
defs: {
severity: 'error'
}
},
{
ignores: ['**/bar.js'],
defs: {
severity: 'warn'
}
}
], { basePath, schema });

configs.normalizeSync();

const config1 = configs.getConfig(matchingFilename);
expect(config1).to.be.an('object');
expect(config1.defs.severity).to.equal('warn');

const config2 = configs.getConfig(notMatchingFilename);
expect(config2).to.be.an('object');
expect(config2.defs.severity).to.equal('error');
});

});

});

describe('isIgnored()', () => {
Expand Down

0 comments on commit 5ed9c2c

Please sign in to comment.