fix(ast/estree): Better align program span start

[therewillbecode]

This PR adjusts the start span of the program node to the first token of the first statement in order to align with TS-EStree serialisation. Previously the program start span was always 0.

The diff removes many mismatches but also adds around 35 new ones. Seem to be mostly related to strict mode directives.

For the mismatches that are added to the snapshot, I wonder if they just happened to not be mismatches because of the fact our Program spans used to always be 0 and the conformance tester incorrectly expected it to also be 0. TS-eslint seems to ignore comments when calculating the Program start span whereas our conformance snapshot cases don't ignore them.

The below code serialises to TS-ESTree with a program start span of 16 (see playground).

// @target: ES5
if (true) {}

However our conformance tester works such that we expect start span to be 0 which doesn't line up with the playground example linked. So I think its actually our conformance test parser thats set up wrong for the ~35 mismatches that this PR adds to the snapshot.

[graphite-app]

How to use the Graphite Merge Queue

Add either label to this PR to merge it via the merge queue:

• 0-merge - adds this PR to the back of the merge queue
• hotfix - for urgent hot fixes, skip the queue and merge this PR next

You must have a Graphite account in order to use the merge queue. Sign up using this link.

An organization admin has enabled the Graphite Merge Queue in this repository.

Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue.

[codspeed-hq]

CodSpeed Instrumentation Performance Report

Merging #10134 will not alter performance

_{Comparing therewillbecode:fix/fix-program-span-start (0b38c8c) with main (f547d76)}

Summary

✅ 36 untouched benchmarks

[overlookmotel]

Hmm. This is really odd!

I've delved into it a bit more...

Further oddities

Acorn includes comments in the Program start span, but both Espree and TS-ESLint do not.

Espree and TS-ESLint also exclude whitespace (line breaks or spaces) from Program start span.

Babel also behaves the same as Acorn (span of the Program is the total length of the file). But Babel has its own AST shape which doesn't conform to ESTree, so that's not very relevant.

But... Espree also does not include any trailing comments or whitespace at end of the file in Program end span.

Espree is the default parser for ESLint, so TS-ESLint should be aiming to be compatible with it. The fact that TS-ESLint behaves differently for trailing comments/whitespace I think should be considered a bug in TS-ESLint.

But... should Espree and TS-ESLint be including comments/whitespace in Program span or not? ESTree spec doesn't seem to say anything about spans/ranges/locations. So there's no "spec" to conform to. So, who's right??

Comments:

• Acorn
• Babel
• Espree
• TS-ESLint

Whitespace:

• Acorn
• Babel
• Espree
• TS-ESLint

What to do?

We should raise an issue on TS-ESLint to point out the discrepancy between it and Espree. Personally, I'd like to see Program span include the whole file (Acorn style). It doesn't make much sense for Acorn and Espree be out of line with each other. But probably no-one will want to make a breaking change.

To fix our tests in meantime, I think doing what you've done in this PR is probably the best way forward. However, you could also take into account directives. That might pass a few more tests.

if let Some(first_directive) = self.directives.first() {
    self.span.start = first_directive.span.start;
} else if let Some(first_stmt) = self.body.first() {
    self.span.start = first_stmt.span.start;
}

As you say, there's likely something else going on the conformance runner, but it'll be easier to figure that out once directives are handled like TS-ESLint does.

[overlookmotel]

Also, please add a comment where you change the start span as to why this is necessary, and a link to this PR (or one of the TS-ESLint AST explorer links above).

[therewillbecode]

Also, please add a comment where you change the start span as to why this is necessary, and a link to this PR (or one of the TS-ESLint AST explorer links above).

Thanks @overlookmotel for looking into this! Yes odd indeed.

I have implemented your suggestions and hopefully my amendment to the comment gives sufficient rationale for the change.

[overlookmotel]

Great. No new failing tests now!

[overlookmotel]

I've opened an issue about TS-ESLint's non-alignment with Espree: typescript-eslint/typescript-eslint#11026