diff --git a/README.md b/README.md index b862d31..1c14214 100644 --- a/README.md +++ b/README.md @@ -21,6 +21,7 @@ Dart interop facade file is written to stdout. `--destination=`: output generated code to destination-dir
`--base-path=`: specify the directory that contains the input d.ts files
`--generate-html`: generate facades for dart:html types rather than importing them
+`--rename-conflicting-types`: rename types to avoid conflicts in cases where a variable and a type have the exact same name, but it is not clear if they are related or not. `--explicit-static`: disables default assumption that properties declared on the anonymous types of top level variable declarations are static `--trust-js-types`: Emits @anonymous tags on classes that have neither constructors nor static members. This prevents the Dart Dev Compiler from checking whether or not objects are truly instances of those classes. This flag should be used if the input JS/TS library has structural types, or is otherwise claiming that types match in cases where the correct JS prototype is not there for DDC to check against. diff --git a/index.js b/index.js index efe3455..3061e06 100755 --- a/index.js +++ b/index.js @@ -4,11 +4,15 @@ const main = require('./build/lib/main.js'); var args = require('minimist')(process.argv.slice(2), { base: 'string', - boolean: ['semantic-diagnostics', 'generate-html', 'explicit-static', 'trust-js-types'], + boolean: [ + 'semantic-diagnostics', 'generate-html', 'rename-conflicting-types', 'explicit-static', + 'trust-js-types' + ], alias: { 'base-path': 'basePath', 'semantic-diagnostics': 'semanticDiagnostics', 'generate-html': 'generateHTML', + 'rename-conflicting-types': 'renameConflictingTypes', 'explicit-static': 'explicitStatic', 'trust-js-types': 'trustJSTypes' } diff --git a/lib/base.ts b/lib/base.ts index 5de5a92..90ad4d2 100644 --- a/lib/base.ts +++ b/lib/base.ts @@ -71,8 +71,6 @@ export class ImportSummary { export type Constructor = ts.ConstructorDeclaration|ts.ConstructSignatureDeclaration; export type ClassLike = ts.ClassLikeDeclaration|ts.InterfaceDeclaration; -export type NamedDeclaration = ClassLike|ts.PropertyDeclaration|ts.VariableDeclaration| - ts.MethodDeclaration|ts.ModuleDeclaration|ts.FunctionDeclaration; /** * Interface extending the true InterfaceDeclaration interface to add optional state we store on @@ -105,13 +103,6 @@ export function isFunctionTypedefLikeInterface(ifDecl: ts.InterfaceDeclaration): ts.isCallSignatureDeclaration(ifDecl.members[0]); } -export function getDeclaration(type: ts.Type): ts.Declaration { - let symbol = type.getSymbol(); - if (!symbol) return null; - if (symbol.valueDeclaration) return symbol.valueDeclaration; - return symbol.declarations && symbol.declarations.length > 0 ? symbol.declarations[0] : null; -} - export function isExtendsClause(heritageClause: ts.HeritageClause) { return heritageClause.token === ts.SyntaxKind.ExtendsKeyword && !ts.isInterfaceDeclaration(heritageClause.parent); diff --git a/lib/declaration.ts b/lib/declaration.ts index 51ae8bb..46fab7f 100644 --- a/lib/declaration.ts +++ b/lib/declaration.ts @@ -158,12 +158,12 @@ export default class DeclarationTranspiler extends base.TranspilerBase { this.visit(name); return; } - // Have to rewrite names in this case as we could have conflicts - // due to needing to support multiple JS modules in a single Dart module + // Have to rewrite names in this case as we could have conflicts due to needing to support + // multiple JS modules in a single Dart module. if (!ts.isIdentifier(name)) { throw 'Internal error: unexpected function name kind:' + name.kind; } - let entry = this.fc.lookupCustomDartTypeName(name); + let entry = this.fc.lookupDartValueName(name); if (entry) { this.emit(entry.name); return; @@ -813,7 +813,7 @@ export default class DeclarationTranspiler extends base.TranspilerBase { if (name.kind !== ts.SyntaxKind.Identifier) { this.reportError(name, 'Unexpected name kind:' + name.kind); } - this.fc.visitTypeName(name); + this.visitName(name); } if (fn.typeParameters && fn.typeParameters.length > 0) { diff --git a/lib/facade_converter.ts b/lib/facade_converter.ts index e9e89c2..2d044c7 100644 --- a/lib/facade_converter.ts +++ b/lib/facade_converter.ts @@ -85,7 +85,7 @@ function hasVarArgs(parameters: ts.ParameterDeclaration[]): boolean { * } * Path: m1.m2.foo */ -function fullJsPath(node: base.NamedDeclaration): string { +function fullJsPath(node: ts.NamedDeclaration): string { const parts: Array = [base.ident(node.name)]; let p: ts.Node = node.parent; while (p != null) { @@ -131,7 +131,7 @@ export class NameRewriter { constructor(private fc: FacadeConverter) {} - private computeName(node: base.NamedDeclaration): DartNameRecord { + private computeName(node: ts.NamedDeclaration): DartNameRecord { const fullPath = fullJsPath(node); if (this.dartTypes.has(fullPath)) { return this.dartTypes.get(fullPath); @@ -178,7 +178,7 @@ export class NameRewriter { } } - lookupName(node: base.NamedDeclaration, context: ts.Node) { + lookupName(node: ts.NamedDeclaration, context: ts.Node) { let name = this.computeName(node).name; return this.fc.resolveImportForSourceFile(node.getSourceFile(), context.getSourceFile(), name); } @@ -610,52 +610,83 @@ export class FacadeConverter extends base.TranspilerBase { } } + replaceNode(original: ts.Node, replacement: ts.Node) { + if (ts.isVariableDeclaration(original) && ts.isClassDeclaration(replacement)) { + // Handle the speical case in mergeVariablesIntoClasses where we upgrade variable declarations + // to classes. + const symbol = this.tc.getSymbolAtLocation(original.name); + symbol.declarations = symbol.getDeclarations().map((declaration: ts.Declaration) => { + // TODO(derekx): Changing the declarations of a symbol like this is a hack. It would be + // cleaner and safer to generate a new Program and TypeChecker after performing + // gatherClasses and mergeVariablesIntoClasses. + if (declaration === original) { + return replacement; + } + return declaration; + }); + } + + super.replaceNode(original, replacement); + } + getSymbolAtLocation(identifier: ts.EntityName) { let symbol = this.tc.getSymbolAtLocation(identifier); while (symbol && symbol.flags & ts.SymbolFlags.Alias) symbol = this.tc.getAliasedSymbol(symbol); return symbol; } - getSymbolDeclaration(symbol: ts.Symbol, n?: ts.Node): ts.Declaration { - if (!symbol || this.tc.isUnknownSymbol(symbol)) return null; - let decl = symbol.valueDeclaration; - if (!decl) { - // In the case of a pure declaration with no assignment, there is no value declared. - // Just grab the first declaration, hoping it is declared once. - if (!symbol.declarations || symbol.declarations.length === 0) { - this.reportError(n, 'no declarations for symbol ' + symbol.name); - return; - } - decl = symbol.declarations[0]; + getValueDeclarationOfSymbol(symbol: ts.Symbol, n?: ts.Node): ts.Declaration|undefined { + if (!symbol || this.tc.isUnknownSymbol(symbol)) { + return undefined; + } + if (!symbol.valueDeclaration) { + this.reportError(n, `no value declaration for symbol ${symbol.name}`); + return undefined; } - return decl; + return symbol.valueDeclaration; + } + + getTypeDeclarationOfSymbol(symbol: ts.Symbol, n?: ts.Node): ts.Declaration|undefined { + if (!symbol || this.tc.isUnknownSymbol(symbol)) { + return undefined; + } + const typeDeclaration = symbol.declarations.find((declaration: ts.Declaration) => { + return ts.isInterfaceDeclaration(declaration) || ts.isClassDeclaration(declaration) || + ts.isTypeAliasDeclaration(declaration) || ts.isTypeParameterDeclaration(declaration); + }); + if (!typeDeclaration) { + this.reportError(n, `no type declarations for symbol ${symbol.name}`); + return undefined; + } + return typeDeclaration; } generateDartName(identifier: ts.EntityName, options: TypeDisplayOptions): string { - let ret = this.lookupCustomDartTypeName(identifier, options); - if (ret) return base.formatType(ret.name, ret.comment, options); + const ret = this.lookupCustomDartTypeName(identifier, options); + if (ret) { + return base.formatType(ret.name, ret.comment, options); + } // TODO(jacobr): handle library import prefixes more robustly. This generally works but is // fragile. return this.maybeAddTypeArguments(base.ident(identifier), options); } /** - * Returns null if declaration cannot be found or is not valid in Dart. + * Resolves TypeReferences to find the declaration of the referenced type that matches the + * predicate. + * + * For example, if the type passed is a reference to X and the predicate passed is + * ts.isInterfaceDeclaration, then this function will will return the declaration of interface X, + * or undefined if there is no such declaration. */ - getDeclaration(identifier: ts.EntityName): ts.Declaration { - let symbol: ts.Symbol; - - symbol = this.getSymbolAtLocation(identifier); - - let declaration = this.getSymbolDeclaration(symbol, identifier); - if (symbol && symbol.flags & ts.SymbolFlags.TypeParameter) { - let kind = declaration.parent.kind; - // Only kinds of TypeParameters supported by Dart. - if (kind !== ts.SyntaxKind.ClassDeclaration && kind !== ts.SyntaxKind.InterfaceDeclaration) { - return null; - } + getDeclarationOfReferencedType( + type: ts.TypeReferenceNode, + predicate: (declaration: ts.Declaration) => boolean): ts.Declaration { + const referenceSymbol = this.tc.getTypeAtLocation(type.typeName).getSymbol(); + if (!referenceSymbol) { + return undefined; } - return declaration; + return referenceSymbol.getDeclarations().find(predicate); } maybeAddTypeArguments(name: string, options: TypeDisplayOptions): string { @@ -667,8 +698,7 @@ export class FacadeConverter extends base.TranspilerBase { } /** - * Returns a custom Dart type name or null if the type isn't a custom Dart - * type. + * Returns a custom Dart type name or null if the type isn't a custom Dart type. */ lookupCustomDartTypeName(identifier: ts.EntityName, options?: TypeDisplayOptions): {name?: string, comment?: string, keep?: boolean} { @@ -678,14 +708,13 @@ export class FacadeConverter extends base.TranspilerBase { insideTypeArgument: this.insideTypeArgument }; } - let ident = base.ident(identifier); + const ident = base.ident(identifier); if (ident === 'Promise' && this.emitPromisesAsFutures) { return {name: this.maybeAddTypeArguments('Future', options)}; } - let symbol: ts.Symbol = this.getSymbolAtLocation(identifier); - let declaration = this.getSymbolDeclaration(symbol, identifier); + const symbol: ts.Symbol = this.getSymbolAtLocation(identifier); if (symbol && symbol.flags & ts.SymbolFlags.TypeParameter) { - let kind = declaration.parent.kind; + const parent = this.getTypeDeclarationOfSymbol(symbol).parent; if (options.resolvedTypeArguments && options.resolvedTypeArguments.has(ident)) { return { name: this.generateDartTypeName( @@ -693,8 +722,8 @@ export class FacadeConverter extends base.TranspilerBase { }; } // Only kinds of TypeParameters supported by Dart. - if (kind !== ts.SyntaxKind.ClassDeclaration && kind !== ts.SyntaxKind.InterfaceDeclaration && - kind !== ts.SyntaxKind.TypeAliasDeclaration) { + if (!ts.isClassDeclaration(parent) && !ts.isInterfaceDeclaration(parent) && + !ts.isTypeAliasDeclaration(parent)) { return {name: 'dynamic', comment: ident}; } } @@ -704,7 +733,7 @@ export class FacadeConverter extends base.TranspilerBase { return null; } - let fileAndName = this.getFileAndName(identifier, symbol); + const fileAndName = this.getFileAndName(identifier, symbol); if (fileAndName) { let fileSubs = TS_TO_DART_TYPENAMES.get(fileAndName.fileName); @@ -727,6 +756,8 @@ export class FacadeConverter extends base.TranspilerBase { } } } + + const declaration = this.getTypeDeclarationOfSymbol(symbol, identifier); if (declaration) { if (symbol.flags & ts.SymbolFlags.Enum) { // We can't treat JavaScript enums as Dart enums in this case. @@ -738,8 +769,7 @@ export class FacadeConverter extends base.TranspilerBase { if (supportedDeclaration) { return { name: this.maybeAddTypeArguments( - this.nameRewriter.lookupName(declaration, identifier), - options), + this.nameRewriter.lookupName(declaration, identifier), options), keep: true }; } @@ -751,13 +781,10 @@ export class FacadeConverter extends base.TranspilerBase { }; } - let kind = declaration.kind; - if (kind === ts.SyntaxKind.ClassDeclaration || kind === ts.SyntaxKind.InterfaceDeclaration || - kind === ts.SyntaxKind.VariableDeclaration || - kind === ts.SyntaxKind.PropertyDeclaration || - kind === ts.SyntaxKind.FunctionDeclaration) { - let name = this.nameRewriter.lookupName(declaration, identifier); - if (kind === ts.SyntaxKind.InterfaceDeclaration && + if (ts.isClassDeclaration(declaration) || ts.isInterfaceDeclaration(declaration) || + ts.isTypeAliasDeclaration(declaration)) { + const name = this.nameRewriter.lookupName(declaration, identifier); + if (ts.isInterfaceDeclaration(declaration) && base.isFunctionTypedefLikeInterface(declaration) && base.getAncestor(identifier, ts.SyntaxKind.HeritageClause)) { // TODO(jacobr): we need to specify a specific call method for this @@ -770,6 +797,32 @@ export class FacadeConverter extends base.TranspilerBase { return null; } + /** + * Looks up an identifier that is used as the name of a value (variable or function). Uses the + * name rewriter to fix naming conflicts. + * + * Returns the original name if it doesn't cause any conflicts, otherwise returns a renamed + * identifier. + */ + lookupDartValueName(identifier: ts.Identifier, options?: TypeDisplayOptions): + {name?: string, comment?: string, keep?: boolean} { + if (!options) { + options = { + insideComment: this.insideCodeComment, + insideTypeArgument: this.insideTypeArgument + }; + } + const symbol: ts.Symbol = this.getSymbolAtLocation(identifier); + const declaration = this.getValueDeclarationOfSymbol(symbol, identifier); + if (declaration) { + if (ts.isVariableDeclaration(declaration) || ts.isPropertyDeclaration(declaration) || + ts.isFunctionDeclaration(declaration)) { + const name = this.nameRewriter.lookupName(declaration, identifier); + return {name: this.maybeAddTypeArguments(name, options), keep: true}; + } + } + } + // TODO(jacobr): performance of this method could easily be optimized. /** * This method works around the lack of Dart support for union types @@ -837,7 +890,7 @@ export class FacadeConverter extends base.TranspilerBase { // TODO(jacobr): property need to prefix the name better. referenceType.typeName = this.createEntityName(symbol); referenceType.typeName.parent = referenceType; - let decl = this.getSymbolDeclaration(symbol); + const decl = this.getTypeDeclarationOfSymbol(symbol); base.copyLocation(decl, referenceType); return referenceType; } @@ -855,7 +908,7 @@ export class FacadeConverter extends base.TranspilerBase { // that is a typedef like interface causes the typescript compiler to stack // overflow. Not sure if this is a bug in the typescript compiler or I am // missing something obvious. - let declaration = base.getDeclaration(type) as ts.InterfaceDeclaration; + const declaration = this.getTypeDeclarationOfSymbol(type.symbol) as ts.InterfaceDeclaration; if (base.isFunctionTypedefLikeInterface(declaration)) { return []; } @@ -923,7 +976,7 @@ export class FacadeConverter extends base.TranspilerBase { private getFileAndName(n: ts.Node, originalSymbol: ts.Symbol): {fileName: string, qname: string} { let symbol = originalSymbol; while (symbol.flags & ts.SymbolFlags.Alias) symbol = this.tc.getAliasedSymbol(symbol); - let decl = this.getSymbolDeclaration(symbol, n); + const decl = this.getTypeDeclarationOfSymbol(symbol, n); const fileName = decl.getSourceFile().fileName; const canonicalFileName = this.getRelativeFileName(fileName) diff --git a/lib/main.ts b/lib/main.ts index dcd2156..a207df3 100644 --- a/lib/main.ts +++ b/lib/main.ts @@ -45,6 +45,11 @@ export interface TranspilerOptions { * Generate browser API facades instead of importing them from dart:html. */ generateHTML?: boolean; + /** + * Rename types to avoid conflicts in cases where a variable and a type have the exact same name, + * but it is not clear if they are related or not. + */ + renameConflictingTypes?: boolean; /** * Do not assume that all properties declared on the anonymous types of top level variable * declarations are static. @@ -288,7 +293,9 @@ export class Transpiler { } this.lastCommentIdx = -1; - merge.normalizeSourceFile(sourceFile, this.fc, fileSet, this.options.explicitStatic); + merge.normalizeSourceFile( + sourceFile, this.fc, fileSet, this.options.renameConflictingTypes, + this.options.explicitStatic); this.pushContext(OutputContext.Default); this.visit(sourceFile); this.popContext(); diff --git a/lib/merge.ts b/lib/merge.ts index e882330..cb1a29b 100644 --- a/lib/merge.ts +++ b/lib/merge.ts @@ -33,10 +33,11 @@ export class MergedType { case ts.SyntaxKind.TypeReference: // We need to follow Alias types as Dart does not support them for non // function types. TODO(jacobr): handle them for Function types? - let typeRef = t; - let decl = this.fc.getDeclaration(typeRef.typeName); - if (decl !== null && ts.isTypeAliasDeclaration(decl)) { - let alias = decl; + const typeRef = t; + const decl = + this.fc.getTypeDeclarationOfSymbol(this.fc.getSymbolAtLocation(typeRef.typeName)); + if (decl && ts.isTypeAliasDeclaration(decl)) { + const alias = decl; if (!base.supportedTypeDeclaration(alias)) { if (typeRef.typeArguments) { console.log( @@ -244,7 +245,8 @@ export class MergedMember { * Normalize a SourceFile */ export function normalizeSourceFile( - f: ts.SourceFile, fc: FacadeConverter, fileSet: Set, explicitStatic = false) { + f: ts.SourceFile, fc: FacadeConverter, fileSet: Set, renameConflictingTypes = false, + explicitStatic = false) { let modules: Map = new Map(); // Merge top level modules. @@ -275,71 +277,59 @@ export function normalizeSourceFile( } /** - * Searches for a constructor member within a TypeNode. If found, it returns that member. - * Otherwise, it returns undefined. + * Searches for a constructor member within a type literal or an interface. If found, it returns + * that member. Otherwise, it returns undefined. */ - function findConstructorInType(type: ts.TypeNode): base.Constructor|undefined { - if (ts.isTypeLiteralNode(type)) { - // Example TypeScript definition matching the type literal case: - // - // declare interface XType { - // a: string; - // b: number; - // c(): boolean; - // } - // - // declare var X: { - // prototype: XType, - // new(a: string, b: number): XType, - // }; - // - // Possible underlying implementation: - // var X = class { - // constructor(public a: string, public b: number) {} - // c(): boolean { return this.b.toString() === this.a } - // } - // - // In TypeScript you could just write new X('abc', 123) and create an instance of - // XType. Dart doesn't support this when X is a variable, so X must be upgraded to be - // a class. - return type.members.find(base.isConstructor) as base.Constructor; - } else if (ts.isTypeReferenceNode(type)) { - const referenceSymbol = fc.tc.getTypeAtLocation(type.typeName).symbol; - if (!referenceSymbol) { - return undefined; - } - for (const declaration of referenceSymbol.declarations) { - if (ts.isTypeLiteralNode(declaration)) { - // An example of the TypeLiteral case is provided above. - return declaration.members.find(base.isConstructor) as base.Constructor; - } else if (ts.isInterfaceDeclaration(declaration)) { - // Example TypeScript definition matching the interface case: - // - // interface XType { - // a: string; - // b: number; - // c(): boolean; - // } - // - // interface X { - // new (a: string, b: number): XType; - // } - // - // declare var X: X; - // - // Possible underlying implementation: - // var X: X = class { - // constructor(public a: string, public b: number) {} - // c(): boolean { return this.b.toString() === this.a } - // } - // - // In TypeScript you could just write new X('abc', 123) and create an instance of - // XType. Dart doesn't support this when X is a variable, so X must be upgraded to - // be a class. - return declaration.members.find(base.isConstructor) as base.Constructor; - } - } - } + function findConstructorInType(declaration: ts.TypeLiteralNode| + ts.InterfaceDeclaration): base.Constructor|undefined { + // Example TypeScript definition matching the type literal case: + // + // declare interface XType { + // a: string; + // b: number; + // c(): boolean; + // } + // + // declare var X: { + // prototype: XType, + // new(a: string, b: number): XType, + // }; + // + // Possible underlying implementation: + // var X = class { + // constructor(public a: string, public b: number) {} + // c(): boolean { return this.b.toString() === this.a } + // } + // + // In TypeScript you could just write new X('abc', 123) and create an instance of + // XType. Dart doesn't support this when X is a variable, so X must be upgraded to be + // a class. + + // Example TypeScript definition matching the interface case: + // + // interface XType { + // a: string; + // b: number; + // c(): boolean; + // } + // + // interface X { + // new (a: string, b: number): XType; + // } + // + // declare var X: X; + // + // Possible underlying implementation: + // var X: X = class { + // constructor(public a: string, public b: number) {} + // c(): boolean { return this.b.toString() === this.a } + // } + // + // In TypeScript you could just write new X('abc', 123) and create an instance of + // XType. Dart doesn't support this when X is a variable, so X must be upgraded to + // be a class. + + return declaration.members.find(base.isConstructor) as base.Constructor; } /** @@ -349,7 +339,7 @@ export function normalizeSourceFile( undefined { const constructedTypeSymbol: ts.Symbol = constructor && ts.isTypeReferenceNode(constructor.type) && - fc.tc.getTypeAtLocation(constructor.type).symbol; + fc.tc.getTypeAtLocation(constructor.type).getSymbol(); if (!constructedTypeSymbol) { return; } @@ -381,18 +371,34 @@ export function normalizeSourceFile( const statement = n; statement.declarationList.declarations.forEach((variableDecl: ts.VariableDeclaration) => { if (ts.isIdentifier(variableDecl.name)) { - if (variableDecl.type) { - const variableType = variableDecl.type; - // Try to find a Constructor within the variable's type. - const constructor: base.Constructor = findConstructorInType(variableType); + const name = base.ident(variableDecl.name); + const variableType = variableDecl.type; + if (!variableType) { + return; + } + + // We need to find the declaration of the variable's type in order to acccess the + // members of that type. + let variableTypeDeclaration: ts.TypeLiteralNode|ts.InterfaceDeclaration; + if (ts.isTypeReferenceNode(variableType)) { + variableTypeDeclaration = + fc.getDeclarationOfReferencedType(variableType, (declaration: ts.Declaration) => { + return ts.isTypeLiteralNode(declaration) || + ts.isInterfaceDeclaration(declaration); + }) as ts.TypeLiteralNode | ts.InterfaceDeclaration; + } else if (ts.isTypeLiteralNode(variableType)) { + variableTypeDeclaration = variableType; + } + if (!variableTypeDeclaration) { + return; + } + // Try to find a Constructor within the variable's type. + const constructor: base.Constructor = findConstructorInType(variableTypeDeclaration); + if (constructor) { // Get the type of object that the constructor creates. const constructedType = getConstructedObjectType(constructor); - if (!constructedType) { - return; - } - const name = base.ident(variableDecl.name); if (classes.has(name)) { const existing = classes.get(name); // If a class with the same name as the variable already exists, we should suppress @@ -421,69 +427,97 @@ export function normalizeSourceFile( clazz.flags = variableDecl.flags; fc.replaceNode(variableDecl, clazz); classes.set(name, clazz); - - const existing = classes.get(name); - const members = existing.members; - const resolvedVariableType = constructor.parent as ts.ObjectTypeDeclaration; - resolvedVariableType.members.forEach((member: ts.TypeElement|ts.ClassElement) => { - // Array.prototype.push is used below as a small hack to get around NodeArrays being - // readonly. - switch (member.kind) { - case ts.SyntaxKind.Constructor: - case ts.SyntaxKind.ConstructorType: - case ts.SyntaxKind.ConstructSignature: { - const clonedConstructor = ts.getMutableClone(member); - clonedConstructor.name = ts.getMutableClone(variableDecl.name) as ts.PropertyName; - clonedConstructor.parent = existing; - - const existingConstructIndex = members.findIndex(base.isConstructor); - if (existingConstructIndex === -1) { - Array.prototype.push.call(members, clonedConstructor); - } else { - Array.prototype.splice.call(members, existingConstructIndex, clonedConstructor); + } else { + if (renameConflictingTypes) { + // If we cannot find a constructor within the variable's type and the + // --rename-conflicting-types flag is set, we need to check whether or not a type + // with the same name as the variable already exists. If it does, we must rename it. + // That type is not directly associated with this variable, so they cannot be + // combined. + const existingSymbol = fc.tc.getSymbolAtLocation(variableDecl.name); + if (existingSymbol.getDeclarations()) { + for (const declaration of existingSymbol.getDeclarations()) { + if (ts.isInterfaceDeclaration(declaration) || + ts.isTypeAliasDeclaration(declaration)) { + declaration.name.escapedText = ts.escapeLeadingUnderscores(name + 'Type'); } - } break; - case ts.SyntaxKind.MethodSignature: - member.parent = existing.parent; - Array.prototype.push.call(members, member); - break; - case ts.SyntaxKind.PropertySignature: - if (!explicitStatic) { - // Finds all existing declarations of this property in the inheritance - // hierarchy of this class. - const existingDeclarations = - findPropertyInHierarchy(base.ident(member.name), existing, classes); - - if (existingDeclarations.size) { - for (const existingDecl of existingDeclarations) { - addModifier(existingDecl, ts.createModifier(ts.SyntaxKind.StaticKeyword)); - } + } + } + return; + } else if (!renameConflictingTypes && classes.has(name)) { + // If we cannot find a constructor and there exists a class with the exact same name, + // we assume by default that the variable and type are related as they have the exact + // same name. Thus, the variable declaration is suppressed and the members of its type + // are merged into the existing class below. + fc.suppressNode(variableDecl); + } + } + + // Merge the members of the variable's type into the existing class. + const existing = classes.get(name); + if (!existing) { + return; + } + + const members = existing.members; + variableTypeDeclaration.members.forEach((member: ts.TypeElement|ts.ClassElement) => { + // Array.prototype.push is used below as a small hack to get around NodeArrays being + // readonly. + switch (member.kind) { + case ts.SyntaxKind.Constructor: + case ts.SyntaxKind.ConstructorType: + case ts.SyntaxKind.ConstructSignature: { + const clonedConstructor = ts.getMutableClone(member); + clonedConstructor.name = ts.getMutableClone(variableDecl.name) as ts.PropertyName; + clonedConstructor.parent = existing; + + const existingConstructIndex = members.findIndex(base.isConstructor); + if (existingConstructIndex === -1) { + Array.prototype.push.call(members, clonedConstructor); + } else { + Array.prototype.splice.call(members, existingConstructIndex, clonedConstructor); + } + } break; + case ts.SyntaxKind.MethodSignature: + member.parent = existing.parent; + Array.prototype.push.call(members, member); + break; + case ts.SyntaxKind.PropertySignature: + if (!explicitStatic) { + // Finds all existing declarations of this property in the inheritance + // hierarchy of this class. + const existingDeclarations = + findPropertyInHierarchy(base.ident(member.name), existing, classes); + + if (existingDeclarations.size) { + for (const existingDecl of existingDeclarations) { + addModifier(existingDecl, ts.createModifier(ts.SyntaxKind.StaticKeyword)); } } + } - // If needed, add declaration of property to the interface that we are - // currently handling. - if (!findPropertyInClass(base.ident(member.name), existing)) { - if (!explicitStatic) { - addModifier(member, ts.createModifier(ts.SyntaxKind.StaticKeyword)); - } - member.parent = existing.parent; - Array.prototype.push.call(members, member); + // If needed, add declaration of property to the interface that we are + // currently handling. + if (!findPropertyInClass(base.ident(member.name), existing)) { + if (!explicitStatic) { + addModifier(member, ts.createModifier(ts.SyntaxKind.StaticKeyword)); } - break; - case ts.SyntaxKind.IndexSignature: - member.parent = existing.parent; - Array.prototype.push.call(members, member); - break; - case ts.SyntaxKind.CallSignature: member.parent = existing.parent; Array.prototype.push.call(members, member); - break; - default: - throw 'Unhandled TypeLiteral member type:' + member.kind; - } - }); - } + } + break; + case ts.SyntaxKind.IndexSignature: + member.parent = existing.parent; + Array.prototype.push.call(members, member); + break; + case ts.SyntaxKind.CallSignature: + member.parent = existing.parent; + Array.prototype.push.call(members, member); + break; + default: + throw 'Unhandled TypeLiteral member type:' + member.kind; + } + }); } else { throw 'Unexpected VariableStatement identifier kind'; } diff --git a/test/declaration_test.ts b/test/declaration_test.ts index cb50dcb..982459e 100644 --- a/test/declaration_test.ts +++ b/test/declaration_test.ts @@ -1089,9 +1089,7 @@ declare interface X { new(a: string, b: number): YType; } -declare type Y = X; - -declare var Y: Y; +declare var Y: X; `).to.equal(`@anonymous @JS() abstract class XType { @@ -1110,7 +1108,6 @@ abstract class X { /*external factory X(String a, num b);*/ } -/*declare type Y = X;*/ @JS() class Y { // @Ignore @@ -1162,18 +1159,50 @@ class X { // End module m1`); }); + }); - // Case where we cannot find a variable matching the interface so it is unsafe to give the - // interface a constructor. - expectTranslate(` + describe('cases where a type and a variable cannot be merged', () => { + it('should handle cases where an interface has no matching variable', () => { + // Case where we cannot find a variable matching the interface so it is unsafe to give the + // interface a constructor. + expectTranslate(` interface X { - new (a: string|bool, b: number): XType; + new (a: string|boolean, b: number): XType; }`).to.equal(`@anonymous @JS() abstract class X { // Constructors on anonymous interfaces are not yet supported. /*external factory X(String|bool a, num b);*/ }`); + }); + }); + it('should merge variables and interfaces with the same name by default', () => { + expectTranslate(` +interface X { + a: string; + b: number; + c(): boolean; +} + +declare var X: { d: number[] }; + +declare var x: X; +`).to.equal(`@anonymous +@JS() +abstract class X { + external String get a; + external set a(String v); + external num get b; + external set b(num v); + external bool c(); + external static List get d; + external static set d(List v); +} + +@JS() +external X get x; +@JS() +external set x(X v);`); }); }); @@ -1335,6 +1364,70 @@ class X { external static set b(String v); }`); }); + + describe('--rename-conflicting-types', () => { + it('should merge variables and interfaces with the same name by default', () => { + expectTranslate(` +interface X { + a: string; + b: number; + c(): boolean; +} + +declare var X: { d: number[] }; + +declare var x: X; +`).to.equal(`@anonymous +@JS() +abstract class X { + external String get a; + external set a(String v); + external num get b; + external set b(num v); + external bool c(); + external static List get d; + external static set d(List v); +} + +@JS() +external X get x; +@JS() +external set x(X v);`); + }); + + it('should rename types that conflict with unrelated variables when the flag is set', () => { + const renameConflictingTypesOpts = {failFast: true, renameConflictingTypes: true}; + expectTranslate( + `interface X { + a: string; + b: number; + c(): boolean; + } + + declare var X: { a: number[], b: number[], c: number[] }; + + declare var x: X;`, + renameConflictingTypesOpts) + .to.equal(`@anonymous +@JS() +abstract class XType { + external String get a; + external set a(String v); + external num get b; + external set b(num v); + external bool c(); +} + +@JS() +external dynamic /*{ a: number[], b: number[], c: number[] }*/ get X; +@JS() +external set X(dynamic /*{ a: number[], b: number[], c: number[] }*/ v); +@JS() +external XType get x; +@JS() +external set x(XType v);`); + }); + }); }); describe('single call signature interfaces', () => { diff --git a/test/js_interop_test.ts b/test/js_interop_test.ts index 3d135b8..2c4c346 100644 --- a/test/js_interop_test.ts +++ b/test/js_interop_test.ts @@ -347,9 +347,9 @@ external ScaleLinear /*ScaleLinear|ScaleLinear*/ scaleLinear/**/();`) expectTranslate(` class X { - F(a:string):num; - F(a:string, b: string|num):string; - F(a2:string, b: string, c: num):string; + F(a: string): number; + F(a: string, b: string|number): string; + F(a2: string, b: string, c: number): string; }`).to.equal(`@JS() class X { // @Ignore @@ -363,9 +363,9 @@ class X { expectTranslate(` class X { - Y(a:string):num {}; - Y(a:string, b: num):string {}; - Y(a2:string, b: string, c: num):string {}; + Y(a: string): number {}; + Y(a: string, b: number):string {}; + Y(a2:string, b: string, c: number):string {}; }`).to.equal(`@JS() class X { // @Ignore @@ -486,11 +486,11 @@ external log([args1, args2, args3, args4, args5]);`); expectTranslate(` interface X { a: string; - b: num; + b: number; c: X; } interface Y extends X { - d: num; + d: number; /* example comment */ e: any; }`).to.equal(`@anonymous diff --git a/test/type_test.ts b/test/type_test.ts index e26c20d..159814b 100644 --- a/test/type_test.ts +++ b/test/type_test.ts @@ -66,27 +66,50 @@ external set x(dynamic /*{a: number, [k: string]: number}*/ v);`); it('should support union types', () => { expectTranslate('function foo() : number | number[];').to.equal(`@JS() external dynamic /*num|List*/ foo();`); - expectTranslate('var x: number|List;').to.equal(`@JS() + expectTranslate('var x: number|Array;').to.equal(`@JS() external dynamic /*num|List*/ get x; @JS() external set x(dynamic /*num|List*/ v);`); - expectTranslate('function x(): number|List<{[k: string]: any}> {};').to.equal(`@JS() + expectTranslate('function x(): number|Array<{[k: string]: any}> {};').to.equal(`@JS() external dynamic /*num|List>*/ x();`); }); it('should support intersection types', () => { - expectTranslate('function foo() : Foo & Bar;').to.equal(`@JS() + expectTranslate(` +interface Foo { a: number, b: string } +interface Bar { b: string } + +function foo() : Foo & Bar; +`).to.equal(`@anonymous +@JS() +abstract class Foo { + external num get a; + external set a(num v); + external String get b; + external set b(String v); + external factory Foo({num a, String b}); +} + +@anonymous +@JS() +abstract class Bar { + external String get b; + external set b(String v); + external factory Bar({String b}); +} + +@JS() external Foo /*Foo&Bar*/ foo();`); }); it('should support parenthesized types', () => { expectTranslate('function foo() : (number | number[]);').to.equal(`@JS() external dynamic /*num|List*/ foo();`); - expectTranslate('var x: (number|List);').to.equal(`@JS() + expectTranslate('var x: (number|Array);').to.equal(`@JS() external dynamic /*num|List*/ get x; @JS() external set x(dynamic /*num|List*/ v);`); - expectTranslate('function x(): number|(List<{[k: string]: any}>) {};').to.equal(`@JS() + expectTranslate('function x(): number|(Array<{[k: string]: any}>) {};').to.equal(`@JS() external dynamic /*num|List>*/ x();`); });