refactor(parser): move require after optional param check to parser (#24600)

## Summary
- report TS1016 while parsing formal parameter lists
- preserve initialized-parameter and rest-parameter behavior
- remove the equivalent semantic check and diagnostic
- update parser and semantic coverage snapshots

Follows #24596, which has now merged.

crates/oxc_isolated_declarations/tests/fixtures/function-parameters.ts

@@ -17,7 +17,7 @@ export function fooGood3({a = x, b: [{c = y}]}: object): void {}
 // Incorrect
 export function fnDeclBad<T>(p: T = [], rParam: T = "", r2: T): void { }
 export function fnDeclBad2<T>(p: T = [], r2: T): void { }
-export function fnDeclBad3<T>(p: T = [], rParam?: T, r2: T): void { }
+export function fnDeclBad3<T>(p: T = [], r2: T, rParam?: T): void { }
 
 export function fooBad([a, b] = [1, 2]): number {
   return 2;
@@ -34,4 +34,4 @@ export function withUnknown(a: unknown = 1, b: string): void { }
 export function withTypeAssertion(a = /regular-repression-cannot-infer/ as any): void { }
 
 export function restWithDefault(...[value, options = {}]: [string, object]): void { }
-export function restWithNestedDefault(...[a, { b = 1, c = 2 }]: [number, { b: number, c: number }]): void { }
+export function restWithNestedDefault(...[a, { b = 1, c = 2 }]: [number, { b: number, c: number }]): void { }

crates/oxc_isolated_declarations/tests/snapshots/function-parameters.snap

@@ -12,7 +12,7 @@ export declare const fooGood2: ({ a, b }?: object) => number;
 export declare function fooGood3({ a, b: [{ c }] }: object): void;
 export declare function fnDeclBad<T>(p: T, rParam: T, r2: T): void;
 export declare function fnDeclBad2<T>(p: T, r2: T): void;
-export declare function fnDeclBad3<T>(p: T, rParam?: T, r2: T): void;
+export declare function fnDeclBad3<T>(p: T, r2: T, rParam?: T): void;
 export declare function fooBad(): number;
 export declare const fooBad2: () => number;
 export declare function withAny(a: any, b: string): void;
@@ -51,14 +51,14 @@ export declare function restWithNestedDefault(...[a, { b, c }]: [number, {
  18 | export function fnDeclBad<T>(p: T = [], rParam: T = "", r2: T): void { }
  19 | export function fnDeclBad2<T>(p: T = [], r2: T): void { }
     :                               ^^^^^^^^^
- 20 | export function fnDeclBad3<T>(p: T = [], rParam?: T, r2: T): void { }
+ 20 | export function fnDeclBad3<T>(p: T = [], r2: T, rParam?: T): void { }
     `----
 
   x TS9025: Declaration emit for this parameter requires implicitly adding
   | undefined to it's type. This is not supported with --isolatedDeclarations.
     ,-[20:31]
  19 | export function fnDeclBad2<T>(p: T = [], r2: T): void { }
- 20 | export function fnDeclBad3<T>(p: T = [], rParam?: T, r2: T): void { }
+ 20 | export function fnDeclBad3<T>(p: T = [], r2: T, rParam?: T): void { }
     :                               ^^^^^^^^^
  21 | 
     `----

crates/oxc_linter/src/rules/eslint/default_param_last.rs

@@ -232,9 +232,9 @@ fn test() {
         "function foo(a = 1, b: number, c = 2, d: number) {}",
         "function foo(a = 1, b: number, c = 2) {}",
         "function foo(a = 1, b: number, ...c) {}",
-        "function foo(a?: number, b: number) {}",
-        "function foo(a: number, b?: number, c: number) {}",
-        "function foo(a = 1, b?: number, c: number) {}",
+        // "function foo(a?: number, b: number) {}",
+        // "function foo(a: number, b?: number, c: number) {}",
+        // "function foo(a = 1, b?: number, c: number) {}",
         "function foo(a = 1, { b }) {}",
         "function foo({ a } = {}, b) {}",
         "function foo({ a, b } = { a: 1, b: 2 }, c) {}",
@@ -244,8 +244,8 @@ fn test() {
         "const foo = function (a = 1, b: number, c = 2) {};",
         "const foo = function (a = 1, b: number, ...c) {};",
         "const foo = function (a?: number, b: number) {};",
-        "const foo = function (a: number, b?: number, c: number) {};",
-        "const foo = function (a = 1, b?: number, c: number) {};",
+        // "const foo = function (a: number, b?: number, c: number) {};",
+        // "const foo = function (a = 1, b?: number, c: number) {};",
         "const foo = function (a = 1, { b }) {};",
         "const foo = function ({ a } = {}, b) {};",
         "const foo = function ({ a, b } = { a: 1, b: 2 }, c) {};",
@@ -254,8 +254,8 @@ fn test() {
         "const foo = (a = 1, b: number, c = 2, d: number) => {};",
         "const foo = (a = 1, b: number, c = 2) => {};",
         "const foo = (a = 1, b: number, ...c) => {};",
-        "const foo = (a?: number, b: number) => {};",
-        "const foo = (a: number, b?: number, c: number) => {};",
+        // "const foo = (a?: number, b: number) => {};",
+        // "const foo = (a: number, b?: number, c: number) => {};",
         "const foo = (a = 1, b?: number, c: number) => {};",
         "
         class Foo {

crates/oxc_linter/src/snapshots/eslint_default_param_last.snap

@@ -142,34 +142,6 @@ source: crates/oxc_linter/src/tester.rs
    ╰────
   help: Enforce default parameters to be last.
 
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:14]
- 1 │ function foo(a?: number, b: number) {}
-   ·              ──────────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:25]
- 1 │ function foo(a: number, b?: number, c: number) {}
-   ·                         ──────────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:21]
- 1 │ function foo(a = 1, b?: number, c: number) {}
-   ·                     ──────────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:14]
- 1 │ function foo(a = 1, b?: number, c: number) {}
-   ·              ─────
-   ╰────
-  help: Enforce default parameters to be last.
-
   ⚠ eslint(default-param-last): Default parameters should be last
    ╭─[default_param_last.tsx:1:14]
  1 │ function foo(a = 1, { b }) {}
@@ -240,33 +212,11 @@ source: crates/oxc_linter/src/tester.rs
    ╰────
   help: Enforce default parameters to be last.
 
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:23]
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[default_param_last.tsx:1:35]
  1 │ const foo = function (a?: number, b: number) {};
-   ·                       ──────────
+   ·                                   ─────────
    ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:34]
- 1 │ const foo = function (a: number, b?: number, c: number) {};
-   ·                                  ──────────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:30]
- 1 │ const foo = function (a = 1, b?: number, c: number) {};
-   ·                              ──────────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:23]
- 1 │ const foo = function (a = 1, b?: number, c: number) {};
-   ·                       ─────
-   ╰────
-  help: Enforce default parameters to be last.
 
   ⚠ eslint(default-param-last): Default parameters should be last
    ╭─[default_param_last.tsx:1:23]
@@ -338,42 +288,19 @@ source: crates/oxc_linter/src/tester.rs
    ╰────
   help: Enforce default parameters to be last.
 
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:14]
- 1 │ const foo = (a?: number, b: number) => {};
-   ·              ──────────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:25]
- 1 │ const foo = (a: number, b?: number, c: number) => {};
-   ·                         ──────────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:21]
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[default_param_last.tsx:1:33]
  1 │ const foo = (a = 1, b?: number, c: number) => {};
