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

feat: add method isFileConfigured #138

Closed
wants to merge 1 commit into from
Closed

feat: add method isFileConfigured #138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions src/config-array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -453,6 +453,9 @@ export const ConfigArraySymbol = {
// used to store calculate data for faster lookup
const dataCache = new WeakMap();

// A value indicating that a file has no matching configuration. Should be a unique object or symbol.
const NoMatchingConfig = { };

/**
* Represents an array of config objects and provides method for working with
* those config objects.
Expand Down Expand Up @@ -759,6 +762,16 @@ export class ConfigArray extends Array {
* @returns {Object} The config object for this file.
*/
getConfig(filePath) {
const config = this.getConfigOrNoMatch(filePath);
if (config !== NoMatchingConfig) return config;
}

/**
* Returns the config object for a given file path or a `NoMatchingConfig` indicator.
* @param {string} filePath The complete path of a file to get a config for.
* @returns {Object} The config object or `NoMatchingConfig` indicator for this file.
*/
getConfigOrNoMatch(filePath) {
Comment on lines +769 to +774
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This method doesn't need to be public, it could be converted into a local function.


assertNormalized(this);

Expand Down Expand Up @@ -885,6 +898,7 @@ export class ConfigArray extends Array {
debug(`No matching configs found for ${filePath}`);

// cache and return result - finalConfig is undefined at this point
finalConfig = NoMatchingConfig;
cache.set(filePath, finalConfig);
return finalConfig;
}
Expand Down Expand Up @@ -937,6 +951,15 @@ export class ConfigArray extends Array {
return this.getConfig(filePath) === undefined;
}

/**
* Determines if the given filepath has a matching config.
* @param {string} filePath The complete path of a file to check.
* @returns {boolean} True if the path has a matching config, false if not.
*/
isFileConfigured(filePath) {
return this.getConfigOrNoMatch(filePath) !== NoMatchingConfig;
}
Comment on lines +954 to +961
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the file is ignored by ignore patterns, this returns true regardless of whether or not it has a matching config?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes. This is technically okay, because either way the error message "File ignored because of a matching ignore pattern." does apply. But maybe the method could have a more descriptive name?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps we could change the return type of getConfig()? Instead of returning config directly, it could return a { config } object with an additional property that would indicate why is config undefined if that's the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's an interesting idea, it would be a breaking change though.


/**
* Determines if the given directory is ignored based on the configs.
* This checks only default `ignores` that don't have `files` in the
Expand Down
62 changes: 60 additions & 2 deletions tests/config-array.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1105,46 +1105,71 @@ describe('ConfigArray', () => {
.throw(/normalized/);
});

});

describe('isFileConfigured()', () => {

it('should throw an error when not normalized', () => {
const filename = path.resolve(basePath, 'foo.js');

expect(() => {
unnormalizedConfigs.isFileConfigured(filename);
})
.to
.throw(/normalized/);
});

});

describe('isFileIgnored() / isFileConfigured()', () => {
Copy link
Contributor Author

@fasttime fasttime Apr 29, 2024 •
edited

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've added assertions for isFileConfigured after those for isFileIgnored, because in ESLint isFileConfigured will be called when isFileIgnored returns true. I think this makes more sense than calling isFileConfigured alone. Alternatively, we could duplicate some test cases to keep the assertions separated.


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

expect(configs.isFileIgnored(filename)).to.be.false;
expect(configs.isFileConfigured(filename)).to.be.true;
});

it('should return true when passed JS filename in parent directory', () => {
const filename = path.resolve(basePath, '../foo.js');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.true;
});

it('should return false when passed HTML filename', () => {
const filename = path.resolve(basePath, 'foo.html');

expect(configs.isFileIgnored(filename)).to.be.false;
expect(configs.isFileConfigured(filename)).to.be.true;
});

it('should return true when passed ignored .gitignore filename', () => {
const filename = path.resolve(basePath, '.gitignore');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.true;
});

it('should return false when passed CSS filename', () => {
const filename = path.resolve(basePath, 'foo.css');

expect(configs.isFileIgnored(filename)).to.be.false;
expect(configs.isFileConfigured(filename)).to.be.true;
});

it('should return true when passed docx filename', () => {
it('should return false when passed docx filename', () => {
const filename = path.resolve(basePath, 'sss.docx');

expect(configs.isFileIgnored(filename)).to.be.false;
expect(configs.isFileConfigured(filename)).to.be.true;
});

it('should return true when passed ignored node_modules filename', () => {
const filename = path.resolve(basePath, 'node_modules/foo.js');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.true;
});

it('should return true when passed matching both files and ignores in a config', () => {
Expand All @@ -1162,6 +1187,7 @@ describe('ConfigArray', () => {
const filename = path.resolve(basePath, 'fixtures/test.xsl');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.false;
});

it('should return false when negated pattern comes after matching pattern', () => {
Expand All @@ -1177,7 +1203,9 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'bar.txt'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'bar.txt'))).to.be.false;
expect(configs.isFileIgnored(path.join(basePath, 'foo.txt'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo.txt'))).to.be.true;
});

it('should return true when negated pattern comes before matching pattern', () => {
Expand All @@ -1192,7 +1220,9 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'bar.txt'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'bar.txt'))).to.be.true;
expect(configs.isFileIgnored(path.join(basePath, 'foo.txt'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo.txt'))).to.be.true;
});

it('should return false when matching files and ignores has a negated pattern comes after matching pattern', () => {
Expand All @@ -1208,10 +1238,12 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'bar.test.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'bar.test.js'))).to.be.false;
expect(configs.isFileIgnored(path.join(basePath, 'foo.test.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo.test.js'))).to.be.true;
});

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([
{
ignores: ['ignoreme']
Expand All @@ -1226,6 +1258,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'ignoreme/foo.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'ignoreme/foo.js'))).to.be.true;
});

it('should return false when file is inside of ignored directory', () => {
Expand All @@ -1246,6 +1279,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/bar/a.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo/bar/a.js'))).to.be.true;
});

it('should return true when file is ignored, unignored, and then reignored', () => {
Expand All @@ -1267,6 +1301,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'a.js'))).to.be.true;
});

it('should return true when the parent directory of a file is ignored', () => {
Expand All @@ -1286,6 +1321,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/bar/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo/bar/a.js'))).to.be.true;
});

it('should return true when an ignored directory is later negated with **', () => {
Expand All @@ -1310,6 +1346,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'node_modules/package/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'node_modules/package/a.js'))).to.be.true;
});

it('should return true when an ignored directory is later negated with *', () => {
Expand All @@ -1334,6 +1371,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'node_modules/package/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'node_modules/package/a.js'))).to.be.true;
});

it('should return true when there are only patterns ending with /*', () => {
Expand Down Expand Up @@ -1362,6 +1400,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.false;
});

it('should return false when files pattern matches and there is a pattern ending with /**', () => {
Expand All @@ -1376,6 +1415,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return false when file has the same name as a directory that is ignored by a pattern that ends with `/`', () => {
Expand All @@ -1395,6 +1435,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo'))).to.be.true;
});

it('should return false when file is in the parent directory of directories that are ignored by a pattern that ends with `/`', () => {
Expand All @@ -1414,6 +1455,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return true when file is in a directory that is ignored by a pattern that ends with `/`', () => {
Expand All @@ -1433,6 +1475,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return true when file is in a directory that is ignored by a pattern that does not end with `/`', () => {
Expand All @@ -1452,6 +1495,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return false when file is in a directory that is ignored and then unignored by pattern that end with `/`', () => {
Expand All @@ -1472,6 +1516,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return true 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 `/`', () => {
Expand All @@ -1494,6 +1539,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return true 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 `/`', () => {
Expand All @@ -1516,6 +1562,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return false 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 `/**`', () => {
Expand All @@ -1538,6 +1585,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

it('should return true when file is ignored by a pattern and there are unignore patterns that target files of a directory with the same name', () => {
Expand All @@ -1559,6 +1607,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo'))).to.be.true;
});

it('should return true when file is in a directory that is ignored even if an unignore pattern that ends with `/*` matches the file', () => {
Expand All @@ -1579,6 +1628,7 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'foo/a.js'))).to.be.true;
});

// https://github.com/eslint/eslint/issues/17964#issuecomment-1879840650
Expand Down Expand Up @@ -1607,9 +1657,13 @@ describe('ConfigArray', () => {
configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'tests/format/foo.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'tests/format/foo.js'))).to.be.true;
expect(configs.isFileIgnored(path.join(basePath, 'tests/format/jsfmt.spec.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'tests/format/jsfmt.spec.js'))).to.be.true;
expect(configs.isFileIgnored(path.join(basePath, 'tests/format/subdir/foo.js'))).to.be.true;
expect(configs.isFileConfigured(path.join(basePath, 'tests/format/subdir/foo.js'))).to.be.true;
expect(configs.isFileIgnored(path.join(basePath, 'tests/format/subdir/jsfmt.spec.js'))).to.be.false;
expect(configs.isFileConfigured(path.join(basePath, 'tests/format/subdir/jsfmt.spec.js'))).to.be.true;
});

// https://github.com/eslint/eslint/pull/16579/files
Expand All @@ -1629,6 +1683,7 @@ describe('ConfigArray', () => {
const filename = path.resolve(basePath, 'node_modules/foo/bar.js');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.false;
});

it('should return true when a subdirectory is ignored and then we try to unignore a file', () => {
Expand All @@ -1645,6 +1700,7 @@ describe('ConfigArray', () => {
const filename = path.resolve(basePath, 'node_modules/foo/bar.js');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.false;
});

it('should return true when all descendant directories are ignored and then we try to unignore a file', () => {
Expand All @@ -1661,6 +1717,7 @@ describe('ConfigArray', () => {
const filename = path.resolve(basePath, 'node_modules/foo/bar.js');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.false;
});

it('should return true when all descendant directories are ignored without leading slash and then we try to unignore a file', () => {
Expand All @@ -1677,6 +1734,7 @@ describe('ConfigArray', () => {
const filename = path.resolve(basePath, 'node_modules/foo/bar.js');

expect(configs.isFileIgnored(filename)).to.be.true;
expect(configs.isFileConfigured(filename)).to.be.true;
});
});

Expand Down