fix(parser): Handle JSDocUnknownType correctly

[leaysgur]

Current behavior:

type G<T> = T;
type C1 = G<?>; // Parse failed 👀

TS parses this as TypeReference > JSDocUnknownType.

REPL: https://www.typescriptlang.org/play/?noCheck=true#code/C4TwDgpgBA4gPAFQHxQLxQQbgFCklAYQEY1Y4B+JTIA
Parser: https://github.com/microsoft/TypeScript/blob/83dc0bb2ed91fe0815ab28dc3ff95fae7425e413/src/compiler/parser.ts#L3847

And also this PR fixes these:

type X = [
  1?,  // TSOptionalType
  ?1,  // JSDocUnknownType 👀
  ?1?, // JSDocUnknownType 👀
  ?1!, // JSDocUnknownType 👀
];

to

type X = [
  1?,  // TSOptionalType
  ?1,  // JSDocNullableType
  ?1?, // JSDocNullableType > JSDocNullableType
  ?1!, // JSDocNullableType > JSDocNonNullableType
];

This will result in the same behavior as TS and ESLint-TS.

[leaysgur]

How should I handle diffs in parser coverage...?

This is the same as #10346 (comment) , another one that "doesn't result in a parse error, but does result in a type check error".

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #10363 will improve performances by 3.58%

_{Comparing fix/jsdocunknowntype (6a9465f) with main (3ed3669)}

Summary

⚡ 1 improvements
✅ 35 untouched benchmarks

Benchmarks breakdown

	Benchmark	BASE	HEAD	Change
⚡	formatter[antd.js]	8.1 ms	7.8 ms	+3.58%

[Boshen]

Thank you. I think I got lazy with these AST nodes at the time of their writing :-)

--

How should I handle diffs in parser coverage...?

Add logic to https://github.com/oxc-project/oxc/blob/main/crates/oxc_semantic/src/checker/typescript.rs if you want to pass some typescript tests.

In terms of the correct jargon to use, there is actually no parser error or type check error, they are all syntax errors.

[overlookmotel]

In terms of the correct jargon to use, there is actually no parser error or type check error, they are all syntax errors.

Maybe I've been using the wrong terms, but there seems to be a difference between the two. I don't understand this JSDocUnknownType business, but using the example from #10332 (comment):

enum Foo {
    [bar()],
}

TS parser (and TS-ESLint parser) will both parse this without error - therefore I don't think it's a syntax error.

But still it's not valid - you can't transform this to JS. That's what I was calling a "typecheck error" because it only gets flagged by TS when you enable type checking.

Please correct me if I've got this wrong. We're probably going to encounter more of these cases as we continue with TS-ESTree compat work, so would be good to understand how we should be handling them.

Taking off the merge label until we resolve this ambiguity.

[Boshen]

They parse (produce an AST) because tsc is a recoverable parser, it's a syntax error regardless where it's reported from.

[overlookmotel]

But doesn't TS usually report syntax error even when it recovers? Like in this case it reports an error during parsing:

enum Foo {

https://www.typescriptlang.org/play/?noCheck=true#code/KYOwrgtgBAYg9nKBvAUEA

Whereas this is parsed without any error:

enum Foo {
    [bar()],
}

https://www.typescriptlang.org/play/?noCheck=true#code/KYOwrgtgBAYg9nKBvAUFdUDaAjAhgJwAoBKAXQBoUBfFIA

Sorry if I'm labouring the point. I just want to make absolutely sure I understand, to avoid us making tons of changes, and later having to revert them.

[Boshen]

noCheck turns off type checking (stop running the checker), but it's still syntax error regardless of where it's reported from.

[graphite-app]

Merge activity

• Apr 12, 5:05 AM UTC: @leaysgur we removed the merge queue label because we could not find a Graphite account associated with your GitHub profile.

You must have a Graphite account in order to use the merge queue. Create an account and try again using this link