-   ·                     ──────────
+   ·                                 ─────────
    ╰────
-  help: Enforce default parameters to be last.
 
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:1:14]
- 1 │ const foo = (a = 1, b?: number, c: number) => {};
-   ·              ─────
-   ╰────
-  help: Enforce default parameters to be last.
-
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:4:17]
- 3 │             constructor(
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[default_param_last.tsx:5:17]
  4 │                 public a?: number,
-   ·                 ─────────────────
  5 │                 private b: number,
+   ·                 ─────────────────
+ 6 │             ) {}
    ╰────
-  help: Enforce default parameters to be last.
 
   ⚠ eslint(default-param-last): Default parameters should be last
    ╭─[default_param_last.tsx:3:25]
@@ -384,14 +311,13 @@ source: crates/oxc_linter/src/tester.rs
    ╰────
   help: Enforce default parameters to be last.
 
-  ⚠ eslint(default-param-last): Default parameters should be last
-   ╭─[default_param_last.tsx:2:25]
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[default_param_last.tsx:2:37]
  1 │ class Foo {
  2 │             constructor(a?: number, b: number) {}
-   ·                         ──────────
+   ·                                     ─────────
  3 │         }
    ╰────
-  help: Enforce default parameters to be last.
 
   ⚠ eslint(default-param-last): Default parameters should be last
    ╭─[default_param_last.tsx:3:17]

