From 2e7eec1e3d1135ab99cfeb024476c2a8cd594fce Mon Sep 17 00:00:00 2001 From: hazzard993 Date: Wed, 19 Jun 2019 22:05:00 +1000 Subject: [PATCH 1/4] Fix noResolution for named imports --- src/LuaTransformer.ts | 22 ++++++++++- src/TSHelper.ts | 25 ++++++++++++ test/unit/require.spec.ts | 81 +++++++++++++++++++++++++++++++++------ 3 files changed, 114 insertions(+), 14 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index a4144e126..9d206c274 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -376,8 +376,26 @@ export class LuaTransformer { throw TSTLErrors.UnsupportedImportType(statement.importClause); } - const type = this.checker.getTypeAtLocation(imports); - const shouldResolve = !tsHelper.getCustomDecorators(type, this.checker).has(DecoratorKind.NoResolution); + let shouldResolve = true; + if (ts.isNamedImports(imports)) { + for (const importSpecifier of imports.elements) { + const parentModule = tsHelper.getImportSpecifierModuleDeclaration(importSpecifier, this.checker); + if (parentModule) { + const type = this.checker.getTypeAtLocation(parentModule); + const decorators = tsHelper.getCustomDecorators(type, this.checker); + if (decorators.has(DecoratorKind.NoResolution)) { + shouldResolve = false; + break; + } + } + } + } else if (ts.isNamespaceImport(imports)) { + const type = this.checker.getTypeAtLocation(imports); + if (tsHelper.getCustomDecorators(type, this.checker).has(DecoratorKind.NoResolution)) { + shouldResolve = false; + } + } + const requireCall = this.createModuleRequire(statement.moduleSpecifier as ts.StringLiteral, shouldResolve); if (ts.isNamedImports(imports)) { diff --git a/src/TSHelper.ts b/src/TSHelper.ts index eeb6373e1..47941a708 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -131,6 +131,10 @@ export class TSHelper { return !((ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Ambient) === 0); } + public static isNonNamespaceModuleDeclaration(node: ts.Node): node is ts.ModuleDeclaration { + return ts.isModuleDeclaration(node) && (node.flags & ts.NodeFlags.Namespace) === 0; + } + public static isStatic(node: ts.Node): boolean { return node.modifiers !== undefined && node.modifiers.some(m => m.kind === ts.SyntaxKind.StaticKeyword); } @@ -354,6 +358,27 @@ export class TSHelper { return directivesMap; } + public static getImportSpecifierModuleDeclaration( + node: ts.ImportSpecifier, + checker: ts.TypeChecker + ): ts.ModuleDeclaration | undefined { + const symbol = checker.getSymbolAtLocation(node.name); + if (symbol) { + const originalSymbol = checker.getAliasedSymbol(symbol); + if (originalSymbol.declarations) { + for (const declaration of originalSymbol.declarations) { + const parentAmbientModule = this.findFirstNodeAbove( + declaration, + this.isNonNamespaceModuleDeclaration + ); + if (parentAmbientModule) { + return parentAmbientModule; + } + } + } + } + } + // Search up until finding a node satisfying the callback public static findFirstNodeAbove( node: ts.Node, diff --git a/test/unit/require.spec.ts b/test/unit/require.spec.ts index b6db8f877..62b9cd539 100644 --- a/test/unit/require.spec.ts +++ b/test/unit/require.spec.ts @@ -91,20 +91,77 @@ test.each([ } ); -test.each([{ comment: "", expectedPath: "src.fake" }, { comment: "/** @noResolution */", expectedPath: "fake" }])( - "noResolution on ambient modules causes no path alterations (%p)", - ({ comment, expectedPath }) => { - const lua = util.transpileString({ - "src/main.ts": `import * as fake from "fake"; fake;`, - "module.d.ts": `${comment} declare module "fake" {}`, - }); - const match = requireRegex.exec(lua); +test.each([ + { + declarationStatement: ` + declare module 'fake' {} + `, + mainCode: "import * as fake from 'fake'; fake;", + expectedPath: "src.fake", + }, + { + declarationStatement: ` + /** @noResolution */ + declare module 'fake' {} + `, + mainCode: "import * as fake from 'fake'; fake;", + expectedPath: "fake", + }, + { + declarationStatement: ` + declare module 'fake' { + export const x: number; + } + `, + mainCode: "import { x } from 'fake'; x;", + expectedPath: "src.fake", + }, + { + declarationStatement: ` + /** @noResolution */ + declare module 'fake' { + export const x: number; + } + `, + mainCode: "import { x } from 'fake'; x;", + expectedPath: "fake", + }, + { + declarationStatement: ` + /** @noResolution */ + declare module 'fake' { + export const x: number; + } + declare module 'fake' { + export const y: number; + } + `, + mainCode: "import { y } from 'fake'; y;", + expectedPath: "fake", + }, + { + declarationStatement: ` + declare module 'fake' { + export const x: number; + } + declare module 'fake' { + export const y: number; + } + `, + mainCode: "import { y } from 'fake'; y;", + expectedPath: "src.fake", + }, +])("noResolution prevents any module path resolution behaviour", ({ declarationStatement, mainCode, expectedPath }) => { + const lua = util.transpileString({ + "src/main.ts": mainCode, + "module.d.ts": declarationStatement, + }); + const match = requireRegex.exec(lua); - if (util.expectToBeDefined(match)) { - expect(match[1]).toBe(expectedPath); - } + if (util.expectToBeDefined(match)) { + expect(match[1]).toBe(expectedPath); } -); +}); test("ImportEquals declaration require", () => { const input = `import foo = require("./foo/bar"); foo;`; From 0dd855df8ac6ef44338c9479a9eb4dcb37cd3046 Mon Sep 17 00:00:00 2001 From: hazzard993 Date: Wed, 19 Jun 2019 23:04:07 +1000 Subject: [PATCH 2/4] Fix no element imports and use alternate symbol resolution solution --- src/LuaTransformer.ts | 41 +++++++++++++++++---------------------- src/TSHelper.ts | 21 -------------------- test/unit/require.spec.ts | 15 ++++++++++++++ 3 files changed, 33 insertions(+), 44 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index 9d206c274..d6b6a3700 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -357,11 +357,28 @@ export class LuaTransformer { scope.importStatements = []; } + let shouldResolve = true; + const moduleOwnerSymbol = this.checker.getSymbolAtLocation(statement.moduleSpecifier); + if (moduleOwnerSymbol) { + for (const declaration of moduleOwnerSymbol.declarations) { + if (tsHelper.isNonNamespaceModuleDeclaration(declaration)) { + const type = this.checker.getTypeAtLocation(declaration); + if (type) { + const decorators = tsHelper.getCustomDecorators(type, this.checker); + if (decorators.has(DecoratorKind.NoResolution)) { + shouldResolve = false; + break; + } + } + } + } + } + const moduleSpecifier = statement.moduleSpecifier as ts.StringLiteral; const importPath = moduleSpecifier.text.replace(new RegExp('"', "g"), ""); + const requireCall = this.createModuleRequire(statement.moduleSpecifier as ts.StringLiteral, shouldResolve); if (!statement.importClause) { - const requireCall = this.createModuleRequire(statement.moduleSpecifier as ts.StringLiteral); result.push(tstl.createExpressionStatement(requireCall)); if (scope.importStatements) { scope.importStatements.push(...result); @@ -376,28 +393,6 @@ export class LuaTransformer { throw TSTLErrors.UnsupportedImportType(statement.importClause); } - let shouldResolve = true; - if (ts.isNamedImports(imports)) { - for (const importSpecifier of imports.elements) { - const parentModule = tsHelper.getImportSpecifierModuleDeclaration(importSpecifier, this.checker); - if (parentModule) { - const type = this.checker.getTypeAtLocation(parentModule); - const decorators = tsHelper.getCustomDecorators(type, this.checker); - if (decorators.has(DecoratorKind.NoResolution)) { - shouldResolve = false; - break; - } - } - } - } else if (ts.isNamespaceImport(imports)) { - const type = this.checker.getTypeAtLocation(imports); - if (tsHelper.getCustomDecorators(type, this.checker).has(DecoratorKind.NoResolution)) { - shouldResolve = false; - } - } - - const requireCall = this.createModuleRequire(statement.moduleSpecifier as ts.StringLiteral, shouldResolve); - if (ts.isNamedImports(imports)) { const filteredElements = imports.elements.filter(e => { const decorators = tsHelper.getCustomDecorators(this.checker.getTypeAtLocation(e), this.checker); diff --git a/src/TSHelper.ts b/src/TSHelper.ts index 47941a708..bc480d653 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -358,27 +358,6 @@ export class TSHelper { return directivesMap; } - public static getImportSpecifierModuleDeclaration( - node: ts.ImportSpecifier, - checker: ts.TypeChecker - ): ts.ModuleDeclaration | undefined { - const symbol = checker.getSymbolAtLocation(node.name); - if (symbol) { - const originalSymbol = checker.getAliasedSymbol(symbol); - if (originalSymbol.declarations) { - for (const declaration of originalSymbol.declarations) { - const parentAmbientModule = this.findFirstNodeAbove( - declaration, - this.isNonNamespaceModuleDeclaration - ); - if (parentAmbientModule) { - return parentAmbientModule; - } - } - } - } - } - // Search up until finding a node satisfying the callback public static findFirstNodeAbove( node: ts.Node, diff --git a/test/unit/require.spec.ts b/test/unit/require.spec.ts index 62b9cd539..3f864b3fc 100644 --- a/test/unit/require.spec.ts +++ b/test/unit/require.spec.ts @@ -151,6 +151,21 @@ test.each([ mainCode: "import { y } from 'fake'; y;", expectedPath: "src.fake", }, + { + declarationStatement: ` + declare module 'fake' {} + `, + mainCode: "import 'fake';", + expectedPath: "src.fake", + }, + { + declarationStatement: ` + /** @noResolution */ + declare module 'fake' {} + `, + mainCode: "import 'fake';", + expectedPath: "fake", + }, ])("noResolution prevents any module path resolution behaviour", ({ declarationStatement, mainCode, expectedPath }) => { const lua = util.transpileString({ "src/main.ts": mainCode, From 073592d6a73a0b8c26827f46f103d7c494d3c429 Mon Sep 17 00:00:00 2001 From: hazzard993 Date: Wed, 19 Jun 2019 23:19:24 +1000 Subject: [PATCH 3/4] Use collectCustomDecorators and remove isNonNamespaceModuleDeclaration --- src/LuaTransformer.ts | 17 +++++------------ src/TSHelper.ts | 4 ---- 2 files changed, 5 insertions(+), 16 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index d6b6a3700..beeef4890 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -1,7 +1,7 @@ import * as path from "path"; import * as ts from "typescript"; import { CompilerOptions, LuaTarget } from "./CompilerOptions"; -import { DecoratorKind } from "./Decorator"; +import { Decorator, DecoratorKind } from "./Decorator"; import * as tstl from "./LuaAST"; import { LuaLibFeature } from "./LuaLib"; import { ContextType, TSHelper as tsHelper } from "./TSHelper"; @@ -360,17 +360,10 @@ export class LuaTransformer { let shouldResolve = true; const moduleOwnerSymbol = this.checker.getSymbolAtLocation(statement.moduleSpecifier); if (moduleOwnerSymbol) { - for (const declaration of moduleOwnerSymbol.declarations) { - if (tsHelper.isNonNamespaceModuleDeclaration(declaration)) { - const type = this.checker.getTypeAtLocation(declaration); - if (type) { - const decorators = tsHelper.getCustomDecorators(type, this.checker); - if (decorators.has(DecoratorKind.NoResolution)) { - shouldResolve = false; - break; - } - } - } + const decMap = new Map(); + tsHelper.collectCustomDecorators(moduleOwnerSymbol, this.checker, decMap); + if (decMap.has(DecoratorKind.NoResolution)) { + shouldResolve = false; } } diff --git a/src/TSHelper.ts b/src/TSHelper.ts index bc480d653..eeb6373e1 100644 --- a/src/TSHelper.ts +++ b/src/TSHelper.ts @@ -131,10 +131,6 @@ export class TSHelper { return !((ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Ambient) === 0); } - public static isNonNamespaceModuleDeclaration(node: ts.Node): node is ts.ModuleDeclaration { - return ts.isModuleDeclaration(node) && (node.flags & ts.NodeFlags.Namespace) === 0; - } - public static isStatic(node: ts.Node): boolean { return node.modifiers !== undefined && node.modifiers.some(m => m.kind === ts.SyntaxKind.StaticKeyword); } From 37a6fbe6076f448efc56389bc1f29437e1b9e278 Mon Sep 17 00:00:00 2001 From: hazzard993 Date: Thu, 20 Jun 2019 08:41:35 +1000 Subject: [PATCH 4/4] Change decorator map abbreviation --- src/LuaTransformer.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/LuaTransformer.ts b/src/LuaTransformer.ts index beeef4890..172687da3 100644 --- a/src/LuaTransformer.ts +++ b/src/LuaTransformer.ts @@ -360,9 +360,9 @@ export class LuaTransformer { let shouldResolve = true; const moduleOwnerSymbol = this.checker.getSymbolAtLocation(statement.moduleSpecifier); if (moduleOwnerSymbol) { - const decMap = new Map(); - tsHelper.collectCustomDecorators(moduleOwnerSymbol, this.checker, decMap); - if (decMap.has(DecoratorKind.NoResolution)) { + const decorators = new Map(); + tsHelper.collectCustomDecorators(moduleOwnerSymbol, this.checker, decorators); + if (decorators.has(DecoratorKind.NoResolution)) { shouldResolve = false; } }