Skip to content
This repository has been archived by the owner on Dec 5, 2022. It is now read-only.

Commit

Permalink
Added --rename-conflicting-types flag, refactored FacadeGenerator to …
Browse files Browse the repository at this point in the history 
…separate handling of type names and value names

Added --rename-conflicting-types flag to allow the user to control whether types with names that conflict with variable names get renamed or get merged with the variable.
Made variables with the same name as types get merged into the types by
default
Separated handling of type names and value names in FacadeConverter
  • Loading branch information
@derekxu16
derekxu16 authored Nov 23, 2019
1 parent b809c53 commit fc55d7a
Show file tree
Hide file tree
Showing 10 changed files with 428 additions and 222 deletions.
1 change: 1 addition & 0 deletions README.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ Dart interop facade file is written to stdout.
`--destination=<destination-dir>`: output generated code to destination-dir<br/>
`--base-path=<input d.ts file directory>`: specify the directory that contains the input d.ts files<br/>
`--generate-html`: generate facades for dart:html types rather than importing them<br/>
`--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.

Expand Down
6 changes: 5 additions & 1 deletion index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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'
}
Expand Down
9 changes: 0 additions & 9 deletions lib/base.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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);
Expand Down
8 changes: 4 additions & 4 deletions lib/declaration.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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) {
Expand Down
157 changes: 105 additions & 52 deletions lib/facade_converter.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<string> = [base.ident(node.name)];
let p: ts.Node = node.parent;
while (p != null) {
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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);
}
Expand Down Expand Up @@ -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 {
Expand All @@ -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} {
Expand All @@ -678,23 +708,22 @@ 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(
options.resolvedTypeArguments.get(ident), removeResolvedTypeArguments(options))
};
}
// 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};
}
}
Expand All @@ -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);
Expand All @@ -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.
Expand All @@ -738,8 +769,7 @@ export class FacadeConverter extends base.TranspilerBase {
if (supportedDeclaration) {
return {
name: this.maybeAddTypeArguments(
this.nameRewriter.lookupName(<base.NamedDeclaration>declaration, identifier),
options),
this.nameRewriter.lookupName(declaration, identifier), options),
keep: true
};
}
Expand All @@ -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(<base.NamedDeclaration>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(<ts.InterfaceDeclaration>declaration) &&
base.getAncestor(identifier, ts.SyntaxKind.HeritageClause)) {
// TODO(jacobr): we need to specify a specific call method for this
Expand All @@ -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
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 [];
}
Expand Down Expand Up @@ -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)
Expand Down
9 changes: 8 additions & 1 deletion lib/main.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down
Loading

0 comments on commit fc55d7a

Please sign in to comment.