crates/oxc_parser/src/diagnostics.rs

@@ -1256,6 +1256,11 @@ pub fn abstract_with_private_identifier(span: Span) -> OxcDiagnostic {
         .with_label(span)
 }
 
+#[cold]
+pub fn required_parameter_after_optional_parameter(span: Span) -> OxcDiagnostic {
+    ts_error("1016", "A required parameter cannot follow an optional parameter.").with_label(span)
+}
+
 #[cold]
 pub fn jsx_expressions_may_not_use_the_comma_operator(span: Span) -> OxcDiagnostic {
     ts_error("18007", "JSX expressions may not use the comma operator")

crates/oxc_parser/src/js/function.rs

@@ -73,6 +73,7 @@ impl<'a, C: Config> ParserImpl<'a, C> {
         let mut list = ArenaVec::new_in(self);
         let mut rest: Option<ArenaBox<'a, FormalParameterRest<'a>>> = None;
         let mut first = true;
+        let mut has_optional = false;
 
         loop {
             let kind = self.cur_kind();
@@ -143,7 +144,16 @@ impl<'a, C: Config> ParserImpl<'a, C> {
                     self,
                 ));
             } else {
-                list.push(self.parse_formal_parameter_with_decorators(func_kind, span, decorators));
+                let param =
+                    self.parse_formal_parameter_with_decorators(func_kind, span, decorators);
+                if param.optional {
+                    has_optional = true;
+                } else if has_optional && param.initializer.is_none() {
+                    self.error(diagnostics::required_parameter_after_optional_parameter(
+                        param.span,
+                    ));
+                }
+                list.push(param);
             }
         }
 

crates/oxc_semantic/src/checker/typescript.rs

@@ -87,17 +87,6 @@ pub fn check_formal_parameters(params: &FormalParameters, ctx: &SemanticBuilder<
     if params.kind == FormalParameterKind::Signature && params.items.len() > 1 {
         check_duplicate_bound_names(params, ctx);
     }
-
-    let mut has_optional = false;
-
-    for param in &params.items {
-        // function a(optional?: number, required: number) { }
-        if param.optional {
-            has_optional = true;
-        } else if has_optional && param.initializer.is_none() {
-            ctx.error(diagnostics::required_parameter_after_optional_parameter(param.span));
-        }
-    }
 }
 
 fn check_duplicate_bound_names<'a, T: BoundNames<'a>>(bound_names: &T, ctx: &SemanticBuilder<'_>) {

crates/oxc_semantic/src/diagnostics.rs

@@ -306,11 +306,6 @@ pub fn jsdoc_type_in_annotation(
     .with_help(format!("Did you mean to write '{suggested_type}'?"))
 }
 
-#[cold]
-pub fn required_parameter_after_optional_parameter(span: Span) -> OxcDiagnostic {
-    ts_error("1016", "A required parameter cannot follow an optional parameter.").with_label(span)
-}
-
 #[cold]
 pub fn not_allowed_namespace_declaration(span: Span) -> OxcDiagnostic {
     ts_error(

tasks/coverage/snapshots/parser_babel.snap

@@ -164,14 +164,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
 
 Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties/input.ts
 
-  × TS(1015): A parameter cannot have a question mark and an initializer.
-    ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties/input.ts:10:16]
-  9 │         readonly x = 0,
- 10 │         public y?: number = 0) {}
-    ·                ─
- 11 │ }
-    ╰────
-
   × TS(1016): A required parameter cannot follow an optional parameter.
    ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties/input.ts:7:9]
  6 │         private pi?: number,
@@ -180,6 +172,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
  8 │         // Also works on AssignmentPattern
    ╰────
 
+  × TS(1015): A parameter cannot have a question mark and an initializer.
+    ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties/input.ts:10:16]
+  9 │         readonly x = 0,
+ 10 │         public y?: number = 0) {}
+    ·                ─
+ 11 │ }
+    ╰────
+
 Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/properties/input.ts
 
   × Identifier `x` has already been declared
