fix(rome_js_parser): allow readonly property with initializer in ambient context

[magic-akari]

Summary

• fix 🐛 Parser: Initializers should be allowed for ambientreadonly in TsPropertySignatureClassMember #2958

Test Plan

[magic-akari]

I am not familiar with this codebase.
I have no idea about why I got failed test result without diagnostics.

Is there a step I'm missing?

[IWANABETHATGUY]

You should run UPDATE_EXPECT=1 cargo test -p rome_js_parser to update the snapshot

[magic-akari]

You should run UPDATE_EXPECT=1 cargo test -p rome_js_parser to update the snapshot

No. It does not work. No rast files generated.
It seems that there are still some errors in the parsing process. But there is no diagnostic output.

failures:

---- tests::parser::ok::ts_property_readonly_initializer_ambient_context stdout ----
thread 'tests::parser::ok::ts_property_readonly_initializer_ambient_context' panicked at 'There should be no errors in the file "/Users/akari/Github/tools/crates/rome_js_parser/test_data/inline/ok/ts_property_readonly_initializer_ambient_context.ts" but the following errors where present:


Parsed tree:
0: JS_MODULE@0..86
  0: (empty)
  1: JS_DIRECTIVE_LIST@0..0
  2: JS_MODULE_ITEM_LIST@0..85
    0: TS_DECLARE_STATEMENT@0..42
      0: DECLARE_KW@0..8 "declare" [] [Whitespace(" ")]
      1: JS_CLASS_DECLARATION@8..42
        0: (empty)
        1: CLASS_KW@8..14 "class" [] [Whitespace(" ")]
        2: JS_IDENTIFIER_BINDING@14..16
          0: IDENT@14..16 "A" [] [Whitespace(" ")]
        3: (empty)
        4: (empty)
        5: (empty)
        6: L_CURLY@16..18 "{" [] [Whitespace(" ")]
        7: JS_CLASS_MEMBER_LIST@18..41
          0: JS_UNKNOWN_MEMBER@18..41
            0: TS_PROPERTY_SIGNATURE_MODIFIER_LIST@18..27
              0: TS_READONLY_MODIFIER@18..27
                0: READONLY_KW@18..27 "readonly" [] [Whitespace(" ")]
            1: JS_LITERAL_MEMBER_NAME@27..32
              0: IDENT@27..32 "prop" [] [Whitespace(" ")]
            2: JS_INITIALIZER_CLAUSE@32..41
              0: EQ@32..34 "=" [] [Whitespace(" ")]
              1: JS_STRING_LITERAL_EXPRESSION@34..41
                0: JS_STRING_LITERAL@34..41 "\"test\"" [] [Whitespace(" ")]
        8: R_CURLY@41..42 "}" [] []
    1: JS_CLASS_DECLARATION@42..85
      0: (empty)
      1: CLASS_KW@42..49 "class" [Newline("\n")] [Whitespace(" ")]
      2: JS_IDENTIFIER_BINDING@49..51
        0: IDENT@49..51 "B" [] [Whitespace(" ")]
      3: (empty)
      4: (empty)
      5: (empty)
      6: L_CURLY@51..53 "{" [] [Whitespace(" ")]
      7: JS_CLASS_MEMBER_LIST@53..84
        0: JS_UNKNOWN_MEMBER@53..84
          0: TS_PROPERTY_SIGNATURE_MODIFIER_LIST@53..70
            0: TS_DECLARE_MODIFIER@53..61
              0: DECLARE_KW@53..61 "declare" [] [Whitespace(" ")]
            1: TS_READONLY_MODIFIER@61..70
              0: READONLY_KW@61..70 "readonly" [] [Whitespace(" ")]
          1: JS_LITERAL_MEMBER_NAME@70..75
            0: IDENT@70..75 "prop" [] [Whitespace(" ")]
          2: JS_INITIALIZER_CLAUSE@75..84
            0: EQ@75..77 "=" [] [Whitespace(" ")]
            1: JS_STRING_LITERAL_EXPRESSION@77..84
              0: JS_STRING_LITERAL@77..84 "\"test\"" [] [Whitespace(" ")]
      8: R_CURLY@84..85 "}" [] []
  3: EOF@85..86 "" [Newline("\n")] []
', crates/rome_js_parser/src/test_utils.rs:53:5


failures:
    tests::parser::ok::ts_property_readonly_initializer_ambient_context

[IWANABETHATGUY]

Looks like I am wrong

[ematipico]

@MichaReiser @xunilrj any chance you can help here?

[IWANABETHATGUY]

The first syntax error happened here.

[ematipico]

The first syntax error happened here.

The issue we are trying to fix is around d.ts files - declaration files. In the playground we don't have that option yet, that's why you see the parser error. This is my guess.

[MichaReiser]

The issue is that the TsPropertySignatureClassMember type doesn't allow for an initializer.

TsPropertySignatureClassMember =
	modifiers: TsPropertySignatureModifierList
	name: JsAnyClassMemberName
	property_annotation: TsAnyPropertySignatureAnnotation?
	';'?

I'm not entirely sure how we want to represent this in the AST because where the initializer is allowed is more subtle:

 declare class A { readonly prop: string = "test" } // <- not OK
 declare class A { readonly prop? = "test" } // OK
 declare class A { readonly prop = "test" } // OK

I'm not sure if this isn't just a bug in the typescript compiler as the rules seem a bit arbitrary (in general, my experience when working on the TS parser was that typescript isn't consistent with error messages inside of ambient contexts).

[IWANABETHATGUY]

I am going to make a pr to resolve the missing initialization of TsPropertySignatureClassMember

[MichaReiser]

I am going to make a pr to resolve the missing initialization of TsPropertySignatureClassMember

I'm not sure if just adding It to TsPropertySignatureMember is the way to go as it isn't valid in many places. It would be nice if it could be expressed by the tree in which places the initializer is valid but I'm doubtful this is possible with typescript's inconsistency (I don't see the benefit of having an initializer in a declaration file).

It may be worth opening a bug report on the typescript repo to check in if this is the expected behaviour or not.

[IWANABETHATGUY]

#3003

[github-actions]

This PR is stale because it has been open 14 days with no activity.

[IWANABETHATGUY]

This pull request should not be closed, since it has some upstream blocking #3003, I will have a look this week, last week I was tackling the instantiation expression

[IWANABETHATGUY]

@ematipico , maybe we should diss about the workflow for the stale issue and pull request.

[nissy-dev]

This issue is solved by #4225