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

Expand the json output of mint lint to have more metadata #465

Open
bbugh opened this issue Jun 13, 2021 · 4 comments
Open

Expand the json output of mint lint to have more metadata #465

bbugh opened this issue Jun 13, 2021 · 4 comments
Labels
enhancement New feature or request tooling Tooling related feature (formatter, documentation, production builder)

Comments

@bbugh
Copy link
Contributor

bbugh commented Jun 13, 2021

I was looking at what it would take to have inline warnings in VSCode, and testing with master mint to use lint --json.

The json that the linter outputs an array of strings with the error messages. In order to effectively use the json output, it would be helpful for it to output an array of objects that include metadata like:

  • filename
  • line number
  • column from
  • column to
  • basic message (like I was looking for a branch of a case expression but found "Result::Ok" instead.)
  • full message

It could be parsed out from the error message, but that would be brittle, and the error message does not include the "end" column number, so any errors in-editor would have to be drawn for the whole line.

@Sija Sija added enhancement New feature or request tooling Tooling related feature (formatter, documentation, production builder) labels Jun 13, 2021
@bbugh
Copy link
Contributor Author

bbugh commented Jun 13, 2021

I took a look at the code, and this is my best guess for the changes.

It appears that the parser would have to send more data when the error occurs:

https://github.com/mint-lang/mint/blob/master/src/parser.cr#L64-L67

And the json output would have to be changed, either by overriding the base class (which appears to be Crystal Exception rather than the Mint::Error, so I'm not sure how to do that) or by adding a block:

https://github.com/mint-lang/mint/blob/master/src/commands/lint.cr#L71

But from there I'm not sure how to get that data into Message so that it can be output in the ^^^ lint code.

@Sija
Copy link
Member

Sija commented Jun 13, 2021

With #443 it should be relatively easy, having Ast::Node#location.

@gdotdesign
Copy link
Member

This would be a good change for the lint command and it's doable but I wonder if it would be better to implement Diagnostics for the language server instead?

@Sija
Copy link
Member

Sija commented Jun 15, 2021

@gdotdesign I think we need both, Diagnostics for LS, and a similar feature for the lint command - mostly useful within the CI env.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request tooling Tooling related feature (formatter, documentation, production builder)
Development

No branches or pull requests

3 participants