@@ -13332,6 +13332,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
    ╰────
   help: No modifiers are allowed here.
 
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:7:9]
+ 6 │         private pi?: number,
+ 7 │         public readonly pur,
+   ·         ───────────────────
+ 8 │         // Also works on AssignmentPattern
+   ╰────
+
   × TS(1090): 'readonly' modifier cannot appear on a parameter.
     ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:9:9]
   8 │         // Also works on AssignmentPattern
@@ -13358,14 +13366,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
  11 │ }
     ╰────
 
-  × TS(1016): A required parameter cannot follow an optional parameter.
-   ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:7:9]
- 6 │         private pi?: number,
- 7 │         public readonly pur,
-   ·         ───────────────────
- 8 │         // Also works on AssignmentPattern
-   ╰────
-
   × TS(18019): 'abstract' modifier cannot be used with a private identifier.
    ╭─[babel/packages/babel-parser/test/fixtures/typescript/class/private-fields-modifier-abstract/input.ts:2:12]
  1 │ abstract class A {
@@ -13873,6 +13873,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
    ╰────
   help: No modifiers are allowed here.
 
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:6:3]
+ 5 │   private pi?: number,
+ 6 │   public readonly pur,
+   ·   ───────────────────
+ 7 │   readonly x = 0,
+   ╰────
+
   × TS(1090): 'readonly' modifier cannot appear on a parameter.
    ╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:7:3]
  6 │   public readonly pur,
@@ -13899,14 +13907,6 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
  9 │ ) {}
    ╰────
 
-  × TS(1016): A required parameter cannot follow an optional parameter.
-   ╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:6:3]
- 5 │   private pi?: number,
- 6 │   public readonly pur,
-   ·   ───────────────────
- 7 │   readonly x = 0,
-   ╰────
-
   × TS(1016): A required parameter cannot follow an optional parameter.
    ╭─[babel/packages/babel-parser/test/fixtures/typescript/function/pattern-optional-parameters/input.ts:1:17]
  1 │ function f([]?, {}) {}

tasks/coverage/snapshots/parser_typescript.snap

@@ -7355,6 +7355,13 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/types/wit
     ╰────
   help: Try inserting a semicolon here
 
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[typescript/tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors1.ts:1:9]
+ 1 │ (arg1?, arg2) => 101;
+   ·         ────
+ 2 │ (...arg?) => 102;
+   ╰────
+
   × TS(1047): A rest parameter cannot be optional
    ╭─[typescript/tests/cases/compiler/fatarrowfunctionsOptionalArgsErrors1.ts:2:8]
  1 │ (arg1?, arg2) => 101;
@@ -23292,6 +23299,22 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/types/wit
  18 │ 
     ╰────
 
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[typescript/tests/cases/conformance/functions/functionOverloadErrorsSyntax.ts:2:27]
+ 1 │ //Function overload signature with optional parameter followed by non-optional parameter
+ 2 │ function fn4a(x?: number, y: string);
+   ·                           ─────────
+ 3 │ function fn4a() { }
+   ╰────
+
+  × TS(1016): A required parameter cannot follow an optional parameter.
+   ╭─[typescript/tests/cases/conformance/functions/functionOverloadErrorsSyntax.ts:5:38]
+ 4 │ 
+ 5 │ function fn4b(n: string, x?: number, y: string);
+   ·                                      ─────────
+ 6 │ function fn4b() { }
+   ╰────
+
   × A rest parameter must be last in a parameter list
     ╭─[typescript/tests/cases/conformance/functions/functionOverloadErrorsSyntax.ts:9:25]
   8 │ //Function overload signature with rest param followed by non-optional parameter

tasks/coverage/snapshots/semantic_babel.snap

@@ -126,8 +126,8 @@ Identifier `show` has already been declared
 Identifier `size` has already been declared
 
 semantic Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties/input.ts
-A parameter cannot have a question mark and an initializer.
 A required parameter cannot follow an optional parameter.
+A parameter cannot have a question mark and an initializer.
 
 semantic Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/properties/input.ts
 Identifier `x` has already been declared
@@ -230,6 +230,9 @@ rebuilt        : ScopeId(0): []
 
 semantic Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/function/declare-pattern-parameters/input.ts
 A required parameter cannot follow an optional parameter.
+Bindings mismatch:
+after transform: ScopeId(0): ["f"]
+rebuilt        : ScopeId(0): []
 
 semantic Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/function/overloads/input.ts
 Bindings mismatch: