Skip to content

Commit 74cf572

Browse files
committed
feat(ast)!: Make source field of TSImportType a StringLiteral (#16114)
- [x] Change `source` field type in `TSImportType` from `TSType<'a>` to `StringLiteral<'a>` in `crates/oxc_ast/src/ast/ts.rs` - [x] Remove `#[estree(via = TSImportTypeSource)]` attribute from the field - [x] Run `just ast` to regenerate all derived code - [x] Update the parser in `crates/oxc_parser/src/ts/types.rs` to parse a `StringLiteral` directly and emit errors for non-string literals - [x] Delete the dead code `TSImportTypeSource` serializer from `crates/oxc_ast/src/serialize/ts.rs` - [x] Add `ts_string_literal_expected` diagnostic (TS1141) - [x] Update semantic snapshot for `import-type-with-type-params.snap` - [x] Add test case for invalid import type source - [x] Verify all tests pass - [x] Simplify error recovery - just report error and create dummy string literal (per review feedback) - [x] Fix spurious "Expected ')'" error by skipping invalid tokens in error recovery <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>AST: Make `source` field of `TSImportType` a `StringLiteral`</issue_title> > <issue_description>Continuation of #16074. > > The expression inside `import(...)` in TS context (`TSImportType`) cannot legally be anything other than a string literal. > > Our parser is too liberal. We currently accept all kind of crazy stuff like: > > ```ts > type A = typeof import(`react`); > type B = typeof import(A); > type C = typeof import(`${A} ${B}`); > type D = typeof import(typeof import('react')); > ``` > > [Playground](https://playground.oxc.rs/#eNptU9tOGzEQ/ZXIQqJIqUSRKqFUfQjQSJVooaUqL31g1jsbDF7bHc/molX+nfEmdkDhyfaZ+5njXmk1UbwOOJqOvo7SxTcj0wZP/OGBEDQ/nHz55waPiwOPabFdHkYf9dPN6Ki/2OwzXB14vX0dDxWPT1KAGiuvJr2izqXDGsdq0oCNOFaNpxaYkQrCBC4muCAmeguM9RVqCwRsvIvFuHw0jDGAxgK14OZ2/9S+DYRxHxK1D2Jm6tJj3Vbe5pdu5ju3zVgFoJga6xWuGF2UuoniKPOAtX75G7kjd9NxNDXOOqdTZzlTKom0wFsgCc3oEPf3/LtjMpJQF0NE6ZqN/kbkaYdKC4mroQW5v6KqV13EP1DtZ2Ko7k3Nj2pyNlbo6pvm2jgZUtlG2g1SjXf281OpZhJDvzrPe5qe4uruHfh/et2SD1JLQfzoEGusJafsyUh780vftpBs1goqc5gyrMyyzPOLfQnrxF1FoJ+R72RnEp6dMwotbjvf1ffVE2q+JwiSI5OaCg29TlmIrDrpEOlNGK4CkmnRMdgbuQJ7uvXRbFeUGFJCadHallQGmqNoU2E8O/30WaoIzVfYSOKZp0sLMc4M2rps7XUVUacfyuytreGC/kCGGliY2q32fVX3KspEQQSC5EB0mcWofY1zHD6QS0Iokn2KtZevv6PcOc9DroxYnGffIUv+C0lR+aPI5D5c4wJL0mfE8FNWkQU2hIpovZ2JgFPEAqnyMROe5nFpUclUbhuB64G+BJfbJuGJZoisNi/WFa5W) > > TypeScript says these are all errors: [TS Playground](https://www.typescriptlang.org/play/?#code/C4TwDgpgBAglC8VSQPYDMoEsC2YUCdgAKAA3wgEMBjYEgSgG4AoZaAIQSXAnS1wOIxGLblADCnVrxx5CpACQBvGAF8oStivrNWUACKTu0-nKkYZAogHJy1YFbrCgA) > > ### AST change > > We should alter the type of the `source` field from `TSType<'a>` to `StringLiteral<'a>`: > > https://github.com/oxc-project/oxc/blob/35d0ee491cbcd271ebceaac7e25cff0490ded5c4/crates/oxc_ast/src/ast/ts.rs#L1382-L1389 > > ```diff > pub struct TSImportType<'a> { > pub span: Span, > - #[estree(via = TSImportTypeSource)] > - pub source: TSType<'a>, > + pub source: StringLiteral<'a>, > pub options: Option<Box<'a, ObjectExpression<'a>>>, > pub qualifier: Option<TSImportTypeQualifier<'a>>, > pub type_arguments: Option<Box<'a, TSTypeParameterInstantiation<'a>>>, > } > ``` > > ### Parser changes > > If we change the AST first and run `just ast` to regenerate traits code, then all the places which rely on the type of `source` field will become apparent. We'll then need to alter the parser to throw an error if it encounters something that isn't a string literal. > > ### `ESTree` serialization > > Currently there's a custom deserializer to alter the AST on JS side from `TSType` to `StringLiteral`: > > https://github.com/oxc-project/oxc/blob/91eb3f2be509cef11cffeff51fac61a3a4fbd12c/crates/oxc_ast/src/serialize/ts.rs#L506-L538 > > Once we've changed the field type in AST, this code can be deleted - it's dead code once `#[estree(via = TSImportTypeSource)]` attribute is removed from the `source` field in AST struct.</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > </comments> > </details> - Fixes #16111 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey).
1 parent c05db06 commit 74cf572

File tree

31 files changed

+235
-181
lines changed

31 files changed

+235
-181
lines changed

apps/oxlint/src-js/generated/deserialize.js

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -5221,16 +5221,11 @@ function deserializeTSImportType(pos) {
52215221
end,
52225222
range: [start, end],
52235223
parent,
5224-
}),
5225-
source = deserializeTSType(pos + 8);
5226-
if (source.type === "TSLiteralType") {
5227-
source = source.literal;
5228-
source.parent = parent;
5229-
}
5230-
node.source = source;
5231-
node.options = deserializeOptionBoxObjectExpression(pos + 24);
5232-
node.qualifier = deserializeOptionTSImportTypeQualifier(pos + 32);
5233-
node.typeArguments = deserializeOptionBoxTSTypeParameterInstantiation(pos + 48);
5224+
});
5225+
node.source = deserializeStringLiteral(pos + 8);
5226+
node.options = deserializeOptionBoxObjectExpression(pos + 56);
5227+
node.qualifier = deserializeOptionTSImportTypeQualifier(pos + 64);
5228+
node.typeArguments = deserializeOptionBoxTSTypeParameterInstantiation(pos + 80);
52345229
parent = previousParent;
52355230
return node;
52365231
}

