Incorrect Span on BindingIdentifier

[DonIsaac]

Spans for binding identifiers cover the identifier itself and the type annotation on BindingPattern. This was introduced in #2624, and is blocking oxc-project/backlog#84.

Expected Behavior

//    ___________ BindingPattern
const foo: number = 1
//    ^^^-------- TSTypeAnnotation
//    ^^^BindingIdentifier

Actual Behavior

//    ___________ BindingPattern and BindingIdentifier
const foo: number = 1
//       -------- TSTypeAnnotation

[Boshen]

This is a weird one ...

[Dunqing]

This is also weird in TypeScript, Why not { pos: 6, end: 7 }?

[Boshen]

Shall we go for the more intuitive version? i.e.

//    ___________ BindingPattern
const foo: number = 1
//       ^^^^^^^^ TSTypeAnnotation
//    ^^^BindingIdentifier

[DonIsaac]

The cause is BindingPattern gets its span from kind, which covers the whole BindingPattern. I see two possible solutions:

1. give BindingPattern its own span field, which would increase its size to 40 bytes in total
2. derive span as kind.span().merge(type_annotation.span), which would save size at the cost of a) GetSpan performance, b) we could not implement GetSpanMut, and c) we'd need to create a custom GetSpan impl for BindingPattern.

[Boshen]

Ahh, I don't like either of these options. What if we accept that it's the way it is and hack it in no-unused-vars alone?

[DonIsaac]

That's also acceptable assuming nobody else needs this.

[magic-akari]

This is weird in SWC.

swc-project/swc#8856

[overlookmotel]

In my opinion, we should add a span field to BindingPattern.

Yes, that costs 8 bytes. But having a weird oddity like this is a footgun, and is bound to lead to trouble. I ran into it on #4473 and couldn't understand what was going on, and Don's also run into it here. That's 2 people tripped up by this in 2 weeks!

If we wanted to, later on we could potentially optimize that field out again by making span access via a span() method only. But we could also do the same thing to various other structs (e.g. BinaryExpression), so I don't think it makes a great deal of sense to only do it on BindingPattern.

[Boshen]

Close as won't fix. Let's try keep this as is and hack no-unused-vars.