From 351ecf2707e31292cfa0deb8f379a50c1cac7f14 Mon Sep 17 00:00:00 2001 From: Dunqing <29533304+Dunqing@users.noreply.github.com> Date: Wed, 17 Jul 2024 03:33:00 +0000 Subject: [PATCH] fix(semantic): incorrect resolve references for `TSTypeQuery` (#4310) ```ts type A = typeof Foo ^^^ Only allow reference to value symbol ``` I have verified the changed snapshot. That's correct --- .../typescript/consistent_type_imports.rs | 96 +++++++++++-------- .../snapshots/consistent_type_imports.snap | 16 ++-- crates/oxc_semantic/src/builder.rs | 8 +- .../fixtures/typescript-eslint/README.md | 1 - .../class/declaration/type-reference.snap | 2 +- .../import/type-default-value.snap | 9 +- .../import/type-inline-value.snap | 9 +- .../import/type-named-value.snap | 9 +- .../type-arguments2.snap | 6 ++ .../type-declaration/function/function2.snap | 9 +- .../type-declaration/index-access3.snap | 9 +- .../type-query-qualified.snap | 9 +- .../type-query-with-parameters.snap | 9 +- .../type-declaration/type-query.snap | 9 +- .../oxc_semantic/tests/integration/symbols.rs | 13 +++ tasks/coverage/transformer_typescript.snap | 4 +- tasks/transform_conformance/babel.snap.md | 5 +- 17 files changed, 140 insertions(+), 83 deletions(-) delete mode 100644 crates/oxc_semantic/tests/fixtures/typescript-eslint/README.md diff --git a/crates/oxc_linter/src/rules/typescript/consistent_type_imports.rs b/crates/oxc_linter/src/rules/typescript/consistent_type_imports.rs index 0f3920bc8e990..c2eb127395a9b 100644 --- a/crates/oxc_linter/src/rules/typescript/consistent_type_imports.rs +++ b/crates/oxc_linter/src/rules/typescript/consistent_type_imports.rs @@ -309,7 +309,23 @@ fn is_only_has_type_references(symbol_id: SymbolId, ctx: &LintContext) -> bool { if peekable_iter.peek().is_none() { return false; } - peekable_iter.all(oxc_semantic::Reference::is_type) + peekable_iter.all(|reference| { + if reference.is_type() { + return true; + } else if reference.is_read() { + for node in ctx.nodes().iter_parents(reference.node_id()).skip(1) { + return match node.kind() { + // CASE 1: + // `type T = typeof foo` will create a value reference because "foo" must be a value type + // however this value reference is safe to use with type-only imports + AstKind::TSTypeQuery(_) => true, + AstKind::TSTypeName(_) | AstKind::TSQualifiedName(_) => continue, + _ => false, + }; + } + } + false + }) } struct FixOptions<'a, 'b> { @@ -1155,7 +1171,7 @@ fn test() { ( " import Type from 'foo'; - + export { Type }; // is a value export export default Type; // is a value export ", @@ -1164,7 +1180,7 @@ fn test() { ( " import type Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1174,7 +1190,7 @@ fn test() { ( " import { Type } from 'foo'; - + export { Type }; // is a value export export default Type; // is a value export ", @@ -1183,7 +1199,7 @@ fn test() { ( " import type { Type } from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1193,7 +1209,7 @@ fn test() { ( " import * as Type from 'foo'; - + export { Type }; // is a value export export default Type; // is a value export ", @@ -1202,7 +1218,7 @@ fn test() { ( " import type * as Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1212,7 +1228,7 @@ fn test() { ( " import Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1222,7 +1238,7 @@ fn test() { ( " import { Type } from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1232,7 +1248,7 @@ fn test() { ( " import * as Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1282,7 +1298,7 @@ fn test() { ( " import type * as constants from './constants'; - + export type Y = { [constants.X]: ReadonlyArray; }; @@ -1796,7 +1812,7 @@ fn test() { ( " import Type from 'foo'; - + export type { Type }; // is a type-only export ", None, @@ -1804,7 +1820,7 @@ fn test() { ( " import { Type } from 'foo'; - + export type { Type }; // is a type-only export ", None, @@ -1812,7 +1828,7 @@ fn test() { ( " import * as Type from 'foo'; - + export type { Type }; // is a type-only export ", None, @@ -1820,7 +1836,7 @@ fn test() { ( " import type Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1830,7 +1846,7 @@ fn test() { ( " import type { Type } from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1840,7 +1856,7 @@ fn test() { ( " import type * as Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -1853,7 +1869,7 @@ fn test() { import type // comment DefType from 'foo'; import type /*comment*/ { Type } from 'foo'; - + type T = { a: AllType; b: DefType; c: Type }; ", Some(serde_json::json!([{ "prefer": "no-type-imports" }])), @@ -1933,7 +1949,7 @@ fn test() { ( " import { A, B } from 'foo'; - + let foo: A; B(); ", @@ -2028,7 +2044,7 @@ fn test() { " import { A, B, C } from 'foo'; import type { D } from 'deez'; - + const foo: A = B(); let bar: C; let baz: D; @@ -2132,7 +2148,7 @@ fn test() { class A { @deco get foo() {} - + set foo(value: Foo) {} } ", @@ -2144,7 +2160,7 @@ fn test() { class A { @deco get foo() {} - + set ['foo'](value: Foo) {} } ", @@ -2660,12 +2676,12 @@ fn test() { ( " import Type from 'foo'; - + export type { Type }; // is a type-only export ", " import type Type from 'foo'; - + export type { Type }; // is a type-only export ", None, @@ -2673,12 +2689,12 @@ fn test() { ( " import { Type } from 'foo'; - + export type { Type }; // is a type-only export ", " import type { Type } from 'foo'; - + export type { Type }; // is a type-only export ", None, @@ -2686,12 +2702,12 @@ fn test() { ( " import * as Type from 'foo'; - + export type { Type }; // is a type-only export ", " import type * as Type from 'foo'; - + export type { Type }; // is a type-only export ", None, @@ -2699,14 +2715,14 @@ fn test() { ( " import type Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export ", " import Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -2716,14 +2732,14 @@ fn test() { ( " import type { Type } from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export ", " import { Type } from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -2733,14 +2749,14 @@ fn test() { ( " import type * as Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export ", " import * as Type from 'foo'; - + export { Type }; // is a type-only export export default Type; // is a type-only export export type { Type }; // is a type-only export @@ -2753,7 +2769,7 @@ fn test() { import type // comment DefType from 'foo'; import type /*comment*/ { Type } from 'foo'; - + type T = { a: AllType; b: DefType; c: Type }; ", " @@ -2761,7 +2777,7 @@ fn test() { import // comment DefType from 'foo'; import /*comment*/ { Type } from 'foo'; - + type T = { a: AllType; b: DefType; c: Type }; ", Some(serde_json::json!([{ "prefer": "no-type-imports" }])), @@ -2890,13 +2906,13 @@ fn test() { ( " import { A, B } from 'foo'; - + let foo: A; B(); ", " import { type A, B } from 'foo'; - + let foo: A; B(); ", @@ -3036,7 +3052,7 @@ fn test() { " import { A, B, C } from 'foo'; import type { D } from 'deez'; - + const foo: A = B(); let bar: C; let baz: D; @@ -3044,7 +3060,7 @@ fn test() { " import { type A, B, type C } from 'foo'; import type { D } from 'deez'; - + const foo: A = B(); let bar: C; let baz: D; diff --git a/crates/oxc_linter/src/snapshots/consistent_type_imports.snap b/crates/oxc_linter/src/snapshots/consistent_type_imports.snap index 2712c36d353fa..cd8247b007eee 100644 --- a/crates/oxc_linter/src/snapshots/consistent_type_imports.snap +++ b/crates/oxc_linter/src/snapshots/consistent_type_imports.snap @@ -310,7 +310,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ 2 │ import Type from 'foo'; · ─────────────────────── - 3 │ + 3 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): All imports in the declaration are only used as types. Use `import type`. @@ -318,7 +318,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ 2 │ import { Type } from 'foo'; · ─────────────────────────── - 3 │ + 3 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): All imports in the declaration are only used as types. Use `import type`. @@ -326,7 +326,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ 2 │ import * as Type from 'foo'; · ──────────────────────────── - 3 │ + 3 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): Use an `import` instead of an `import type`. @@ -334,7 +334,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ 2 │ import type Type from 'foo'; · ──────────────────────────── - 3 │ + 3 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): Use an `import` instead of an `import type`. @@ -342,7 +342,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ 2 │ import type { Type } from 'foo'; · ──────────────────────────────── - 3 │ + 3 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): Use an `import` instead of an `import type`. @@ -350,7 +350,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ 2 │ import type * as Type from 'foo'; · ───────────────────────────────── - 3 │ + 3 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): Use an `import` instead of an `import type`. @@ -374,7 +374,7 @@ source: crates/oxc_linter/src/tester.rs 4 │ DefType from 'foo'; 5 │ import type /*comment*/ { Type } from 'foo'; · ──────────────────────────────────────────── - 6 │ + 6 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): Imports Rest are only used as type. @@ -446,7 +446,7 @@ source: crates/oxc_linter/src/tester.rs 1 │ 2 │ import { A, B } from 'foo'; · ─────────────────────────── - 3 │ + 3 │ ╰──── ⚠ typescript-eslint(consistent-type-imports): Imports A are only used as type. diff --git a/crates/oxc_semantic/src/builder.rs b/crates/oxc_semantic/src/builder.rs index 723333d7f53b5..6179d027ff3d2 100644 --- a/crates/oxc_semantic/src/builder.rs +++ b/crates/oxc_semantic/src/builder.rs @@ -1714,8 +1714,14 @@ impl<'a> SemanticBuilder<'a> { } AstKind::TSTypeName(_) => { match self.nodes.parent_kind(self.current_node_id) { - Some(AstKind::TSModuleReference(_)) => { + Some( + // type A = typeof a; + // ^^^^^^^^ + AstKind::TSTypeQuery(_) // import A = a; + // ^ + | AstKind::TSModuleReference(_) + ) => { self.current_reference_flag = ReferenceFlag::Read; } Some(AstKind::TSQualifiedName(_)) => { diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/README.md b/crates/oxc_semantic/tests/fixtures/typescript-eslint/README.md deleted file mode 100644 index 0941baecb0aae..0000000000000 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/README.md +++ /dev/null @@ -1 +0,0 @@ -These fixtures were copied from https://github.com/typescript-eslint/typescript-eslint/tree/a5b652da1ebb09ecbca8f4b032bf05ebc0e03dc9/packages/scope-manager/tests/fixtures. We used them to test out semantic [ScopeTree](../../src/scope.rs) and [SymbolTable](../../src/symbol.rs) \ No newline at end of file diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/type-reference.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/type-reference.snap index a81404cea790f..34be306653cfe 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/type-reference.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declaration/type-reference.snap @@ -44,7 +44,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/class/declarati "node_id": 8 }, { - "flag": "ReferenceFlag(Type)", + "flag": "ReferenceFlag(Read)", "id": 1, "name": "A", "node_id": 13 diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-default-value.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-default-value.snap index 4d5e3d90e7099..61035781caaaa 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-default-value.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-default-value.snap @@ -22,14 +22,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-def "id": 0, "name": "foo", "node": "ImportDefaultSpecifier", - "references": [ - { - "flag": "ReferenceFlag(Type)", - "id": 0, - "name": "foo", - "node_id": 10 - } - ] + "references": [] }, { "flag": "SymbolFlags(TypeAlias)", diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-inline-value.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-inline-value.snap index 2fbff989f792c..bfa74a794e660 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-inline-value.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-inline-value.snap @@ -22,14 +22,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-inl "id": 0, "name": "foo", "node": "ImportSpecifier", - "references": [ - { - "flag": "ReferenceFlag(Type)", - "id": 0, - "name": "foo", - "node_id": 11 - } - ] + "references": [] }, { "flag": "SymbolFlags(TypeAlias)", diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-named-value.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-named-value.snap index 589331c4c31bc..989e6b9f6bdc0 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-named-value.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-named-value.snap @@ -22,14 +22,7 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/import/type-nam "id": 0, "name": "foo", "node": "ImportSpecifier", - "references": [ - { - "flag": "ReferenceFlag(Type)", - "id": 0, - "name": "foo", - "node_id": 11 - } - ] + "references": [] }, { "flag": "SymbolFlags(TypeAlias)", diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/instantiation-expressions/type-arguments2.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/instantiation-expressions/type-arguments2.snap index ed72d2a10c6a6..b050a79c0e400 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/instantiation-expressions/type-arguments2.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/instantiation-expressions/type-arguments2.snap @@ -78,6 +78,12 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/instantiation-e "flag": "ReferenceFlag(Read)", "id": 0, "name": "makeBox", + "node_id": 27 + }, + { + "flag": "ReferenceFlag(Read)", + "id": 1, + "name": "makeBox", "node_id": 36 } ] diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/function/function2.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/function/function2.snap index 275c4b1a0910b..1830abb5a568a 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/function/function2.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/function/function2.snap @@ -22,7 +22,14 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaratio "id": 0, "name": "arg", "node": "VariableDeclarator", - "references": [] + "references": [ + { + "flag": "ReferenceFlag(Read)", + "id": 0, + "name": "arg", + "node_id": 15 + } + ] }, { "flag": "SymbolFlags(TypeAlias)", diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/index-access3.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/index-access3.snap index aec344e7030e1..b66e0db5c670c 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/index-access3.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/index-access3.snap @@ -43,7 +43,14 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaratio "id": 1, "name": "k", "node": "VariableDeclarator", - "references": [] + "references": [ + { + "flag": "ReferenceFlag(Read)", + "id": 0, + "name": "k", + "node_id": 21 + } + ] }, { "flag": "SymbolFlags(TypeAlias)", diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-qualified.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-qualified.snap index 2dbd737326256..63c6d8c69e262 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-qualified.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-qualified.snap @@ -29,7 +29,14 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaratio "id": 0, "name": "x", "node": "VariableDeclarator", - "references": [] + "references": [ + { + "flag": "ReferenceFlag(Read)", + "id": 0, + "name": "x", + "node_id": 21 + } + ] }, { "flag": "SymbolFlags(TypeAlias)", diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-with-parameters.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-with-parameters.snap index 648b81cef84a2..5d901eed89ccd 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-with-parameters.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query-with-parameters.snap @@ -73,7 +73,14 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaratio "id": 0, "name": "foo", "node": "Function(foo)", - "references": [] + "references": [ + { + "flag": "ReferenceFlag(Read)", + "id": 0, + "name": "foo", + "node_id": 29 + } + ] }, { "flag": "SymbolFlags(Export | TypeAlias)", diff --git a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query.snap b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query.snap index b76264903b238..e95a6cea4cd66 100644 --- a/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query.snap +++ b/crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaration/type-query.snap @@ -29,7 +29,14 @@ input_file: crates/oxc_semantic/tests/fixtures/typescript-eslint/type-declaratio "id": 0, "name": "x", "node": "VariableDeclarator", - "references": [] + "references": [ + { + "flag": "ReferenceFlag(Read)", + "id": 0, + "name": "x", + "node_id": 9 + } + ] }, { "flag": "SymbolFlags(TypeAlias)", diff --git a/crates/oxc_semantic/tests/integration/symbols.rs b/crates/oxc_semantic/tests/integration/symbols.rs index b879758c9c11f..6b4f861aaadc6 100644 --- a/crates/oxc_semantic/tests/integration/symbols.rs +++ b/crates/oxc_semantic/tests/integration/symbols.rs @@ -279,3 +279,16 @@ fn test_type_used_as_value() { .has_number_of_reads(0) .test(); } + +#[test] +fn test_type_query() { + SemanticTester::ts( + " + type T = number; + let x: typeof T; + ", + ) + .has_some_symbol("T") + .has_number_of_reads(0) + .test(); +} diff --git a/tasks/coverage/transformer_typescript.snap b/tasks/coverage/transformer_typescript.snap index 94311ce35a448..61250e368a477 100644 --- a/tasks/coverage/transformer_typescript.snap +++ b/tasks/coverage/transformer_typescript.snap @@ -2,8 +2,10 @@ commit: d8086f14 transformer_typescript Summary: AST Parsed : 6456/6456 (100.00%) -Positive Passed: 6452/6456 (99.94%) +Positive Passed: 6450/6456 (99.91%) Mismatch: "compiler/constEnumNamespaceReferenceCausesNoImport2.ts" Mismatch: "compiler/elidedEmbeddedStatementsReplacedWithSemicolon.ts" +Mismatch: "conformance/dynamicImport/importCallExpressionReturnPromiseOfAny.ts" Mismatch: "conformance/externalModules/typeOnly/exportDeclaration.ts" +Mismatch: "conformance/externalModules/typeOnly/namespaceImportTypeQuery2.ts" Mismatch: "conformance/jsx/inline/inlineJsxAndJsxFragPragmaOverridesCompilerOptions.tsx" diff --git a/tasks/transform_conformance/babel.snap.md b/tasks/transform_conformance/babel.snap.md index 962d2d27ba8ec..d4bb055b52117 100644 --- a/tasks/transform_conformance/babel.snap.md +++ b/tasks/transform_conformance/babel.snap.md @@ -1,6 +1,6 @@ commit: 12619ffe -Passed: 473/927 +Passed: 472/927 # All Passed: * babel-preset-react @@ -445,13 +445,14 @@ Passed: 473/927 * opts/optimizeConstEnums/input.ts * opts/rewriteImportExtensions/input.ts -# babel-plugin-transform-typescript (129/151) +# babel-plugin-transform-typescript (128/151) * class/accessor-allowDeclareFields-false/input.ts * class/accessor-allowDeclareFields-true/input.ts * enum/mix-references/input.ts * enum/ts5.0-const-foldable/input.ts * exports/declared-types/input.ts * exports/interface/input.ts +* imports/elide-typeof/input.ts * imports/elision-locations/input.ts * imports/only-remove-type-imports/input.ts * imports/type-only-export-specifier-2/input.ts