Expose rule URLs to formatters

[pmcelhaney]

ESLint Version: 4.15.0

Now that url has been added to meta.docs (#6582) we can use that field to share more information about a rule in the output.

The most obvious place to start is the HTML formatter, which includes links to rule documentation. Currently the links only work for core rules because eslint.org is hard-coded in the URL.

I opened a pull request (#9840) to get an idea of what kind of changes would be needed, and @not-an-aardvark has some great comments, which I'm just going to copy verbatim:

• I like the idea here -- it seems like it would be useful to provide url info to formatters.
• If we do add a second argument to formatters, I think it should be an object (perhaps something like { rules: ruleMap }) rather than the rule map itself. That way, if we want to provide additional information to formatters in a later enhancement, we could add it to the same object, so formatters wouldn't need to remember the argument order and do something like function(results, unused, newInfo) {}.
• getRules is currently somewhat slow, because it requires it requires loading all core rules from the filesystem, and copying a map of rules with ~250 elements. (Currently, we only load a core rule for the first time when it's actually used, which reduces startup time.) I wonder if it would be better to pass an object like { getRule: ruleName => rule } to the formatter, so that it would only need to load the rules that are needed.
• As an open question, would it be better to just give formatters access to rule.meta or rule.meta.docs rather than giving access to a rule itself? Giving formatters access to a rule object itself would allow them to do things such as rerunning the rule, which seems like it could be confusing because formatters are just supposed to apply a transformation to the results. (Admittedly, I'm not sure why a formatter would want to rerun the rule, but ideally I think we should provide the minimal API needed to be useful.)

[j-f1]

getRules is currently somewhat slow, because it requires it requires loading all core rules from the filesystem, and copying a map of rules with ~250 elements.

What if we built a custom Map object here that only loaded a rule when the user calls get, delaying execution until it’s actually needed?

[not-an-aardvark]

We sort of already do that with Rules(used internally). There are a few problems with exposing it externally:

• The function is documented in the API to return a regular JavaScript Map object.
• We need to copy the map because if we didn't do that, users would be able to tamper with ESLint's internal state by mutating the map.

I think it would have been better if we had provided accessor functions directly like getRule() on Linter, but it's too late to change it now.

[j-f1]

You could make a custom Map object that has its own real Map for keys folks call set() on, and get from the instance-internal map, then the global map if it’s not found.

[not-an-aardvark]

That could work, although I feel like it's a lot of complexity for something that users shouldn't need to do anyway. Maybe a better solution would be to make a breaking change and not allow users to mutate anything (maybe by exposing a similar interface that doesn't have any mutator methods, or adding something like getRule to Linter to get a single rule without copying the map).

[j-f1]

We could add getRule and deprecate getRules now, then 🔥 getRules in v5.

EDIT: we’d also need a ruleNames or similar method to get the list of valid rule names.

[ilyavolodin]

I'm not sure I fully understand the discussion here. ESLint already attaches some internal information when report is called, so it would be available to formatters (like filename, for example), why not attach rule's metadata to the same object at that time? It's already available in the execution context, the rule is already loaded, so no overhead is required. Why do we need to use getRules at all in this case?

[not-an-aardvark]

why not attach rule's metadata to the same object at that time?

This would work. We should consider where we put the metadata on the result object; for example, if we add metadata to every reported problem, we will end up with a lot of duplication.

As a sidenote, I'm wondering if it would be better to provide meta.docs instead, rather than meta. meta contains a lot of information that formatters don't need, such as the rule schema and whether a rule is fixable.

Why do we need to use getRules at all in this case?

We don't need to do this if we attach the metadata to the result object. The implementation in #9840 proposed adding a separate argument to the formatter API, containing a map of all loaded rules.

[ilyavolodin]

I see. I don't have an objection to attaching meta.docs instead of meta, although, I could imagine a scenario where somebody wanted to write a formatter that would not only report problems, but also notify user which of them might be fixable, but that could be done by other means.

Yes, there's a potential for number of repeated data on a large set of errors. Not sure we need to optimism for that right away, but maybe we should.

[pmcelhaney]

Why the concern about repeated data? It’s just a bunch of pointers in memory, right?

The original use case for exposing a rule’s docs is to provide contextual help when reporting a problem. Attaching that info to the problem object has always made sense to me.

[not-an-aardvark]

You're correct that the rule metadata would only appear once in memory initially, but formatters that serialize the result object (like json) would generally end up duplicating the metadata for each problem.

When running a big lint job which reports a lot of errors, the formatter output can get extremely large and can cause the process to run out of memory (e.g. #4731). I think we try to keep the result object reasonably small to prevent this from happening.

[platinumazure]

I could imagine a scenario where somebody wanted to write a formatter that would not only report problems, but also notify user which of them might be fixable, but that could be done by other means.

The message object contains fix information-- the value isn't useful to the user, but the presence of the fix property is enough to indicate that the message in question has a fix that could be applied. (I think-- this assumes that we don't remove the fix property when applying the fixes to disk, which would be silly.) See CLIEngine#executeOnFiles() docs for more info.

[platinumazure]

Personally, I would love to see an extra argument be added for this and other information.

As a side note: This might finally allow my proposal to support arbitrary formatter options (with ESLint not knowing or caring about the structure of the options and just passing them through to formatters) to get off the ground...

[not-an-aardvark]

I was originally in favor of adding an extra argument for this, but on reconsideration I'm not sure it's a good idea. Presumably, anyone using a formatter like json is planning to operate on the data somewhere outside of the ESLint process (e.g. within an editor integration). If we're just using a second argument to keep the object small, then we're effectively not allowing those integrations to use the new info.

One nice thing about having a single serializable argument for formatters is that it allows linting results to be portable. If you use the json formatter, you can effectively defer formatting until later and pass the results to an editor, another process, over the network to a browser, etc. Adding an additional argument would make it difficult to do this.

[pmcelhaney]

Can the json formatter filter out noisy data? Or use $ref?

[not-an-aardvark]

I don't think filtering out the noisy data is a good idea for the reasons described in #9841 (comment) (it would effectively be deleting information, which would prevent consumers from using that information).

It could use something like $ref, although to me it seems like it would be simpler just to add the info to the report object in a manner that doesn't duplicate it for every report.

[not-an-aardvark]

As a side note: This might finally allow my proposal to support arbitrary formatter options (with ESLint not knowing or caring about the structure of the options and just passing them through to formatters) to get off the ground...

It's not directly related, but if we added something like this then it could partially solve the problem of report objects being too large, because users of the json formatter could filter out properties that they don't care about (if we added an implementation for it).

[pmcelhaney]

I’m simply proposing the JSON formatter’s behavior wouldn’t change. It would output no more or less information than it does now.

[not-an-aardvark]

I realize that, but I don't think it's ideal because it would prevent external integrations from taking advantage of rule metadata.

[pmcelhaney]

Is there a reason the JSON has to match the exact structure of the API?

[not-an-aardvark]

I don't think it's an absolute requirement, but it makes things more convenient because the results object is the same regardless of whether you (a) use a custom formatter, or (b) use the json formatter and then apply custom formatting somewhere else.

[platinumazure]

@pmcelhaney @not-an-aardvark Where are we at on this?

[pmcelhaney]

If I remember correctly, we want to pass a copy of a rule’s meta to each reported problem in the results array. We can’t do that because it would bloat results when it’s serialized to JSON.

If we keep the metadata out of the results object by passing the rules as a second argument, we cheat the JSON object out of valuable information.

It now occurs to me that we could create a “v2” JSON formatter that serializes an object with two top level properties: results and rules.

Would that work?

[platinumazure]

@pmcelhaney That's a good idea and worth considering.

Another might be to add a second argument to formatters (the options thing), and the existing formatter could use that to decide whether to include the new top level or just include the results only (default behavior).

Thanks for your input! Hopefully we can get this moving again 😄

[not-an-aardvark]

It now occurs to me that we could create a “v2” JSON formatter that serializes an object with two top level properties: results and rules.

That seems like a reasonable compromise to me, although I think it would be better to give the formatter a name like json-with-metadata rather than "v2" to avoid implying that the original json formatter would be obsoleted.

[nzakas]

As this is a core change, it falls under our new RFC process. I'd suggest that if we want to move this forward, that someone put together an RFC. Details can be found here:
https://github.com/eslint/rfcs

Thanks!

[eslint-deprecated]

Unfortunately, it looks like there wasn't enough interest from the team
or community to implement this change. While we wish we'd be able to
accommodate everyone's requests, we do need to prioritize. We've found
that issues failing to reach accepted status after 21 days tend to
never be accepted, and as such, we close those issues.
This doesn't mean the idea isn't interesting or useful, just that it's
not something the team can commit to.

Thanks for contributing to ESLint and we appreciate your understanding.