Allow processor API to be configurable and to formally be able to lint both a file and its blocks

[brettz9]

Update: see updated description below

The version of ESLint you are using.

7.29.0

The problem you want to solve.

A few facts:

1. Per the v7.29.0 blog release, further changes are anticpated toward dropping CLIEngine.

2. Its replacement, the ESLint class, relies on some async-only methods.

3. However, ESLint rules do not, and per @nzakas in this comment, there have been no discussions or apparent interest in making Linter async so that asynchronous rules could be supported.

This all means that if CLIEngine is dropped, rules cannot take advantage of ESLint linting within their own rules.

Why would one want to run linting within a linting rule?

In eslint-plugin-jsdoc, we allow it in three cases all within our jsdoc/check-examples rule:

1. To lint JavaScript code within @example tags

2. To lint a JavaScript value within @default/@defaultvalue tags

3. To lint a JavaScript expression within the likes of @param {type} [name=default] or @property {type} [name=default]

Your take on the correct solution to problem.

Besides adding synchronous methods, I would think that allowing some config to be passed to the ESLint class which triggered use of the synchronous rather than asynchronous methods would be sufficient.

Are you willing to submit a pull request to implement this change?

Yes (if my health-based limits on energy and concentration allow).

[nzakas]

Can you explain how you're using CLIEngine in the use cases you mention? This is the first I've heard of someone using CLIEngine within a rule, so it would be helpful to have some context as to why you're doing that instead of using Linter.

[brettz9]

Sure, basically we allow users to store the configuration they want to be used for their @example, etc., as ESLint overrides, and we leverage its useEslintrc option (or configFile) to be able to do the checking.

While users can pass in a baseConfig object with their rules instead (and/or optionally leverage some suggested default rules), disabling eslintrc for the performance benefit, the RC approach has the advantage of being able to set a filename which can be shared with other overrides which may utilize the same kind of configuration.

For example, the default behavior for @example, is to pass in a filename to CLIEngine#getConfigForFile, using the filename of the file being linted, but switching its extension to .md/*.js. This is because for those linting Markdown with such as eslint-plugin-markdown, the types of rules one wishes to disable, e.g., strict, eol-last, no-unused-vars, etc., while great in normal files, tend to be bulky or require unneeded boilerplate when the intent is to just show a snippet within tutorials or within an @example.

[btmills]

This use case sounds very similar to #14198. Here, we want to lint a JS file and separately lint expressions from JSDoc comments. #14198 wants to lint a Markdown file and separately lint the code blocks within.

I’m not thrilled about the API proposed in #14198. Similarly, the API proposed in this issue has some downsides including performance as you’ve already mentioned.

Looking at these two use cases together gave me an idea:

eslint-plugin-jsdoc wants to extract chunks from JSDoc comments. That’s exactly what ESLint’s processor API is for. However, that API assumes that we lint either a file XOR the blocks from a processor. Because of that restriction, eslint-plugin-JSDoc cannot be written as a processor today. If it were, it would prevent linting the parent JS file.

What if we removed that limitation? If we could lint both parent.js and run a JSDoc processor on it to lint JSDoc expressions as parent.js/x_y.jsdoc, eslint-plugin-jsdoc wouldn’t need to call back into CLIEngine from within a rule anymore. That removes performance overhead, allows it to use the built-in config system for the JSDoc expressions, and probably simplifies the plugin/rule implementation considerably. It also makes #14198 a first-class supported use case.

[JounQin]

Because of that restriction, eslint-plugin-JSDoc cannot be written as a processor today. If it were, it would prevent linting the parent JS file.

@btmills If I don't understand incorrectly, it is not true.

const jsoc = {
  preprocess(text, filename) {
    return [
      text, // presents the parent file
      ...extractExamplesInJsDoc(text), // lint examples at the same time
    ]
  }
}

We're using this pattern at eslint-plugin-mdx.

[JounQin]

This issue is very similar with my original issue #14203, and I just ended up by using my pattern above.

But I'm not quite sure why #14198 is related, #14198 is for reading more info from messages in postprocess.

[brettz9]

Ok, good info and sounds promising as a possibility, but a few concerns with the processor approach at this point (not sure anything else would come up) are:

