Skip to content

Commit f067159

Browse files
authored
fix(parser): forbid readonly in parser instead of semantic (#13905)
Closes #13883 This also fixes duplicate essentially same diagnostics coming from parser and semantic each.
1 parent 55775ce commit f067159

File tree

5 files changed

+71
-691
lines changed

5 files changed

+71
-691
lines changed

crates/oxc_parser/src/js/function.rs

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -70,12 +70,11 @@ impl<'a> ParserImpl<'a> {
7070
let decorators = self.parse_decorators();
7171
let modifiers = self.parse_modifiers(false, false);
7272
if self.is_ts {
73-
let mut allowed_modifiers = ModifierFlags::READONLY;
74-
if func_kind == FunctionKind::Constructor {
75-
allowed_modifiers = allowed_modifiers
76-
.union(ModifierFlags::ACCESSIBILITY)
77-
.union(ModifierFlags::OVERRIDE);
78-
}
73+
let allowed_modifiers = if func_kind == FunctionKind::Constructor {
74+
ModifierFlags::ACCESSIBILITY | ModifierFlags::OVERRIDE | ModifierFlags::READONLY
75+
} else {
76+
ModifierFlags::empty()
77+
};
7978
self.verify_modifiers(
8079
&modifiers,
8180
allowed_modifiers,

crates/oxc_semantic/src/checker/typescript.rs

Lines changed: 2 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -77,32 +77,19 @@ fn required_parameter_after_optional_parameter(span: Span) -> OxcDiagnostic {
7777
.with_label(span)
7878
}
7979

80-
fn parameter_property_outside_constructor(span: Span) -> OxcDiagnostic {
81-
OxcDiagnostic::error("A parameter property is only allowed in a constructor implementation.")
82-
.with_label(span)
83-
}
84-
8580
pub fn check_formal_parameters(params: &FormalParameters, ctx: &SemanticBuilder<'_>) {
8681
if params.kind == FormalParameterKind::Signature && params.items.len() > 1 {
8782
check_duplicate_bound_names(params, ctx);
8883
}
8984

90-
let is_inside_constructor =
91-
!params.kind.is_signature() && ctx.current_scope_flags().is_constructor();
9285
let mut has_optional = false;
9386

9487
for param in &params.items {
9588
// function a(optional?: number, required: number) { }
96-
if has_optional && !param.pattern.optional && !param.pattern.kind.is_assignment_pattern() {
97-
ctx.error(required_parameter_after_optional_parameter(param.span));
98-
}
9989
if param.pattern.optional {
10090
has_optional = true;
101-
}
102-
103-
// function a(public x: number) { }
104-
if !is_inside_constructor && param.has_modifier() {
105-
ctx.error(parameter_property_outside_constructor(param.span));
91+
} else if has_optional && !param.pattern.kind.is_assignment_pattern() {
92+
ctx.error(required_parameter_after_optional_parameter(param.span));
10693
}
10794
}
10895
}

tasks/coverage/snapshots/parser_babel.snap

Lines changed: 46 additions & 110 deletions
Original file line numberDiff line numberDiff line change
@@ -12870,6 +12870,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1287012870
9 │ readonly *[e?.e]?() { }
1287112871
╰────
1287212872

12873+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
12874+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:3:9]
12875+
2 │ not_constructor(
12876+
3 │ readonly r,
12877+
· ────────
12878+
4 │ public pu: number,
12879+
╰────
12880+
1287312881
× TS(1090): 'public' modifier cannot appear on a parameter.
1287412882
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:4:9]
1287512883
3 │ readonly r,
@@ -12902,78 +12910,38 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1290212910
8 │ // Also works on AssignmentPattern
1290312911
╰────
1290412912

12905-
× TS(1090): 'public' modifier cannot appear on a parameter.
12906-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:10:9]
12907-
9 │ readonly x = 0,
12908-
10 │ public y?: number = 0) {}
12909-
· ──────
12910-
11 │ }
12911-
╰────
12912-
12913-
× A parameter property is only allowed in a constructor implementation.
12914-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:3:9]
12915-
2 │ not_constructor(
12916-
3 │ readonly r,
12917-
· ──────────
12918-
4 │ public pu: number,
12919-
╰────
12920-
12921-
× A parameter property is only allowed in a constructor implementation.
12922-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:4:9]
12923-
3 │ readonly r,
12924-
4 │ public pu: number,
12925-
· ─────────────────
12926-
5 │ protected po?,
12927-
╰────
12928-
12929-
× A parameter property is only allowed in a constructor implementation.
12930-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:5:9]
12931-
4 │ public pu: number,
12932-
5 │ protected po?,
12933-
· ─────────────
12934-
6 │ private pi?: number,
12935-
╰────
12936-
12937-
× A parameter property is only allowed in a constructor implementation.
12938-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:6:9]
12939-
5 │ protected po?,
12940-
6 │ private pi?: number,
12941-
· ───────────────────
12942-
7 │ public readonly pur,
12943-
╰────
12944-
12945-
× A required parameter cannot follow an optional parameter.
12946-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:7:9]
12913+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
12914+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:7:16]
1294712915
6 │ private pi?: number,
1294812916
7 │ public readonly pur,
12949-
· ───────────────────
12950-
8 │ // Also works on AssignmentPattern
12951-
╰────
12952-
12953-
× A parameter property is only allowed in a constructor implementation.
12954-
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:7:9]
12955-
6 │ private pi?: number,
12956-
7 │ public readonly pur,
12957-
· ───────────────────
12917+
· ────────
1295812918
8 │ // Also works on AssignmentPattern
1295912919
╰────
1296012920

