refactor(parser): improve parsing of parse_function_or_constructor_type (#3892)

Boshen · Boshen · commit 187f0782c1ad · 2024-06-25T03:43:02.000Z
part of #3502
diff --git a/crates/oxc_parser/src/js/binding.rs b/crates/oxc_parser/src/js/binding.rs
@@ -31,7 +31,7 @@ impl<'a> ParserImpl<'a> {
         Ok(self.ast.binding_pattern(self.end_span(span), kind, type_annotation, optional))
     }
 
-    pub(super) fn parse_binding_pattern_kind(&mut self) -> Result<BindingPatternKind<'a>> {
+    pub(crate) fn parse_binding_pattern_kind(&mut self) -> Result<BindingPatternKind<'a>> {
         match self.cur_kind() {
             Kind::LCurly => self.parse_object_binding_pattern(),
             Kind::LBrack => self.parse_array_binding_pattern(),
diff --git a/crates/oxc_parser/src/ts/statement.rs b/crates/oxc_parser/src/ts/statement.rs
@@ -396,7 +396,6 @@ impl<'a> ParserImpl<'a> {
 
     pub(crate) fn parse_ts_type_assertion(&mut self) -> Result<Expression<'a>> {
         let span = self.start_span();
-        self.re_lex_ts_l_angle();
         self.expect(Kind::LAngle)?;
         let type_annotation = self.parse_ts_type()?;
         self.expect(Kind::RAngle)?;
diff --git a/crates/oxc_parser/src/ts/types.rs b/crates/oxc_parser/src/ts/types.rs
@@ -9,7 +9,6 @@ use super::list::{
 };
 use crate::{
     diagnostics,
-    js::list::{ArrayPatternList, ObjectPatternProperties},
     lexer::Kind,
     list::{NormalList, SeparatedList},
     modifiers::ModifierFlags,
@@ -19,18 +18,91 @@ use crate::{
 
 impl<'a> ParserImpl<'a> {
     pub(crate) fn parse_ts_type(&mut self) -> Result<TSType<'a>> {
-        if self.is_at_constructor_type() {
-            return self.parse_ts_constructor_type();
+        if self.is_start_of_function_type_or_constructor_type() {
+            return self.parse_function_or_constructor_type();
         }
+        let left_span = self.start_span();
+        let left = self.parse_ts_union_type()?;
+        self.parse_ts_conditional_type(left_span, left)
+    }
+
+    fn parse_function_or_constructor_type(&mut self) -> Result<TSType<'a>> {
+        let span = self.start_span();
+        let r#abstract = self.eat(Kind::Abstract);
+        let is_constructor_type = self.eat(Kind::New);
+        let type_parameters = self.parse_ts_type_parameters()?;
+        let (this_param, params) = self.parse_formal_parameters(FormalParameterKind::Signature)?;
+        self.expect(Kind::Arrow)?;
+        let return_type = {
+            let return_type_span = self.start_span();
+            let return_type = self.parse_ts_return_type()?;
+            self.ast.ts_type_annotation(self.end_span(return_type_span), return_type)
+        };
 
-        if self.is_at_function_type() {
-            return self.parse_ts_function_type();
+        let span = self.end_span(span);
+        Ok(if is_constructor_type {
+            if let Some(this_param) = &this_param {
+                // type Foo = new (this: number) => any;
+                self.error(diagnostics::ts_constructor_this_parameter(this_param.span));
+            }
+            self.ast.ts_constructor_type(span, r#abstract, params, return_type, type_parameters)
+        } else {
+            self.ast.ts_function_type(span, this_param, params, return_type, type_parameters)
+        })
+    }
+
+    fn is_start_of_function_type_or_constructor_type(&mut self) -> bool {
+        if self.at(Kind::LAngle) {
+            return true;
+        }
+        if self.at(Kind::LParen) && self.lookahead(Self::is_unambiguously_start_of_function_type) {
+            return true;
         }
+        self.at(Kind::New) || (self.at(Kind::Abstract) && self.peek_at(Kind::New))
+    }
 
-        let left_span = self.start_span();
-        let left = self.parse_ts_union_type()?;
+    fn is_unambiguously_start_of_function_type(&mut self) -> bool {
+        self.bump_any();
+        // ( )
+        // ( ...
+        if matches!(self.cur_kind(), Kind::RParen | Kind::Dot3) {
+            return true;
+        }
+        if self.skip_parameter_start() {
+            // ( xxx :
+            // ( xxx ,
+            // ( xxx ?
+            // ( xxx =
+            if matches!(self.cur_kind(), Kind::Colon | Kind::Comma | Kind::Question | Kind::Eq) {
+                return true;
+            }
+            // ( xxx ) =>
+            if self.eat(Kind::RParen) && self.at(Kind::Arrow) {
+                return true;
+            }
+        }
+        false
+    }
 
-        self.parse_ts_conditional_type(left_span, left)
+    fn skip_parameter_start(&mut self) -> bool {
+        // Skip modifiers
+        loop {
+            if self.cur_kind().is_modifier_kind() && !self.peek_at(Kind::Comma) {
+                self.bump_any();
+            } else {
+                break;
+            }
+        }
+        if self.cur_kind().is_identifier() || self.at(Kind::This) {
+            self.bump_any();
+            return true;
+        }
+        if matches!(self.cur_kind(), Kind::LBrack | Kind::LCurly)
+            && self.parse_binding_pattern_kind().is_ok()
+        {
+            return true;
+        }
+        false
     }
 
     pub(crate) fn parse_ts_type_parameters(
@@ -162,10 +234,6 @@ impl<'a> ParserImpl<'a> {
         Ok(left)
     }
 
-    fn is_at_constructor_type(&mut self) -> bool {
-        self.at(Kind::New) || (self.at(Kind::Abstract) && self.peek_at(Kind::New))
-    }
-
     // test ts ts_union_type
     // type A = string | number;
     // type B = | A | void | null;
@@ -474,51 +542,6 @@ impl<'a> ParserImpl<'a> {
         Ok(self.ast.ts_tuple_type(self.end_span(span), elements))
     }
 
-    fn is_at_function_type(&mut self) -> bool {
-        if self.at(Kind::LAngle) {
-            return true;
-        }
-
-        if !self.at(Kind::LParen) {
-            return false;
-        }
-
-        let checkpoint = self.checkpoint();
-
-        self.bump_any(); // bump (
-
-        if self.at(Kind::RParen) || self.at(Kind::Dot3) {
-            self.rewind(checkpoint);
-            return true;
-        }
-
-        let mut is_function_parameter_start =
-            self.at(Kind::This) || self.cur_kind().is_binding_identifier();
-
-        if is_function_parameter_start {
-            self.bump_any();
-        }
-
-        if match self.cur_kind() {
-            Kind::LBrack => ArrayPatternList::parse(self).is_ok(),
-            Kind::LCurly => ObjectPatternProperties::parse(self).is_ok(),
-            _ => false,
-        } {
-            is_function_parameter_start = true;
-        }
-
-        let result = if is_function_parameter_start {
-            matches!(self.cur_kind(), Kind::Colon | Kind::Eq | Kind::Comma | Kind::Question)
-                || (self.at(Kind::RParen) && self.peek_at(Kind::Arrow))
-        } else {
-            false
-        };
-
-        self.rewind(checkpoint);
-
-        result
-    }
-
     fn is_at_mapped_type(&mut self) -> bool {
         if !self.at(Kind::LCurly) {
             return false;
@@ -731,49 +754,6 @@ impl<'a> ParserImpl<'a> {
         Ok(TSImportAttributes { span, elements })
     }
 
-    fn parse_ts_constructor_type(&mut self) -> Result<TSType<'a>> {
-        let span = self.start_span();
-        let r#abstract = self.eat(Kind::Abstract);
-        self.expect(Kind::New)?;
-        let type_parameters = self.parse_ts_type_parameters()?;
-        let (this_param, params) = self.parse_formal_parameters(FormalParameterKind::Signature)?;
-
-        if let Some(this_param) = this_param {
-            // type Foo = new (this: number) => any;
-            self.error(diagnostics::ts_constructor_this_parameter(this_param.span));
-        }
-
-        self.expect(Kind::Arrow)?;
-        let return_type_span = self.start_span();
-        let return_type = self.parse_ts_return_type()?;
-        let return_type = self.ast.ts_type_annotation(self.end_span(return_type_span), return_type);
-
-        Ok(self.ast.ts_constructor_type(
-            self.end_span(span),
-            r#abstract,
-            params,
-            return_type,
-            type_parameters,
-        ))
-    }
-
-    fn parse_ts_function_type(&mut self) -> Result<TSType<'a>> {
-        let span = self.start_span();
-        let type_parameters = self.parse_ts_type_parameters()?;
-        let (this_param, params) = self.parse_formal_parameters(FormalParameterKind::Signature)?;
-        let return_type_span = self.start_span();
-        self.expect(Kind::Arrow)?;
-        let return_type = self.parse_ts_return_type()?;
-        let return_type = self.ast.ts_type_annotation(self.end_span(return_type_span), return_type);
-        Ok(self.ast.ts_function_type(
-            self.end_span(span),
-            this_param,
-            params,
-            return_type,
-            type_parameters,
-        ))
-    }
-
     fn parse_ts_infer_type(&mut self) -> Result<TSType<'a>> {
         let span = self.start_span();
         self.expect(Kind::Infer)?;
diff --git a/crates/oxc_semantic/src/checker/typescript.rs b/crates/oxc_semantic/src/checker/typescript.rs
@@ -48,7 +48,8 @@ pub fn check_formal_parameters(params: &FormalParameters, ctx: &SemanticBuilder<
         check_duplicate_bound_names(params, ctx);
     }
 
-    let is_inside_constructor = ctx.current_scope_flags().is_constructor();
+    let is_inside_constructor =
+        !params.kind.is_signature() && ctx.current_scope_flags().is_constructor();
     let mut has_optional = false;
 
     for item in &params.items {
diff --git a/tasks/coverage/parser_typescript.snap b/tasks/coverage/parser_typescript.snap
@@ -3999,20 +3999,18 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  2 │ }
    ╰────
 
-  × Expected `)` but found `Identifier`
-   ╭─[compiler/ParameterList5.ts:1:23]
+  × A parameter property is only allowed in a constructor implementation.
+   ╭─[compiler/ParameterList5.ts:1:16]
  1 │ function A(): (public B) => C {
-   ·                       ┬
-   ·                       ╰── `)` expected
+   ·                ────────
  2 │ }
    ╰────
 
-  × Expected `)` but found `Identifier`
-   ╭─[compiler/ParameterList6.ts:2:26]
+  × A parameter property is only allowed in a constructor implementation.
+   ╭─[compiler/ParameterList6.ts:2:19]
  1 │ class C {
  2 │   constructor(C: (public A) => any) {
-   ·                          ┬
-   ·                          ╰── `)` expected
+   ·                   ────────
  3 │   }
    ╰────
 
@@ -17725,20 +17723,18 @@ Expect to Parse: "conformance/salsa/plainJSRedeclare3.ts"
  2 │ }
    ╰────
 
-  × Expected `)` but found `Identifier`
-   ╭─[conformance/parser/ecmascript5/ParameterLists/parserParameterList5.ts:1:23]
+  × A parameter property is only allowed in a constructor implementation.
+   ╭─[conformance/parser/ecmascript5/ParameterLists/parserParameterList5.ts:1:16]
  1 │ function A(): (public B) => C {
-   ·                       ┬
-   ·                       ╰── `)` expected
+   ·                ────────
  2 │ }
    ╰────
 
-  × Expected `)` but found `Identifier`
-   ╭─[conformance/parser/ecmascript5/ParameterLists/parserParameterList6.ts:2:26]
+  × A parameter property is only allowed in a constructor implementation.
+   ╭─[conformance/parser/ecmascript5/ParameterLists/parserParameterList6.ts:2:19]
  1 │ class C {
  2 │   constructor(C: (public A) => any) {
-   ·                          ┬
-   ·                          ╰── `)` expected
+   ·                   ────────
  3 │   }
    ╰────
 

Original file line number	Diff line number	Diff line change
`@@ -31,7 +31,7 @@ impl<'a> ParserImpl<'a> {`
`31`	`31`	`Ok(self.ast.binding_pattern(self.end_span(span), kind, type_annotation, optional))`
`32`	`32`	`}`
`33`	`33`
`34`		`- pub(super) fn parse_binding_pattern_kind(&mut self) -> Result<BindingPatternKind<'a>> {`
	`34`	`+ pub(crate) fn parse_binding_pattern_kind(&mut self) -> Result<BindingPatternKind<'a>> {`
`35`	`35`	`match self.cur_kind() {`
`36`	`36`	`Kind::LCurly => self.parse_object_binding_pattern(),`
`37`	`37`	`Kind::LBrack => self.parse_array_binding_pattern(),`
Original file line number	Diff line number	Diff line change
`@@ -48,7 +48,8 @@ pub fn check_formal_parameters(params: &FormalParameters, ctx: &SemanticBuilder<`
`48`	`48`	`check_duplicate_bound_names(params, ctx);`
`49`	`49`	`}`
`50`	`50`
`51`		`- let is_inside_constructor = ctx.current_scope_flags().is_constructor();`
	`51`	`+ let is_inside_constructor =`
	`52`	`+ !params.kind.is_signature() && ctx.current_scope_flags().is_constructor();`
`52`	`53`	`let mut has_optional = false;`
`53`	`54`
`54`	`55`	`for item in &params.items {`