[feature] Add custom data into LintMessage

[JounQin]

The version of ESLint you are using.

v7.21.0

The problem you want to solve.

I want to get custom data which reported with context.report({ data }) info in LintMessage in postprocess.

Your take on the correct solution to problem.

For now, we got no way to get the custom data, I have to hack to use the message property with JSON.stringify/parse instead.

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

Yes

[nzakas]

@aladdin-add please only move to triaging if you have left a comment.

@JounQin can you explain your use case? We intentionally don’t allow extra data to pass through to our results, so we need an idea about why you are requesting this.

[JounQin]

@nzakas

https://github.com/mdx-js/eslint-mdx/blob/master/packages/eslint-plugin-mdx/src/rules/remark.ts#L78
https://github.com/mdx-js/eslint-mdx/blob/master/packages/eslint-plugin-mdx/src/processors/remark.ts#L30

I'm writing a plugin which emits reporting from other tools like remark-lint, so I need to modify the ruleId and severity of lint message in postprocess method.

For now, I have to hack to use the message property with JSON.stringify/parse instead.

[nzakas]

Sorry, I’m having a hard time understanding what it is you’re trying to do and how it relates to custom data in our lint messages. Can you please provide a full description of your use case? More than one or two sentences would be great.

[JounQin]

I don't know if my English and good enough to explain clearly, but I'll try!

When I write rule mdx/remark by integrating with remark-lint, it runs well with reporting ESLint's ruleId and severity. But I want to display the original remark's reporting messages, so I need to get the remark's messages in postprocess method to override the ESLint's one.

For now, no data is provided so I hack to serialize the remark's message with JSON.stringify as message property of ESLint's LintMessage and JSON.parse it back in postprocessor.

It works well too. But it would be better to add data into LintMessage without sterilization and deserialization.

[nzakas]

Ah okay, thank you. I understand now. You’re looking for a way to pass data from the rules into the processor to determine how to represent the errors better.

That’s an interesting idea. I’m not sure if adding such data would be a breaking change, so I’ll run this by the team and see what they think.

@btmills as our resident processor expert, what do you think?

[nzakas]

@eslint/eslint-tsc still looking for feedback on this.

[JounQin]

I won't say it as a BREAKING CHANGE, because the plugin authors control data and message by themselves.

[btmills]

To make sure I understand, @JounQin: you've figured out how to wrap ESLint as a runner around another (non-JS) linter by combining a rule and a processor. It would be more convenient if the rule could pass data to the processor without serialization. Is that correct?

That's a creative solution. Passing data from a rule through messages to a processor isn't the API I'd come up with if we wanted to implement support for wrapping other linters as a first-class feature, but I'm not opposed to doing this little thing to enable it using otherwise existing API.

If it's possible for another processor to sit in between your rule and processor, this might be breaking. For example, if your data-containing message was first processed by a processor that didn't know anything about the data key, there's no guarantee that it gets passed through. Processors are likely implemented as return { ...message, line: adjustedLine, column: adjustedColumn };, but there's no guarantee that they aren't explicitly setting each key. If we add a new key to the message type, requiring processors to use object spread or explicitly copy the new key seems breaking.

[JounQin]

@btmills The message list are returning back in postprocess, eslint itself does not care about data, it only makes sense for plugin authors themselves, I still can't understand why it would break the processor.

But it's still fine to me if you'd like to mark this as BREAKING.

---

Passing data from a rule through messages to a processor isn't the API I'd come up with if we wanted to implement support for wrapping other linters as a first-class feature

Then do you have any better solution to implement this? Current implementation is really poor because we can not use // eslint-disable-other-linter-rule even if it reports other-linter-rule now.

[btmills]

I'll elaborate a bit on my hypothetical breaking change scenario so we can see if it has any holes:

1. On *.foo files, a user configures a processor in eslint-plugin-foo.
2. eslint-plugin-foo includes a rule, foo/rule, that sets data on its reports.
3. On *.bar files, the user configures a processor in eslint-plugin-bar.
4. eslint-plugin-bar includes a rule ,bar/rule, that sets data on its reports.
5. The user lints a file, test.foo.
6. The eslint-plugin-foo processor runs on test.foo.
   1. preprocess() returns the entire file contents as a code block also with the foo syntax. (This is the trick you discovered, correct?)
      1. ESLint runs foo/rule on the returned code block, test.foo/0_0.foo, and receives lint messages containing the data property.
   2. eslint-plugin-foo's preprocess() also returns code blocks with the bar syntax.
      1. ESLint runs the eslint-plugin-bar processor on those code blocks as test.foo/1_1.bar thru test.foo/n_n.bar.
         1. eslint-plugin-bar's preprocess() returns a code block also with the bar syntax.
            1. ESLint runs bar/rule on the returned code block, test.foo/1_1.bar/0_0.bar, and receives lint messages containing the data property.
         2. eslint-plugin-bar's preprocess() also returns other nested code blocks, which ESLint lints as test.foo/1_1.bar/1_1.js and so on.
         3. eslint-plugin-bar's postprocess() maps the messages, some of which contain data from bar/rule, back to their locations in test.foo/0_0.foo.
   3. eslint-plugin-foo's postprocess() maps the messages, some of which contain data from foo/rule and some of which came from bar/rule, back to their locations in test.foo.