crates/oxc_ast/src/ast/ts.rs

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1381,8 +1381,7 @@ pub enum TSTypeQueryExprName<'a> {
13811381
#[generate_derive(CloneIn, Dummy, TakeIn, GetSpan, GetSpanMut, ContentEq, ESTree, UnstableAddress)]
13821382
pub struct TSImportType<'a> {
13831383
pub span: Span,
1384-
#[estree(via = TSImportTypeSource)]
1385-
pub source: TSType<'a>,
1384+
pub source: StringLiteral<'a>,
13861385
pub options: Option<Box<'a, ObjectExpression<'a>>>,
13871386
pub qualifier: Option<TSImportTypeQualifier<'a>>,
13881387
pub type_arguments: Option<Box<'a, TSTypeParameterInstantiation<'a>>>,

crates/oxc_ast/src/generated/assert_layouts.rs

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1449,13 +1449,13 @@ const _: () = {
14491449
assert!(align_of::<TSTypeQueryExprName>() == 8);
14501450

14511451
// Padding: 0 bytes
1452-
assert!(size_of::<TSImportType>() == 56);
1452+
assert!(size_of::<TSImportType>() == 88);
14531453
assert!(align_of::<TSImportType>() == 8);
14541454
assert!(offset_of!(TSImportType, span) == 0);
14551455
assert!(offset_of!(TSImportType, source) == 8);
1456-
assert!(offset_of!(TSImportType, options) == 24);
1457-
assert!(offset_of!(TSImportType, qualifier) == 32);
1458-
assert!(offset_of!(TSImportType, type_arguments) == 48);
1456+
assert!(offset_of!(TSImportType, options) == 56);
1457+
assert!(offset_of!(TSImportType, qualifier) == 64);
1458+
assert!(offset_of!(TSImportType, type_arguments) == 80);
14591459

14601460
assert!(size_of::<TSImportTypeQualifier>() == 16);
14611461
assert!(align_of::<TSImportTypeQualifier>() == 8);
@@ -3065,13 +3065,13 @@ const _: () = if cfg!(target_family = "wasm") || align_of::<u64>() == 8 {
30653065
assert!(align_of::<TSTypeQueryExprName>() == 4);
30663066

30673067
// Padding: 0 bytes
3068-
assert!(size_of::<TSImportType>() == 32);
3068+
assert!(size_of::<TSImportType>() == 52);
30693069
assert!(align_of::<TSImportType>() == 4);
30703070
assert!(offset_of!(TSImportType, span) == 0);
30713071
assert!(offset_of!(TSImportType, source) == 8);
3072-
assert!(offset_of!(TSImportType, options) == 16);
3073-
assert!(offset_of!(TSImportType, qualifier) == 20);
3074-
assert!(offset_of!(TSImportType, type_arguments) == 28);
3072+
assert!(offset_of!(TSImportType, options) == 36);
3073+
assert!(offset_of!(TSImportType, qualifier) == 40);
3074+
assert!(offset_of!(TSImportType, type_arguments) == 48);
30753075

30763076
assert!(size_of::<TSImportTypeQualifier>() == 8);
30773077
assert!(align_of::<TSImportTypeQualifier>() == 4);

crates/oxc_ast/src/generated/ast_builder.rs

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -10629,7 +10629,7 @@ impl<'a> AstBuilder<'a> {
1062910629
pub fn ts_type_import_type<T1, T2>(
1063010630
self,
1063110631
span: Span,
10632-
source: TSType<'a>,
10632+
source: StringLiteral<'a>,
1063310633
options: T1,
1063410634
qualifier: Option<TSImportTypeQualifier<'a>>,
1063510635
type_arguments: T2,
@@ -14024,7 +14024,7 @@ impl<'a> AstBuilder<'a> {
1402414024
pub fn ts_type_query_expr_name_import_type<T1, T2>(
1402514025
self,
1402614026
span: Span,
14027-
source: TSType<'a>,
14027+
source: StringLiteral<'a>,
1402814028
options: T1,
1402914029
qualifier: Option<TSImportTypeQualifier<'a>>,
1403014030
type_arguments: T2,
@@ -14057,7 +14057,7 @@ impl<'a> AstBuilder<'a> {
1405714057
pub fn ts_import_type<T1, T2>(
1405814058
self,
1405914059
span: Span,
14060-
source: TSType<'a>,
14060+
source: StringLiteral<'a>,
1406114061
options: T1,
1406214062
qualifier: Option<TSImportTypeQualifier<'a>>,
1406314063
type_arguments: T2,
@@ -14090,7 +14090,7 @@ impl<'a> AstBuilder<'a> {
1409014090
pub fn alloc_ts_import_type<T1, T2>(
1409114091
self,
1409214092
span: Span,
14093-
source: TSType<'a>,
14093+
source: StringLiteral<'a>,
1409414094
options: T1,
1409514095
qualifier: Option<TSImportTypeQualifier<'a>>,
1409614096
type_arguments: T2,

crates/oxc_ast/src/generated/derive_dummy.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2638,7 +2638,7 @@ impl<'a> Dummy<'a> for TSTypeQueryExprName<'a> {
26382638
impl<'a> Dummy<'a> for TSImportType<'a> {
26392639
/// Create a dummy [`TSImportType`].
26402640
///
2641-
/// Has cost of making 1 allocation (8 bytes).
2641+
/// Does not allocate any data into arena.
26422642
fn dummy(allocator: &'a Allocator) -> Self {
26432643
Self {
26442644
span: Dummy::dummy(allocator),

crates/oxc_ast/src/generated/derive_estree.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2982,7 +2982,7 @@ impl ESTree for TSImportType<'_> {
29822982
fn serialize<S: Serializer>(&self, serializer: S) {
29832983
let mut state = serializer.serialize_struct();
29842984
state.serialize_field("type", &JsonSafeString("TSImportType"));
2985-
state.serialize_field("source", &crate::serialize::ts::TSImportTypeSource(self));
2985+
state.serialize_field("source", &self.source);
29862986
state.serialize_field("options", &self.options);
29872987
state.serialize_field("qualifier", &self.qualifier);
29882988
state.serialize_field("typeArguments", &self.type_arguments);

crates/oxc_ast/src/serialize/ts.rs

Lines changed: 0 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -503,39 +503,6 @@ impl ESTree for TSFunctionTypeParams<'_, '_> {
503503
}
504504
}
505505

506-
/// Serializer for `source` field of `TSImportType`.
507-
///
508-
/// Serialized as a `StringLiteral` - all other values are illegal syntax.
509-
#[ast_meta]
510-
#[estree(
511-
ts_type = "StringLiteral",
512-
raw_deser = "
513-
let source = DESER[TSType](POS_OFFSET.source);
514-
if (source.type === 'TSLiteralType') {
515-
source = source.literal;
516-
if (PARENT) source.parent = parent;
517-
} else {
518-
// Should be unreachable - illegal syntax
519-
}
520-
source
521-
"
522-
)]
523-
pub struct TSImportTypeSource<'a, 'b>(pub &'b TSImportType<'a>);
524-
525-
impl ESTree for TSImportTypeSource<'_, '_> {
526-
fn serialize<S: Serializer>(&self, serializer: S) {
527-
let source = &self.0.source;
528-
if let TSType::TSLiteralType(ts_lit_type) = source
529-
&& let TSLiteral::StringLiteral(str_lit) = &ts_lit_type.literal
530-
{
531-
str_lit.serialize(serializer);
532-
return;
533-
}
534-
// Should be unreachable - illegal syntax
535-
source.serialize(serializer);
536-
}
537-
}
538-
539506
/// Converter for [`TSParenthesizedType`].
540507
///
541508
/// In raw transfer, do not produce a `TSParenthesizedType` node in AST if `PRESERVE_PARENS` is false.

crates/oxc_ast_visit/src/generated/visit.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3955,7 +3955,7 @@ pub mod walk {
39553955
let kind = AstKind::TSImportType(visitor.alloc(it));
39563956
visitor.enter_node(kind);
39573957
visitor.visit_span(&it.span);
3958-
visitor.visit_ts_type(&it.source);
3958+
visitor.visit_string_literal(&it.source);
39593959
if let Some(options) = &it.options {
39603960
visitor.visit_object_expression(options);
39613961
}

crates/oxc_ast_visit/src/generated/visit_mut.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4164,7 +4164,7 @@ pub mod walk_mut {
41644164
let kind = AstType::TSImportType;
41654165
visitor.enter_node(kind);
41664166
visitor.visit_span(&mut it.span);
4167-
visitor.visit_ts_type(&mut it.source);
4167+
visitor.visit_string_literal(&mut it.source);
41684168
if let Some(options) = &mut it.options {
41694169
visitor.visit_object_expression(options);
41704170
}

crates/oxc_codegen/src/gen.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3515,7 +3515,7 @@ impl Gen for TSTypeQueryExprName<'_> {
35153515
impl Gen for TSImportType<'_> {
35163516
fn r#gen(&self, p: &mut Codegen, ctx: Context) {
35173517
p.print_str("import(");
3518-
self.source.print(p, ctx);
3518+
p.print_string_literal(&self.source, false);
35193519
if let Some(options) = &self.options {
35203520
p.print_str(", ");
35213521
options.print_expr(p, Precedence::Lowest, ctx);

0 commit comments

Comments
 (0)