Teal Check: Lint-Friendly Output

tooolbox commented 3 years ago

With a couple hours of hacking I got tl check working as a linter in Sublime, via SublimeLinter. There are a couple of adjustments to the output that would increase the quality of the linting experience:

Warning/Error By Line

Rather than separating out warnings and errors into different sections, include "warning" or "error" (or "w" and "e", etc.) on each line. SublimeLinter works by processing each line of the output, with no way to detect "sections".

Stdout/Stderr Distinction

I believe that all output from the checker which pertains to problems within the material should go to stdout, and all problems with running the checker itself should go to stderr.

Currently:

argument 1: got integer, expected string is an error and goes to stderr.
unused variable apple: string is a warning and goes to stdout.

Ideally:

argument 1: got integer, expected string is an error and goes to stdout.
unused variable apple: string is a warning and goes to stdout.

Then, stderr would be reserved for such things as invalid-file.lua: No such file or directory and others that indicate a total failure to perform a check, rather than problems detected in your code while checking.

All Together

Taken together, I would like to see this sort of output on stdout:

[err] my-file.lua:3:31: argument 1: got integer, expected string
[wrn] my-file.lua:5:21: unused variable apple: string

Perhaps this style of output could be gated behind a flag, such as tl check -lint or another command, tl lint, to preserve compatibility with any existing tools of workflows.

Thanks! 👋

lenscas commented 3 years ago

though I agree that warnings and errors should both use stdout, if for no other reason than to clearly separate problems with the compiler itself vs problems with your programs, I don't think tl check should have this output format.

tl check is not a linter, so setting it up as one seems weird to me. Especially with people already working on proper language servers. If some machine readable output is desired then json seems like a better choice to me than a custom format.

tooolbox commented 3 years ago

Question: how far off is the language server?

Regardless, tl check is already being used as a linter for Lite. Adding “err” and “warn” in front of each line just makes it a little more usable, not sure that’s too much to ask for?

lenscas commented 3 years ago

Question: how far off is the language server? I don't know to be honest :( There is a vscode plugin that works pretty well, but not sure how the language server is developing.

Regardless, tl check is already being used as a linter for Lite. Adding “err” and “warn” in front of each line just makes it a little more usable, not sure that’s too much to ask for?

My problem isn't necessary with adding that, it is more that if you add that to let sublime use it, then what stops someone from requesting a change for another editor, and then another? All the while the output becomes harder and harder to read for humans as more and more noise gets added.

Instead I would propose to have an optional json output. Then it becomes easy to use it for all kinds of tools. Just make a program that forwards the arguments to tl check , read the produces json and prints it in whatever format is needed.

tooolbox commented 3 years ago

I don't know to be honest :( There is a vscode plugin that works pretty well, but not sure how the language server is developing.

Gotcha, alright.

My problem isn't necessary with adding that, it is more that if you add that to let sublime use it, then what stops someone from requesting a change for another editor, and then another? All the while the output becomes harder and harder to read for humans as more and more noise gets added.

Instead I would propose to have an optional json output. Then it becomes easy to use it for all kinds of tools. Just make a program that forwards the arguments to tl check , read the produces json and prints it in whatever format is needed.

I do get where you're coming from. It's true that JSON is a standard format, but not so much for linters. It seems to be pretty common that they output one line per issue found, and the line contains all the relevant information.

Examples:

Eslint has various formatting options, but it has for example compact.
Go vet does this.
Flake8 does this.

Furthermore, that's exactly how the VSCode plugin for Teal works, and the linting plugin for Lite: by reading the lines from Teal's output. The fact that SublimeLinter (which has linters for many languages) expects line-by-line output indicates this isn't a Sublime thing, it's a linter thing.

Anyway, not to harp on it too thoroughly, but I think the output is already 90% of the way there, and adding the severity on a per-line basis should improve the linting experience in all editors, not just Sublime.

lenscas commented 3 years ago

tl check isn't a linter though and having it spit json out would help the vscode plugin and Lite. As it becomes easier to parse for both of them. And what if someone makes a PR that improves the errors and thus may start to look more like

error[E0308]: mismatched types
 --> src/main.rs:4:16
  |
4 |     do_nothing(1);
  |                ^
  |                |
  |                expected struct `String`, found integer
  |                help: try using a conversion method: `1.to_string()`

or changes them in some other way.

if tl check can spit out json then something like this becomes A LOT easier to parse than if it needs to deal with the human readable text and it becomes a lot easier to make a wrapper for it to add support to whatever format is needed.

Either way, if I am correctly up to date with the plans then the plan is that we will interact more with cyan than tl for most project, which I believe already changes the errors to some other format anyway, so this entire discussion may be useless.

tooolbox commented 3 years ago

Well, I guess it’s not a linter, it’s a typechecker? It’s being used as a linter and is specifically called out in the Teal read me as offering linting for Lite. So this may be a matter of semantics.

I’ll note that your rust example has “error” at the start of that line, which is all I’m asking for. I don’t particularly disagree with making the output more human readable, Rust sets a good example there. It could maybe tweak output formats based on a flag as I said before. If the maintainer wants to switch to Json that’s his prerogative, including changing the various plugins that rely on its current format, and in that case I would live with it. All I’m saying is that a minor tweak to what currently exists and the way the output currently works would give a little more value to the plug-in I figured out, and shouldn’t require rewriting other plugins. That may be worthwhile at this stage of the game.

teal-language / tl