Skip to content

Commit

Permalink
Add --preserveValueImports (#44619)
Browse files Browse the repository at this point in the history 
* Add compiler option

* Require es2015+

* Do not elide any imports or exports in preserve-exact

* Add errors for writing imports/exports that reference elided names

* Improve diagnostics wording

* Update API baselines

* Redo as noEraslingImportedNames

* Update option category

* Update baselines

* Lint

* Fix up transformer comments

* Fix errors from merge

* Update other error code baseline

* Rename to "preserveValueImports"

* Clean up, reword diagnostics

* Update API baselines

* Update other baseline affected by error message reword

* Update tsconfig baselines

* Add debug assertion instead of !
  • Loading branch information
@andrewbranch
andrewbranch authored Sep 8, 2021
1 parent b9eeb74 commit 8610ff5
Show file tree
Hide file tree
Showing 55 changed files with 1,367 additions and 40 deletions.
77 changes: 62 additions & 15 deletions src/compiler/checker.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2198,17 +2198,25 @@ namespace ts {
const message = isExport
? Diagnostics._0_cannot_be_used_as_a_value_because_it_was_exported_using_export_type
: Diagnostics._0_cannot_be_used_as_a_value_because_it_was_imported_using_import_type;
const relatedMessage = isExport
? Diagnostics._0_was_exported_here
: Diagnostics._0_was_imported_here;
const unescapedName = unescapeLeadingUnderscores(name);
addRelatedInfo(
addTypeOnlyDeclarationRelatedInfo(
error(useSite, message, unescapedName),
createDiagnosticForNode(typeOnlyDeclaration, relatedMessage, unescapedName));
typeOnlyDeclaration,
unescapedName);
}
}
}

function addTypeOnlyDeclarationRelatedInfo(diagnostic: Diagnostic, typeOnlyDeclaration: TypeOnlyCompatibleAliasDeclaration | undefined, unescapedName: string) {
if (!typeOnlyDeclaration) return diagnostic;
return addRelatedInfo(
diagnostic,
createDiagnosticForNode(
typeOnlyDeclaration,
typeOnlyDeclarationIsExport(typeOnlyDeclaration) ? Diagnostics._0_was_exported_here : Diagnostics._0_was_imported_here,
unescapedName));
}

