Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: remove support for ignore files in FlatESLint #16355

Merged
merged 7 commits into from Oct 3, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
4 changes: 2 additions & 2 deletions Makefile.js
Expand Up @@ -518,11 +518,11 @@ target.lint = function([fix = false] = []) {
* when analyzing `require()` calls from CJS modules in the `docs` directory. Since our release process does not run `npm install`
* in the `docs` directory, linting would fail and break the release. Also, working on the main `eslint` package does not require
* installing dependencies declared in `docs/package.json`, so most contributors will not have `docs/node_modules` locally.
* Therefore, we add `--ignore-pattern docs` to exclude linting the `docs` directory from this command.
* Therefore, we add `--ignore-pattern "docs/**"` to exclude linting the `docs` directory from this command.
* There is a separate command `target.lintDocsJS` for linting JavaScript files in the `docs` directory.
*/
echo("Validating JavaScript files");
lastReturn = exec(`${ESLINT}${fix ? "--fix" : ""} . --ignore-pattern docs`);
lastReturn = exec(`${ESLINT}${fix ? "--fix" : ""} . --ignore-pattern "docs/**"`);
Copy link
Member Author

Choose a reason for hiding this comment

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

I've updated this to be how we expect ignore patterns to work, but it would work the same without this change because of #16354.

if (lastReturn.code !== 0) {
errors++;
}
Expand Down
26 changes: 13 additions & 13 deletions docs/src/user-guide/configuring/configuration-files-new.md
Expand Up @@ -76,7 +76,7 @@ You can limit which files a configuration object applies to by specifying a comb

```js
export default [
{
{
files: ["src/**/*.js"],
rules: {
semi: "error"
Expand All @@ -89,7 +89,7 @@ Here, only the JavaScript files in the `src` directory will have the `semi` rule

```js
export default [
{
{
files: ["src/**/*.js"],
ignores: ["**/*.config.js"],
rules: {
Expand All @@ -103,7 +103,7 @@ This configuration object matches all JavaScript files in the `src` directory ex

```js
export default [
{
{
files: ["src/**/*.js"],
ignores: ["**/*.config.js", "!**/eslint.config.js"],
rules: {
Expand All @@ -119,7 +119,7 @@ If `ignores` is used without `files` and any other setting, then the configurati

```js
export default [
{
{
ignores: ["**/*.config.js"],
rules: {
semi: "error"
Expand All @@ -132,17 +132,17 @@ This configuration object applies to all files except those ending with `.config

#### Globally ignoring files with `ignores`

If `ignores` is used without any other keys in the configuration object, then the patterns act as additional global ignores, similar to those found in `.eslintignore`. Here's an example:
If `ignores` is used without any other keys in the configuration object, then the patterns act as global ignores. Here's an example:

```js
export default [
{
{
ignores: [".config/*"]
}
];
```

This configuration specifies that all of the files in the `.config` directory should be ignored. This pattern is added after the patterns found in `.eslintignore`.
This configuration specifies that all of the files in the `.config` directory should be ignored. This pattern is added after the default patterns, which are `["**/node_modules/**", ".git/**"]`.

#### Cascading configuration objects

Expand All @@ -156,16 +156,16 @@ export default [
globals: {
MY_CUSTOM_GLOBAL: "readonly"
}
}
}
},
{
{
files: ["tests/**/*.js"],
languageOptions: {
globals: {
it: "readonly",
describe: "readonly"
}
}
}
}
];
```
Expand Down Expand Up @@ -343,7 +343,7 @@ export default [
rules: {
"jsdoc/require-description": "error",
"jsdoc/check-values": "error"
}
}
}
];
```
Expand All @@ -364,7 +364,7 @@ export default [
rules: {
"jsdoc/require-description": "error",
"jsdoc/check-values": "error"
}
}
}
];
```
Expand All @@ -383,7 +383,7 @@ export default [
rules: {
"jsd/require-description": "error",
"jsd/check-values": "error"
}
}
}
];
```
Expand Down
17 changes: 17 additions & 0 deletions eslint.config.js
Expand Up @@ -78,6 +78,23 @@ function createInternalFilesPatterns(pattern = null) {

module.exports = [
...compat.extends("eslint"),
{
ignores: [
"build/**",
"coverage/**",
"docs/**",
"!docs/.eleventy.js",
"jsdoc/**",
"templates/**",
"tests/bench/**",
"tests/fixtures/**",
"tests/performance/**",
"tmp/**",
"tools/internal-rules/node_modules/**",
"**/test.js",
"!**/.eslintrc.js"
Copy link
Member

Choose a reason for hiding this comment

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

Just double-checking: this still doesn't unignore .eslintrc.js correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Correct. When doing a glob search with a pattern that doesn't have an explicit dot that matches leading dots in file names, .eslintrc.js will still be ignored because globby doesn't support negative patterns in ignore (in fact, I believe it removes leading ! so this actually ignores **/.eslintrc.js), and in general globby's dotfiles matching behavior seems to depend only on the content of search patterns.

On the other hand, when directly using our config array to evaluate whether or not .eslintrc.js is ignored (e.g., when passing a file path), it's not ignored regardless of this line. This behavior needs further analysis (#16265).

Overall, this line currently does not affect anything, but it may when we fix #16265 so I've left it in for now.

Copy link
Member

Choose a reason for hiding this comment

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

That’s what I figured. 👍

]
},
{
plugins: {
"internal-rules": internalPlugin,
Expand Down
5 changes: 2 additions & 3 deletions lib/cli.js
Expand Up @@ -25,7 +25,6 @@ const fs = require("fs"),
RuntimeInfo = require("./shared/runtime-info");
const { Legacy: { naming } } = require("@eslint/eslintrc");
const { findFlatConfigFile } = require("./eslint/flat-eslint");
const { gitignoreToMinimatch } = require("@humanwhocodes/gitignore-to-minimatch");
const { ModuleImporter } = require("@humanwhocodes/module-importer");

const debug = require("debug")("eslint:cli");
Expand Down Expand Up @@ -145,7 +144,7 @@ async function translateOptions({

if (ignorePattern) {
overrideConfig.push({
ignores: ignorePattern.map(gitignoreToMinimatch)
ignores: ignorePattern
});
}

Expand Down Expand Up @@ -182,7 +181,6 @@ async function translateOptions({
fix: (fix || fixDryRun) && (quiet ? quietFixPredicate : true),
fixTypes: fixType,
ignore,
ignorePath,
overrideConfig,
overrideConfigFile,
reportUnusedDisableDirectives: reportUnusedDisableDirectives ? "error" : void 0
Expand All @@ -193,6 +191,7 @@ async function translateOptions({
options.rulePaths = rulesdir;
options.useEslintrc = eslintrc;
options.extensions = ext;
options.ignorePath = ignorePath;
}

return options;
Expand Down
8 changes: 3 additions & 5 deletions lib/eslint/eslint-helpers.js
Expand Up @@ -410,7 +410,6 @@ function processOptions({
fixTypes = null, // ← should be null by default because if it's an array then it suppresses rules that don't have the `meta.type` property.
globInputPaths = true,
ignore = true,
ignorePath = null, // ← should be null by default because if it's a string then it may throw ENOENT.
ignorePatterns = null,
overrideConfig = null,
overrideConfigFile = null,
Expand Down Expand Up @@ -441,6 +440,9 @@ function processOptions({
if (unknownOptionKeys.includes("globals")) {
errors.push("'globals' has been removed. Please use the 'overrideConfig.languageOptions.globals' option instead.");
}
if (unknownOptionKeys.includes("ignorePath")) {
errors.push("'ignorePath' has been removed.");
}
if (unknownOptionKeys.includes("ignorePattern")) {
errors.push("'ignorePattern' has been removed. Please use the 'overrideConfig.ignorePatterns' option instead.");
}
Expand Down Expand Up @@ -493,9 +495,6 @@ function processOptions({
if (typeof ignore !== "boolean") {
errors.push("'ignore' must be a boolean.");
}
if (!isNonEmptyString(ignorePath) && ignorePath !== null) {
errors.push("'ignorePath' must be a non-empty string or null.");
}
if (typeof overrideConfig !== "object") {
errors.push("'overrideConfig' must be an object or null.");
}
Expand Down Expand Up @@ -538,7 +537,6 @@ function processOptions({
fixTypes,
globInputPaths,
ignore,
ignorePath,
ignorePatterns,
reportUnusedDisableDirectives
};
Expand Down
50 changes: 3 additions & 47 deletions lib/eslint/flat-eslint.js
Expand Up @@ -16,7 +16,6 @@ const findUp = require("find-up");
const { version } = require("../../package.json");
const { Linter } = require("../linter");
const { getRuleFromConfig } = require("../config/flat-config-helpers");
const { gitignoreToMinimatch } = require("@humanwhocodes/gitignore-to-minimatch");
const {
Legacy: {
ConfigOps: {
Expand All @@ -28,7 +27,6 @@ const {
} = require("@eslint/eslintrc");

const {
fileExists,
findFiles,
getCacheFile,

Expand Down Expand Up @@ -76,9 +74,8 @@ const LintResultCache = require("../cli-engine/lint-result-cache");
* @property {boolean|Function} [fix] Execute in autofix mode. If a function, should return a boolean.
* @property {string[]} [fixTypes] Array of rule types to apply fixes for.
* @property {boolean} [globInputPaths] Set to false to skip glob resolution of input file paths to lint (default: true). If false, each input file paths is assumed to be a non-glob path to an existing file.
* @property {boolean} [ignore] False disables use of .eslintignore.
* @property {string} [ignorePath] The ignore file to use instead of .eslintignore.
* @property {string[]} [ignorePatterns] Ignore file patterns to use in addition to .eslintignore.
* @property {boolean} [ignore] False disables all ignore patterns except for the default ones.
* @property {string[]} [ignorePatterns] Ignore file patterns to use in addition to config ignores.
* @property {ConfigData} [overrideConfig] Override config object, overrides all configs used with this instance
* @property {boolean|string} [overrideConfigFile] Searches for default config file when falsy;
* doesn't do any config file lookup when `true`; considered to be a config filename
Expand Down Expand Up @@ -151,30 +148,6 @@ function calculateStatsPerRun(results) {
});
}

/**
* Loads global ignore patterns from an ignore file (usually .eslintignore).
* @param {string} filePath The filename to load.
* @returns {ignore} A function encapsulating the ignore patterns.
* @throws {Error} If the file cannot be read.
* @private
*/
async function loadIgnoreFilePatterns(filePath) {
debug(`Loading ignore file: ${filePath}`);

try {
const ignoreFileText = await fs.readFile(filePath, { encoding: "utf8" });

return ignoreFileText
.split(/\r?\n/gu)
.filter(line => line.trim() !== "" && !line.startsWith("#"));

} catch (e) {
debug(`Error reading ignore file: ${filePath}`);
e.message = `Cannot read ignore file: ${filePath}\nError: ${e.message}`;
throw e;
}
}

/**
* Create rulesMeta object.
* @param {Map<string,Rule>} rules a map of rules from which to generate the object.
Expand Down Expand Up @@ -319,7 +292,6 @@ async function calculateConfigArray(eslint, {
overrideConfig,
configFile,
ignore: shouldIgnore,
ignorePath,
ignorePatterns
}) {

Expand Down Expand Up @@ -364,22 +336,6 @@ async function calculateConfigArray(eslint, {
configs.push(...slots.defaultConfigs);

let allIgnorePatterns = [];
let ignoreFilePath;

// load ignore file if necessary
if (shouldIgnore) {
if (ignorePath) {
ignoreFilePath = path.resolve(cwd, ignorePath);
allIgnorePatterns = await loadIgnoreFilePatterns(ignoreFilePath);
} else {
ignoreFilePath = path.resolve(cwd, ".eslintignore");

// no error if .eslintignore doesn't exist`
if (fileExists(ignoreFilePath)) {
allIgnorePatterns = await loadIgnoreFilePatterns(ignoreFilePath);
}
}
}

// append command line ignore patterns
if (ignorePatterns) {
Expand Down Expand Up @@ -428,7 +384,7 @@ async function calculateConfigArray(eslint, {
* so they can override default ignores.
*/
configs.push({
ignores: allIgnorePatterns.map(gitignoreToMinimatch)
ignores: allIgnorePatterns
});
}

Expand Down
16 changes: 11 additions & 5 deletions lib/options.js
Expand Up @@ -129,6 +129,16 @@ module.exports = function(usingFlatConfig) {
};
}

let ignorePathFlag;

if (!usingFlatConfig) {
ignorePathFlag = {
option: "ignore-path",
type: "path::String",
description: "Specify path of ignore file"
};
}

return optionator({
prepend: "eslint [options] file.js [file.js] [dir]",
defaults: {
Expand Down Expand Up @@ -203,11 +213,7 @@ module.exports = function(usingFlatConfig) {
{
heading: "Ignoring files"
},
{
option: "ignore-path",
type: "path::String",
description: "Specify path of ignore file"
},
ignorePathFlag,
{
option: "ignore",
type: "Boolean",
Expand Down
1 change: 0 additions & 1 deletion package.json
Expand Up @@ -57,7 +57,6 @@
"dependencies": {
"@eslint/eslintrc": "^1.3.2",
"@humanwhocodes/config-array": "^0.10.5",
"@humanwhocodes/gitignore-to-minimatch": "^1.0.2",
"@humanwhocodes/module-importer": "^1.0.1",
"ajv": "^6.10.0",
"chalk": "^4.0.0",
Expand Down
Empty file.
3 changes: 3 additions & 0 deletions tests/fixtures/cli-engine/eslint.config_with_ignores2.js
@@ -0,0 +1,3 @@
module.exports = [{
ignores: ["**/fixtures/**"]
}];
8 changes: 8 additions & 0 deletions tests/fixtures/eslint.config_with_ignores.js
@@ -0,0 +1,8 @@
const eslintConfig = require("./eslint.config.js");

module.exports = [
eslintConfig,
{
ignores: ["**/*.json", "**/*.js"]
}
];
8 changes: 8 additions & 0 deletions tests/fixtures/eslint.config_with_ignores2.js
@@ -0,0 +1,8 @@
const eslintConfig = require("./eslint.config.js");

module.exports = [
eslintConfig,
{
ignores: ["**/undef.js", "undef2.js", "**/undef3.js"]
}
];