1. It does thankfully look per @JounQin 's example that we can pass on the whole text (since we wouldn't our other rules to be unaware of the @example blocks' presence), but are there restrictions on a processor targeting js files?
2. I'm unclear from the docs on whether one can get away with say targeting just *.md or say *.md/* to say apply the same semi rule to both ts and js blocks. (I would suspect so, and this is more a curiosity than a critical need, but I would like the ability for things to be specified generically as well as specifically if going this route.)
3. It seems a processor cannot lint files of one extension and then claim to be equivalent to another as we have effectively been doing. This is what has allowed us to have these JS-within-JS blocks be linted by default as filename.md/*.js allowing for the same rules to be applied. I guess one can set-and-forget an overrides entry to lint both that and *.js/*.js.

[JounQin]

I think code block with custom filename + overrides maybe is all you want.

const jsdoc = {
  preprocess() {
    return [
      text,
      {
        filename: 'jsdoc-example' + getExtFromJsdoc(text),
      },
    ]
  },
}

{
  "overrides": [
    {
      "files": ["*.js", "*.ts"],
      "processor": "jsdoc/example" // a pretended value here
    },
    {
      "files": [
        "*.js/*_jsdoc-example.js",
        "*.ts/*_jsdoc-example.js",
        "*.js/*_jsdoc-example.ts"
      ],
      "rules": {
        // specific rules for examples in jsdoc only here
        // And other rules for `.js` and `.ts` will also be enabled for them
      }
    }
  ]
}

[btmills]

1. We shouldn’t be special casing JS files in processor handling. Returning the file’s entire text as an unnamed first block in @JounQin‘s example works around the “lint file XOR lint blocks” assumption today. We know it works on MD files, but I’m not aware of anyone trying it on JS files yet. We’d want to make sure the parent JS file looks to the rules during its lint run just as it would if there hadn’t been any processor. Right now ESLint calls processors that return strings the “legacy” API, and processors using the newer API return objects that give blocks virtual filenames. Mixing them as in @JounQin’s example wasn’t considered. If that’s the API we adopt for this use case, even if it happens to work already, we’ll need to add tests for it. If the processor approach is satisfactory, I’m open to a different API as well, so returning the file as a plain string block first can be the first option in that pool.
2. With the new processor API where blocks have virtual filenames, users can configure them specifically. Is that what you’re asking? So for example, a JSDoc processor could return blocks with annotation-specific extensions like example.jsdoc.js or returns.jsdoc, and configs could target *.js/*.example.jsdoc.js or *.ts/*.returns.jsdoc. If you give it the JS extension, it gets run through ESLint as a regular JS file. For expressions that wouldn’t parse as JS, if you end the extension in .jsdoc, then you could even supply your own parser for those JSDoc fragments and configure it on *.JSDoc files.
3. We had a bug, overrides with processor works unexpectedly #14207, where ESLint would skip blocks that had the same extension as their parent file, but @JounQin fixed it. Is that what you mean? Blocks with virtual filenames like parent.js/0_0.js should be linted just fine now.

[brettz9]

1. We shouldn’t be special casing JS files in processor handling. Returning the file’s entire text as an unnamed first block in @JounQin‘s example works around the “lint file XOR lint blocks” assumption today. We know it works on MD files, but I’m not aware of anyone trying it on JS files yet. We’d want to make sure the parent JS file looks to the rules during its lint run just as it would if there hadn’t been any processor. Right now ESLint calls processors that return strings the “legacy” API, and processors using the newer API return objects that give blocks virtual filenames. Mixing them as in @JounQin’s example wasn’t considered. If that’s the API we adopt for this use case, even if it happens to work already, we’ll need to add tests for it. If the processor approach is satisfactory, I’m open to a different API as well, so returning the file as a plain string block first can be the first option in that pool.

Ok, good to know, and can hopefully look into it on our end to see if it works within the next week or two.

But yeah, we would definitely need the full content supplied again, as we wouldn't want chunks taken out of the JSDoc blocks, at least before the rest of eslint-plugin-jsdoc gets to them.

2. With the new processor API where blocks have virtual filenames, users can configure them specifically. Is that what you’re asking? So for example, a JSDoc processor could return blocks with annotation-specific extensions like `example.jsdoc.js` or `returns.jsdoc`, and configs could target `*.js/*.example.jsdoc.js` or `*.ts/*.returns.jsdoc`. If you give it the JS extension, it gets run through ESLint as a regular JS file. For expressions that wouldn’t parse as JS, if you end the extension in `.jsdoc`, then you could even supply your own parser for those JSDoc fragments and configure it on `*.JSDoc` files.

I was just curious if an expression like *.md/* would work to catch all fenced blocks within Markdown (or that *.js/* would do so for us). It seems it would, but even if not, setting-and-forgetting something like {overrides: [{files: '*.md/*.js', '*.md/*.ts'}]} is not too big of a deal.

Helpful to consider the idea though of passing on a special case for any unrecognized blocks, e.g., perhaps we can still extract out the contents within the likes of: @example ```python ... ``` or @example ```md ... ``` (head is starting to spin considering the recursive possibilities!).

3. We had a bug, [`overrides` with processor works unexpectedly #14207](https://github.com/eslint/eslint/issues/14207), where ESLint would skip blocks that had the same extension as their parent file, but @JounQin fixed it. Is that what you mean? Blocks with virtual filenames like `parent.js/0_0.js` should be linted just fine now.

No, thanks, I meant that since the rules for @example that we parse are often similar to those for Markdown, we could provide the default option for the processor output filenames to be treated as filename.md/*.js. Although processors don't, AFAICT, seem to be able to accept or read config, we could I suppose provide one processor which produces filename.js/*.js and another the user could choose that produced filename.*.md/*.js. In any case, I don't think it is too critical with the ability for overrides to be set up which target both.

[btmills]

Aha, now I think I see how Markdown figures in. When you see an @example in a JSDoc comment, you want to ESLint to treat the example’s contents as Markdown, which could further contain JS, TS, or any other blocks. Do I have that right? (If I’m still not quite there, a snippet of such a comment might help.)

Given a file parent.js, today the plugin would run a JSDoc example’s contents through CLIEngine with the filename set to parent.md? If the plugin were implemented as a processor, ESLint would instead treat the JSDoc example block as parent.js/${index}.md, or any extension you chose in place of .md.

I might recommend getting even more specific in case someone needs to configure JSDoc blocks separately from regular Markdown blocks. For example, you could return .example.jsdoc, and the virtual filename for that block would be parent.js/${index}.example.jsdoc. Users or your recommended config could then override *.js/*.jsdoc or more specifically *.js/*.example.jsdoc (to distinguish it from a hypothetical .returns.jsdoc) to apply a Markdown processor to all JSDoc @examples. The Markdown processor could then find JS or TS code blocks, whose virtual file names would follow the pattern parent.js/${index}.example.jsdoc/${index}.ts. (Filenames don’t have to end in .md to run through the Markdown processor. ESLint uses whatever the use or your recommended config says.)

If instead someone wrote their examples in TS without wrapping them in Markdown code blocks, they could configure *.js/*.example.jsdoc to use @typescript-eslint’s parser and plugins.

Using a specific extension gives the user the ability to target JSDoc’s code blocks specifically in their config.

[brettz9]

Aha, now I think I see how Markdown figures in. When you see an @example in a JSDoc comment, you want to ESLint to treat the example’s contents as Markdown, which could further contain JS, TS, or any other blocks. Do I have that right? (If I’m still not quite there, a snippet of such a comment might help.)

You've gotten the important gist in the rest of your message, but FWIW, the main reasons I've mentioned Markdown are:

1. It is an already existing processor, so in my mind, it was clearer to use it as the example, but I can see in the context of this issue, my side-discussing it may have been confusing.
2. We simulate it (as you correctly say in the except below) because Markdown like:

## Usage
Here's how to use it:

```js
const result = runMe();
```

...tends to need to have similar rules disabled (such as no-unused-vars) as does @example:

/**
 * @example
 * const result = runMe();
 */
 function runMe () {
 }

