From 0b6d6c712678d85752a515d0964881578270ada1 Mon Sep 17 00:00:00 2001 From: theseanl Date: Sun, 22 Nov 2020 10:14:15 +0900 Subject: [PATCH] Fix externs for export defaults in declaration files Export defaults are the same ts.ExportAssignment nodes as export equals statement, which tsickle already supports. Adopted existing code for export equals to support externs generation for export defaults, which tsickle previously skipped. In addition, both export equals assignments and export defaults can export not mere identifiers but also qualified names (property accesses). Tsickle had a test for it but it has never worked. This fixes the issue so that proper externs are generated for qualified names too. In doing so, a problem arised is that the previous check for `isInGlobalAugmentation` stopped working when one default exports nested namespaces in declared in global augmentation. Thus, the check for `isInGisInGlobalAugmentation` was changed to search arbitrary depth, which caused another issue with underscore-style exports, which was resolved subsequently. --- src/externs.ts | 70 +++++++++++++------ src/transformer_util.ts | 20 ++++++ .../declare_export_global_nested.d.ts | 5 +- .../declare_export_global/dtsdiagnostics.txt | 1 - test_files/declare_export_global/externs.js | 2 +- test_files/declare_import/externs.js | 2 + .../export_default_dts/export_default.d.ts | 12 ++++ test_files/export_default_dts/externs.js | 19 +++++ 8 files changed, 105 insertions(+), 26 deletions(-) delete mode 100644 test_files/declare_export_global/dtsdiagnostics.txt create mode 100755 test_files/export_default_dts/export_default.d.ts create mode 100755 test_files/export_default_dts/externs.js diff --git a/src/externs.ts b/src/externs.ts index 431c0a2e3..1e27ca39a 100644 --- a/src/externs.ts +++ b/src/externs.ts @@ -74,7 +74,7 @@ import * as jsdoc from './jsdoc'; import {escapeForComment, maybeAddHeritageClauses, maybeAddTemplateClause} from './jsdoc_transformer'; import {ModuleTypeTranslator} from './module_type_translator'; import * as path from './path'; -import {getEntityNameText, getIdentifierText, hasModifierFlag, isAmbient, isDtsFileName, reportDiagnostic} from './transformer_util'; +import {getEntityNameText, getIdentifierText, hasModifierFlag, isAmbient, isEntityNameExpression, isDtsFileName, reportDiagnostic, getEntityNameExpressionText} from './transformer_util'; import {isValidClosurePropertyName} from './type_translator'; /** @@ -139,14 +139,17 @@ export function getGeneratedExterns( } /** - * isInGlobalAugmentation returns true if declaration is the immediate child of a 'declare global' + * isInGlobalAugmentation returns true if declaration is the descendent of a 'declare global' * block. */ function isInGlobalAugmentation(declaration: ts.Declaration): boolean { // declare global { ... } creates a ModuleDeclaration containing a ModuleBlock containing the // declaration, with the ModuleDeclaration having the GlobalAugmentation flag set. - if (!declaration.parent || !declaration.parent.parent) return false; - return (declaration.parent.parent.flags & ts.NodeFlags.GlobalAugmentation) !== 0; + let node: ts.Node = declaration; + while (node = node.parent) { + if ((node.flags & ts.NodeFlags.GlobalAugmentation) !== 0) return true; + } + return false; } /** @@ -217,9 +220,11 @@ export function generateExterns( * If `someName` is `declare global { namespace someName {...} }`, tsickle must not qualify access * to it with the mangled module namespace as it is emitted in the global namespace. Similarly, if * the symbol is declared in a non-module context, it must not be mangled. + * Typescript parses `someName` as an EntityNameExpression for `export`, and as `EntityName` for + * `import`. */ - function qualifiedNameToMangledIdentifier(name: ts.Identifier|ts.QualifiedName) { - const entityName = getEntityNameText(name); + function qualifiedNameToMangledIdentifier( + name: ts.EntityName|ts.EntityNameExpression, entityName: string) { let symbol = typeChecker.getSymbolAtLocation(name); if (symbol) { // If this is an aliased name (e.g. from an import), use the alias to refer to it. @@ -228,7 +233,9 @@ export function generateExterns( } const alias = mtt.symbolsToAliasedNames.get(symbol); if (alias) return alias; - const isGlobalSymbol = symbol && symbol.declarations && symbol.declarations.some(d => { + // If at least one declaration is local to a module, we can always use that and there won't be + // any name clash in generating externs for `export as namespace ${output}`. See test_files/underscore. + const isGlobalSymbol = symbol && symbol.declarations && symbol.declarations.every(d => { if (isInGlobalAugmentation(d)) return true; // If the declaration's source file is not a module, it must be global. // If it is a module, the identifier must be local to this file, or handled above via the @@ -240,6 +247,22 @@ export function generateExterns( return rootNamespace + '.' + entityName; } + function extractExportedNamespaceFromExportAssignment(exportAssignment:ts.ExportAssignment) { + const {expression} = exportAssignment; + if (isEntityNameExpression(expression)) { + // E.g. export = someName, export default someName; + // If someName is "declare global { namespace someName {...} }", tsickle must not qualify + // access to it with module namespace as it is emitted in the global namespace. + return qualifiedNameToMangledIdentifier(expression, getEntityNameExpressionText(expression)); + } else { + reportDiagnostic( + diagnostics, expression, + `export =/default expression must be a qualified name, got ${ + ts.SyntaxKind[exportAssignment.expression.kind]}.`); + return rootNamespace; + } + } + if (output && isExternalModule) { // If tsickle generated any externs and this is an external module, prepend the namespace // declaration for it. @@ -247,18 +270,7 @@ export function generateExterns( let exportedNamespace = rootNamespace; if (exportAssignment && hasExportEquals) { - if (ts.isIdentifier(exportAssignment.expression) || - ts.isQualifiedName(exportAssignment.expression)) { - // E.g. export = someName; - // If someName is "declare global { namespace someName {...} }", tsickle must not qualify - // access to it with module namespace as it is emitted in the global namespace. - exportedNamespace = qualifiedNameToMangledIdentifier(exportAssignment.expression); - } else { - reportDiagnostic( - diagnostics, exportAssignment.expression, - `export = expression must be a qualified name, got ${ - ts.SyntaxKind[exportAssignment.expression.kind]}.`); - } + exportedNamespace = extractExportedNamespaceFromExportAssignment(exportAssignment); // Assign the actually exported namespace object (which lives somewhere under rootNamespace) // into the module's namespace. emit(`/**\n * export = ${exportAssignment.expression.getText()}\n * @const\n */\n`); @@ -537,6 +549,18 @@ export function generateExterns( } } + function writeExportAssignment( + exportAssignment: ts.ExportAssignment, namespace:ReadonlyArray) { + // export = ... is handled at the file level. + if (exportAssignment.isExportEquals) return; + // export default + emit('/** @const */\n'); + writeVariableStatement( + "default", namespace, + extractExportedNamespaceFromExportAssignment(exportAssignment) + ); + } + /** * Adds aliases for the symbols imported in the given declaration, so that their types get * printed as the fully qualified name, and not just as a reference to the local import alias. @@ -766,7 +790,8 @@ export function generateExterns( addImportAliases(importEquals); break; } - const qn = qualifiedNameToMangledIdentifier(importEquals.moduleReference); + const qn = qualifiedNameToMangledIdentifier(importEquals.moduleReference, + getEntityNameText(importEquals.moduleReference)); // @const so that Closure Compiler understands this is an alias. emit('/** @const */\n'); writeVariableStatement(localName, namespace, qn); @@ -805,9 +830,12 @@ export function generateExterns( addImportAliases(node as ts.ImportDeclaration); break; case ts.SyntaxKind.NamespaceExportDeclaration: - case ts.SyntaxKind.ExportAssignment: // Handled on the file level. break; + case ts.SyntaxKind.ExportAssignment: + const exportAssignment = node as ts.ExportAssignment; + writeExportAssignment(exportAssignment, namespace); + break; case ts.SyntaxKind.ExportDeclaration: const exportDeclaration = node as ts.ExportDeclaration; writeExportDeclaration(exportDeclaration, namespace); diff --git a/src/transformer_util.ts b/src/transformer_util.ts index f2c749837..de8c79b7a 100644 --- a/src/transformer_util.ts +++ b/src/transformer_util.ts @@ -60,6 +60,26 @@ export function getEntityNameText(name: ts.EntityName): string { return getEntityNameText(name.left) + '.' + getIdentifierText(name.right); } +export function getEntityNameExpressionText(name:ts.EntityNameExpression) :string{ + if (ts.isIdentifier(name)) { + return getIdentifierText(name); + } + return getEntityNameExpressionText(name.expression) + '.' + getIdentifierText(name.name); +} + +/** + * Returns true if node is ts.EntityNameExpression. This internal API of typescript is replicated here + * in order to be used in externs.ts. + */ +export function isEntityNameExpression(node: ts.Node): node is ts.EntityNameExpression { + return node.kind === ts.SyntaxKind.Identifier || isPropertyAccessEntityNameExpression(node); +} + +function isPropertyAccessEntityNameExpression(node: ts.Node): node is ts.PropertyAccessEntityNameExpression { + return ts.isPropertyAccessExpression(node) && ts.isIdentifier(node.name) && + isEntityNameExpression(node.expression); +} + /** * Converts an escaped TypeScript name into the original source name. */ diff --git a/test_files/declare_export_global/declare_export_global_nested.d.ts b/test_files/declare_export_global/declare_export_global_nested.d.ts index 775c8a7db..f5e07b69a 100644 --- a/test_files/declare_export_global/declare_export_global_nested.d.ts +++ b/test_files/declare_export_global/declare_export_global_nested.d.ts @@ -1,6 +1,6 @@ /** * @fileoverview Tests declaring a namespace in a module .d.ts file, as a globally available symbol - * using a `declare global` block, where the . + * using a `declare global` block, where the symbol is nested in several namespaces. */ declare global { @@ -12,6 +12,5 @@ declare global { } // tsickle must emit the `globalParentNamespace` reference below as a global name, not the mangled -// module scoped name. This is currently unsupported, tsickle reports an error for this pattern (see -// dtsdiagnostics.txt). +// module scoped name. export = globalParentNamespace.globalNestedNamespace; diff --git a/test_files/declare_export_global/dtsdiagnostics.txt b/test_files/declare_export_global/dtsdiagnostics.txt deleted file mode 100644 index 549e4468e..000000000 --- a/test_files/declare_export_global/dtsdiagnostics.txt +++ /dev/null @@ -1 +0,0 @@ -test_files/declare_export_global/declare_export_global_nested.d.ts(17,10): error TS0: export = expression must be a qualified name, got PropertyAccessExpression. diff --git a/test_files/declare_export_global/externs.js b/test_files/declare_export_global/externs.js index cf91c63f9..9caa8b21f 100644 --- a/test_files/declare_export_global/externs.js +++ b/test_files/declare_export_global/externs.js @@ -28,4 +28,4 @@ globalParentNamespace.globalNestedNamespace.x; * export = globalParentNamespace.globalNestedNamespace * @const */ -var test_files$declare_export_global$declare_export_global_nested = test_files$declare_export_global$declare_export_global_nested_; +var test_files$declare_export_global$declare_export_global_nested = globalParentNamespace.globalNestedNamespace; diff --git a/test_files/declare_import/externs.js b/test_files/declare_import/externs.js index 731aa8def..43a5cf19b 100644 --- a/test_files/declare_import/externs.js +++ b/test_files/declare_import/externs.js @@ -61,3 +61,5 @@ var goog_imported$closure$Class = {}; * @struct */ goog_imported$closure$Class.ClosureClazz = function() {}; +/** @const */ +goog_imported$closure$Class.default = ClosureClazz; diff --git a/test_files/export_default_dts/export_default.d.ts b/test_files/export_default_dts/export_default.d.ts new file mode 100755 index 000000000..7e721ad7c --- /dev/null +++ b/test_files/export_default_dts/export_default.d.ts @@ -0,0 +1,12 @@ +/** + * @fileoverview Tests that default exports from .d.ts are defined on the root namespace with + * "default" property. Expressions in default export in d.ts are limited to identifiers and + * qualified names, and we test here that qualified name works. + */ +declare namespace innerNamespace { + class DefaultExportedClass{ + property:string + } +} + +export default innerNamespace.DefaultExportedClass; diff --git a/test_files/export_default_dts/externs.js b/test_files/export_default_dts/externs.js new file mode 100755 index 000000000..7fc468215 --- /dev/null +++ b/test_files/export_default_dts/externs.js @@ -0,0 +1,19 @@ +/** + * @externs + * @suppress {duplicate,checkTypes,missingOverride} + */ +// NOTE: generated by tsickle, do not edit. +// externs from test_files/export_default_dts/export_default.d.ts: +/** @const */ +var test_files$export_default_dts$export_default = {}; +/** @const */ +test_files$export_default_dts$export_default.innerNamespace = {}; +/** + * @constructor + * @struct + */ +test_files$export_default_dts$export_default.innerNamespace.DefaultExportedClass = function() {}; +/** @type {string} */ +test_files$export_default_dts$export_default.innerNamespace.DefaultExportedClass.prototype.property; +/** @const */ +test_files$export_default_dts$export_default.default = test_files$export_default_dts$export_default.innerNamespace.DefaultExportedClass;