Clean up, reword diagnostics

microsoft · andrewbranch · Sep 8, 2021 · May 7, 2021 · May 7, 2021 · May 7, 2021
commit 9e2e4dc39d67fcc9674001586c8af3f9ac0921f3
diff --git a/src/compiler/checker.ts b/src/compiler/checker.ts
@@ -38607,49 +38607,45 @@ namespace ts {
                     error(node, message, symbolToString(symbol));
                 }
 
-                // Type assertion to defeat the type predicate aliasing of `node` here because
-                // `isTypeOnlyImportOrExportDeclaration` is a one-sided type guard so `!isDeclaredTypeOnly`
-                // does not actually imply any narrowing effect on `node`.
-                const isDeclaredTypeOnly = isTypeOnlyImportOrExportDeclaration(node as never);
                 if (compilerOptions.isolatedModules
-                    && !isDeclaredTypeOnly
-                    && !(node.flags & NodeFlags.Ambient)
-                    && (!(target.flags & SymbolFlags.Value) || getTypeOnlyAliasDeclaration(symbol))) {
-                    const isType = !(target.flags & SymbolFlags.Value);
+                    && !isTypeOnlyImportOrExportDeclaration(node)
+                    && !(node.flags & NodeFlags.Ambient)) {
                     const typeOnlyAlias = getTypeOnlyAliasDeclaration(symbol);
-
-                    switch (node.kind) {
-                        case SyntaxKind.ImportClause:
-                        case SyntaxKind.ImportSpecifier:
-                        case SyntaxKind.ImportEqualsDeclaration: {
-                            if (compilerOptions.preserveValueImports) {
-                                const message = isType
-                                    ? Diagnostics._0_is_a_type_and_must_be_imported_with_a_type_only_import_when_preserveValueImports_and_isolatedModules_are_both_enabled
-                                    : Diagnostics._0_resolves_to_a_type_only_declaration_and_must_be_imported_with_a_type_only_import_when_preserveValueImports_and_isolatedModules_are_both_enabled;
-                                const name = idText(node.kind === SyntaxKind.ImportSpecifier ? node.propertyName || node.name : node.name!);
-                                addTypeOnlyDeclarationRelatedInfo(
-                                    error(node, message, name),
-                                    isType ? undefined : typeOnlyAlias,
-                                    name
-                                );
+                    const isType = !(target.flags & SymbolFlags.Value);
+                    if (isType || typeOnlyAlias) {
+                        switch (node.kind) {
+                            case SyntaxKind.ImportClause:
+                            case SyntaxKind.ImportSpecifier:
+                            case SyntaxKind.ImportEqualsDeclaration: {
+                                if (compilerOptions.preserveValueImports) {
+                                    const message = isType
+                                        ? Diagnostics._0_is_a_type_and_must_be_imported_using_a_type_only_import_when_preserveValueImports_and_isolatedModules_are_both_enabled
+                                        : Diagnostics._0_resolves_to_a_type_only_declaration_and_must_be_imported_using_a_type_only_import_when_preserveValueImports_and_isolatedModules_are_both_enabled;
+                                    const name = idText(node.kind === SyntaxKind.ImportSpecifier ? node.propertyName || node.name : node.name!);
+                                    addTypeOnlyDeclarationRelatedInfo(
+                                        error(node, message, name),
+                                        isType ? undefined : typeOnlyAlias,
+                                        name
+                                    );
+                                }
+                                break;
                             }
-                            break;
-                        }
-                        case SyntaxKind.ExportSpecifier: {
-                            // Don't allow re-exporting an export that will be elided when `--isolatedModules` is set.
-                            // The exception is that `import type { A } from './a'; export { A }` is allowed
-                            // because single-file analysis can determine that the export should be dropped.
-                            if (getSourceFileOfNode(typeOnlyAlias) !== getSourceFileOfNode(node)) {
-                                const message = isType
-                                    ? Diagnostics.Re_exporting_a_type_when_the_isolatedModules_flag_is_provided_requires_using_export_type
-                                    : Diagnostics._0_resolves_to_a_type_only_declaration_and_must_be_re_exported_with_a_type_only_re_export_when_isolatedModules_is_enabled;
-                                const name = idText(node.propertyName || node.name);
-                                addTypeOnlyDeclarationRelatedInfo(
-                                    error(node, message, name),
-                                    isType ? undefined : typeOnlyAlias,
-                                    name
-                                );
-                                return;
+                            case SyntaxKind.ExportSpecifier: {
+                                // Don't allow re-exporting an export that will be elided when `--isolatedModules` is set.
+                                // The exception is that `import type { A } from './a'; export { A }` is allowed
+                                // because single-file analysis can determine that the export should be dropped.
+                                if (getSourceFileOfNode(typeOnlyAlias) !== getSourceFileOfNode(node)) {
+                                    const message = isType
+                                        ? Diagnostics.Re_exporting_a_type_when_the_isolatedModules_flag_is_provided_requires_using_export_type
+                                        : Diagnostics._0_resolves_to_a_type_only_declaration_and_must_be_re_exported_using_a_type_only_re_export_when_isolatedModules_is_enabled;
+                                    const name = idText(node.propertyName || node.name);
+                                    addTypeOnlyDeclarationRelatedInfo(
+                                        error(node, message, name),
+                                        isType ? undefined : typeOnlyAlias,
+                                        name
+                                    );
+                                    return;
+                                }
                             }
                         }
                     }
@@ -40433,13 +40429,13 @@ namespace ts {
         function isValueAliasDeclaration(node: Node): boolean {
             switch (node.kind) {
                 case SyntaxKind.ImportEqualsDeclaration:
-                    return isAliasResolvedToValue(getSymbolOfNode(node) || unknownSymbol);
+                    return isAliasResolvedToValue(getSymbolOfNode(node));
                 case SyntaxKind.ImportClause:
                 case SyntaxKind.NamespaceImport:
                 case SyntaxKind.ImportSpecifier:
                 case SyntaxKind.ExportSpecifier:
-                    const symbol = getSymbolOfNode(node) || unknownSymbol;
-                    return isAliasResolvedToValue(symbol) && !getTypeOnlyAliasDeclaration(symbol);
+                    const symbol = getSymbolOfNode(node);
+                    return !!symbol && isAliasResolvedToValue(symbol) && !getTypeOnlyAliasDeclaration(symbol);
                 case SyntaxKind.ExportDeclaration:
                     const exportClause = (node as ExportDeclaration).exportClause;
                     return !!exportClause && (
@@ -40448,7 +40444,7 @@ namespace ts {
                     );
                 case SyntaxKind.ExportAssignment:
                     return (node as ExportAssignment).expression && (node as ExportAssignment).expression.kind === SyntaxKind.Identifier ?
-                        isAliasResolvedToValue(getSymbolOfNode(node) || unknownSymbol) :
+                        isAliasResolvedToValue(getSymbolOfNode(node)) :
                         true;
             }
             return false;
@@ -40465,8 +40461,8 @@ namespace ts {
             return isValue && node.moduleReference && !nodeIsMissing(node.moduleReference);
         }
 
-        function isAliasResolvedToValue(symbol: Symbol): boolean {
-            if (symbol === unknownSymbol) {
+        function isAliasResolvedToValue(symbol: Symbol | undefined): boolean {
+            if (!symbol) {
                 return false;
             }
             const target = resolveAlias(symbol);

diff --git a/src/compiler/commandLineParser.ts b/src/compiler/commandLineParser.ts
@@ -1173,7 +1173,7 @@ namespace ts {
             type: "boolean",
             affectsEmit: true,
             category: Diagnostics.Emit,
-            description: Diagnostics.Disable_the_removal_of_unused_imported_identifiers_from_the_JavaScript_output
+            description: Diagnostics.Preserve_unused_imported_values_in_the_JavaScript_output_that_would_otherwise_be_removed,
         },
 
         {

diff --git a/src/compiler/diagnosticMessages.json b/src/compiler/diagnosticMessages.json
@@ -1408,19 +1408,19 @@
         "category": "Error",
         "code": 1443
     },
-    "'{0}' is a type and must be imported with a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.": {
+    "'{0}' is a type and must be imported using a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.": {
         "category": "Error",
         "code": 1444
     },
-    "'{0}' resolves to a type-only declaration and must be imported with a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.": {
+    "'{0}' resolves to a type-only declaration and must be imported using a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.": {
         "category": "Error",
         "code": 1446
     },
-    "'{0}' resolves to a type-only declaration and must be re-exported with a type-only re-export when 'isolatedModules' is enabled.": {
+    "'{0}' resolves to a type-only declaration and must be re-exported using a type-only re-export when 'isolatedModules' is enabled.": {
         "category": "Error",
         "code": 1448
     },
-    "Disable the removal of unused imported identifiers from the JavaScript output.": {
+    "Preserve unused imported values in the JavaScript output that would otherwise be removed.": {
         "category": "Message",
         "code": 1449
     },
@@ -4054,7 +4054,7 @@
         "category": "Error",
         "code": 5094
     },
-    "Option 'preserveValueImports' may be enabled only when 'module' is set to 'es2015' or later.": {
+    "Option 'preserveValueImports' can only be used when 'module' is set to 'es2015' or later.": {
         "category": "Error",
         "code": 5095
     },

diff --git a/src/compiler/program.ts b/src/compiler/program.ts
@@ -3308,7 +3308,7 @@ namespace ts {
             }
 
             if (options.preserveValueImports && getEmitModuleKind(options) < ModuleKind.ES2015) {
-                createOptionValueDiagnostic("importsNotUsedAsValues", Diagnostics.Option_preserveValueImports_may_be_enabled_only_when_module_is_set_to_es2015_or_later);
+                createOptionValueDiagnostic("importsNotUsedAsValues", Diagnostics.Option_preserveValueImports_can_only_be_used_when_module_is_set_to_es2015_or_later);
             }
 
             // If the emit is enabled make sure that every output file is unique and not overwriting any of the input files

diff --git a/src/compiler/transformers/ts.ts b/src/compiler/transformers/ts.ts
@@ -46,7 +46,6 @@ namespace ts {
         const strictNullChecks = getStrictOptionValue(compilerOptions, "strictNullChecks");
         const languageVersion = getEmitScriptTarget(compilerOptions);
         const moduleKind = getEmitModuleKind(compilerOptions);
-        const { importsNotUsedAsValues } = compilerOptions;
 
         // Save the previous transformation hooks.
         const previousOnEmitNode = context.onEmitNode;
@@ -2810,8 +2809,8 @@ namespace ts {
             // Elide the declaration if the import clause was elided.
             const importClause = visitNode(node.importClause, visitImportClause, isImportClause);
             return importClause ||
-                importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
-                importsNotUsedAsValues === ImportsNotUsedAsValues.Error
+                compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve ||
+                compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Error
                 ? factory.updateImportDeclaration(
                     node,
                     /*decorators*/ undefined,
@@ -2966,7 +2965,7 @@ namespace ts {
             if (isExternalModuleImportEqualsDeclaration(node)) {
                 const isReferenced = shouldEmitAliasDeclaration(node);
                 // If the alias is unreferenced but we want to keep the import, replace with 'import "mod"'.
-                if (!isReferenced && importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve) {
+                if (!isReferenced && compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve) {
                     return setOriginalNode(
                         setTextRange(
                             factory.createImportDeclaration(

diff --git a/src/compiler/types.ts b/src/compiler/types.ts
@@ -3104,6 +3104,14 @@ namespace ts {
         | ImportOrExportSpecifier
         ;
 
+    export type TypeOnlyAliasDeclaration =
+        | ImportClause & { readonly isTypeOnly: true, readonly name: Identifier }
+        | ImportEqualsDeclaration & { readonly isTypeOnly: true }
+        | NamespaceImport & { readonly parent: ImportClause & { readonly isTypeOnly: true } }
+        | ImportSpecifier & { readonly parent: NamedImports & { readonly parent: ImportClause & { readonly isTypeOnly: true } } }
+        | ExportSpecifier & { readonly parent: NamedExports & { readonly parent: ExportDeclaration & { readonly isTypeOnly: true } } }
+        ;
+
     /**
      * This is either an `export =` or an `export default` declaration.
      * Unless `isExportEquals` is set, this node was parsed as an `export default`.

diff --git a/src/compiler/utilitiesPublic.ts b/src/compiler/utilitiesPublic.ts
@@ -1126,7 +1126,7 @@ namespace ts {
         return isImportSpecifier(node) || isExportSpecifier(node);
     }
 
-    export function isTypeOnlyImportOrExportDeclaration(node: Node): node is TypeOnlyCompatibleAliasDeclaration {
+    export function isTypeOnlyImportOrExportDeclaration(node: Node): node is TypeOnlyAliasDeclaration {
         switch (node.kind) {
             case SyntaxKind.ImportSpecifier:
             case SyntaxKind.ExportSpecifier:

diff --git a/tests/baselines/reference/noErasingImportedNames_module(module=amd).errors.txt b/tests/baselines/reference/noErasingImportedNames_module(module=amd).errors.txt
diff --git a/tests/baselines/reference/noErasingImportedNames_module(module=commonjs).errors.txt b/tests/baselines/reference/noErasingImportedNames_module(module=commonjs).errors.txt
diff --git a/tests/baselines/reference/noErasingImportedNames_module(module=commonjs).js b/tests/baselines/reference/noErasingImportedNames_module(module=commonjs).js
diff --git a/tests/baselines/reference/noErasingImportedNames_module(module=es2015).js b/tests/baselines/reference/noErasingImportedNames_module(module=es2015).js
diff --git a/tests/baselines/reference/noErasingImportedNames_module(module=system).errors.txt b/tests/baselines/reference/noErasingImportedNames_module(module=system).errors.txt
diff --git a/...edNames(isolatedmodules=false).errors.txt → ...Imports(isolatedmodules=false).errors.txt b/...edNames(isolatedmodules=false).errors.txt → ...Imports(isolatedmodules=false).errors.txt
diff --git a/...ngImportedNames(isolatedmodules=false).js → ...rveValueImports(isolatedmodules=false).js b/...ngImportedNames(isolatedmodules=false).js → ...rveValueImports(isolatedmodules=false).js
@@ -1,4 +1,4 @@
-//// [tests/cases/conformance/externalModules/typeOnly/noErasingImportedNames.ts] ////
+//// [tests/cases/conformance/externalModules/typeOnly/preserveValueImports.ts] ////
 
 //// [a.ts]
 export default {};

diff --git a/...ortedNames(isolatedmodules=false).symbols → ...lueImports(isolatedmodules=false).symbols b/...ortedNames(isolatedmodules=false).symbols → ...lueImports(isolatedmodules=false).symbols
diff --git a/...mportedNames(isolatedmodules=false).types → ...ValueImports(isolatedmodules=false).types b/...mportedNames(isolatedmodules=false).types → ...ValueImports(isolatedmodules=false).types
diff --git a/...tedNames(isolatedmodules=true).errors.txt → ...eImports(isolatedmodules=true).errors.txt b/...tedNames(isolatedmodules=true).errors.txt → ...eImports(isolatedmodules=true).errors.txt
@@ -1,4 +1,4 @@
-tests/cases/conformance/externalModules/typeOnly/b.ts(1,19): error TS1444: 'D' is a type and must be imported with a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.
+tests/cases/conformance/externalModules/typeOnly/b.ts(1,19): error TS1444: 'D' is a type and must be imported using a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.
 tests/cases/conformance/externalModules/typeOnly/d.ts(1,1): error TS1203: Export assignment cannot be used when targeting ECMAScript modules. Consider using 'export default' or another module format instead.
 tests/cases/conformance/externalModules/typeOnly/e.ts(1,1): error TS1202: Import assignment cannot be used when targeting ECMAScript modules. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.
 tests/cases/conformance/externalModules/typeOnly/e.ts(2,1): error TS1202: Import assignment cannot be used when targeting ECMAScript modules. Consider using 'import * as ns from "mod"', 'import {a} from "mod"', 'import d from "mod"', or another module format instead.
@@ -13,7 +13,7 @@ tests/cases/conformance/externalModules/typeOnly/e.ts(2,1): error TS1202: Import
 ==== tests/cases/conformance/externalModules/typeOnly/b.ts (1 errors) ====
     import a, { b, c, D } from "./a";
                       ~
-!!! error TS1444: 'D' is a type and must be imported with a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.
+!!! error TS1444: 'D' is a type and must be imported using a type-only import when 'preserveValueImports' and 'isolatedModules' are both enabled.
 
 ==== tests/cases/conformance/externalModules/typeOnly/c.ts (0 errors) ====
     import * as a from "./a";

diff --git a/...ingImportedNames(isolatedmodules=true).js → ...erveValueImports(isolatedmodules=true).js b/...ingImportedNames(isolatedmodules=true).js → ...erveValueImports(isolatedmodules=true).js
@@ -1,4 +1,4 @@
-//// [tests/cases/conformance/externalModules/typeOnly/noErasingImportedNames.ts] ////
+//// [tests/cases/conformance/externalModules/typeOnly/preserveValueImports.ts] ////
 
 //// [a.ts]
 export default {};

diff --git a/...portedNames(isolatedmodules=true).symbols → ...alueImports(isolatedmodules=true).symbols b/...portedNames(isolatedmodules=true).symbols → ...alueImports(isolatedmodules=true).symbols
diff --git a/...ImportedNames(isolatedmodules=true).types → ...eValueImports(isolatedmodules=true).types b/...ImportedNames(isolatedmodules=true).types → ...eValueImports(isolatedmodules=true).types
diff --git a/...rtedNames_errors(isolatedmodules=true).js → ...eImports_errors(isolatedmodules=false).js b/...rtedNames_errors(isolatedmodules=true).js → ...eImports_errors(isolatedmodules=false).js
@@ -1,4 +1,4 @@
-//// [tests/cases/conformance/externalModules/typeOnly/noErasingImportedNames_errors.ts] ////
+//// [tests/cases/conformance/externalModules/typeOnly/preserveValueImports_errors.ts] ////
 
 //// [a.ts]
 export type A = {};

diff --git a/...mes_errors(isolatedmodules=false).symbols → ...rts_errors(isolatedmodules=false).symbols b/...mes_errors(isolatedmodules=false).symbols → ...rts_errors(isolatedmodules=false).symbols
diff --git a/...Names_errors(isolatedmodules=false).types → ...ports_errors(isolatedmodules=false).types b/...Names_errors(isolatedmodules=false).types → ...ports_errors(isolatedmodules=false).types