Skip to content

Assume files with imports to be modules #776

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
Perryvw merged 1 commit into from
Dec 26, 2019
Merged

Assume files with imports to be modules #776

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 1 addition & 2 deletions src/transformation/context/context.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import * as ts from "typescript";
import { CompilerOptions, LuaTarget } from "../../CompilerOptions";
import * as lua from "../../LuaAST";
import { unwrapVisitorResult } from "../utils/lua-ast";
import { isFileModule } from "../utils/typescript";
import { ExpressionLikeNode, ObjectVisitor, StatementLikeNode, VisitorMap } from "./visitors";

export interface EmitResolver {
Expand All @@ -23,7 +22,7 @@ export class TransformationContext {

public readonly options: CompilerOptions = this.program.getCompilerOptions();
public readonly luaTarget = this.options.luaTarget ?? LuaTarget.LuaJIT;
public readonly isModule = isFileModule(this.sourceFile);
public readonly isModule = ts.isExternalModule(this.sourceFile);
Copy link
Copy Markdown
Member

@lolleko lolleko Dec 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if all imports are elided is a file still considered a module in that case?

Copy link
Copy Markdown
Contributor Author

@ark120202 ark120202 Dec 19, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes

public readonly isStrict =
(this.options.alwaysStrict ?? this.options.strict) ||
(this.isModule && this.options.target !== undefined && this.options.target >= ts.ScriptTarget.ES2015);
Expand Down
15 changes: 3 additions & 12 deletions src/transformation/utils/export.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import { TransformationContext } from "../context";
import { createModuleLocalNameIdentifier } from "../visitors/namespace";
import { createExportsIdentifier } from "./lua-ast";
import { getSymbolInfo } from "./symbols";
import { findFirstNodeAbove, isFileModule } from "./typescript";
import { findFirstNodeAbove } from "./typescript";

export function hasDefaultExportModifier(node: ts.Node): boolean {
return (node.modifiers ?? []).some(modifier => modifier.kind === ts.SyntaxKind.DefaultKeyword);
Expand Down Expand Up @@ -75,17 +75,8 @@ export function getExportedSymbolsFromScope(
context: TransformationContext,
scope: ts.SourceFile | ts.ModuleDeclaration
): ts.Symbol[] {
if (ts.isSourceFile(scope) && !isFileModule(scope)) {
return [];
}

let scopeSymbol = context.checker.getSymbolAtLocation(scope);
if (scopeSymbol === undefined) {
// TODO: Necessary?
scopeSymbol = context.checker.getTypeAtLocation(scope).getSymbol();
}

if (scopeSymbol === undefined || scopeSymbol.exports === undefined) {
const scopeSymbol = context.checker.getSymbolAtLocation(ts.isSourceFile(scope) ? scope : scope.name);
if (scopeSymbol?.exports === undefined) {
return [];
}

Expand Down
17 changes: 0 additions & 17 deletions src/transformation/utils/typescript/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,28 +1,11 @@
import * as ts from "typescript";
import { TransformationContext } from "../../context";
import { isDeclaration } from "./nodes";

export * from "./nodes";
export * from "./types";

// TODO: Move to separate files?

export function isFileModule(sourceFile: ts.SourceFile): boolean {
return sourceFile.statements.some(isStatementExported);
}

function isStatementExported(statement: ts.Statement): boolean {
if (ts.isExportAssignment(statement) || ts.isExportDeclaration(statement)) {
return true;
}
if (ts.isVariableStatement(statement)) {
return statement.declarationList.declarations.some(
declaration => (ts.getCombinedModifierFlags(declaration) & ts.ModifierFlags.Export) !== 0
);
}
return isDeclaration(statement) && (ts.getCombinedModifierFlags(statement) & ts.ModifierFlags.Export) !== 0;
}

export function hasExportEquals(sourceFile: ts.SourceFile): boolean {
return sourceFile.statements.some(node => ts.isExportAssignment(node) && node.isExportEquals);
}
Expand Down
16 changes: 0 additions & 16 deletions src/transformation/utils/typescript/nodes.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,22 +16,6 @@ export function isAmbientNode(node: ts.Declaration): boolean {
return (ts.getCombinedModifierFlags(node) & ts.ModifierFlags.Ambient) !== 0;
}

export function isDeclaration(node: ts.Node): node is ts.Declaration {
return (
ts.isEnumDeclaration(node) ||
ts.isClassDeclaration(node) ||
ts.isExportDeclaration(node) ||
ts.isImportDeclaration(node) ||
ts.isMethodDeclaration(node) ||
ts.isModuleDeclaration(node) ||
ts.isFunctionDeclaration(node) ||
ts.isVariableDeclaration(node) ||
ts.isInterfaceDeclaration(node) ||
ts.isTypeAliasDeclaration(node) ||
ts.isNamespaceExportDeclaration(node)
);
}

export function isInDestructingAssignment(node: ts.Node): boolean {
return (
node.parent &&
Expand Down
42 changes: 29 additions & 13 deletions test/translation/__snapshots__/transformation.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -126,18 +126,23 @@ end"
`;

exports[`Transformation (modulesImportAll) 1`] = `
"local Test = require(\\"test\\")
local ____ = Test"
"local ____exports = {}
local Test = require(\\"test\\")
local ____ = Test
return ____exports"
`;

exports[`Transformation (modulesImportNamed) 1`] = `
"local ____test = require(\\"test\\")
"local ____exports = {}
local ____test = require(\\"test\\")
local TestClass = ____test.TestClass
local ____ = TestClass"
local ____ = TestClass
return ____exports"
`;

exports[`Transformation (modulesImportNamedSpecialChars) 1`] = `
"local ____kebab_2Dmodule = require(\\"kebab-module\\")
"local ____exports = {}
local ____kebab_2Dmodule = require(\\"kebab-module\\")
local TestClass1 = ____kebab_2Dmodule.TestClass1
local ____dollar_24module = require(\\"dollar$module\\")
local TestClass2 = ____dollar_24module.TestClass2
Expand All @@ -151,17 +156,21 @@ local ____ = TestClass1
local ____ = TestClass2
local ____ = TestClass3
local ____ = TestClass4
local ____ = TestClass5"
local ____ = TestClass5
return ____exports"
`;

exports[`Transformation (modulesImportRenamed) 1`] = `
"local ____test = require(\\"test\\")
"local ____exports = {}
local ____test = require(\\"test\\")
local RenamedClass = ____test.TestClass
local ____ = RenamedClass"
local ____ = RenamedClass
return ____exports"
`;

exports[`Transformation (modulesImportRenamedSpecialChars) 1`] = `
"local ____kebab_2Dmodule = require(\\"kebab-module\\")
"local ____exports = {}
local ____kebab_2Dmodule = require(\\"kebab-module\\")
local RenamedClass1 = ____kebab_2Dmodule.TestClass
local ____dollar_24module = require(\\"dollar$module\\")
local RenamedClass2 = ____dollar_24module.TestClass
Expand All @@ -175,10 +184,15 @@ local ____ = RenamedClass1
local ____ = RenamedClass2
local ____ = RenamedClass3
local ____ = RenamedClass4
local ____ = RenamedClass5"
local ____ = RenamedClass5
return ____exports"
`;

exports[`Transformation (modulesImportWithoutFromClause) 1`] = `"require(\\"test\\")"`;
exports[`Transformation (modulesImportWithoutFromClause) 1`] = `
"local ____exports = {}
require(\\"test\\")
return ____exports"
`;

exports[`Transformation (modulesNamespaceExport) 1`] = `
"local ____exports = {}
Expand Down Expand Up @@ -244,6 +258,8 @@ end"
`;

exports[`Transformation (unusedDefaultWithNamespaceImport) 1`] = `
"local x = require(\\"module\\")
local ____ = x"
"local ____exports = {}
local x = require(\\"module\\")
local ____ = x
return ____exports"
`;
2 changes: 2 additions & 0 deletions test/transpile/__snapshots__/project.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,12 @@ return ____exports
Object {
"name": "index.lua",
"text": "--[[ Generated with https://github.com/TypeScriptToLua/TypeScriptToLua ]]
local ____exports = {}
local ____otherFile = require(\\"otherFile\\")
local getNumber = ____otherFile.getNumber
local myNumber = getNumber(nil)
SetAPIValue(myNumber * 5)
return ____exports
",
},
]
Expand Down
66 changes: 28 additions & 38 deletions test/unit/hoisting.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -243,55 +243,45 @@ test.each([
});

test("Import hoisting (named)", () => {
const importCode = `
const bar = foo;
import {foo} from "myMod";`;
const luaHeader = `
package.loaded["myMod"] = {foo = "foobar"}
${util.transpileString(importCode)}`;
const tsHeader = "declare const bar: any;";
const code = "return bar;";
expect(util.transpileAndExecute(code, undefined, luaHeader, tsHeader)).toBe("foobar");
util.testBundle`
export const result = foo;
import { foo } from "./module";
`
.addExtraFile("module.ts", "export const foo = true;")
.expectToEqual({ result: true });
});

test("Import hoisting (namespace)", () => {
const importCode = `
const bar = myMod.foo;
import * as myMod from "myMod";`;
const luaHeader = `
package.loaded["myMod"] = {foo = "foobar"}
${util.transpileString(importCode)}`;
const tsHeader = "declare const bar: any;";
const code = "return bar;";
expect(util.transpileAndExecute(code, undefined, luaHeader, tsHeader)).toBe("foobar");
util.testBundle`
export const result = module.foo;
import * as module from "./module";
`
.addExtraFile("module.ts", "export const foo = true;")
.expectToEqual({ result: true });
});

test("Import hoisting (side-effect)", () => {
const importCode = `
const bar = foo;
import "myMod";`;
const luaHeader = `
package.loaded["myMod"] = {_ = (function() foo = "foobar" end)()}
${util.transpileString(importCode)}`;
const tsHeader = "declare const bar: any;";
const code = "return bar;";
expect(util.transpileAndExecute(code, undefined, luaHeader, tsHeader)).toBe("foobar");
util.testBundle`
export const result = (globalThis as any).result;
import "./module";
`
.addExtraFile("module.ts", "(globalThis as any).result = true; export {};")
.expectToEqual({ result: true });
});

test("Import hoisted before function", () => {
const importCode = `
let bar: any;
import {foo} from "myMod";
util.testBundle`
export let result: any;

baz();
function baz() {
bar = foo;
}`;
const luaHeader = `
package.loaded["myMod"] = {foo = "foobar"}
${util.transpileString(importCode)}`;
const tsHeader = "declare const bar: any;";
const code = "return bar;";
expect(util.transpileAndExecute(code, undefined, luaHeader, tsHeader)).toBe("foobar");
result = foo;
}

import { foo } from "./module";
`
.addExtraFile("module.ts", "export const foo = true;")
.expectToEqual({ result: true });
});

test("Hoisting Shorthand Property", () => {
Expand Down