12961-
× A parameter property is only allowed in a constructor implementation.
12921+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
1296212922
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:9:9]
1296312923
8 │ // Also works on AssignmentPattern
1296412924
9 │ readonly x = 0,
12965-
· ──────────────
12925+
· ────────
1296612926
10 │ public y?: number = 0) {}
1296712927
╰────
1296812928

12969-
× A parameter property is only allowed in a constructor implementation.
12929+
× TS(1090): 'public' modifier cannot appear on a parameter.
1297012930
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:10:9]
1297112931
9 │ readonly x = 0,
1297212932
10 │ public y?: number = 0) {}
12973-
· ─────────────────────
12933+
· ──────
1297412934
11 │ }
1297512935
╰────
1297612936

12937+
× A required parameter cannot follow an optional parameter.
12938+
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/parameter-properties-not-constructor/input.ts:7:9]
12939+
6 │ private pi?: number,
12940+
7 │ public readonly pur,
12941+
· ───────────────────
12942+
8 │ // Also works on AssignmentPattern
12943+
╰────
12944+
1297712945
× TS(18010): An accessibility modifier cannot be used with a private identifier.
1297812946
╭─[babel/packages/babel-parser/test/fixtures/typescript/class/private-fields-modifier-private/input.ts:2:3]
1297912947
1 │ class A {
@@ -13158,6 +13126,14 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1315813126
· ──
1315913127
╰────
1316013128

13129+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
13130+
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:2:3]
13131+
1 │ function foo(
13132+
2 │ readonly r,
13133+
· ────────
13134+
3 │ public pu: number,
13135+
╰────
13136+
1316113137
× TS(1090): 'public' modifier cannot appear on a parameter.
1316213138
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:3:3]
1316313139
2 │ readonly r,
@@ -13190,78 +13166,38 @@ Expect to Parse: tasks/coverage/babel/packages/babel-parser/test/fixtures/typesc
1319013166
7 │ readonly x = 0,
1319113167
╰────
1319213168

