feat: add method isFileConfigured

[fasttime]

This PR adds a method isFileConfigured to ConfigArray, 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

This PR introduces the same changes as humanwhocodes/config-array#138, plus the reformatting done by prettier in the commit hook.

See also the discussion in humanwhocodes/config-array#138.

[nzakas]

I think we should revisit the expected behavior of isFileIgnored() and isExplicitMatch() before we add another method. Specifically:

1. isFileIgnored() - this should probably return true only if a file matches an ignores pattern somewhere along the way, as opposed to right now, where it also returns true if there isn't a files pattern that matches.
2. isExplicitMatch() - this should probably return true only if there's matching files pattern somewhere such that a config will be generated. Maybe rename to isFileMatched()?

So then we could do:

const isFileConfigured = !isFileIgnored && !isExplicitMatch;

[fasttime]

I think we should revisit the expected behavior of isFileIgnored() and isExplicitMatch() before we add another method. Specifically:

1. `isFileIgnored()` - this should probably return `true` only if a file matches an `ignores` pattern somewhere along the way, as opposed to right now, where it also returns `true` if there isn't a `files` pattern that matches.

2. `isExplicitMatch()` - this should probably return `true` only if there's matching `files` pattern somewhere such that a config will be generated. Maybe rename to `isFileMatched()`?

So then we could do:

const isFileConfigured = !isFileIgnored && !isExplicitMatch;

Sounds good 👍🏻I can change the methods and update ESLint to see how that works in practice. This will be a breaking change.

[nzakas]

@mdjermanovic what do you think of #7 (comment)?

[mdjermanovic]

The current isExplicitMatch() differs from other methods in that it can return true in some cases where the config won't be generated (i.e., the file is essentially ignored):

humanwhocodes/config-array#138 (comment).

In ESLint, we're using isExplicitMatch() only for filtering code blocks:

https://github.com/eslint/eslint/blob/d23574c5c0275c8b3714a7a6d3e8bf2108af60f1/lib/eslint/eslint.js#L510-L512

Now, before changing the behavior of isExplicitMatch(), I think we should first figure out whether the mentioned current specifics of it actually make sense for this use case. In other words, is the current behavior where a code block matched only by patterns that end with /* or /** ends up producing this lint error intentional and desirable, or should such code block just be silently skipped.

Edit: I tried making an example where the current behavior would be easily observable with eslint-plugin-markdown, but the lint error we're generating has line: 0 (which seems wrong because lines are 1-based) and then eslint-plugin-markdown's postprocess filters it out as out-of-range.

[nzakas]

Now it's all coming back to me. :) So isExplicitMatch() is important to work as-is in order to allow us to filter out code blocks, otherwise all of the code blocks would always match any pattern ending with * due to its parent filename.

So then we are really talking about modifying isFileIgnored() so that it returns true only for files that explicitly ignored via an ignores entry, whereas isFileConfigured() returns true if getConfig() would return a config object?

[fasttime]

On closer look at the implementation, I think that getConfig() could return null for any of three unrelated reasons:

1. The file is ignored by a global ignore pattern
2. The file has no matching config
3. The file is outside the base path

or some combination of the above. Currently, isFileIgnored will return true for any of the above reasons . My original idea for adding isFileConfigured is that is would return true only if both 1. and 2. are false, i.e. if a file is not globally ignored and has a matching config. I thought this would be sufficient for providing a meaningful error message in ESLint, but, as it seems, I didn't consider reason 3.

[mdjermanovic]

On closer look at the implementation, I think that getConfig() could return null for any of three unrelated reasons:

1. The file is ignored by a global ignore pattern
2. The file has no matching config
3. The file is outside the base path

or some combination of the above. Currently, isFileIgnored will return true for any of the above reasons . My original idea for adding isFileConfigured is that is would return true only if both 1. and 2. are false, i.e. if a file is not globally ignored and has a matching config. I thought this would be sufficient for providing a meaningful error message in ESLint, but, as it seems, I didn't consider reason 3.

Yeah, while it can be argued that 3. is a subset of 2. because all patterns are considered relative to the base path so no pattern can match a file outside of it, it would still be good to show a distinct message for 3.

[mdjermanovic]

Now it's all coming back to me. :) So isExplicitMatch() is important to work as-is in order to allow us to filter out code blocks, otherwise all of the code blocks would always match any pattern ending with * due to its parent filename.

Can you provide an example to clarify why the specific logic of isExplicitMatch() is needed?

I tried replacing isExplicitMatch() with getConfig() here:

https://github.com/eslint/eslint/blob/b67eba4514026ef7e489798fd883beb678817a46/lib/eslint/eslint.js#L510-L512

filterCodeBlock(blockFilename) {
-    return configs.isExplicitMatch(blockFilename);
+    return configs.getConfig(blockFilename) !== void 0;
}

and all tests we have in eslint/eslint are still passing.

[nzakas]

I believe the case was when we had a files pattern such as "foo/**". If we had a file named foo/bar.md that then produced foo/bar.md/0.js, then the files pattern still matched against the block.

[mdjermanovic]

What is the expected behavior in the following case:

// eslint.config.js
export default [
    {
        plugins: {
            "my-plugin": {
                processors: {
                    "my-processor": {

                        // dummy processor that assumes that the whole content of a file is a single ```ts code block 
                        preprocess(text) {
                            const lines = text.split("\n");
                            return [{
                                filename: "foo.ts",
                                text: lines.slice(1, -1).join("\n")
                            }]
                        },

                        // just increment lines to adjust for the first ```ts line
                        postprocess(messages) {
                            const retv = messages[0]; // there's always exactly 1 code block

                            retv.forEach(message => {
                                message.line++
                                message.endLine++; 
                            });

                            return retv;
                        }
                    }
                }
            }
        }
    },
    {
        files: ["**/*.md"],
        processor: "my-plugin/my-processor"
    },
    {
        files: ["**/*"],
        rules: {
            "no-undef": 2
        }
    }
];

a.md:

```ts
bar
```

The virtual .ts file is matched only by **/*. Should the code block be silently skipped or reported as not-configured?

The current behavior is:

C:\projects\tmp\tmp\a.md
  1:0  warning  No matching configuration found for C:\projects\tmp\tmp\a.md\0_foo.ts

✖ 1 problem (0 errors, 1 warning)

[nzakas]

The virtual .ts file is matched only by **/*. Should the code block be silently skipped or reported as not-configured?

Yes, so this is exactly the case I was targeting. IMHO, we should silently skip code blocks that don't have an explicit match. It's quite possible that people won't want to lint every code block, especially if it's not JavaScript. Outputting a warning for every code block that doesn't have a matching config seems likely to only create noise and frustration. Think of a Markdown file that has code examples in multiple languages, most of which ESLint does not lint.

[mdjermanovic]

The virtual .ts file is matched only by **/*. Should the code block be silently skipped or reported as not-configured?

Yes, so this is exactly the case I was targeting. IMHO, we should silently skip code blocks that don't have an explicit match. It's quite possible that people won't want to lint every code block, especially if it's not JavaScript. Outputting a warning for every code block that doesn't have a matching config seems likely to only create noise and frustration. Think of a Markdown file that has code examples in multiple languages, most of which ESLint does not lint.

I agree, but isExplicitMatch() doesn't work that way. In the above example, it returns true for the virtual .ts file because it matches **/*, and at the end ESLint outputs No matching configuration found for C:\projects\tmp\tmp\a.md\0_foo.ts warning.