function getIsDeferredContext(location: Node, lastLocation: Node | undefined): boolean {
if (location.kind !== SyntaxKind.ArrowFunction && location.kind !== SyntaxKind.FunctionExpression) {
// initializers in instance property declaration of class like entities are executed in constructor and thus deferred
Expand Down Expand Up @@ -38813,13 +38821,49 @@ namespace ts {
error(node, message, symbolToString(symbol));
}

// Don't allow to re-export something with no value side when `--isolatedModules` is set.
if (compilerOptions.isolatedModules
&& node.kind === SyntaxKind.ExportSpecifier
&& !node.parent.parent.isTypeOnly
&& !(target.flags & SymbolFlags.Value)
&& !isTypeOnlyImportOrExportDeclaration(node)
&& !(node.flags & NodeFlags.Ambient)) {
error(node, Diagnostics.Re_exporting_a_type_when_the_isolatedModules_flag_is_provided_requires_using_export_type);
const typeOnlyAlias = getTypeOnlyAliasDeclaration(symbol);
const isType = !(target.flags & SymbolFlags.Value);
if (isType || typeOnlyAlias) {
switch (node.kind) {
case SyntaxKind.ImportClause:
case SyntaxKind.ImportSpecifier:
case SyntaxKind.ImportEqualsDeclaration: {
if (compilerOptions.preserveValueImports) {
Debug.assertIsDefined(node.name, "An ImportClause with a symbol should have a name");
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;
}
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;
}
}
}
}
}

if (isImportSpecifier(node) && target.declarations?.every(d => !!(getCombinedNodeFlags(d) & NodeFlags.Deprecated))) {
Expand Down Expand Up @@ -40600,13 +40644,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 && (
Expand All @@ -40615,7 +40659,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;
Expand All @@ -40632,7 +40676,10 @@ namespace ts {
return isValue && node.moduleReference && !nodeIsMissing(node.moduleReference);
}

function isAliasResolvedToValue(symbol: Symbol): boolean {
function isAliasResolvedToValue(symbol: Symbol | undefined): boolean {
if (!symbol) {
return false;
}
const target = resolveAlias(symbol);
if (target === unknownSymbol) {
return true;
Expand Down
10 changes: 9 additions & 1 deletion src/compiler/commandLineParser.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -571,7 +571,7 @@ namespace ts {
type: new Map(getEntries({
remove: ImportsNotUsedAsValues.Remove,
preserve: ImportsNotUsedAsValues.Preserve,
error: ImportsNotUsedAsValues.Error
error: ImportsNotUsedAsValues.Error,
})),
affectsEmit: true,
affectsSemanticDiagnostics: true,
Expand Down Expand Up @@ -1169,6 +1169,14 @@ namespace ts {
description: Diagnostics.Emit_ECMAScript_standard_compliant_class_fields,
defaultValueDescription: Diagnostics.true_for_ES2022_and_above_including_ESNext
},
{
name: "preserveValueImports",
type: "boolean",
affectsEmit: true,
category: Diagnostics.Emit,
description: Diagnostics.Preserve_unused_imported_values_in_the_JavaScript_output_that_would_otherwise_be_removed,
},

{
name: "keyofStringsOnly",
type: "boolean",
Expand Down
20 changes: 20 additions & 0 deletions src/compiler/diagnosticMessages.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,22 @@
"category": "Error",
"code": 1443
},
"'{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 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 using a type-only re-export when 'isolatedModules' is enabled.": {
"category": "Error",
"code": 1448
},
"Preserve unused imported values in the JavaScript output that would otherwise be removed.": {
"category": "Message",
"code": 1449
},

"The types of '{0}' are incompatible between these types.": {
"category": "Error",
Expand Down Expand Up @@ -3910,6 +3926,10 @@
"category": "Error",
"code": 5094
},
"Option 'preserveValueImports' can only be used when 'module' is set to 'es2015' or later.": {
"category": "Error",
"code": 5095
},

"Generates a sourcemap for each corresponding '.d.ts' file.": {
"category": "Message",
Expand Down
10 changes: 7 additions & 3 deletions src/compiler/program.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3305,6 +3305,10 @@ namespace ts {
}
}

if (options.preserveValueImports && getEmitModuleKind(options) < ModuleKind.ES2015) {
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
if (!options.noEmit && !options.suppressOutputPathCheck) {
const emitHost = getEmitHost();
Expand Down Expand Up @@ -3575,7 +3579,7 @@ namespace ts {
createDiagnosticForOption(/*onKey*/ true, option1, option2, message, option1, option2, option3);
}

function createOptionValueDiagnostic(option1: string, message: DiagnosticMessage, arg0: string) {
function createOptionValueDiagnostic(option1: string, message: DiagnosticMessage, arg0?: string) {
createDiagnosticForOption(/*onKey*/ false, option1, /*option2*/ undefined, message, arg0);
}

Expand All @@ -3590,7 +3594,7 @@ namespace ts {
}
}

function createDiagnosticForOption(onKey: boolean, option1: string, option2: string | undefined, message: DiagnosticMessage, arg0: string | number, arg1?: string | number, arg2?: string | number) {
function createDiagnosticForOption(onKey: boolean, option1: string, option2: string | undefined, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number) {
const compilerOptionsObjectLiteralSyntax = getCompilerOptionsObjectLiteralSyntax();
const needCompilerDiagnostic = !compilerOptionsObjectLiteralSyntax ||
!createOptionDiagnosticInObjectLiteralSyntax(compilerOptionsObjectLiteralSyntax, onKey, option1, option2, message, arg0, arg1, arg2);
Expand All @@ -3616,7 +3620,7 @@ namespace ts {
return _compilerOptionsObjectLiteralSyntax || undefined;
}

function createOptionDiagnosticInObjectLiteralSyntax(objectLiteral: ObjectLiteralExpression, onKey: boolean, key1: string, key2: string | undefined, message: DiagnosticMessage, arg0: string | number, arg1?: string | number, arg2?: string | number): boolean {
function createOptionDiagnosticInObjectLiteralSyntax(objectLiteral: ObjectLiteralExpression, onKey: boolean, key1: string, key2: string | undefined, message: DiagnosticMessage, arg0?: string | number, arg1?: string | number, arg2?: string | number): boolean {
const props = getPropertyAssignment(objectLiteral, key1, key2);
for (const prop of props) {
programDiagnostics.add(createDiagnosticForNodeInSourceFile(options.configFile!, onKey ? prop.name : prop.initializer, message, arg0, arg1, arg2));
Expand Down
32 changes: 17 additions & 15 deletions src/compiler/transformers/ts.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2791,7 +2791,7 @@ namespace ts {
}

/**
* Visits an import declaration, eliding it if it is not referenced and `importsNotUsedAsValues` is not 'preserve'.
* Visits an import declaration, eliding it if it is type-only or if it has an import clause that may be elided.
*
* @param node The import declaration node.
*/
Expand Down Expand Up @@ -2821,29 +2821,27 @@ namespace ts {
}

/**
* Visits an import clause, eliding it if it is not referenced.
* Visits an import clause, eliding it if its `name` and `namedBindings` may both be elided.
*
* @param node The import clause node.
*/
function visitImportClause(node: ImportClause): VisitResult<ImportClause> {
if (node.isTypeOnly) {
return undefined;
}
Debug.assert(!node.isTypeOnly);
// Elide the import clause if we elide both its name and its named bindings.
const name = resolver.isReferencedAliasDeclaration(node) ? node.name : undefined;
const name = shouldEmitAliasDeclaration(node) ? node.name : undefined;
const namedBindings = visitNode(node.namedBindings, visitNamedImportBindings, isNamedImportBindings);
return (name || namedBindings) ? factory.updateImportClause(node, /*isTypeOnly*/ false, name, namedBindings) : undefined;
}

/**
* Visits named import bindings, eliding it if it is not referenced.
* Visits named import bindings, eliding them if their targets, their references, and the compilation settings allow.
*
* @param node The named import bindings node.
*/
function visitNamedImportBindings(node: NamedImportBindings): VisitResult<NamedImportBindings> {
if (node.kind === SyntaxKind.NamespaceImport) {
// Elide a namespace import if it is not referenced.
return resolver.isReferencedAliasDeclaration(node) ? node : undefined;
return shouldEmitAliasDeclaration(node) ? node : undefined;
}
else {
// Elide named imports if all of its import specifiers are elided.
Expand All @@ -2853,13 +2851,12 @@ namespace ts {
}

/**
* Visits an import specifier, eliding it if it is not referenced.
* Visits an import specifier, eliding it if its target, its references, and the compilation settings allow.
*
* @param node The import specifier node.
*/
function visitImportSpecifier(node: ImportSpecifier): VisitResult<ImportSpecifier> {
// Elide an import specifier if it is not referenced.
return resolver.isReferencedAliasDeclaration(node) ? node : undefined;
return shouldEmitAliasDeclaration(node) ? node : undefined;
}

/**
Expand All @@ -2876,8 +2873,7 @@ namespace ts {
}

/**
* Visits an export declaration, eliding it if it does not contain a clause that resolves
* to a value.
* Visits an export declaration, eliding it if it does not contain a clause that resolves to a value.
*
* @param node The export declaration node.
*/
Expand Down Expand Up @@ -2950,7 +2946,7 @@ namespace ts {
// preserve old compiler's behavior: emit 'var' for import declaration (even if we do not consider them referenced) when
// - current file is not external module
// - import declaration is top level and target is value imported by entity name
return resolver.isReferencedAliasDeclaration(node)
return shouldEmitAliasDeclaration(node)
|| (!isExternalModule(currentSourceFile)
&& resolver.isTopLevelValueImportEqualsWithEntityName(node));
}
Expand All @@ -2967,7 +2963,7 @@ namespace ts {
}

if (isExternalModuleImportEqualsDeclaration(node)) {
const isReferenced = resolver.isReferencedAliasDeclaration(node);
const isReferenced = shouldEmitAliasDeclaration(node);
// If the alias is unreferenced but we want to keep the import, replace with 'import "mod"'.
if (!isReferenced && compilerOptions.importsNotUsedAsValues === ImportsNotUsedAsValues.Preserve) {
return setOriginalNode(
Expand Down Expand Up @@ -3358,5 +3354,11 @@ namespace ts {

return isPropertyAccessExpression(node) || isElementAccessExpression(node) ? resolver.getConstantValue(node) : undefined;
}

function shouldEmitAliasDeclaration(node: Node): boolean {
return compilerOptions.preserveValueImports
? resolver.isValueAliasDeclaration(node)
: resolver.isReferencedAliasDeclaration(node);
}
}
}
11 changes: 10 additions & 1 deletion src/compiler/types.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3109,6 +3109,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`.
Expand Down Expand Up @@ -6057,6 +6065,7 @@ namespace ts {
preserveConstEnums?: boolean;
noImplicitOverride?: boolean;
preserveSymlinks?: boolean;
preserveValueImports?: boolean;
/* @internal */ preserveWatchOutput?: boolean;
project?: string;
/* @internal */ pretty?: boolean;
Expand Down Expand Up @@ -6150,7 +6159,7 @@ namespace ts {
export const enum ImportsNotUsedAsValues {
Remove,
Preserve,
Error
Error,
}

export const enum NewLineKind {
Expand Down
2 changes: 1 addition & 1 deletion src/compiler/utilitiesPublic.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
Loading

0 comments on commit 8610ff5

Please sign in to comment.