Skip to content

Commit

Permalink
feat(parser): better handling of invalid modifiers (#6482)
Browse files Browse the repository at this point in the history
## What This PR Does

1. Recover on, and provide a better message for, invalid `export` modifier on constructor parameters. Before, an `unexpected token` error would be produced and the parser would panic. Now, the parser recovers and produces a message saying `'export' modifier cannot appear on a parameter.`
  ```ts
class Foo {
    constructor(export x: number) {}
}
  ```

2. Recover on, and provide a better message for, invalid modifiers on index signatures. Same recovery/message characteristics as above.
  ```ts
class Foo {
    public [x: string]: string;
}
```
  • Loading branch information
DonIsaac committed Oct 13, 2024
1 parent 8ea6b72 commit 58467a5
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 16 deletions.
6 changes: 6 additions & 0 deletions crates/oxc_parser/src/diagnostics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,12 @@ pub fn cannot_appear_on_a_parameter(modifier: &Modifier) -> OxcDiagnostic {
.with_label(modifier.span)
}

/// TS(1071)
pub fn cannot_appear_on_an_index_signature(modifier: &Modifier) -> OxcDiagnostic {
ts_error("1071", format!("'{}' modifier cannot appear on an index signature.", modifier.kind))
.with_label(modifier.span)
}

/// TS(18010)
#[cold]
pub fn accessibility_modifier_on_private_property(modifier: &Modifier) -> OxcDiagnostic {
Expand Down
1 change: 1 addition & 0 deletions crates/oxc_parser/src/ts/statement.rs
Original file line number Diff line number Diff line change
Expand Up @@ -255,6 +255,7 @@ impl<'a> ParserImpl<'a> {
| Kind::Readonly
| Kind::Declare
| Kind::Override
| Kind::Export
)) {
return false;
}
Expand Down
16 changes: 8 additions & 8 deletions crates/oxc_parser/src/ts/types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1269,14 +1269,14 @@ impl<'a> ParserImpl<'a> {

pub(crate) fn parse_ts_index_signature_member(&mut self) -> Result<TSSignature<'a>> {
let span = self.start_span();
let mut readonly = false;
while self.is_nth_at_modifier(0, false) {
if self.eat(Kind::Readonly) {
readonly = true;
} else {
return Err(self.unexpected());
}
}

let modifiers = self.parse_class_element_modifiers(false);
self.verify_modifiers(
&modifiers,
ModifierFlags::READONLY,
diagnostics::cannot_appear_on_an_index_signature,
);
let readonly = modifiers.contains(ModifierKind::Readonly);

self.bump(Kind::LBrack);
let index_name = self.parse_ts_index_signature_name()?;
Expand Down
31 changes: 23 additions & 8 deletions tasks/coverage/snapshots/parser_typescript.snap
Original file line number Diff line number Diff line change
Expand Up @@ -6629,7 +6629,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
3 │ }
╰────

× Identifier expected. 'export' is a reserved word that cannot be used here.
× TS(1090): 'export' modifier cannot appear on a parameter.
╭─[typescript/tests/cases/compiler/constructorArgsErrors5.ts:2:18]
1 │ class foo {
2 │ constructor (export a: number) {
Expand Down Expand Up @@ -9223,7 +9223,7 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
6 │ };
╰────

× Unexpected token
× TS(1071): 'public' modifier cannot appear on an index signature.
╭─[typescript/tests/cases/compiler/modifiersOnInterfaceIndexSignature1.ts:2:3]
1 │ interface I {
2 │ public [a: string]: number;
Expand Down Expand Up @@ -15079,22 +15079,38 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
9 │ }
╰────

× Unexpected token
× TS(1071): 'static' modifier cannot appear on an index signature.
╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature4.ts:12:5]
11 │ interface IB {
12 │ static [s: string]: number;
· ──────
13 │ static [s: number]: 42 | 233;
╰────

× Unexpected token
× TS(1071): 'static' modifier cannot appear on an index signature.
╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature4.ts:13:5]
12 │ static [s: string]: number;
13 │ static [s: number]: 42 | 233;
· ──────
14 │ }
╰────

× TS(1071): 'static' modifier cannot appear on an index signature.
╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature5.ts:7:5]
6 │ interface I {
7 │ static readonly [s: string]: number;
· ──────
8 │ static readonly [s: number]: 42 | 233
╰────

× TS(1071): 'static' modifier cannot appear on an index signature.
╭─[typescript/tests/cases/conformance/classes/staticIndexSignature/staticIndexSignature5.ts:8:5]
7 │ static readonly [s: string]: number;
8 │ static readonly [s: number]: 42 | 233
· ──────
9 │ }
╰────

× Expected `,` but found `is`
╭─[typescript/tests/cases/conformance/controlFlow/assertionTypePredicates1.ts:163:20]
162 │ get p1(): this is string;
Expand Down Expand Up @@ -20758,14 +20774,13 @@ Expect to Parse: tasks/coverage/typescript/tests/cases/conformance/salsa/private
· ─
╰────

× Expected a semicolon or an implicit semicolon after a statement, but found none
╭─[typescript/tests/cases/conformance/parser/ecmascript5/IndexMemberDeclarations/parserIndexMemberDeclaration9.ts:2:10]
× TS(1071): 'export' modifier cannot appear on an index signature.
╭─[typescript/tests/cases/conformance/parser/ecmascript5/IndexMemberDeclarations/parserIndexMemberDeclaration9.ts:2:4]
1 │ class C {
2 │ export [x: string]: string;
·
· ──────
3 │ }
╰────
help: Try insert a semicolon here

× Unexpected token
╭─[typescript/tests/cases/conformance/parser/ecmascript5/IndexSignatures/parserIndexSignature1.ts:2:4]
Expand Down

0 comments on commit 58467a5

Please sign in to comment.