Skip to content

Commit

Permalink
More fixes for enum type translation
Browse files Browse the repository at this point in the history
In TS5.0, enum types are represented as a union of enum literals.
Each literal in the enum is a ts.TypeFlags.Enum. Resolve enum members
to their enum type name before translating the type.

FUTURE_COPYBARA_INTEGRATE_REVIEW=TGP
PiperOrigin-RevId: 543829593
  • Loading branch information
mhausner authored and copybara-github committed Jun 27, 2023
1 parent 7c15472 commit 8e25305
Show file tree
Hide file tree
Showing 12 changed files with 60 additions and 18 deletions.
2 changes: 1 addition & 1 deletion demo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
"dependencies": {
"minimist": "^1.2.3",
"tsickle": "file:../",
"typescript": "4.9.4"
"typescript": "5.0.4"
},
"devDependencies": {
"@types/minimist": "1.2.0",
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
"out/src/*"
],
"peerDependencies": {
"typescript": "~4.9.4"
"typescript": "~5.0.4"
},
"devDependencies": {
"@types/diff-match-patch": "^1.0.32",
Expand All @@ -28,7 +28,7 @@
"source-map-support": "^0.5.19",
"tslib": "^2.2.0",
"tslint": "^6.1.3",
"typescript": "4.9.4"
"typescript": "5.0.4"
},
"scripts": {
"build": "tsc",
Expand Down
3 changes: 0 additions & 3 deletions src/closure_externs.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,9 +52,6 @@ var NodeListOf;
*/
var RegExpExecArray;

/** @typedef {!Map} */
var ReadonlyMap;

/** @typedef {!Set} */
var ReadonlySet;

Expand Down
10 changes: 10 additions & 0 deletions src/externs.ts
Original file line number Diff line number Diff line change
Expand Up @@ -456,6 +456,16 @@ export function generateExterns(
reportDiagnostic(diagnostics, decl, 'anonymous type in externs');
return;
}

// gbigint, as defined in
// google3/third_party/java_src/clutz/src/resources/closure.lib.d.ts, is
// defined separately in TypeScript and JavaScript.
if (name.escapedText === 'gbigint'
// Just the terminal filename so we can test this.
&& decl.getSourceFile().fileName.endsWith('closure.lib.d.ts')) {
return;
}

const typeName = namespace.concat([name.getText()]).join('.');
if (PREDECLARED_CLOSURE_EXTERNS_LIST.indexOf(typeName) >= 0) return;

Expand Down
1 change: 1 addition & 0 deletions src/jsdoc.ts
Original file line number Diff line number Diff line change
Expand Up @@ -126,6 +126,7 @@ const CLOSURE_ALLOWED_JSDOC_TAGS_OUTPUT = new Set([
'requires',
'return',
'returns',
'sassGeneratedCssTs',
'see',
'struct',
'suppress',
Expand Down
6 changes: 6 additions & 0 deletions src/ts_migration_exports_shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,12 @@ export function createTsMigrationExportsShimTransformerFactory(
src, srcIds, typeChecker, host, manifest, tsickleDiagnostics);
const tsmesFile = srcIds.google3PathWithoutExtension() + '.tsmes.js';
const dtsFile = srcIds.google3PathWithoutExtension() + '.tsmes.d.ts';
if (!host.generateTsMigrationExportsShim) {
// we need to create the Generator to make sure there aren't any shim
// related function calls if generateTsMigrationExportsShim isn't true,
// but we don't want to actually write any files, so return
return src;
}
if (!generator.foundMigrationExportsShim()) {
// If there is no export shims calls, we still need to generate empty
// files, so that we always produce a predictable set of files.
Expand Down
23 changes: 23 additions & 0 deletions src/type_translator.ts
Original file line number Diff line number Diff line change
Expand Up @@ -511,6 +511,13 @@ export class TypeTranslator {
this.warn(`EnumType without a symbol`);
return '?';
}
// In TS5.0, enum types are represented as a union of enum literals.
// Each literal in the enum is a ts.TypeFlags.Enum. If the symbol is
// marked as being an enum member, translate it as the parent type (the
// name of the enum type).
if (type.symbol.flags & ts.SymbolFlags.EnumMember) {
return this.translateEnumLiteral(type);
}
return this.symbolToString(type.symbol) || '?';
case ts.TypeFlags.ESSymbol:
case ts.TypeFlags.UniqueESSymbol:
Expand Down Expand Up @@ -592,6 +599,13 @@ export class TypeTranslator {
}
return innerSymbol ?? '?';
}
// gbigint, as defined in
// google3/third_party/java_src/clutz/src/resources/closure.lib.d.ts, is
// defined separately in TypeScript and JavaScript.
// In JS, gbigint is treated as an object so needs !.
if (type.aliasSymbol?.escapedName === 'gbigint') {
return '!gbigint';
}
this.warn(`unhandled type flags: ${ts.TypeFlags[type.flags]}`);
return '?';
case ts.TypeFlags.Index:
Expand All @@ -609,6 +623,15 @@ export class TypeTranslator {
// Note also that in a more complex union, e.g. boolean|number, then it's a union of three
// things (true|false|number) and ts.TypeFlags.Boolean doesn't show up at all.
if (type.flags & ts.TypeFlags.Union) {
if (type.flags === (ts.TypeFlags.EnumLiteral | ts.TypeFlags.Union) &&
type.symbol) {
// TS5.0 started to treat number enums as a union of its values. We
// want the name of the enum type if possible, not the union of the
// values.
const name = this.symbolToString(type.symbol);
return name ? '!' + name :
this.translateUnion(type as ts.UnionType);
}
return this.translateUnion(type as ts.UnionType);
}

Expand Down
10 changes: 4 additions & 6 deletions test/googmodule_test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -518,7 +518,7 @@ describe('convertCommonJsToGoogModule', () => {
goog.module('project.file');
var module = module || { id: 'project/file.ts' };
const tslib_1 = goog.require('tslib');
(() => tslib_1.__awaiter(this, void 0, void 0, function* () {
(() => tslib_1.__awaiter(void 0, void 0, void 0, function* () {
const starImport = yield goog.requireDynamic('project.relpath');
}))();
`).split(/\n/g));
Expand All @@ -539,9 +539,8 @@ describe('convertCommonJsToGoogModule', () => {
expect(beforeLines).toEqual(outdent(`
goog.module('project.file');
var module = module || { id: 'project/file.ts' };
var _this = this;
var tslib_1 = goog.require('tslib');
(function () { return tslib_1.__awaiter(_this, void 0, void 0, function () {
(function () { return tslib_1.__awaiter(void 0, void 0, void 0, function () {
var starImport;
return tslib_1.__generator(this, function (_a) {
switch (_a.label) {
Expand Down Expand Up @@ -571,7 +570,7 @@ describe('convertCommonJsToGoogModule', () => {
goog.module('project.file');
var module = module || { id: 'project/file.ts' };
const tslib_1 = goog.require('tslib');
(() => tslib_1.__awaiter(this, void 0, void 0, function* () {
(() => tslib_1.__awaiter(void 0, void 0, void 0, function* () {
const { Foo } = yield goog.requireDynamic('project.relpath');
}))();
`).split(/\n/g));
Expand All @@ -592,9 +591,8 @@ describe('convertCommonJsToGoogModule', () => {
expect(beforeLines).toEqual(outdent(`
goog.module('project.file');
var module = module || { id: 'project/file.ts' };
var _this = this;
var tslib_1 = goog.require('tslib');
(function () { return tslib_1.__awaiter(_this, void 0, void 0, function () {
(function () { return tslib_1.__awaiter(void 0, void 0, void 0, function () {
var Foo;
return tslib_1.__generator(this, function (_a) {
switch (_a.label) {
Expand Down
9 changes: 8 additions & 1 deletion test/test_support.ts
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,9 @@ export const baseCompilerOptions: ts.CompilerOptions = {
experimentalDecorators: true,
module: ts.ModuleKind.CommonJS,
strictNullChecks: true,
noImplicitUseStrict: true,
// TODO: b/285332972 - noImplicitUseStrict is deprecated. Investigate failing
// async_functions test.
ignoreDeprecations: '5.0',
allowJs: false,
importHelpers: true,
noEmitHelpers: true,
Expand Down Expand Up @@ -181,6 +183,11 @@ export function createSourceCachingHost(
return originalFileExists.call(host, fileName);
};

// The default CompilerHost created by the compiler resolves symlinks.
// We don't want to do that here. see also
// https://github.com/Microsoft/TypeScript/pull/12020.
host.realpath = (s: string): string => s;

return host;
}

Expand Down
6 changes: 3 additions & 3 deletions test_files/async_functions/async_functions.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ const asyncTopLevelArrowFunction = (/**
* @param {string} param
* @return {!Promise<number>}
*/
(param) => tslib_1.__awaiter(this, void 0, void 0, function* () {
(param) => tslib_1.__awaiter(void 0, void 0, void 0, function* () {
/** @type {number} */
const s = yield 3;
return s;
Expand Down Expand Up @@ -122,12 +122,12 @@ if (false) {
const asyncArrowFn = (/**
* @return {!Promise<void>}
*/
() => tslib_1.__awaiter(this, void 0, void 0, function* () { }));
() => tslib_1.__awaiter(void 0, void 0, void 0, function* () { }));
/** @type {function(): !Promise<void>} */
exports.exportedAsyncArrowFn = (/**
* @return {!Promise<void>}
*/
() => tslib_1.__awaiter(this, void 0, void 0, function* () { }));
() => tslib_1.__awaiter(void 0, void 0, void 0, function* () { }));
/**
* @return {function(): !Promise<number>}
* @this {*}
Expand Down
2 changes: 1 addition & 1 deletion test_files/readonly/readonly.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,8 @@ class Class {
this.normalProp = '';
}
}
exports.Class = Class;
Class.staticProp = '';
exports.Class = Class;
/* istanbul ignore if */
if (false) {
/**
Expand Down
2 changes: 1 addition & 1 deletion test_files/type_and_value/type_and_value.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ let extUseAsType;
let extUseAsValue = ExtTypeAndValue;
/** @type {?} */
let extUseEnumAsValue = ExtEnum;
/** @type {ExtEnum} */
/** @type {!ExtEnum} */
let extUseEnumAsType;
// ImplementsTypeAndValue implements a symbol that resolves to both a type and a
// value, and thus the @implements clause is dropped by tsickle.
Expand Down

0 comments on commit 8e25305

Please sign in to comment.