Skip to content

Commit 1199cee

Browse files
camc314camc314
committed
fix(parser): reject invalid modifiers on parameter properties with binding patterns (#16083)
Fixes #16077
1 parent a18c065 commit 1199cee

File tree

11 files changed

+211
-27
lines changed

11 files changed

+211
-27
lines changed

‎crates/oxc_parser/src/diagnostics.rs‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -813,6 +813,12 @@ pub fn cannot_appear_on_a_parameter(
813813
.with_allowed_modifier_help(allowed)
814814
}
815815

816+
#[cold]
817+
pub fn parameter_property_cannot_be_binding_pattern(span: Span) -> OxcDiagnostic {
818+
ts_error("1187", "A parameter property may not be declared using a binding pattern.")
819+
.with_label(span)
820+
}
821+
816822
pub fn cannot_appear_on_an_index_signature(
817823
modifier: &Modifier,
818824
allowed: Option<ModifierFlags>,

‎crates/oxc_parser/src/js/function.rs‎

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -93,6 +93,25 @@ impl<'a> ParserImpl<'a> {
9393
);
9494
}
9595
let pattern = self.parse_binding_pattern_with_initializer();
96+
97+
if modifiers.accessibility().is_some()
98+
|| modifiers.contains_readonly()
99+
|| modifiers.contains_override()
100+
{
101+
let is_valid_pattern = match &pattern.kind {
102+
BindingPatternKind::BindingIdentifier(_) => true,
103+
BindingPatternKind::AssignmentPattern(assignment) => {
104+
assignment.left.kind.is_binding_identifier()
105+
}
106+
_ => false,
107+
};
108+
if !is_valid_pattern {
109+
self.error(diagnostics::parameter_property_cannot_be_binding_pattern(Span::new(
110+
span,
111+
self.prev_token_end,
112+
)));
113+
}
114+
}
96115
let are_decorators_allowed =
97116
matches!(func_kind, FunctionKind::ClassMethod | FunctionKind::Constructor)
98117
&& self.is_ts;

‎tasks/coverage/misc/fail/oxc-16077.ts‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
class A {constructor(override []){}}
2+
class B {constructor(private []){}}
3+
class C {constructor(protected []){}}
4+
class D {constructor(public []){}}
5+
class E {constructor(readonly []){}}
6+
class F {constructor(private {}){}}
7+
class G {constructor(public {a} = {}){}}
8+
class H {constructor(readonly [b] = []){}}

‎tasks/coverage/misc/pass/oxc-16077.ts‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
// Valid parameter properties with simple identifiers
2+
class ValidA {constructor(private x){}}
3+
class ValidB {constructor(public y){}}
4+
class ValidC {constructor(protected z){}}
5+
class ValidD {constructor(readonly w){}}
6+
class ValidF {constructor(private x = 1){}}
7+
class ValidG {constructor(public y: number = 2){}}

‎tasks/coverage/snapshots/codegen_misc.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
codegen_misc Summary:
2-
AST Parsed : 51/51 (100.00%)
3-
Positive Passed: 51/51 (100.00%)
2+
AST Parsed : 52/52 (100.00%)
3+
Positive Passed: 52/52 (100.00%)

‎tasks/coverage/snapshots/formatter_misc.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,3 @@
11
formatter_misc Summary:
2-
AST Parsed : 51/51 (100.00%)
3-
Positive Passed: 51/51 (100.00%)
2+
AST Parsed : 52/52 (100.00%)
3+
Positive Passed: 52/52 (100.00%)

‎tasks/coverage/snapshots/parser_babel.snap‎

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ commit: 99dcba5e
33
parser_babel Summary:
44
AST Parsed : 2422/2440 (99.26%)
55
Positive Passed: 2396/2440 (98.20%)
6-
Negative Passed: 1689/1752 (96.40%)
6+
Negative Passed: 1690/1752 (96.46%)
77
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/es2022/private-in/invalid-private-followed-by-in-2/input.js
88

99
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/es2026/async-explicit-resource-management/invalid-script-top-level-using-binding/input.js
@@ -40,8 +40,6 @@ Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/ty
4040

4141
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/modifiers-invalid-order/input.ts
4242

43-
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-binding-patterns/input.ts
44-
4543
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/class/private-fields-modifier-abstract/input.ts
4644

4745
Expect Syntax Error: tasks/coverage/babel/packages/babel-parser/test/fixtures/typescript/declare/invalid-namespace-var/input.ts
@@ -13407,6 +13405,22 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1340713405
9 │ readonly *[e?.e]?() { }
1340813406
╰────
1340913407

13408+
× TS(1187): A parameter property may not be declared using a binding pattern.
13409+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-binding-patterns/input.ts:2:17]
13410+
1 │ class C {
13411+
2 │ constructor(public []) {}
13412+
· ─────────
13413+
3 │ }
13414+
╰────
13415+
13416+
× TS(1187): A parameter property may not be declared using a binding pattern.
13417+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-binding-patterns/input.ts:5:17]
13418+
4 │ class D {
13419+
5 │ constructor(public {}) {}
13420+
· ─────────
13421+
6 │ }
13422+
╰────
13423+
1341013424
× TS(1090): 'readonly' modifier cannot appear on a parameter.
1341113425
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:3:9]
1341213426
2 │ not_constructor(

‎tasks/coverage/snapshots/parser_misc.snap‎

Lines changed: 65 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
parser_misc Summary:
2-
AST Parsed : 51/51 (100.00%)
3-
Positive Passed: 51/51 (100.00%)
4-
Negative Passed: 118/118 (100.00%)
2+
AST Parsed : 52/52 (100.00%)
3+
Positive Passed: 52/52 (100.00%)
4+
Negative Passed: 119/119 (100.00%)
55

66
× Cannot assign to 'arguments' in strict mode
77
╭─[misc/fail/arguments-eval.ts:1:10]
@@ -3143,6 +3143,68 @@ Negative Passed: 118/118 (100.00%)
31433143
· ╰── `{` expected
31443144
╰────
31453145
3146+
× TS(1187): A parameter property may not be declared using a binding pattern.
3147+
╭─[misc/fail/oxc-16077.ts:1:22]
3148+
1 │ class A {constructor(override []){}}
3149+
· ───────────
3150+
2 │ class B {constructor(private []){}}
3151+
╰────
3152+
3153+
× TS(1187): A parameter property may not be declared using a binding pattern.
3154+
╭─[misc/fail/oxc-16077.ts:2:22]
3155+
1 │ class A {constructor(override []){}}
3156+
2 │ class B {constructor(private []){}}
3157+
· ──────────
3158+
3 │ class C {constructor(protected []){}}
3159+
╰────
3160+
3161+
× TS(1187): A parameter property may not be declared using a binding pattern.
3162+
╭─[misc/fail/oxc-16077.ts:3:22]
3163+
2 │ class B {constructor(private []){}}
3164+
3 │ class C {constructor(protected []){}}
3165+
· ────────────
3166+
4 │ class D {constructor(public []){}}
3167+
╰────
3168+
3169+
× TS(1187): A parameter property may not be declared using a binding pattern.
3170+
╭─[misc/fail/oxc-16077.ts:4:22]
3171+
3 │ class C {constructor(protected []){}}
3172+
4 │ class D {constructor(public []){}}
3173+
· ─────────
3174+
5 │ class E {constructor(readonly []){}}
3175+
╰────
3176+
3177+
× TS(1187): A parameter property may not be declared using a binding pattern.
3178+
╭─[misc/fail/oxc-16077.ts:5:22]
3179+
4 │ class D {constructor(public []){}}
3180+
5 │ class E {constructor(readonly []){}}
3181+
· ───────────
3182+
6 │ class F {constructor(private {}){}}
3183+
╰────
3184+
3185+
× TS(1187): A parameter property may not be declared using a binding pattern.
3186+
╭─[misc/fail/oxc-16077.ts:6:22]
3187+
5 │ class E {constructor(readonly []){}}
3188+
6 │ class F {constructor(private {}){}}
3189+
· ──────────
3190+
7 │ class G {constructor(public {a} = {}){}}
3191+
╰────
3192+
3193+
× TS(1187): A parameter property may not be declared using a binding pattern.
3194+
╭─[misc/fail/oxc-16077.ts:7:22]
3195+
6 │ class F {constructor(private {}){}}
3196+
7 │ class G {constructor(public {a} = {}){}}
3197+
· ───────────────
3198+
8 │ class H {constructor(readonly [b] = []){}}
3199+
╰────
3200+
3201+
× TS(1187): A parameter property may not be declared using a binding pattern.
3202+
╭─[misc/fail/oxc-16077.ts:8:22]
3203+
7 │ class G {constructor(public {a} = {}){}}
3204+
8 │ class H {constructor(readonly [b] = []){}}
3205+
· ─────────────────
3206+
╰────
3207+
31463208
× Unexpected token
31473209
╭─[misc/fail/oxc-169.js:2:1]
31483210
1 │ 1<(V=82<<t-j0<(V=$<LBI<(V=ut<I<(V=$<LBI<(V=uIV=82<<t-j0<(V=$<LBI<(V=ut<I<(V=$<LBI<(V<II>

‎tasks/coverage/snapshots/parser_typescript.snap‎

Lines changed: 81 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,7 @@ commit: 669c25c0
33
parser_typescript Summary:
44
AST Parsed : 9821/9822 (99.99%)
55
Positive Passed: 9812/9822 (99.90%)
6-
Negative Passed: 1457/2547 (57.20%)
6+
Negative Passed: 1463/2547 (57.44%)
77
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ExportAssignment7.ts
88

99
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/ExportAssignment8.ts
@@ -218,8 +218,6 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/declFileWith
218218

219219
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/declarationEmitComputedPropertyNameEnum3.ts
220220

221-
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/declarationEmitDestructuringParameterProperties.ts
222-
223221
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/declarationEmitDestructuringWithOptionalBindingParameters.ts
224222

225223
Expect Syntax Error: tasks/coverage/typescript/tests/cases/compiler/declarationEmitIndexTypeNotFound.ts
@@ -1308,16 +1306,6 @@ Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destr
13081306

13091307
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/destructuringParameterDeclaration1ES5iterable.ts
13101308

1311-
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts
1312-
1313-
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts
1314-
1315-
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts
1316-
1317-
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties4.ts
1318-
1319-
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties5.ts
1320-
13211309
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/destructuringSpread.ts
13221310

13231311
Expect Syntax Error: tasks/coverage/typescript/tests/cases/conformance/es6/destructuring/missingAndExcessProperties.ts
@@ -4914,6 +4902,30 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc
49144902
4 │ function h1([a, [b], [[c]], {x = 10, y = [1, 2, 3], z: {a1, b1}}]){ }
49154903
╰────
49164904

4905+
× TS(1187): A parameter property may not be declared using a binding pattern.
4906+
╭─[typescript/tests/cases/compiler/declarationEmitDestructuringParameterProperties.ts:2:17]
4907+
1 │ class C1 {
4908+
2 │ constructor(public [x, y, z]: string[]) {
4909+
· ──────────────────────────
4910+
3 │ }
4911+
╰────
4912+
4913+
× TS(1187): A parameter property may not be declared using a binding pattern.
4914+
╭─[typescript/tests/cases/compiler/declarationEmitDestructuringParameterProperties.ts:8:17]
4915+
7 │ class C2 {
4916+
8 │ constructor(public [x, y, z]: TupleType1) {
4917+
· ────────────────────────────
4918+
9 │ }
4919+
╰────
4920+
4921+
× TS(1187): A parameter property may not be declared using a binding pattern.
4922+
╭─[typescript/tests/cases/compiler/declarationEmitDestructuringParameterProperties.ts:14:17]
4923+
13 │ class C3 {
4924+
14 │ constructor(public { x, y, z }: ObjType1) {
4925+
· ────────────────────────────
4926+
15 │ }
4927+
╰────
4928+
49174929
× TS(2499): An interface can only extend an identifier/qualified-name with optional type arguments.
49184930
╭─[typescript/tests/cases/compiler/declarationEmitInterfaceWithNonEntityNameExpressionHeritage.ts:2:25]
49194931
1 │ class A { }
@@ -15746,6 +15758,62 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/parser/ecmasc
1574615758
8 │ function a1({public}) { }
1574715759
╰────
1574815760

15761+
× TS(1187): A parameter property may not be declared using a binding pattern.
15762+
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts:2:17]
15763+
1 │ class C1 {
15764+
2 │ constructor(public [x, y, z]: string[]) {
15765+
· ──────────────────────────
15766+
3 │ }
15767+
╰────
15768+
15769+
× TS(1187): A parameter property may not be declared using a binding pattern.
15770+
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts:9:17]
15771+
8 │ class C2 {
15772+
9 │ constructor(public [x, y, z]: TupleType1) {
15773+
· ────────────────────────────
15774+
10 │ }
15775+
╰────
15776+
15777+
× TS(1187): A parameter property may not be declared using a binding pattern.
15778+
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties1.ts:16:17]
15779+
15 │ class C3 {
15780+
16 │ constructor(public { x, y, z }: ObjType1) {
15781+
· ────────────────────────────
15782+
17 │ }
15783+
╰────
15784+
15785+
× TS(1187): A parameter property may not be declared using a binding pattern.
15786+
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties2.ts:2:36]
15787+
1 │ class C1 {
15788+
2 │ constructor(private k: number, private [a, b, c]: [number, string, boolean]) {
15789+
· ────────────────────────────────────────────
15790+
3 │ if ((b === undefined && c === undefined) || (this.b === undefined && this.c === undefined)) {
15791+
╰────
15792+
15793+
× TS(1187): A parameter property may not be declared using a binding pattern.
15794+
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties3.ts:2:31]
15795+
1 │ class C1<T, U, V> {
15796+
2 │ constructor(private k: T, private [a, b, c]: [T,U,V]) {
15797+
· ──────────────────────────
15798+
3 │ if ((b === undefined && c === undefined) || (this.b === undefined && this.c === undefined)) {
15799+
╰────
15800+
15801+
× TS(1187): A parameter property may not be declared using a binding pattern.
15802+
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties4.ts:2:31]
15803+
1 │ class C1<T, U, V> {
15804+
2 │ constructor(private k: T, protected [a, b, c]: [T,U,V]) {
15805+
· ────────────────────────────
15806+
3 │ if ((b === undefined && c === undefined) || (this.b === undefined && this.c === undefined)) {
15807+
╰────
15808+
15809+
× TS(1187): A parameter property may not be declared using a binding pattern.
15810+
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringParameterProperties5.ts:5:17]
15811+
4 │ class C1 {
15812+
5 │ constructor(public [{ x1, x2, x3 }, y, z]: TupleType1) {
15813+
· ─────────────────────────────────────────
15814+
6 │ var foo: any = x1 || x2 || x3 || y || z;
15815+
╰────
15816+
1574915817
× Identifier `foo1` has already been declared
1575015818
╭─[typescript/tests/cases/conformance/es6/destructuring/destructuringSameNames.ts:21:7]
1575115819
20 │

‎tasks/coverage/snapshots/semantic_misc.snap‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
semantic_misc Summary:
2-
AST Parsed : 51/51 (100.00%)
3-
Positive Passed: 32/51 (62.75%)
2+
AST Parsed : 52/52 (100.00%)
3+
Positive Passed: 33/52 (63.46%)
44
semantic Error: tasks/coverage/misc/pass/declare-let-private.ts
55
Bindings mismatch:
66
after transform: ScopeId(0): ["private"]

0 commit comments

Comments
 (0)