Handle compiler errors in Rust.

[kitsonk]

Resolves #1738

This has gotten to the point where it is ready for light review and feedback. Instead of the compiler outputting errors and exiting, it returns TypeScript errors to Rust which then will log them to the console and exit.

The biggest change on the Rust side is that JSError has been renamed to DenoDiagnostic and both compiler errors and V8 errors are unified. It might make sense to also unify other errors/panics to this format over time, so we have a unified error structure.

Things that still need to be done:

• Tests

• Handle multiple errors returning from the compiler for a given module, currently only the first error is output

• Handle diagnostic changes, the TypeScript compiler can output a chain of errors which need to be formatted and displayed.

• Enhance all outputs to be more aligned to the way TypeScript outputs errors:

  Currently the output for an error looks like this:

  /Users/kkelly/github/deno/tests/config.ts:2:4
  if (map.get("bar").foo) {
      ^^^^^^^^^^^^^^
  Object is possibly 'undefined'.
  

  Where we actually want it to look more like this:

  /Users/kkelly/github/deno/tests/config.ts:3:5 - error TS2532: Object is possibly 'undefined'.
  
  3 if (map.get("bar").foo) {
        ~~~~~~~~~~~~~~
  

If you want to see the branch in action, build it and then run something like:

$ deno run --reload --config tests/config.tsconfig.json tests/config.ts

[ry]

Nice!

[kitsonk]

This is a more complete PR, but still a bit of work to do, I just wanted to push an update. Now core/diagnostics.rs displays all diagnostics properly. I need to then apply some color formatting to the errors in cli/diagnostics.rs.

I also think they we should consider conforming all internal user facing logging to this format, which in theory would include stuff from Rust. The structure is pretty robust now, being able to handle complex sets of diagnostics as well as stacks of frames.

[kitsonk]

Slow progress, but getting there:

So, diagnostics output in Rust now roughly mirror what is done by TypeScript.

Next big objective is to allow a compilation to return a vector of errors which can be output, I am hoping it is fairly straight forward. Then it is updating tests.

[kitsonk]

Ok, it is functionally complete now, I think. I need to add some more tests in Rust and rebaseline the outputs of a lot of other error related tests.

[kitsonk]

@ry I think this if finally ready for review...

[ry]

This also should have a unit test.

[kitsonk]

Oooops, I forgot to add this one, it isn't my code, it was from core/js_error.rs originally and doesn't have a unit test there, as it is from the V8 error, but I will add one.

[ry]

@kitsonk I suggest not making changes in core .. the JSError as it is works with the JSON we get from V8. In cli we can have Diagnostic be the main error struct - we just need impl From<JSError> for Diagnostic.

[kitsonk]

I suggest not making changes in core .. the JSError as it is works with the JSON we get from V8. In cli we can have Diagnostic be the main error struct - we just need impl From for Diagnostic.

Hmmm... I missed this comment. That would be undoing a fair amount of work and divergent from the conversations we had along the way, which was to merge JSError and the TS Diagnostics, which is what this PR does, using the same structure. I am going to finish off the other feedback and have rebase on the changes to compiler. I argue that it is overall cleaner that we have a single structure named Diagnostic.

[kitsonk]

@ry @bartlomieju I think I have addressed all feedback (plus rebased on the compiler changes).

[ry]

Output looks good and (mostly) passing for me locally.

It's not clear to me why any changes need to be made to core? The core/js_errors.rs module is nice because it's designed just to consume the JSON blob from V8, without further complicating things. It's nice to have that independence for testing and also for users who don't want TS. I think cli/diagnostics.rs would just implement From<deno::JSError> ...

[kitsonk]

@ry Yeah. I see the problem. I still think there is value in there being a core diagnostic structure, versus an opinionated one that can be used for any "userland" errors, even using them when ops are bad in Rust land or there are bad arguments.

I will try to separate out the logic for the TS diagnostics. Because clearly that should only exist in CLI.

[kitsonk]

I have been thinking about this quite a bit. I think there is an argument for starting over with this and not trying to merge JSError and the TypeScript diagnostics. It is just too clunky from a Rust perspective, and my head was a little too OO. If we really wanted to do it, properly, we would abstract out features into traits.

So I suspect I will close this and start a new branch PR that just focuses on adding the diagnostics to the compiler in CLI and doesn't touch core.

[bartlomieju]

@kitsonk consider using serde_derive in CLI code - it should simplify JSON deserialization. I believe it should also greatly simplify parsing V8 exceptions, but we'd need to enable it in core as well. To be discussed

[kitsonk]

@bartlomieju it does and it doesn't... As far as the TypeScript diagnostics, there are some strange structures in there (like the message chain) which don't deserialise to Rust well that the procedural code around serde_json works fairly well. I will take a look again though.

[kitsonk]

Closing in lieu of #2445.