But also, yes, the (Markdown-inspired) fenced block syntax is something which we've optionally allowed to denote the coding language within @example (by our allowing whitelist/blacklist regexes to indicate the portion of @example which should be linted).

Btw, another problem I see with our going with a processor approach is that with a rule we can have configuration like these optional regexes (important in this case since JSDoc doesn't really formalize how @example contents should be interpreted--it's own default documenter does auto-detection, but that is less than satisfying for some, but not every project wants to require use of fenced blocks either).

We also have config (that a processor couldn't implement without some ability to supply configuraton to it) for:

1. Whether the JSDoc-blessed use of <caption> within @example is used
2. A padded intent option to assume a given fixed amount of padding (so that ESLint's indent is not wrongly triggered).
3. Whether to apply some default disabling of rules like no-unused-vars
4. Whether to check @example, @default, and/or param/property defaults.

Given a file parent.js, today the plugin would run a JSDoc example’s contents through CLIEngine with the filename set to parent.md? If the plugin were implemented as a processor, ESLint would instead treat the JSDoc example block as parent.js/${index}.md, or any extension you chose in place of .md.

Almost exactly except that, being aware of the more recent ESLint processor syntax, we've already switched from simulating (in CLIEngine) the filename of parent.md to become: parent.md/*.js. But we make this configurable so the user can override this behavior to have a filename of whatever they want, including avoiding .md, and going, as you say, to something like .example.jsdoc.

I might recommend getting even more specific in case someone needs to configure JSDoc blocks separately from regular Markdown blocks. For example, you could return .example.jsdoc, and the virtual filename for that block would be parent.js/${index}.example.jsdoc. Users or your recommended config could then override *.js/*.jsdoc or more specifically *.js/*.example.jsdoc (to distinguish it from a hypothetical .returns.jsdoc) to apply a Markdown processor to all JSDoc @examples. The Markdown processor could then find JS or TS code blocks, whose virtual file names would follow the pattern parent.js/${index}.example.jsdoc/${index}.ts. (Filenames don’t have to end in .md to run through the Markdown processor. ESLint uses whatever the use or your recommended config says.)

If instead someone wrote their examples in TS without wrapping them in Markdown code blocks, they could configure *.js/*.example.jsdoc to use @typescript-eslint’s parser and plugins.

Using a specific extension gives the user the ability to target JSDoc’s code blocks specifically in their config.

Sure. As a processor that approach makes sense (and in fact we do add custom extensions currently for @default and param/property defaults, since those tend to require even laxer rules than Markdown, e.g., allowing plain expressions). But with config that can simulate this already, we've just defaulted to parent.md/*.js though for the benefits that out-of-the-box, the easiest scenario is to assume that the rules for @example need not have full context, just like in typical Markdown examples.

[btmills]

Thank you for getting me up to speed here.

As I understand it, there are two limitations of the processor API that would need to be resolved before it’s a good fit for eslint-plugin-jsdoc:

1. There needs to be an officially-recommended way to run a file through a processor to lint its blocks while still linting the file itself. We could standardize the return [text, …blocks]; approach or define some other API.
2. Users need to be able to configure a processor’s behavior.

Digging into some of your configuration cases, some might be possible today by configuring rules without having to configure a processor:

Whether to apply some default disabling of rules like no-unused-vars

This could be configured by overrides for *.js/*.jsdoc extending e.g. eslint-plugin-jsdoc/commonly-disabled.

Whether to check @example, @default, and/or param/property defaults.

If the processor were to return specific extensions, these could be configured via overrides as well that might match *.example.jsdoc and *.default.property.jsdoc but omit others like *.default.param.jsdoc.

The other cases (whether to allow <caption> and applying fixed leading indent) would require more thought. They feel like parser options, as in “how should the processor’s parser behave?”

1. Is my understanding of those two primary blockers correct?
2. Given what we’ve determined so far about how it could work, stepping back a bit, does the processor approach seem viable to you? If we can answer the remaining questions, does this seem like the right approach for eslint-plugin-jsdoc?

[brettz9]

As I understand it, there are two limitations of the processor API that would need to be resolved before it’s a good fit for eslint-plugin-jsdoc:

1. There needs to be an officially-recommended way to run a file through a processor to lint its blocks while still linting the file itself. We could standardize the `return [text, …blocks];` approach or define some other API.

2. Users need to be able to configure a processor’s behavior.

Sounds right.

Digging into some of your configuration cases, some might be possible today by configuring rules without having to configure a processor:

Whether to apply some default disabling of rules like no-unused-vars

This could be configured by overrides for *.js/*.jsdoc extending e.g. eslint-plugin-jsdoc/commonly-disabled.

Ah, elegant, hadn't thought of that, thanks. I kind of like how it is all in one place now, but your approach feels right being as it is a kind of config, and could be reusable (e.g., even to just apply the rules elsewhere instead, like with Markdown).

Whether to check @example, @default, and/or param/property defaults.

If the processor were to return specific extensions, these could be configured via overrides as well that might match *.example.jsdoc and *.default.property.jsdoc but omit others like *.default.param.jsdoc.

Sure, makes sense too.

The other cases (whether to allow <caption> and applying fixed leading indent) would require more thought. They feel like parser options, as in “how should the processor’s parser behave?”

Yeah. I guess a processorOptions or such would seem suitable from my perspective.

Also, FWIW, I wonder if having such config might help with allowing eslint-plugin-html to use a proper processor as the issue of config came up there too.

1. Is my understanding of those two primary blockers correct?

Yup.

2. Given what we’ve determined so far about how it could work, stepping back a bit, does the processor approach seem viable to you? If we can answer the remaining questions, does this seem like the right approach for `eslint-plugin-jsdoc`?

Yes, I think it sounds very good. And glad the rule won't be blocking, as the RC reading does make the rule more processor intensive than the other rules.

[nzakas]

What’s the next step here? If there’s an agreement on the category of this issue, it should be moved out of Triaging.

[brettz9]

To adapt @btmills ' reply #14745 (comment) accurate summary of the remaining needs, it would be to address two limitations of the processor API:

1. Adopt an officially-recommended way to run a file through a processor to lint its blocks while still linting the file itself. (This would be usable, e.g., by our jsdoc plugin, to lint the JavaScript within @example while linting the rest of the file.)

Need to determine whether to standardize the current informal return [text, …blocks]; approach (and document it as such) or define some other API.

2. Means to configure a processor’s behavior, supplying it config, e.g., for the jsdoc plugin to conditionally parse out <caption> or apply a fixed leading indent.

[nzakas]

So is the next step here to wait for an RFC? @btmills

[btmills]

Yes, assuming there are no objections, I need to write an RFC that will allow linting a file and extracting its blocks with a processor and linting those too.

There's already an unofficial way to do that mentioned in #14198, so the RFC may just pave that cow path and add make it configurable.

The second half of this is asking for processor configuration, but if the RFC unblocks the first half, I believe the second half can be achieved with parser options for the processor's parser and may not need any further changes.

[nzakas]

@eladavron thats too off topic for this issue. Please open a discussion if you’d like to discuss further.

[btmills]

I looked at standardizing the existing workaround in which preprocess() returns the full file text as the first "block" in the array. However, that mean would mean the processor and not the user determines when to further lint the file. In contrast, the processor API change released in ESLint v6 was all about giving users control over processors.

I'm now thinking the processor key should optionally accept an array of multiple processors to apply, and the special value "passthrough" would tell ESLint to lint the file as normal in addition to running any other processors:

const config = {
    files: ["*.js"],
    processor: [
        // Continue to lint this file as normal...
        "passthrough",
        // ...but also extract and lint any JSDoc blocks.
        "jsdoc/processor"
    ]
};

[nzakas]

I like the idea behind this, though I’m not thrilled with having a special valid to consider. And would it be possible to specify additional processors too? Like let’s say we have a markdown file with both JS and CSS codeblocks?

[btmills]

would it be possible to specify additional processors too? Like let’s say we have a markdown file with both JS and CSS codeblocks?

Sure! Your specific example, a Markdown file containing JS and CSS code blocks doesn’t need it because eslint-plugin-markdown already extracts all fenced code blocks, and ESLint implicitly ignores example.md/*.css unless someone configures an override for .css files.

However, let’s say you wanted to extract all fenced code blocks using eslint-plugin-markdown and wrote a plugin to extract indented code as blocks as well: processor: [require(“eslint-plugin-markdown”).processor, require(“./markdown-indented-code”).processor({ extension: “js” })]

Or, if you had a JSDoc-annotated JS file that had embedded GraphQL queries and CSS-in-JS rules, you’d want to lint the JS and extract the JSDoc examples, GraphQL queries, and CSS rules to run with their own sub-linters: processor: [“passthrough”, “jsdoc”, “graphql”, “css-in-js”]

I’m not thrilled with having a special valid to consider.

Assuming you meant “value” about “passthrough”, yeah, I’m not sold on that precisely either. In terms of implementation, the proof of concept is simple enough as an if inside the processor array iteration that calls _verifyWithoutProcessor(…).

Another option I considered was shipping a built-in import { passthroughProcessor } from “eslint”;. However, to ensure filename and physicalFilename match for the passthrough, Linter would have to have some special case handling when it detects that.

I also considered a boolean lintPhysicalFileAlongsideProcessor option (name could use some work), but that felt worse than a special ”passthrough” value.

[nzakas]

Maybe “eslint:default” would be a better fit than “passthrough” (matching “eslint:recommended”)?

In any event, I like the direction. Being able to lint all of the various code blocks inside of a file is a super power I’ve wanted for a long time.

[btmills]

Thanks for the feedback. Since you’re on board with the direction, that’s enough for me to continue sketching out a prototype implementation and write up an RFC. It’ll be easy to change the special value, so I’ll explore the tradeoffs of each option in the RFC and we can decide which way to go there.

[antfu]

Proposed a solution to improve this: eslint/rfcs#115, feedback appreciated.

[JounQin]

@antfu #14745 (comment)

This feature has already been supported but not well-documented.

Please read the context more carefully.

[nzakas]

The solution to this problem will likely be based on this RFC: eslint/rfcs#105