13193-
× TS(1090): 'public' modifier cannot appear on a parameter.
13194-
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:8:3]
13195-
7 │ readonly x = 0,
13196-
8 │ public y?: number = 0
13197-
· ──────
13198-
9 │ ) {}
13199-
╰────
13200-
13201-
× A parameter property is only allowed in a constructor implementation.
13202-
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:2:3]
13203-
1 │ function foo(
13204-
2 │ readonly r,
13205-
· ──────────
13206-
3 │ public pu: number,
13207-
╰────
13208-
13209-
× A parameter property is only allowed in a constructor implementation.
13210-
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:3:3]
13211-
2 │ readonly r,
13212-
3 │ public pu: number,
13213-
· ─────────────────
13214-
4 │ protected po?,
13215-
╰────
13216-
13217-
× A parameter property is only allowed in a constructor implementation.
13218-
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:4:3]
13219-
3 │ public pu: number,
13220-
4 │ protected po?,
13221-
· ─────────────
13222-
5 │ private pi?: number,
13223-
╰────
13224-
13225-
× A parameter property is only allowed in a constructor implementation.
13226-
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:5:3]
13227-
4 │ protected po?,
13228-
5 │ private pi?: number,
13229-
· ───────────────────
13230-
6 │ public readonly pur,
13231-
╰────
13232-
13233-
× A required parameter cannot follow an optional parameter.
13234-
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:6:3]
13235-
5 │ private pi?: number,
13236-
6 │ public readonly pur,
13237-
· ───────────────────
13238-
7 │ readonly x = 0,
13239-
╰────
13240-
13241-
× A parameter property is only allowed in a constructor implementation.
13242-
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:6:3]
13169+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
13170+
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:6:10]
1324313171
5 │ private pi?: number,
1324413172
6 │ public readonly pur,
13245-
· ───────────────────
13173+
· ────────
1324613174
7 │ readonly x = 0,
1324713175
╰────
1324813176

13249-
× A parameter property is only allowed in a constructor implementation.
13177+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
1325013178
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:7:3]
1325113179
6 │ public readonly pur,
1325213180
7 │ readonly x = 0,
13253-
· ──────────────
13181+
· ────────
1325413182
8 │ public y?: number = 0
1325513183
╰────
1325613184

13257-
× A parameter property is only allowed in a constructor implementation.
13185+
× TS(1090): 'public' modifier cannot appear on a parameter.
1325813186
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:8:3]
1325913187
7 │ readonly x = 0,
1326013188
8 │ public y?: number = 0
13261-
· ─────────────────────
13189+
· ──────
1326213190
9 │ ) {}
1326313191
╰────
1326413192

13193+
× A required parameter cannot follow an optional parameter.
13194+
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/parameter-properties/input.ts:6:3]
13195+
5 │ private pi?: number,
13196+
6 │ public readonly pur,
13197+
· ───────────────────
13198+
7 │ readonly x = 0,
13199+
╰────
13200+
1326513201
× A required parameter cannot follow an optional parameter.
1326613202
╭─[babel/packages/babel-parser/test/fixtures/typescript/function/pattern-optional-parameters/input.ts:1:17]
1326713203
1 │ function f([]?, {}) {}

tasks/coverage/snapshots/parser_misc.snap

Lines changed: 4 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -309,16 +309,16 @@ Negative Passed: 90/90 (100.00%)
309309
3 │ }
310310
╰────
311311
312-
× A parameter property is only allowed in a constructor implementation.
312+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
313313
╭─[misc/fail/oxc-11713-26.ts:1:14]
314314
1function foo(readonly parameter) {}
315-
· ──────────────────
315+
· ────────
316316
╰────
317317
318-
× A parameter property is only allowed in a constructor implementation.
318+
× TS(1090): 'readonly' modifier cannot appear on a parameter.
319319
╭─[misc/fail/oxc-11713-27.ts:1:20]
320320
1class Foo { method(readonly parameter) {} }
321-
· ──────────────────
321+
· ────────
322322
╰────
323323
324324
× TS(1090): 'private' modifier cannot appear on a parameter.
@@ -327,12 +327,6 @@ Negative Passed: 90/90 (100.00%)
327327
· ───────
328328
╰────
329329
330-
× A parameter property is only allowed in a constructor implementation.
331-
╭─[misc/fail/oxc-11713-3.ts:1:14]
332-
1function foo(private parameter) {}
333-
· ─────────────────
334-
╰────
335-
336330
× 'declare' modifier cannot be used here.
337331
╭─[misc/fail/oxc-11713-4.ts:2:2]
338332
1class Foo {

0 commit comments

Comments
 (0)