Inconsistant Span of Ident inside BindingIdent

[caoccao]

Describe the bug

I found the span of Ident inside BindingIdent is inconsistant. Here are 2 cases.

Case 1: BindingIdent doesn't Include TsTypeAnn

• Parse the following code.

const a: number

• The span of BindingIdent is as follows.

Span { lo: BytePos(7), hi: BytePos(8), ctxt: #0 }

• The AST is as follows.

BindingIdent {
    id: Ident {
        span: Span {
            lo: BytePos(
                7,
            ),
            hi: BytePos(
                8,
            ),
            ctxt: #0,
        },
        sym: "a",
        optional: false,
    },
    type_ann: Some(
        TsTypeAnn {
            span: Span {
                lo: BytePos(
                    8,
                ),
                hi: BytePos(
                    16,
                ),
                ctxt: #0,
            },
            type_ann: TsKeywordType(
                TsKeywordType {
                    span: Span {
                        lo: BytePos(
                            10,
                        ),
                        hi: BytePos(
                            16,
                        ),
                        ctxt: #0,
                    },
                    kind: TsNumberKeyword,
                },
            ),
        },
    ),
}

Case 2: BindingIdent Includes TsTypeAnn

• Parse the following code.

function b(a: number = 1) {}

• The span of BindingIdent is as follows.

Span { lo: BytePos(12), hi: BytePos(21), ctxt: #0 }

• The AST is as follows.

BindingIdent {
    id: Ident {
        span: Span {
            lo: BytePos(
                12,
            ),
            hi: BytePos(
                21,
            ),
            ctxt: #0,
        },
        sym: "a",
        optional: false,
    },
    type_ann: Some(
        TsTypeAnn {
            span: Span {
                lo: BytePos(
                    13,
                ),
                hi: BytePos(
                    21,
                ),
                ctxt: #0,
            },
            type_ann: TsKeywordType(
                TsKeywordType {
                    span: Span {
                        lo: BytePos(
                            15,
                        ),
                        hi: BytePos(
                            21,
                        ),
                        ctxt: #0,
                    },
                    kind: TsNumberKeyword,
                },
            ),
        },
    ),
}

Conclusions

• In case 1, the BindingIdent (7, 8) doesn't include the TsTypeAnn (8, 16) and the Ident (7, 8) is expected.
• In case 2, the BindingIdent (12, 21) includes the TsTypeAnn (13, 21) and the Ident (12, 21) is not expected because it includes the TsTypeAnn (13, 21) too.

Input code

function b(a: number = 1) {}

Config

{
  "jsc": {
    "parser": {
      "syntax": "typescript",
      "tsx": false
    },
    "target": "es2020",
    "loose": false,
    "minify": {
      "compress": false,
      "mangle": false
    }
  },
  "module": {
    "type": "es6"
  },
  "minify": false,
  "isModule": false
}

Playground link (or link to the minimal reproduction)

• 1.3.100

https://play.swc.rs/?version=1.3.100&code=H4sIAAAAAAAAA0srzUsuyczPU0jSSLRSyCvNTUotUrBVMNRUqK4FAOamXN8cAAAA&config=H4sIAAAAAAAAA1VPOw7DIAzdcwrkuUOUoUPv0EMg6kRE%2FIRdqSjK3QsB0maz38fveRuEgJUUPMSWx7wEGQnjuWeEkmP5yQhwCkgq6sBw6yxToWZpCA9orwywjAtycSFN4zQ2BxjvCbujYVY7Paf%2FTOVtiEh0FRapdIvBa%2BLQUsH61%2Fsg2y%2Blb21wh5%2Boh52HQdOzO%2BvZ%2FQtlF66JGAEAAA%3D%3D

• 1.4.13

https://play.swc.rs/?version=1.4.13&code=H4sIAAAAAAAAA0srzUsuyczPU0jSSLRSyCvNTUotUrBVMNRUqK4FAOamXN8cAAAA&config=H4sIAAAAAAAAA1VPOw7DIAzdcwrkuUOUoUPv0EMg6kRE%2FIRdqSjK3QsB0maz38fveRuEgJUUPMSWx7wEGQnjuWeEkmP5yQhwCkgq6sBw6yxToWZpCA9orwywjAtycSFN4zQ2BxjvCbujYVY7Paf%2FTOVtiEh0FRapdIvBa%2BLQUsH61%2Fsg2y%2Blb21wh5%2Boh52HQdOzO%2BvZ%2FQtlF66JGAEAAA%3D%3D

SWC Info output

No response

Expected behavior

The span of BindingIdent and Ident should be consistant.

• Either the BindingIdent includes the TsTypeAnn in both cases, or the BindingIdent doesn't include the TsTypeAnn in both cases.
• The Ident doesn't include the TsTypeAnn in both cases.

Actual behavior

No response

Version

• 1.3.100
• 1.4.13

Additional context

No response

[caoccao]

It seems this bug fix doesn't match my expectation.

The current behavior is as follows.

const a: number = 0;
// binding ident: 7, 8
//   ident: 7, 8
//   typeAnn: 10, 16

That seems to be a bit weird because the span of typeAnn is out of the span of binding ident though the typeAnn belongs to the binding ident.

My expectation is as follows.

const a: number = 0;
// binding ident: 7, 16
//   ident: 7, 8
//   typeAnn: 10, 16

[kdy1]

It' s not possible because BindingIdent does not have a span.

[caoccao]

It' s not possible because BindingIdent does not have a span.

That's still confusing to me. BindingIdent supports .span(). The return of that call is weird to me. I wonder if it's possible to fix that call?

[kdy1]

I don't think it's good idea. BindingIdent should deref to Id and it will make .span and .span() different

[caoccao]

Alright, thanks for the explanation.

[caoccao]

I just fixed that at my side by this commit.

let span = if let Some(type_ann) = self.type_ann.as_ref() {
  Span {
    lo: self.id.span.lo,
    hi: type_ann.as_ref().span.hi,
    ctxt: self.id.span.ctxt,
  }
} else {
  self.span()
};