-
-
Notifications
You must be signed in to change notification settings - Fork 11
Conversation
/** | ||
* 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) { |
There was a problem hiding this comment.
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.
|
||
}); | ||
|
||
describe('isFileIgnored() / isFileConfigured()', () => { |
There was a problem hiding this comment.
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.
Does |
I tried that but it breaks the tests in ESLint. It looks like configs = new ConfigArray([
{
ignores: ['**/bar.*']
},
{
rules: { semi: 'error' }
}
]);
configs.normalizeSync();
console.log(configs.isFileIgnored('bar.txt')); // true
console.log(configs.isExplicitMatch('bar.txt')); // false
console.log(configs.isFileConfigured('bar.txt')); // true |
/** | ||
* 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; | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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.
Note: there's a functional difference between
const configs1 = new ConfigArray([
{
files: ["foo/*.txt"]
}
]);
configs1.normalizeSync();
console.log(configs1.getConfig('foo/bar.txt')); // {}
console.log(configs1.isFileIgnored('foo/bar.txt')); // false
console.log(configs1.isExplicitMatch('foo/bar.txt')); // true
const configs2 = new ConfigArray([
{
files: ["foo/*"]
}
]);
configs2.normalizeSync();
console.log(configs2.getConfig('foo/bar.txt')); // undefined
console.log(configs2.isFileIgnored('foo/bar.txt')); // true
console.log(configs2.isExplicitMatch('foo/bar.txt')); // true! |
Let's move this over to https://github.com/eslint/rewrite. I'd like to start using the official ESLint version going forward. |
I've created a PR in the rewrite repo: eslint/rewrite#7 |
This PR adds a method
isFileConfigured
toConfigArray
, to determine if a file has matching configuration or not. This could be used in ESLint to provide a better message for ignored files that don't have a matching configuration.Refs eslint/eslint#18263