Parse errors from SWC could be better

[bartlomieju]

Currently if SWC parser encounters error its output is pretty minimal:

import b, { c } from "./b.js";

console.log(b);
console.log(c

error: Unexpected eof at file:///Users/biwanczuk/dev/deno/a.js:4:13

It'd be great if produced error could contain snippet of code pointing to problematic bit of code, similar to errors from TS and V8:

import b, { c } from "./b.js";

foo
console.log(b);
console.log(c);

error: Uncaught ReferenceError: foo is not defined
foo
^
    at file:///Users/biwanczuk/dev/deno/a.js:3:1

CC @kdy1

[kdy1]

Did you provide Lrc<SourceMap> to the Handler from swc_common?
If then, it should point the error just like rustc.

There are some error reporting tests.
See: https://github.com/swc-project/swc/tree/8ca3d1160db804322724a6e30181e9c86bdd396a/ecmascript/parser/tests/typescript-errors

[bartlomieju]

@kdy1 doesn't look like it, here's our handler:

deno/cli/ast.rs

Lines 381 to 388 in d9b4182

	let handler = Handler::with_emitter_and_flags(
	Box::new(error_buffer.clone()),
	HandlerFlags {
	can_emit_warnings: true,
	dont_buffer_diagnostics: true,
	..HandlerFlags::default()
	},
	);

Unfortunately I can't find API that would allow to pass Lrc<SourceMap> and not emit to tty.

[kdy1]

Oh, you are correct. It accepts Lrc<SourceMap> only if tty-emitter is enabled.

I'll see if I can make a handler with cm but without tty.

[bartlomieju]

Oh, you are correct. It accepts Lrc<SourceMap> only if tty-emitter is enabled.

I'll see if I can make a handler with cm but without tty.

Thanks @kdy1!

[kdy1]

You can now update swc_common and use EmitterWriter

I verified that it works by disabling tty-emitter from all crates and run the error reporting test of parser.

[bartlomieju]

@kdy1 thank you, that was very fast turnaround!

[bartlomieju]

Reference implementation: https://github.com/swc-project/swc/blob/46b553ecc3efd3ddcaff6531535b27526dc16781/testing/src/string_errors.rs#L11-L30

[kdy1]

Does it work? I expect it to work correctly as the parser test successed, but just to be sure.

Ah.. I misread it.

[stale]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed in 7 days if no further activity occurs. Thank you for your contributions.