From d6fae6b6cee777a1a2135c3d6a4f069dc193d1e4 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 30 May 2019 13:39:05 -0600 Subject: [PATCH 1/2] Handling exports referenced in different scopes fixes #617 Exported identifiers referenced in other places were not checking what scope they were declared in when prepending their export table. This led to a notable amount of refactoring to clean up the situation. --- src/LuaTransformer.ts | 184 +++++++++++++++++----------------- src/TSHelper.ts | 8 ++ test/unit/identifiers.spec.ts | 47 +++++++++ 3 files changed, 149 insertions(+), 90 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index bb269d570..7437cff07 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -544,7 +544,7 @@ export class LuaTransformer { throw TSTLErrors.InvalidExtensionMetaExtension(statement); } - if ((isExtension || isMetaExtension) && this.isIdentifierExported(className)) { + if ((isExtension || isMetaExtension) && this.getIdentifierExportScope(className) !== undefined) { // Cannot export extension classes throw TSTLErrors.InvalidExportsExtension(statement); } @@ -772,12 +772,13 @@ export class LuaTransformer { const classVar = this.createLocalOrExportedOrGlobalDeclaration(className, classTable, statement); result.push(...classVar); - if (this.isIdentifierExported(className)) { + const exportScope = this.getIdentifierExportScope(className); + if (exportScope) { // local localClassName = ____exports.className result.push( tstl.createVariableDeclarationStatement( localClassName, - this.addExportToIdentifier(tstl.cloneIdentifier(className)) + this.createExportedIdentifier(tstl.cloneIdentifier(className), exportScope) ) ); } @@ -1500,9 +1501,7 @@ export class LuaTransformer { : tstl.createTableIndexExpression(tableExpression, tstl.createNumericLiteral(index + 1)); result.push(...this.createLocalOrExportedOrGlobalDeclaration(variableName, expression)); if (element.initializer) { - const identifier = this.shouldExportIdentifier(variableName) - ? this.createExportedIdentifier(variableName) - : variableName; + const identifier = this.addExportToIdentifier(variableName); result.push( tstl.createIfStatement( tstl.createBinaryExpression( @@ -1561,45 +1560,25 @@ export class LuaTransformer { const nameIdentifier = this.transformIdentifier(statement.name as ts.Identifier); if (isFirstDeclaration) { - const isExported = (ts.getCombinedModifierFlags(statement) & ts.ModifierFlags.Export) !== 0; - if (isExported && this.currentNamespace) { - // outerNS.innerNS = {} - const namespaceDeclaration = tstl.createAssignmentStatement( - tstl.createTableIndexExpression( - this.createModuleLocalNameIdentifier(this.currentNamespace), - tstl.createStringLiteral(nameIdentifier.text) - ), - tstl.createTableExpression() - ); + const exportScope = this.getIdentifierExportScope(nameIdentifier); + if (exportScope) { + const exportTable = ts.isModuleDeclaration(exportScope) + ? this.createModuleLocalNameIdentifier(exportScope) + : this.createExportsIdentifier(); - result.push(namespaceDeclaration); - - if (hasExports && tsHelper.moduleHasEmittedBody(statement)) { - // local innerNS = outerNS.innerNS - const localDeclaration = this.createHoistableVariableDeclarationStatement( - this.createModuleLocalNameIdentifier(statement), - tstl.createTableIndexExpression( - this.createModuleLocalNameIdentifier(this.currentNamespace), - tstl.createStringLiteral(nameIdentifier.text) - ) - ); - - result.push(localDeclaration); - } - } else if (isExported && !this.currentNamespace && this.isModule) { - // exports.NS = {} + // exportTable.NS = {} const namespaceDeclaration = tstl.createAssignmentStatement( - this.createExportedIdentifier(nameIdentifier), + tstl.createTableIndexExpression(exportTable, tstl.createStringLiteral(nameIdentifier.text)), tstl.createTableExpression() ); result.push(namespaceDeclaration); if (hasExports && tsHelper.moduleHasEmittedBody(statement)) { - // local NS = exports.NS + // local NS = exportTable.NS const localDeclaration = this.createHoistableVariableDeclarationStatement( this.createModuleLocalNameIdentifier(statement), - this.createExportedIdentifier(tstl.cloneIdentifier(nameIdentifier, statement.name)) + tstl.createTableIndexExpression(exportTable, tstl.createStringLiteral(nameIdentifier.text)) ); result.push(localDeclaration); @@ -1985,9 +1964,7 @@ export class LuaTransformer { statement.name.elements.forEach(element => { if (!ts.isOmittedExpression(element) && element.initializer) { const variableName = this.transformIdentifier(element.name as ts.Identifier); - const identifier = this.shouldExportIdentifier(variableName) - ? this.createExportedIdentifier(variableName) - : variableName; + const identifier = this.addExportToIdentifier(variableName); statements.push( tstl.createIfStatement( tstl.createBinaryExpression( @@ -3347,10 +3324,7 @@ export class LuaTransformer { properties.push(tstl.createTableFieldExpression(expression, name, element)); } else if (ts.isShorthandPropertyAssignment(element)) { const valueSymbol = this.checker.getShorthandAssignmentValueSymbol(element); - let identifier = this.createShorthandIdentifier(valueSymbol, element.name); - if (tstl.isIdentifier(identifier) && valueSymbol !== undefined && this.isSymbolExported(valueSymbol)) { - identifier = this.createExportedIdentifier(identifier); - } + const identifier = this.createShorthandIdentifier(valueSymbol, element.name); properties.push(tstl.createTableFieldExpression(identifier, name, element)); } else if (ts.isMethodDeclaration(element)) { const expression = this.transformFunctionExpression(element); @@ -4640,8 +4614,10 @@ export class LuaTransformer { private transformIdentifierExpression(expression: ts.Identifier): tstl.Expression { const identifier = this.transformIdentifier(expression); - if (this.isIdentifierExported(identifier)) { - return this.createExportedIdentifier(identifier); + + const exportScope = this.getIdentifierExportScope(identifier); + if (exportScope) { + return this.createExportedIdentifier(identifier, exportScope); } switch (this.getIdentifierText(expression)) { @@ -4664,28 +4640,44 @@ export class LuaTransformer { return identifier; } - protected isIdentifierExported(identifier: tstl.Identifier): boolean { - const symbolInfo = identifier.symbolId && this.symbolInfo.get(identifier.symbolId); - if (!symbolInfo) { - return false; + protected getSymbolFromIdentifier(identifier: tstl.Identifier): ts.Symbol | undefined { + if (identifier.symbolId !== undefined) { + const symbolInfo = this.symbolInfo.get(identifier.symbolId); + if (symbolInfo !== undefined) { + return symbolInfo.symbol; + } + } + return undefined; + } + + protected getIdentifierExportScope(identifier: tstl.Identifier): ts.SourceFile | ts.ModuleDeclaration | undefined { + const symbol = this.getSymbolFromIdentifier(identifier); + if (!symbol) { + return undefined; } - return this.isSymbolExported(symbolInfo.symbol); + return this.getSymbolExportScope(symbol); } protected isSymbolExported(symbol: ts.Symbol): boolean { - if (!this.isModule && !this.currentNamespace) { + if (tsHelper.getExportedSymbolDeclaration(symbol) !== undefined) { + return true; + } else if (this.currentSourceFile) { + // Symbol may have been exported separately (e.g. 'const foo = "bar"; export { foo }') + return this.isSymbolExportedFromScope(symbol, this.currentSourceFile); + } else { return false; } + } - const currentScope = this.currentNamespace ? this.currentNamespace : this.currentSourceFile; - if (currentScope === undefined) { - throw TSTLErrors.UndefinedScope(); + protected isSymbolExportedFromScope(symbol: ts.Symbol, scope: ts.SourceFile | ts.ModuleDeclaration): boolean { + if (ts.isSourceFile(scope) && !tsHelper.isFileModule(scope)) { + return false; } - let scopeSymbol = this.checker.getSymbolAtLocation(currentScope); + let scopeSymbol = this.checker.getSymbolAtLocation(scope); if (scopeSymbol === undefined) { - scopeSymbol = this.checker.getTypeAtLocation(currentScope).getSymbol(); + scopeSymbol = this.checker.getTypeAtLocation(scope).getSymbol(); } if (scopeSymbol === undefined || scopeSymbol.exports === undefined) { @@ -4705,25 +4697,44 @@ export class LuaTransformer { } protected addExportToIdentifier(identifier: tstl.Identifier): tstl.AssignmentLeftHandSideExpression { - if (this.isIdentifierExported(identifier)) { - return this.createExportedIdentifier(identifier); + const exportScope = this.getIdentifierExportScope(identifier); + if (!exportScope) { + return identifier; } - return identifier; + + return this.createExportedIdentifier(identifier, exportScope); } - protected createExportedIdentifier(identifier: tstl.Identifier): tstl.TableIndexExpression { - let exportTable: tstl.Identifier; - if (this.currentNamespace !== undefined) { - if (this.isUnsafeName(this.currentNamespace.name.text)) { - exportTable = this.createModuleLocalNameIdentifier(this.currentNamespace); - } else { - exportTable = this.transformIdentifier(this.currentNamespace.name as ts.Identifier); - } - } else { - exportTable = this.createExportsIdentifier(); + protected createExportedIdentifier( + identifier: tstl.Identifier, + exportScope?: ts.SourceFile | ts.ModuleDeclaration + ): tstl.AssignmentLeftHandSideExpression { + const exportTable = + exportScope && ts.isModuleDeclaration(exportScope) + ? this.createModuleLocalNameIdentifier(exportScope) + : this.createExportsIdentifier(); + return tstl.createTableIndexExpression(exportTable, tstl.createStringLiteral(identifier.text)); + } + + protected getSymbolExportScope(symbol: ts.Symbol): ts.SourceFile | ts.ModuleDeclaration | undefined { + const exportedDeclaration = tsHelper.getExportedSymbolDeclaration(symbol); + if (!exportedDeclaration) { + return undefined; } - return tstl.createTableIndexExpression(exportTable, tstl.createStringLiteral(identifier.text)); + const scope = tsHelper.findFirstNodeAbove( + exportedDeclaration, + (n): n is ts.SourceFile | ts.ModuleDeclaration => ts.isSourceFile(n) || ts.isModuleDeclaration(n) + ); + if (!scope) { + return undefined; + } + + if (!this.isSymbolExportedFromScope(symbol, scope)) { + return undefined; + } + + return scope; } protected transformLuaLibFunction( @@ -4827,17 +4838,6 @@ export class LuaTransformer { return filePath.replace(/\.\//g, "").replace(/\//g, "."); } - protected shouldExportIdentifier(identifier: tstl.Identifier | tstl.Identifier[]): boolean { - if (!this.isModule && !this.currentNamespace) { - return false; - } - if (Array.isArray(identifier)) { - return identifier.some(i => this.isIdentifierExported(i)); - } else { - return this.isIdentifierExported(identifier); - } - } - protected createSelfIdentifier(tsOriginal?: ts.Node): tstl.Identifier { return tstl.createIdentifier("self", tsOriginal); } @@ -4857,20 +4857,19 @@ export class LuaTransformer { const functionDeclaration = tsOriginal && ts.isFunctionDeclaration(tsOriginal) ? tsOriginal : undefined; - if (this.shouldExportIdentifier(lhs)) { + const identifiers = Array.isArray(lhs) ? lhs : [lhs]; + if (identifiers.length === 0) { + return []; + } + + const exportScope = this.getIdentifierExportScope(identifiers[0]); + if (exportScope) { // exported if (!rhs) { return []; - } else if (Array.isArray(lhs)) { - assignment = tstl.createAssignmentStatement( - lhs.map(i => this.createExportedIdentifier(i)), - rhs, - tsOriginal, - parent - ); } else { assignment = tstl.createAssignmentStatement( - this.createExportedIdentifier(lhs), + identifiers.map(i => this.createExportedIdentifier(i, exportScope)), rhs, tsOriginal, parent @@ -5120,10 +5119,15 @@ export class LuaTransformer { name = this.hasUnsafeIdentifierName(propertyIdentifier) ? this.createSafeName(propertyName) : propertyName; } - const identifier = this.transformIdentifierExpression(ts.createIdentifier(name)); + let identifier = this.transformIdentifierExpression(ts.createIdentifier(name)); tstl.setNodeOriginal(identifier, propertyIdentifier); if (valueSymbol !== undefined && tstl.isIdentifier(identifier)) { identifier.symbolId = this.symbolIds.get(valueSymbol); + + const exportScope = this.getSymbolExportScope(valueSymbol); + if (exportScope) { + identifier = this.createExportedIdentifier(identifier, exportScope); + } } return identifier; } diff --git a/src/TSHelper.ts b/src/TSHelper.ts index ad63e2e19..8d701512b 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -72,6 +72,14 @@ export class TSHelper { ); } + public static getExportedSymbolDeclaration(symbol: ts.Symbol): ts.Declaration | undefined { + const declarations = symbol.getDeclarations(); + if (declarations) { + return declarations.find(d => (ts.getCombinedModifierFlags(d) & ts.ModifierFlags.Export) !== 0); + } + return undefined; + } + public static isDeclaration(node: ts.Node): node is ts.Declaration { return ( ts.isEnumDeclaration(node) || diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts index 8aa2bc432..e4677c5dc 100644 --- a/test/unit/identifiers.spec.ts +++ b/test/unit/identifiers.spec.ts @@ -148,6 +148,53 @@ test.each(validTsInvalidLuaNames)("exported values with invalid lua identifier n expect(util.executeLua(`return (function() ${lua} end)()["${name}"]`)).toBe("foobar"); }); +test("exported identifiers referenced in namespace (%p)", () => { + const code = ` + export const foo = "foobar"; + namespace NS { + export const bar = foo; + } + export const baz = NS.bar;`; + expect(util.transpileExecuteAndReturnExport(code, "baz")).toBe("foobar"); +}); + +test("exported namespace identifiers referenced in different namespace (%p)", () => { + const tsHeader = ` + namespace A { + export const foo = "foobar"; + namespace B { + export const bar = foo; + } + export const baz = B.bar; + }`; + expect(util.transpileAndExecute("return A.baz", undefined, undefined, tsHeader)).toBe("foobar"); +}); + +test("exported identifiers referenced in nested scope (%p)", () => { + const code = ` + export const foo = "foobar"; + namespace A { + export namespace B { + export const bar = foo; + } + } + export const baz = A.B.bar;`; + expect(util.transpileExecuteAndReturnExport(code, "baz")).toBe("foobar"); +}); + +test.each(validTsInvalidLuaNames)( + "exported values with invalid lua identifier names referenced in different scope (%p)", + name => { + const code = ` + export const ${name} = "foobar"; + namespace NS { + export const foo = ${name}; + } + export const bar = NS.foo;`; + expect(util.transpileExecuteAndReturnExport(code, "bar")).toBe("foobar"); + } +); + test.each(validTsInvalidLuaNames)("class with invalid lua name has correct name property", name => { const code = ` class ${name} {} From 7b394060cc24cb4076b11e83fa89550ab1d96173 Mon Sep 17 00:00:00 2001 From: Tom <26638278+tomblind@users.noreply.github.com> Date: Thu, 30 May 2019 14:00:23 -0600 Subject: [PATCH 2/2] simplified transformModuleDeclaration further --- src/LuaTransformer.ts | 45 +++++++++++++++---------------------------- 1 file changed, 15 insertions(+), 30 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 7437cff07..8a5fac2ea 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1560,37 +1560,23 @@ export class LuaTransformer { const nameIdentifier = this.transformIdentifier(statement.name as ts.Identifier); if (isFirstDeclaration) { - const exportScope = this.getIdentifierExportScope(nameIdentifier); - if (exportScope) { - const exportTable = ts.isModuleDeclaration(exportScope) - ? this.createModuleLocalNameIdentifier(exportScope) - : this.createExportsIdentifier(); - - // exportTable.NS = {} - const namespaceDeclaration = tstl.createAssignmentStatement( - tstl.createTableIndexExpression(exportTable, tstl.createStringLiteral(nameIdentifier.text)), - tstl.createTableExpression() - ); - - result.push(namespaceDeclaration); + // local NS = {} or exportTable.NS = {} + const localDeclaration = this.createLocalOrExportedOrGlobalDeclaration( + nameIdentifier, + tstl.createTableExpression() + ); - if (hasExports && tsHelper.moduleHasEmittedBody(statement)) { - // local NS = exportTable.NS - const localDeclaration = this.createHoistableVariableDeclarationStatement( - this.createModuleLocalNameIdentifier(statement), - tstl.createTableIndexExpression(exportTable, tstl.createStringLiteral(nameIdentifier.text)) - ); + result.push(...localDeclaration); - result.push(localDeclaration); - } - } else { - // local NS = {} - const localDeclaration = this.createLocalOrExportedOrGlobalDeclaration( + const exportScope = this.getIdentifierExportScope(nameIdentifier); + if (exportScope && hasExports && tsHelper.moduleHasEmittedBody(statement)) { + // local NS = exportTable.NS + const localDeclaration = this.createHoistableVariableDeclarationStatement( this.createModuleLocalNameIdentifier(statement), - tstl.createTableExpression() + this.createExportedIdentifier(nameIdentifier, exportScope) ); - result.push(...localDeclaration); + result.push(localDeclaration); } } @@ -4698,11 +4684,10 @@ export class LuaTransformer { protected addExportToIdentifier(identifier: tstl.Identifier): tstl.AssignmentLeftHandSideExpression { const exportScope = this.getIdentifierExportScope(identifier); - if (!exportScope) { - return identifier; + if (exportScope) { + return this.createExportedIdentifier(identifier, exportScope); } - - return this.createExportedIdentifier(identifier, exportScope); + return identifier; } protected createExportedIdentifier(