If eslint-plugin-bar's postprocess() fails to remove data from bar/rule's messages, eslint-plugin-foo's postprocess() will see messages with "foreign" data.

1. Is there any scenario where eslint-plugin-bar's postprocess() should preserve the data property on a lint message for use by a different postprocess()?
2. If yes, we'll need to figure out how to resolve conflicts on a shared property, which gets hairy quickly. The whole point of data is that it's outside the standard schema, so defining a schema isn't an option.
3. If no, and we go ahead with this approach, should we mandate that data must be removed from any messages returned by postprocess()?
   1. If postprocess() always removes data from messages, is there any scenario where a plugin's postprocess() could process a message containing data from a different plugin's rule? If that's impossible, then this isn't breaking.

Then do you have any better solution to implement this?

To clarify my meaning a bit, I said that because I'm impressed that you've come up with something that works without any support from a first-class API. Linting non-JS code in ESLint is a relatively unexplored concept. RFC56 showed a way to write ESLint rules to run on non-ESTree syntax trees. This is the first time we've talked about using ESLint to wrap another linter as a black box of sorts. With RFC56, ESLint would still be calling the parser, traversing the AST, and calling rules. With what you've figured out, ESLint isn't even aware that there's another linter. That means eslint-* comments don't work as you've discovered. A first-class API to wrap another linter might be able to solve that? Not sure if that would be possible. Either way, that'd be a much bigger effort, and adding data makes your use case more convenient for much less work.

[JounQin]

@btmills Wow, I'm getting sidetracked by your example. 😅

eslint-plugin-foo's postprocess() maps the messages, some of which contain data from foo/rule and some of which came from bar/rule, back to their locations in test.foo.

But is that true that foo processor will get lint messages from another bar processor?

I was thinking that foo processor will only get lint messages from its own preprocess like you mentioned at eslint/eslint-plugin-markdown#183 (comment).

And foo processor should only take cares about its own messages from its own rule in this case.

[btmills]

Yeah, that scenario was dense, but you got the exact point I'm trying to figure out. I think if a bar rule and processor run inside a block extracted from a foo processor, and if bar's postprocess() doesn't remove data from the messages, foo's postprocess() would then see those messages with data that it's not expecting. That's because foo's postprocess() would run on every message from each nested code block, some of which would have reports from bar/rule.

[JounQin]

@btmills

I think BREAKING means it breaks current runnable codes, now that there is no data at all, how could it break current codes?

[btmills]

I agree it's at the very edge of what might be considered breaking. In the strictest sense, any change that adds a requirement to third-party code or weakens a guarantee that we previously made third-party code is breaking. Here, we'd add a new requirement that processors preserve message.data, unless we can be sure that other processors won't ever see a message with data. If we can be sure of that, then other processors don't need to change anything, and this wouldn't be a breaking change.

[btmills]

@JounQin I haven’t really gotten comfortable with the API we’ve been discussing in this issue. However, #14745 looks to be an adjacent use case, and it gave me an idea:

Right now, ESLint either lints a file XOR runs it through a processor and lints its blocks. This issue and #14745 want to lint both a file and chunks within that file. What if we changed the processor API to allow both? Would that solve your use case here?

[JounQin]

What if we changed the processor API to allow both? Would that solve your use case here?

This already allowed with [text, ...blocks] as I described in your mentioned PR.

But this issue is not same as that one exactly.

This issue is about reporting better message when integrating with 3rd party linters/tools.

[github-actions]

Oops! It looks like we lost track of this issue. @eslint/eslint-tsc what do we want to do here? This issue will auto-close in 7 days without an update.

[JounQin]

ping

[nzakas]

@JounQin please don’t leave nonsense comments. It just adds to the noise.

@btmills where do we want to go with this?

[github-actions]

Oops! It looks like we lost track of this issue. What do we want to do here? This issue will auto-close in 7 days without an update.

[btmills]

This only auto-closed because I forgot to self-assign. Sorry about that.

[saberduck]

hello,

I am working on https://github.com/SonarSource/SonarJS which is a static analyzer based on ESLint used in SonarQube/SonarCloud/SonarLint. We would be also interested in the API which would allow passing additional data when reporting the problem from the rule. In Sonar ecosystem we have the concept of additional locations in the source code which helps to understand more complex problems (imagine taint analysis). Here is one example which shows dataflow in SQL injection issue in juice shop https://sonarcloud.io/project/issues?id=saberduck_juice-shop&issues=AX6N9VGbKcvyCvHU0eOA&open=AX6N9VGbKcvyCvHU0eOA

Currently, we have implemented a similar hack, we encode JSON object in the rule message. However this is ugly and we can't use messageIds properly, so I would like to migrate from this.

I will be more than happy to help with the implementation if we can agree together on the design.

[nzakas]

Current status of this: we are looking for someone to create an RFC for this feature. If anyone is interested, please comment here.

[saberduck]

@nzakas I would like to give it a try. I can try to allocate some time by the end of August. Could you pls provide some pointers about RFC process? How RFC should look like?

[nzakas]

@saberduck check out https://github.com/eslint/rfcs. There's a README and you can also read through previous RFCs.