Add isolatedModules error for ambiguous imports referenced in decorator metadata

src/compiler/checker.ts

@@ -34841,21 +34841,32 @@ namespace ts {
          * marked as referenced to prevent import elision.
          */
         function markTypeNodeAsReferenced(node: TypeNode) {
-            markEntityNameOrEntityExpressionAsReference(node && getEntityNameFromTypeNode(node));
+            markEntityNameOrEntityExpressionAsReference(node && getEntityNameFromTypeNode(node), /*forDecoratorMetadata*/ false);
         }
 
-        function markEntityNameOrEntityExpressionAsReference(typeName: EntityNameOrEntityNameExpression | undefined) {
+        function markEntityNameOrEntityExpressionAsReference(typeName: EntityNameOrEntityNameExpression | undefined, forDecoratorMetadata: boolean) {
             if (!typeName) return;
 
             const rootName = getFirstIdentifier(typeName);
             const meaning = (typeName.kind === SyntaxKind.Identifier ? SymbolFlags.Type : SymbolFlags.Namespace) | SymbolFlags.Alias;
-            const rootSymbol = resolveName(rootName, rootName.escapedText, meaning, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined, /*isReference*/ true);
-            if (rootSymbol
-                && rootSymbol.flags & SymbolFlags.Alias
-                && symbolIsValue(rootSymbol)
-                && !isConstEnumOrConstEnumOnlyModule(resolveAlias(rootSymbol))
-                && !getTypeOnlyAliasDeclaration(rootSymbol)) {
-                markAliasSymbolAsReferenced(rootSymbol);
+            const rootSymbol = resolveName(rootName, rootName.escapedText, meaning, /*nameNotFoundMessage*/ undefined, /*nameArg*/ undefined, /*isRefernce*/ true);
+            if (rootSymbol && rootSymbol.flags & SymbolFlags.Alias) {
+                if (symbolIsValue(rootSymbol)
+                    && !isConstEnumOrConstEnumOnlyModule(resolveAlias(rootSymbol))
+                    && !getTypeOnlyAliasDeclaration(rootSymbol)) {
+                    markAliasSymbolAsReferenced(rootSymbol);
+                }
+                else if (forDecoratorMetadata
+                    && compilerOptions.isolatedModules
+                    && getEmitModuleKind(compilerOptions) >= ModuleKind.ES2015
+                    && !symbolIsValue(rootSymbol)
+                    && !some(rootSymbol.declarations, isTypeOnlyImportOrExportDeclaration)) {
+                    const diag = error(typeName, Diagnostics.A_type_referenced_in_a_decorated_signature_must_be_imported_with_import_type_or_a_namespace_import_when_isolatedModules_and_emitDecoratorMetadata_are_enabled);
+                    const aliasDeclaration = find(rootSymbol.declarations || emptyArray, isAliasSymbolDeclaration);
+                    if (aliasDeclaration) {
+                        addRelatedInfo(diag, createDiagnosticForNode(aliasDeclaration, Diagnostics._0_was_imported_here, idText(rootName)));
+                    }
+                }
             }
         }
 
@@ -34869,7 +34880,7 @@ namespace ts {
         function markDecoratorMedataDataTypeNodeAsReferenced(node: TypeNode | undefined): void {
             const entityName = getEntityNameForDecoratorMetadata(node);
             if (entityName && isEntityName(entityName)) {
-                markEntityNameOrEntityExpressionAsReference(entityName);
+                markEntityNameOrEntityExpressionAsReference(entityName, /*forDecoratorMetadata*/ true);
             }
         }
 

src/compiler/diagnosticMessages.json

@@ -891,6 +891,10 @@
         "category": "Error",
         "code": 1268
     },
+    "A type referenced in a decorated signature must be imported with 'import type' or a namespace import when 'isolatedModules' and 'emitDecoratorMetadata' are enabled.": {
+        "category": "Error",
+        "code": 1269
+    },
 
     "'with' statements are not allowed in an async function block.": {
         "category": "Error",
@@ -7111,6 +7115,7 @@
         "code": 95166
     },
 
+
     "No value exists in scope for the shorthand property '{0}'. Either declare one or provide an initializer.": {
         "category": "Error",
         "code": 18004

src/services/codefixes/fixUnreferenceableDecoratorMetadata.ts

@@ -0,0 +1,70 @@
+/* @internal */
+namespace ts.codefix {
+    const fixId = "fixUnreferenceableDecoratorMetadata";
+    const errorCodes = [Diagnostics.A_type_referenced_in_a_decorated_signature_must_be_imported_with_import_type_or_a_namespace_import_when_isolatedModules_and_emitDecoratorMetadata_are_enabled.code];
+    registerCodeFix({
+        errorCodes,
+        getCodeActions: context => {
+            const importDeclaration = getImportDeclaration(context.sourceFile, context.program, context.span.start);
+            if (!importDeclaration) return;
+
+            const namespaceChanges = textChanges.ChangeTracker.with(context, t => importDeclaration.kind === SyntaxKind.ImportSpecifier && doNamespaceImportChange(t, context.sourceFile, importDeclaration, context.program));
+            const typeOnlyChanges = textChanges.ChangeTracker.with(context, t => doTypeOnlyImportChange(t, context.sourceFile, importDeclaration, context.program));
+            let actions: CodeFixAction[] | undefined;
+            if (namespaceChanges.length) {
+                actions = append(actions, createCodeFixActionWithoutFixAll(fixId, namespaceChanges, Diagnostics.Convert_named_imports_to_namespace_import));
+            }
+            if (typeOnlyChanges.length) {
+                actions = append(actions, createCodeFixActionWithoutFixAll(fixId, typeOnlyChanges, Diagnostics.Convert_to_type_only_import));
+            }
+            return actions;
+        },
+        fixIds: [fixId],
+    });
+
+    function getImportDeclaration(sourceFile: SourceFile, program: Program, start: number): ImportClause | ImportSpecifier | ImportEqualsDeclaration | undefined {
+        const identifier = tryCast(getTokenAtPosition(sourceFile, start), isIdentifier);
+        if (!identifier || identifier.parent.kind !== SyntaxKind.TypeReference) return;
+
+        const checker = program.getTypeChecker();
+        const symbol = checker.getSymbolAtLocation(identifier);
+        return find(symbol?.declarations || emptyArray, or(isImportClause, isImportSpecifier, isImportEqualsDeclaration) as (n: Node) => n is ImportClause | ImportSpecifier | ImportEqualsDeclaration);
+    }
+
+    // Converts the import declaration of the offending import to a type-only import,
+    // only if it can be done without affecting other imported names. If the conversion
+    // cannot be done cleanly, we could offer to *extract* the offending import to a
+    // new type-only import declaration, but honestly I doubt anyone will ever use this
+    // codefix at all, so it's probably not worth the lines of code.
+    function doTypeOnlyImportChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, importDeclaration: ImportClause | ImportSpecifier | ImportEqualsDeclaration, program: Program) {
+        if (importDeclaration.kind === SyntaxKind.ImportEqualsDeclaration) {
+            changes.insertModifierBefore(sourceFile, SyntaxKind.TypeKeyword, importDeclaration.name);
+            return;
+        }
+
+        const importClause = importDeclaration.kind === SyntaxKind.ImportClause ? importDeclaration : importDeclaration.parent.parent;
+        if (importClause.name && importClause.namedBindings) {
+            // Cannot convert an import with a default import and named bindings to type-only
+            // (it's a grammar error).
+            return;
+        }
+
+        const checker = program.getTypeChecker();
+        const importsValue = !!forEachImportClauseDeclaration(importClause, decl => {
+            if (skipAlias(decl.symbol, checker).flags & SymbolFlags.Value) return true;
+        });
+
+        if (importsValue) {
+            // Assume that if someone wrote a non-type-only import that includes some values,
+            // they intend to use those values in value positions, even if they haven't yet.
+            // Don't convert it to type-only.
+            return;
+        }
+
+        changes.insertModifierBefore(sourceFile, SyntaxKind.TypeKeyword, importClause);
+    }
+
+    function doNamespaceImportChange(changes: textChanges.ChangeTracker, sourceFile: SourceFile, importDeclaration: ImportSpecifier, program: Program) {
+        refactor.doChangeNamedToNamespace(sourceFile, program.getTypeChecker(), changes, importDeclaration.parent);
+    }
+}

src/services/refactors/convertImport.ts

@@ -134,7 +134,7 @@ namespace ts.refactor {
         return isPropertyAccessExpression(propertyAccessOrQualifiedName) ? propertyAccessOrQualifiedName.expression : propertyAccessOrQualifiedName.left;
     }
 
-    function doChangeNamedToNamespace(sourceFile: SourceFile, checker: TypeChecker, changes: textChanges.ChangeTracker, toConvert: NamedImports): void {
+    export function doChangeNamedToNamespace(sourceFile: SourceFile, checker: TypeChecker, changes: textChanges.ChangeTracker, toConvert: NamedImports): void {
         const importDecl = toConvert.parent.parent;
         const { moduleSpecifier } = importDecl;
 

src/services/tsconfig.json

@@ -86,6 +86,7 @@
         "codefixes/fixExtendsInterfaceBecomesImplements.ts",
         "codefixes/fixForgottenThisPropertyAccess.ts",
         "codefixes/fixInvalidJsxCharacters.ts",
+        "codefixes/fixUnreferenceableDecoratorMetadata.ts",
         "codefixes/fixUnusedIdentifier.ts",
         "codefixes/fixUnreachableCode.ts",
         "codefixes/fixUnusedLabel.ts",

tests/baselines/reference/emitDecoratorMetadata_isolatedModules(module=commonjs).js

@@ -0,0 +1,108 @@
+//// [tests/cases/compiler/emitDecoratorMetadata_isolatedModules.ts] ////
+
+//// [type1.ts]
+interface T1 {}
+export type { T1 }
+
+//// [type2.ts]
+export interface T2 {}
+
+//// [class3.ts]
+export class C3 {}
+
+//// [index.ts]
+import { T1 } from "./type1";
+import * as t1 from "./type1";
+import type { T2 } from "./type2";
+import { C3 } from "./class3";
+declare var EventListener: any;
+
+class HelloWorld {
+  @EventListener('1')
+  handleEvent1(event: T1) {} // Error
+
+  @EventListener('2')
+  handleEvent2(event: T2) {} // Ok
+
+  @EventListener('1')
+  p1!: T1; // Error
+
+  @EventListener('1')
+  p1_ns!: t1.T1; // Ok
+
+  @EventListener('2')
+  p2!: T2; // Ok
+
+  @EventListener('3')
+  handleEvent3(event: C3): T1 { return undefined! } // Ok, Error
+}
+
+
+//// [type1.js]
+"use strict";
+exports.__esModule = true;
+//// [type2.js]
+"use strict";
+exports.__esModule = true;
+//// [class3.js]
+"use strict";
+exports.__esModule = true;
+exports.C3 = void 0;
+var C3 = /** @class */ (function () {
+    function C3() {
+    }
+    return C3;
+}());
+exports.C3 = C3;
+//// [index.js]
+"use strict";
+var __decorate = (this && this.__decorate) || function (decorators, target, key, desc) {
+    var c = arguments.length, r = c < 3 ? target : desc === null ? desc = Object.getOwnPropertyDescriptor(target, key) : desc, d;
+    if (typeof Reflect === "object" && typeof Reflect.decorate === "function") r = Reflect.decorate(decorators, target, key, desc);
+    else for (var i = decorators.length - 1; i >= 0; i--) if (d = decorators[i]) r = (c < 3 ? d(r) : c > 3 ? d(target, key, r) : d(target, key)) || r;
+    return c > 3 && r && Object.defineProperty(target, key, r), r;
+};
+var __metadata = (this && this.__metadata) || function (k, v) {
+    if (typeof Reflect === "object" && typeof Reflect.metadata === "function") return Reflect.metadata(k, v);
+};
+exports.__esModule = true;
+var t1 = require("./type1");
+var class3_1 = require("./class3");
+var HelloWorld = /** @class */ (function () {
+    function HelloWorld() {
+    }
+    HelloWorld.prototype.handleEvent1 = function (event) { }; // Error
+    HelloWorld.prototype.handleEvent2 = function (event) { }; // Ok
+    HelloWorld.prototype.handleEvent3 = function (event) { return undefined; }; // Ok, Error
+    __decorate([
+        EventListener('1'),
+        __metadata("design:type", Function),
+        __metadata("design:paramtypes", [Object]),
+        __metadata("design:returntype", void 0)
+    ], HelloWorld.prototype, "handleEvent1");
+    __decorate([
+        EventListener('2'),
+        __metadata("design:type", Function),
+        __metadata("design:paramtypes", [Object]),
+        __metadata("design:returntype", void 0)
+    ], HelloWorld.prototype, "handleEvent2");
+    __decorate([
+        EventListener('1'),
+        __metadata("design:type", Object)
+    ], HelloWorld.prototype, "p1");
+    __decorate([
+        EventListener('1'),
+        __metadata("design:type", Object)
+    ], HelloWorld.prototype, "p1_ns");
+    __decorate([
+        EventListener('2'),
+        __metadata("design:type", Object)
+    ], HelloWorld.prototype, "p2");
+    __decorate([
+        EventListener('3'),
+        __metadata("design:type", Function),
+        __metadata("design:paramtypes", [class3_1.C3]),
+        __metadata("design:returntype", Object)
+    ], HelloWorld.prototype, "handleEvent3");
+    return HelloWorld;
+}());

tests/baselines/reference/emitDecoratorMetadata_isolatedModules(module=commonjs).symbols

@@ -0,0 +1,83 @@
+=== tests/cases/compiler/type1.ts ===
+interface T1 {}
+>T1 : Symbol(T1, Decl(type1.ts, 0, 0))
+
+export type { T1 }
+>T1 : Symbol(T1, Decl(type1.ts, 1, 13))
+
+=== tests/cases/compiler/type2.ts ===
+export interface T2 {}
+>T2 : Symbol(T2, Decl(type2.ts, 0, 0))
+
+=== tests/cases/compiler/class3.ts ===
+export class C3 {}
+>C3 : Symbol(C3, Decl(class3.ts, 0, 0))
+
+=== tests/cases/compiler/index.ts ===
+import { T1 } from "./type1";
+>T1 : Symbol(T1, Decl(index.ts, 0, 8))
+
+import * as t1 from "./type1";
+>t1 : Symbol(t1, Decl(index.ts, 1, 6))
+
+import type { T2 } from "./type2";
+>T2 : Symbol(T2, Decl(index.ts, 2, 13))
+
+import { C3 } from "./class3";
+>C3 : Symbol(C3, Decl(index.ts, 3, 8))
+
+declare var EventListener: any;
+>EventListener : Symbol(EventListener, Decl(index.ts, 4, 11))
+
+class HelloWorld {
+>HelloWorld : Symbol(HelloWorld, Decl(index.ts, 4, 31))
+
+  @EventListener('1')
+>EventListener : Symbol(EventListener, Decl(index.ts, 4, 11))
+
+  handleEvent1(event: T1) {} // Error
+>handleEvent1 : Symbol(HelloWorld.handleEvent1, Decl(index.ts, 6, 18))
+>event : Symbol(event, Decl(index.ts, 8, 15))
+>T1 : Symbol(T1, Decl(index.ts, 0, 8))
+
+  @EventListener('2')
+>EventListener : Symbol(EventListener, Decl(index.ts, 4, 11))
+
+  handleEvent2(event: T2) {} // Ok
+>handleEvent2 : Symbol(HelloWorld.handleEvent2, Decl(index.ts, 8, 28))
+>event : Symbol(event, Decl(index.ts, 11, 15))
+>T2 : Symbol(T2, Decl(index.ts, 2, 13))
+
+  @EventListener('1')
+>EventListener : Symbol(EventListener, Decl(index.ts, 4, 11))
+
+  p1!: T1; // Error
+>p1 : Symbol(HelloWorld.p1, Decl(index.ts, 11, 28))
+>T1 : Symbol(T1, Decl(index.ts, 0, 8))
+
+  @EventListener('1')
+>EventListener : Symbol(EventListener, Decl(index.ts, 4, 11))
+
+  p1_ns!: t1.T1; // Ok
+>p1_ns : Symbol(HelloWorld.p1_ns, Decl(index.ts, 14, 10))
+>t1 : Symbol(t1, Decl(index.ts, 1, 6))
+>T1 : Symbol(t1.T1, Decl(type1.ts, 1, 13))
+
+  @EventListener('2')
+>EventListener : Symbol(EventListener, Decl(index.ts, 4, 11))
+
+  p2!: T2; // Ok
+>p2 : Symbol(HelloWorld.p2, Decl(index.ts, 17, 16))
+>T2 : Symbol(T2, Decl(index.ts, 2, 13))
+
+  @EventListener('3')
+>EventListener : Symbol(EventListener, Decl(index.ts, 4, 11))
+
+  handleEvent3(event: C3): T1 { return undefined! } // Ok, Error
+>handleEvent3 : Symbol(HelloWorld.handleEvent3, Decl(index.ts, 20, 10))
+>event : Symbol(event, Decl(index.ts, 23, 15))
+>C3 : Symbol(C3, Decl(index.ts, 3, 8))
+>T1 : Symbol(T1, Decl(index.ts, 0, 8))
+>undefined : Symbol(undefined)
+}
+