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

fix: behavior of global ignores #126

Merged
nzakas merged 6 commits into from
Jan 10, 2024
Merged

fix: behavior of global ignores #126

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
add6fe0
fix: behavior of global `ignores`
mdjermanovic Jan 7, 2024
0789125
Update tests/config-array.test.js
mdjermanovic Jan 9, 2024
68ddedd
Update tests/config-array.test.js
mdjermanovic Jan 9, 2024
765ada7
Update tests/config-array.test.js
mdjermanovic Jan 9, 2024
a4bae7f
fix syntax error
mdjermanovic Jan 9, 2024
ff0ccfe
Update tests/config-array.test.js
mdjermanovic Jan 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 1 addition & 32 deletions src/config-array.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -523,38 +523,7 @@ export class ConfigArray extends Array {
* are additional keys, then ignores act like exclusions.
*/
if (config.ignores && Object.keys(config).length === 1) {

/*
* If there are directory ignores, then we need to double up
* the patterns to be ignored. For instance, `foo` will also
* need `foo/**` in order to account for subdirectories.
*/
config.ignores.forEach(ignore => {

result.push(ignore);

if (typeof ignore === 'string') {

// unignoring files won't work unless we unignore directories too
if (ignore.startsWith('!')) {

if (ignore.endsWith('/**')) {
result.push(ignore.slice(0, ignore.length - 3));
} else if (ignore.endsWith('/*')) {
result.push(ignore.slice(0, ignore.length - 2));
}
}

// directories should work with or without a trailing slash
if (ignore.endsWith('/')) {
result.push(ignore.slice(0, ignore.length - 1));
result.push(ignore + '**');
} else if (!ignore.endsWith('*')) {
result.push(ignore + '/**');
}

}
});
result.push(...config.ignores);
}
}

Expand Down
288 changes: 287 additions & 1 deletion tests/config-array.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1291,6 +1291,240 @@ describe('ConfigArray', () => {
expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.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([
{
files: ['**/foo']
},
{
ignores: [
'foo/'
]
}
], {
basePath
});

configs.normalizeSync();

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

it('should return false 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();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.false;
});
Comment on lines +1294 to +1330
Copy link
Contributor Author

@mdjermanovic mdjermanovic Jan 7, 2024

Choose a reason for hiding this comment

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

These two tests were failing before this change (the files were considered ignored).


it('should return true 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();

expect(configs.isFileIgnored(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 `/`', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'foo'
]
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isFileIgnored(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 `/`', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'foo/',
'!foo/'
]
}
], {
basePath
});

configs.normalizeSync();

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

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 `/`', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'foo/**',

// only the directory is unignored, files are not
'!foo/'
]
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isFileIgnored(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 `/`', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'foo/**',

// only the directory is unignored, files are not
'!foo'
]
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isFileIgnored(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 `/**`', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'foo/**',

// both the directory and the files are unignored
'!foo/**'
]
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isFileIgnored(path.join(basePath, 'foo/a.js'))).to.be.false;
});
Comment on lines +1434 to +1454
Copy link
Contributor Author

@mdjermanovic mdjermanovic Jan 7, 2024

Choose a reason for hiding this comment

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

This one was already passing before the change.


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', () => {
configs = new ConfigArray([
{
files: ['**/foo']
},
{
ignores: [
'foo',
'!foo/*',
'!foo/**'
]
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isFileIgnored(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', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'foo',
'!foo/*'
]
}
], {
basePath
});

configs.normalizeSync();

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

// https://github.com/eslint/eslint/issues/17964#issuecomment-1879840650
it('should return true 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();

expect(configs.isFileIgnored(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.isFileIgnored(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;
});
Comment on lines +1456 to +1526
Copy link
Contributor Author

@mdjermanovic mdjermanovic Jan 7, 2024

Choose a reason for hiding this comment

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

These three tests were failing before this change (the files that should have been ignored were considered not ignored).


// https://github.com/eslint/eslint/pull/16579/files
describe('gitignore-style unignores', () => {

Expand Down Expand Up @@ -1555,7 +1789,7 @@ describe('ConfigArray', () => {
});


it('should return true when an ignored directory is later negated with **', () => {
it('should return true when a directory in an ignored directory is later negated with **', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
Expand All @@ -1567,6 +1801,8 @@ describe('ConfigArray', () => {
},
{
ignores: [

// this unignores `node_modules/package/`, but its parent `node_modules/` is still ignored
'!node_modules/package/**'
]
}
Expand All @@ -1580,6 +1816,56 @@ describe('ConfigArray', () => {
expect(configs.isDirectoryIgnored(path.join(basePath, 'node_modules/package/'))).to.be.true;
});

it('should return false when a directory is later negated with **', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'**/node_modules/**'
]
},
{
ignores: [
'!node_modules/**'
]
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isDirectoryIgnored(path.join(basePath, 'node_modules'))).to.be.false;
expect(configs.isDirectoryIgnored(path.join(basePath, 'node_modules/'))).to.be.false;
});
Comment on lines +1819 to +1842
Copy link
Contributor Author

@mdjermanovic mdjermanovic Jan 7, 2024

Choose a reason for hiding this comment

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

This one was already passing before this change.


it('should return true when a directory\'s content is later negated with *', () => {
configs = new ConfigArray([
{
files: ['**/*.js']
},
{
ignores: [
'**/node_modules/**'
]
},
{
ignores: [
'!node_modules/*'
]
}
], {
basePath
});

configs.normalizeSync();

expect(configs.isDirectoryIgnored(path.join(basePath, 'node_modules'))).to.be.true;
expect(configs.isDirectoryIgnored(path.join(basePath, 'node_modules/'))).to.be.true;
});

it('should return true when an ignored directory is later unignored with *', () => {
configs = new ConfigArray([
{
Expand Down