diff --git a/src/jsdoc_transformer.ts b/src/jsdoc_transformer.ts index 93d14e621..d143f692d 100644 --- a/src/jsdoc_transformer.ts +++ b/src/jsdoc_transformer.ts @@ -35,7 +35,7 @@ import * as jsdoc from './jsdoc'; import {ModuleTypeTranslator} from './module_type_translator'; import * as transformerUtil from './transformer_util'; import {symbolIsValue} from './transformer_util'; -import {isValidClosurePropertyName} from './type_translator'; +import {isClutzType, isValidClosurePropertyName} from './type_translator'; function addCommentOn(node: ts.Node, tags: jsdoc.Tag[], escapeExtraTags?: Set) { const comment = jsdoc.toSynthesizedComment(tags, escapeExtraTags); @@ -162,11 +162,17 @@ export function maybeAddHeritageClauses( } } } else if (sym.flags & ts.SymbolFlags.Value) { - // If it's something other than a class in the value namespace, then it will - // not be a type in the Closure output (because Closure collapses - // the type and value namespaces). - warn(decl, `dropped ${relation} of a type/value conflict: ${expr.getText()}`); - return; + // If the symbol came from tsickle emit and it's something other than a class in the value + // namespace, then tsickle may not have emitted the type. + // TODO(#1072): some other symbols we ought to be worrying about here: + // - if the symbol is a TS builtin? we can emit it; + // - if the symbol comes from a tsickle-transpiled file, either .ts or + // .d.ts with externs generation? then maybe we can emit it with + // name mangling. + if (!isClutzType(sym)) { + warn(decl, `dropped ${relation} of a type/value conflict: ${expr.getText()}`); + return; + } } else if (sym.flags & ts.SymbolFlags.TypeLiteral) { // A type literal is a type like `{foo: string}`. // These can come up as the output of a mapped type. diff --git a/src/type_translator.ts b/src/type_translator.ts index 269087a77..4ee84b9cb 100644 --- a/src/type_translator.ts +++ b/src/type_translator.ts @@ -50,6 +50,22 @@ function isClosureProvidedType(symbol: ts.Symbol): boolean { symbol.declarations.some(n => isBuiltinLibDTS(n.getSourceFile().fileName)); } +/** + * Returns true if the source file is generated by Clutz, i.e. has the magic + * Clutz header. + */ +function isClutzDts(sourceFile: ts.SourceFile): boolean { + return sourceFile.text.startsWith('//!! generated by clutz.'); +} + +/** + * Returns true if the symbol is defined in a Clutz-generated d.ts file. + */ +export function isClutzType(symbol: ts.Symbol): boolean { + return symbol.declarations != null && + symbol.declarations.some(n => isClutzDts(n.getSourceFile())); +} + export function typeToDebugString(type: ts.Type): string { let debugString = `flags:0x${type.flags.toString(16)}`; @@ -558,7 +574,7 @@ export class TypeTranslator { } if (type.symbol.flags & ts.SymbolFlags.Value) { // The symbol is both a type and a value. - // For user-defined types in this state, we don't have a Closure name + // For user-defined types in this state, we may not have a Closure name // for the type. See the type_and_value test. if (!isClosureProvidedType(type.symbol)) { this.warn(`type/symbol conflict for ${type.symbol.name}, using {?} for now`); diff --git a/test/e2e_closure_test.ts b/test/e2e_closure_test.ts index 89899061d..16205a686 100644 --- a/test/e2e_closure_test.ts +++ b/test/e2e_closure_test.ts @@ -28,6 +28,7 @@ describe('golden file tests', () => { goldenJs.push('test_files/augment/shim.js'); goldenJs.push('test_files/clutz.no_externs/some_name_space.js'); goldenJs.push('test_files/clutz.no_externs/some_other.js'); + goldenJs.push('test_files/clutz_type_value.no_externs/type_value.js'); goldenJs.push('test_files/declare/shim.js'); goldenJs.push('test_files/declare_export_dts/shim.js'); goldenJs.push('test_files/import_from_goog/closure_Module.js'); diff --git a/test_files/clutz.no_externs/clutz_typings.d.ts b/test_files/clutz.no_externs/clutz_typings.d.ts index 19b3cba14..3bb62e97c 100644 --- a/test_files/clutz.no_externs/clutz_typings.d.ts +++ b/test_files/clutz.no_externs/clutz_typings.d.ts @@ -1,3 +1,4 @@ +//!! generated by clutz. declare namespace ಠ_ಠ.clutz.some.name.space { class ClutzedClass { field: ಠ_ಠ.clutz.some.other.TypeAlias; } function clutzedFn(param: ಠ_ಠ.clutz.some.other.TypeAlias); diff --git a/test_files/clutz_type_value.no_externs/clutz.d.ts b/test_files/clutz_type_value.no_externs/clutz.d.ts new file mode 100644 index 000000000..83b71ecbb --- /dev/null +++ b/test_files/clutz_type_value.no_externs/clutz.d.ts @@ -0,0 +1,23 @@ +//!! generated by clutz. + +// This file recreates the clutz output that has a type/value conflict. + +declare namespace ಠ_ಠ.clutz { + interface module$contents$type_value$IFace { + field: string; + } +} +declare namespace ಠ_ಠ.clutz.module$contents$type_value$IFace { + class Class { + otherField: string; + } +} + +declare namespace ಠ_ಠ.clutz.type_value { + export import IFace = ಠ_ಠ.clutz.module$contents$type_value$IFace; +} + +declare module 'goog:type_value' { +import IFace = ಠ_ಠ.clutz.type_value.IFace; + export default IFace; +} diff --git a/test_files/clutz_type_value.no_externs/type_value.js b/test_files/clutz_type_value.no_externs/type_value.js new file mode 100644 index 000000000..0f9d16955 --- /dev/null +++ b/test_files/clutz_type_value.no_externs/type_value.js @@ -0,0 +1,20 @@ +goog.module('type_value'); + +/** @record */ +class IFace { + constructor() { + /** @type {string} */ + this.field; + } +} + +class Class { + constructor() { + /** @type {string} */ + this.otherField; + } +} + +IFace.Class = Class; + +exports = IFace; diff --git a/test_files/clutz_type_value.no_externs/user.js b/test_files/clutz_type_value.no_externs/user.js new file mode 100644 index 000000000..045eec586 --- /dev/null +++ b/test_files/clutz_type_value.no_externs/user.js @@ -0,0 +1,25 @@ +/** + * + * @fileoverview This test verifies that a type/value-conflict symbol that + * occurs in a clutz file still can be used in a heritage clause. + * + * @suppress {checkTypes,constantProperty,extraRequire,missingOverride,missingReturn,unusedPrivateMembers,uselessCode} checked by tsc + */ +goog.module('test_files.clutz_type_value.no_externs.user'); +var module = module || { id: 'test_files/clutz_type_value.no_externs/user.ts' }; +module = module; +exports = {}; +const tsickle_goog_type_value_1 = goog.requireType("type_value"); +// We expect IFace to show up in the @implements tag. +/** + * @implements {tsickle_goog_type_value_1} + */ +class C { + constructor() { + this.field = 'abc'; + } +} +if (false) { + /** @type {string} */ + C.prototype.field; +} diff --git a/test_files/clutz_type_value.no_externs/user.ts b/test_files/clutz_type_value.no_externs/user.ts new file mode 100644 index 000000000..1546cd7b6 --- /dev/null +++ b/test_files/clutz_type_value.no_externs/user.ts @@ -0,0 +1,11 @@ +/** + * @fileoverview This test verifies that a type/value-conflict symbol that + * occurs in a clutz file still can be used in a heritage clause. + */ + +import IFace from 'goog:type_value'; + +// We expect IFace to show up in the @implements tag. +class C implements IFace { + field = 'abc'; +}