Skip to content

Commit 680585e

Browse files
committed
Add diagnostic reporting and convert forbidden for...in array error
1 parent 3bd4333 commit 680585e

10 files changed

Lines changed: 57 additions & 33 deletions

File tree

src/cli/report.ts

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,9 @@
11
import * as ts from "typescript";
22

3+
export const prepareDiagnosticForFormatting = (diagnostic: ts.Diagnostic) =>
4+
diagnostic.source === "typescript-to-lua" ? { ...diagnostic, code: "TL" as any } : diagnostic;
5+
36
export function createDiagnosticReporter(pretty: boolean, system = ts.sys): ts.DiagnosticReporter {
47
const reporter = ts.createDiagnosticReporter(system, pretty);
5-
return diagnostic => {
6-
if (diagnostic.source === "typescript-to-lua") {
7-
diagnostic = { ...diagnostic, code: ("TL" + diagnostic.code) as any };
8-
}
9-
10-
reporter(diagnostic);
11-
};
8+
return diagnostic => reporter(prepareDiagnosticForFormatting(diagnostic));
129
}

src/transformation/context/context.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ export interface DiagnosticsProducingTypeChecker extends ts.TypeChecker {
1818
}
1919

2020
export class TransformationContext {
21+
public readonly diagnostics: ts.Diagnostic[] = [];
2122
public readonly checker: DiagnosticsProducingTypeChecker = (this
2223
.program as any).getDiagnosticsProducingTypeChecker();
2324
public readonly resolver: EmitResolver;

src/transformation/index.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,15 @@ export function transformSourceFile(
5353
): TransformSourceFileResult {
5454
const context = new TransformationContext(program, sourceFile, visitorMap);
5555

56+
// TODO: Remove once we'll get rid of all `TranspileError`s
5657
try {
5758
const [luaAst] = context.transformNode(sourceFile) as [lua.Block];
58-
const luaLibFeatures = getUsedLuaLibFeatures(context);
5959

60-
return { luaAst, luaLibFeatures, diagnostics: [] };
60+
return {
61+
luaAst,
62+
luaLibFeatures: getUsedLuaLibFeatures(context),
63+
diagnostics: context.diagnostics,
64+
};
6165
} catch (error) {
6266
if (!(error instanceof TranspileError)) throw error;
6367

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
import * as ts from "typescript";
2+
3+
const createDiagnosticFactory = <TArgs extends any[]>(
4+
message: string | ((...args: TArgs) => string),
5+
category = ts.DiagnosticCategory.Error
6+
) => (node: ts.Node, ...args: TArgs): ts.Diagnostic => ({
7+
file: node.getSourceFile(),
8+
start: node.getStart(),
9+
length: node.getWidth(),
10+
category,
11+
code: 0,
12+
source: "typescript-to-lua",
13+
messageText: typeof message === "string" ? message : message(...args),
14+
});
15+
16+
export const forbiddenForIn = createDiagnosticFactory(`Iterating over arrays with 'for ... in' is not allowed.`);

src/transformation/utils/errors.ts

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,6 @@ export class TranspileError extends Error {
1010

1111
const getLuaTargetName = (version: LuaTarget) => (version === LuaTarget.LuaJIT ? "LuaJIT" : `Lua ${version}`);
1212

13-
export const ForbiddenForIn = (node: ts.Node) =>
14-
new TranspileError(`Iterating over arrays with 'for ... in' is not allowed.`, node);
15-
1613
export const ForbiddenLuaTableNonDeclaration = (node: ts.Node) =>
1714
new TranspileError(`Classes with the '@luaTable' annotation must be declared.`, node);
1815

src/transformation/visitors/loops/for-in.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,14 +1,15 @@
11
import * as ts from "typescript";
22
import * as lua from "../../../LuaAST";
33
import { FunctionVisitor } from "../../context";
4-
import { ForbiddenForIn, UnsupportedForInVariable } from "../../utils/errors";
4+
import { forbiddenForIn } from "../../utils/diagnostics";
5+
import { UnsupportedForInVariable } from "../../utils/errors";
56
import { isArrayType } from "../../utils/typescript";
67
import { transformIdentifier } from "../identifier";
78
import { transformLoopBody } from "./body";
89

910
export const transformForInStatement: FunctionVisitor<ts.ForInStatement> = (statement, context) => {
1011
if (isArrayType(context, context.checker.getTypeAtLocation(statement.expression))) {
11-
throw ForbiddenForIn(statement);
12+
context.diagnostics.push(forbiddenForIn(statement));
1213
}
1314

1415
// Transpile expression

test/setup.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
11
import * as ts from "typescript";
2+
import * as tstl from "../src";
23
import * as util from "./util";
34

45
declare global {
@@ -35,11 +36,10 @@ expect.extend({
3536
// @ts-ignore
3637
const matcherHint = this.utils.matcherHint("toHaveDiagnostics", undefined, "", this);
3738

38-
const diagnosticMessages = ts.formatDiagnosticsWithColorAndContext(diagnostics, {
39-
getCurrentDirectory: () => "",
40-
getCanonicalFileName: fileName => fileName,
41-
getNewLine: () => "\n",
42-
});
39+
const diagnosticMessages = ts.formatDiagnosticsWithColorAndContext(
40+
diagnostics.map(tstl.prepareDiagnosticForFormatting),
41+
{ getCurrentDirectory: () => "", getCanonicalFileName: fileName => fileName, getNewLine: () => "\n" }
42+
);
4343

4444
return {
4545
pass: diagnostics.length > 0,
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
// Jest Snapshot v1, https://goo.gl/fbAQLP
2+
3+
exports[`forin[Array] 1`] = `"main.ts(3,9): error TSTL: Iterating over arrays with 'for ... in' is not allowed."`;

test/unit/loops.spec.ts

Lines changed: 6 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,6 @@
11
import * as ts from "typescript";
22
import * as tstl from "../../src";
3-
import {
4-
ForbiddenForIn,
5-
UnsupportedForTarget,
6-
UnsupportedObjectDestructuringInForOf,
7-
} from "../../src/transformation/utils/errors";
3+
import { UnsupportedForTarget, UnsupportedObjectDestructuringInForOf } from "../../src/transformation/utils/errors";
84
import * as util from "../util";
95

106
test.each([{ inp: [0, 1, 2, 3], expected: [1, 2, 3, 4] }])("while (%p)", ({ inp, expected }) => {
@@ -235,15 +231,11 @@ test.each([
235231
expect(JSON.parse(result)).toEqual(expected);
236232
});
237233

238-
test.each([{ inp: [1, 2, 3] }])("forin[Array] (%p)", ({ inp }) => {
239-
expect(() =>
240-
util.transpileString(
241-
`let arrTest = ${JSON.stringify(inp)};
242-
for (let key in arrTest) {
243-
arrTest[key]++;
244-
}`
245-
)
246-
).toThrowExactError(ForbiddenForIn(util.nodeStub));
234+
test("forin[Array]", () => {
235+
util.testFunction`
236+
const array = [];
237+
for (const key in array) {}
238+
`.expectDiagnosticsToMatchSnapshot();
247239
});
248240

249241
test.each([{ inp: { a: 0, b: 1, c: 2, d: 3, e: 4 }, expected: { a: 0, b: 0, c: 2, d: 0, e: 4 } }])(

test/util.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -363,6 +363,19 @@ export abstract class TestBuilder {
363363
return this;
364364
}
365365

366+
public expectDiagnosticsToMatchSnapshot(): this {
367+
this.expectToHaveDiagnostics();
368+
369+
const diagnosticMessages = ts.formatDiagnostics(
370+
this.getLuaDiagnostics().map(tstl.prepareDiagnosticForFormatting),
371+
{ getCurrentDirectory: () => "", getCanonicalFileName: fileName => fileName, getNewLine: () => "\n" }
372+
);
373+
374+
expect(diagnosticMessages.trim()).toMatchSnapshot();
375+
376+
return this;
377+
}
378+
366379
public expectNoExecutionError(): this {
367380
const luaResult = this.getLuaExecutionResult();
368381
if (luaResult instanceof ExecutionError) {

0 commit comments

Comments
 (0)