Improve some diagnostic code spans

ark120202 · ark120202 · commit e533ed7d8a91 · 2020-03-09T15:59:19.000Z
diff --git a/src/transformation/builtins/array.ts b/src/transformation/builtins/array.ts
@@ -79,7 +79,7 @@ export function transformArrayPrototypeCall(
         case "flatMap":
             return transformLuaLibFunction(context, LuaLibFeature.ArrayFlatMap, node, caller, ...params);
         default:
-            context.diagnostics.push(unsupportedProperty(node, "array", expressionName));
+            context.diagnostics.push(unsupportedProperty(expression.name, "array", expressionName));
     }
 }
 
diff --git a/src/transformation/builtins/console.ts b/src/transformation/builtins/console.ts
@@ -61,6 +61,6 @@ export function transformConsoleCall(
             );
             return lua.createCallExpression(lua.createIdentifier("print"), [debugTracebackCall]);
         default:
-            context.diagnostics.push(unsupportedProperty(expression, "console", methodName));
+            context.diagnostics.push(unsupportedProperty(method.name, "console", methodName));
     }
 }
diff --git a/src/transformation/builtins/function.ts b/src/transformation/builtins/function.ts
@@ -27,6 +27,6 @@ export function transformFunctionPrototypeCall(
         case "call":
             return transformLuaLibFunction(context, LuaLibFeature.FunctionCall, node, caller, ...params);
         default:
-            context.diagnostics.push(unsupportedProperty(node, "function", expressionName));
+            context.diagnostics.push(unsupportedProperty(expression.name, "function", expressionName));
     }
 }
diff --git a/src/transformation/builtins/math.ts b/src/transformation/builtins/math.ts
@@ -26,7 +26,7 @@ export function transformMathProperty(
             return lua.createNumericLiteral(Math[name], node);
 
         default:
-            context.diagnostics.push(unsupportedProperty(node, "Math", name));
+            context.diagnostics.push(unsupportedProperty(node.name, "Math", name));
     }
 }
 
@@ -99,6 +99,6 @@ export function transformMathCall(
         }
 
         default:
-            context.diagnostics.push(unsupportedProperty(expression, "Math", expressionName));
+            context.diagnostics.push(unsupportedProperty(expression.name, "Math", expressionName));
     }
 }
diff --git a/src/transformation/builtins/number.ts b/src/transformation/builtins/number.ts
@@ -20,7 +20,7 @@ export function transformNumberPrototypeCall(
                 ? lua.createCallExpression(lua.createIdentifier("tostring"), [caller], node)
                 : transformLuaLibFunction(context, LuaLibFeature.NumberToString, node, caller, ...params);
         default:
-            context.diagnostics.push(unsupportedProperty(node, "number", expressionName));
+            context.diagnostics.push(unsupportedProperty(expression.name, "number", expressionName));
     }
 }
 
@@ -37,6 +37,6 @@ export function transformNumberConstructorCall(
         case "isFinite":
             return transformLuaLibFunction(context, LuaLibFeature.NumberIsFinite, expression, ...parameters);
         default:
-            context.diagnostics.push(unsupportedProperty(expression, "Number", methodName));
+            context.diagnostics.push(unsupportedProperty(method.name, "Number", methodName));
     }
 }
diff --git a/src/transformation/builtins/object.ts b/src/transformation/builtins/object.ts
@@ -24,7 +24,7 @@ export function transformObjectConstructorCall(
         case "values":
             return transformLuaLibFunction(context, LuaLibFeature.ObjectValues, expression, ...parameters);
         default:
-            context.diagnostics.push(unsupportedProperty(expression, "Object", methodName));
+            context.diagnostics.push(unsupportedProperty(method.name, "Object", methodName));
     }
 }
 
diff --git a/src/transformation/builtins/string.ts b/src/transformation/builtins/string.ts
@@ -110,7 +110,7 @@ export function transformStringPrototypeCall(
         case "padEnd":
             return transformLuaLibFunction(context, LuaLibFeature.StringPadEnd, node, caller, ...params);
         default:
-            context.diagnostics.push(unsupportedProperty(node, "string", expressionName));
+            context.diagnostics.push(unsupportedProperty(expression.name, "string", expressionName));
     }
 }
 
@@ -132,7 +132,7 @@ export function transformStringConstructorCall(
             );
 
         default:
-            context.diagnostics.push(unsupportedProperty(node, "String", expressionName));
+            context.diagnostics.push(unsupportedProperty(expression.name, "String", expressionName));
     }
 }
 
@@ -145,6 +145,6 @@ export function transformStringProperty(
             const expression = context.transformExpression(node.expression);
             return lua.createUnaryExpression(expression, lua.SyntaxKind.LengthOperator, node);
         default:
-            context.diagnostics.push(unsupportedProperty(node, "string", node.name.text));
+            context.diagnostics.push(unsupportedProperty(node.name, "string", node.name.text));
     }
 }
diff --git a/src/transformation/builtins/symbol.ts b/src/transformation/builtins/symbol.ts
@@ -20,6 +20,6 @@ export function transformSymbolConstructorCall(
             const functionIdentifier = lua.createIdentifier(`__TS__SymbolRegistry${upperMethodName}`);
             return lua.createCallExpression(functionIdentifier, parameters, expression);
         default:
-            context.diagnostics.push(unsupportedProperty(expression, "Symbol", methodName));
+            context.diagnostics.push(unsupportedProperty(method.name, "Symbol", methodName));
     }
 }
diff --git a/src/transformation/utils/diagnostics.ts b/src/transformation/utils/diagnostics.ts
@@ -97,12 +97,12 @@ export const luaIteratorForbiddenUsage = createDiagnosticFactory(
         "the '@tupleReturn' annotation."
 );
 
-export const unsupportedNullishCoalescing = createDiagnosticFactory("Nullish coalescing is not supported.");
-
 export const unsupportedAccessorInObjectLiteral = createDiagnosticFactory(
     "Accessors in object literal are not supported."
 );
 
+export const unsupportedNullishCoalescing = createDiagnosticFactory("Nullish coalescing is not supported.");
+
 export const unsupportedRightShiftOperator = createDiagnosticFactory(
     "Right shift operator is not supported. Use `>>>` instead."
 );
@@ -117,10 +117,6 @@ export const unsupportedProperty = createDiagnosticFactory(
     (parentName: string, property: string) => `${parentName}.${property} is unsupported.`
 );
 
-export const forOfUnsupportedObjectDestructuring = createDiagnosticFactory(
-    `Unsupported object destructuring in for...of statement.`
-);
-
 export const invalidAmbientIdentifierName = createDiagnosticFactory(
     (text: string) => `Invalid ambient identifier name '${text}'. Ambient identifiers must be valid lua identifiers.`
 );
diff --git a/src/transformation/utils/safe-names.ts b/src/transformation/utils/safe-names.ts
@@ -55,6 +55,19 @@ const luaBuiltins: ReadonlySet<string> = new Set([
 
 export const isUnsafeName = (name: string) => !isValidLuaIdentifier(name) || luaBuiltins.has(name);
 
+function checkName(context: TransformationContext, name: string, node: ts.Node): boolean {
+    const isInvalid = !isValidLuaIdentifier(name);
+
+    if (isInvalid) {
+        // Empty identifier is a TypeScript error
+        if (name !== "") {
+            context.diagnostics.push(invalidAmbientIdentifierName(node, name));
+        }
+    }
+
+    return isInvalid;
+}
+
 export function hasUnsafeSymbolName(
     context: TransformationContext,
     symbol: ts.Symbol,
@@ -63,8 +76,7 @@ export function hasUnsafeSymbolName(
     const isAmbient = symbol.declarations && symbol.declarations.some(d => isAmbientNode(d));
 
     // Catch ambient declarations of identifiers with bad names
-    if (!isValidLuaIdentifier(symbol.name) && isAmbient) {
-        context.diagnostics.push(invalidAmbientIdentifierName(tsOriginal, symbol.name));
+    if (isAmbient && checkName(context, symbol.name, tsOriginal)) {
         return true;
     }
 
@@ -84,12 +96,7 @@ export function hasUnsafeIdentifierName(
         }
     }
 
-    if (!isValidLuaIdentifier(identifier.text)) {
-        context.diagnostics.push(invalidAmbientIdentifierName(identifier, identifier.text));
-        return true;
-    }
-
-    return false;
+    return checkName(context, identifier.text, identifier);
 }
 
 const fixInvalidLuaIdentifier = (name: string) =>
diff --git a/src/transformation/visitors/lua-table.ts b/src/transformation/visitors/lua-table.ts
@@ -70,7 +70,7 @@ export function transformLuaTableExpressionStatement(
                 expression
             );
         default:
-            context.diagnostics.push(unsupportedProperty(expression, "LuaTable", methodName));
+            context.diagnostics.push(unsupportedProperty(expression.expression.name, "LuaTable", methodName));
     }
 }
 
@@ -93,7 +93,7 @@ export function transformLuaTableCallExpression(
         case "get":
             return lua.createTableIndexExpression(luaTable, params[0] ?? lua.createNilLiteral(), node);
         default:
-            context.diagnostics.push(unsupportedProperty(node, "LuaTable", methodName));
+            context.diagnostics.push(unsupportedProperty(node.expression.name, "LuaTable", methodName));
     }
 }
 
@@ -109,7 +109,7 @@ export function transformLuaTablePropertyAccessExpression(
         return lua.createUnaryExpression(luaTable, lua.SyntaxKind.LengthOperator, node);
     }
 
-    context.diagnostics.push(unsupportedProperty(node, "LuaTable", propertyName));
+    context.diagnostics.push(unsupportedProperty(node.name, "LuaTable", propertyName));
 }
 
 export function transformLuaTablePropertyAccessInAssignment(
@@ -127,7 +127,7 @@ export function transformLuaTablePropertyAccessInAssignment(
         return lua.createTableIndexExpression(luaTable, lua.createStringLiteral(propertyName), node);
     }
 
-    context.diagnostics.push(unsupportedProperty(node, "LuaTable", propertyName));
+    context.diagnostics.push(unsupportedProperty(node.name, "LuaTable", propertyName));
 }
 
 export function validateLuaTableElementAccessExpression(
diff --git a/src/transformation/visitors/variable-declaration.ts b/src/transformation/visitors/variable-declaration.ts
@@ -237,7 +237,9 @@ export function transformVariableDeclaration(
 
 export function checkVariableDeclarationList(context: TransformationContext, node: ts.VariableDeclarationList): void {
     if ((node.flags & (ts.NodeFlags.Let | ts.NodeFlags.Const)) === 0) {
-        context.diagnostics.push(unsupportedVarDeclaration(node));
+        const token = node.getFirstToken();
+        assert(token);
+        context.diagnostics.push(unsupportedVarDeclaration(token));
     }
 }
 
diff --git a/test/unit/__snapshots__/assignments.spec.ts.snap b/test/unit/__snapshots__/assignments.spec.ts.snap
@@ -0,0 +1,47 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`var declaration in for loop is disallowed: code 1`] = `
+"local ____exports = {}
+function ____exports.__main(self)
+    do
+        local foo = 0
+        while true do
+        end
+    end
+end
+return ____exports"
+`;
+
+exports[`var declaration in for loop is disallowed: diagnostics 1`] = `"main.ts(2,14): error TSTL: \`var\` declarations are not supported. Use \`let\` or \`const\` instead."`;
+
+exports[`var declaration in for...in loop is disallowed: code 1`] = `
+"local ____exports = {}
+function ____exports.__main(self)
+    for foo in pairs({}) do
+    end
+end
+return ____exports"
+`;
+
+exports[`var declaration in for...in loop is disallowed: diagnostics 1`] = `"main.ts(2,14): error TSTL: \`var\` declarations are not supported. Use \`let\` or \`const\` instead."`;
+
+exports[`var declaration in for...of loop is disallowed: code 1`] = `
+"local ____exports = {}
+function ____exports.__main(self)
+    for ____, foo in ipairs({}) do
+    end
+end
+return ____exports"
+`;
+
+exports[`var declaration in for...of loop is disallowed: diagnostics 1`] = `"main.ts(2,14): error TSTL: \`var\` declarations are not supported. Use \`let\` or \`const\` instead."`;
+
+exports[`var declaration is disallowed: code 1`] = `
+"local ____exports = {}
+function ____exports.__main(self)
+    local foo = true
+end
+return ____exports"
+`;
+
+exports[`var declaration is disallowed: diagnostics 1`] = `"main.ts(2,9): error TSTL: \`var\` declarations are not supported. Use \`let\` or \`const\` instead."`;
diff --git a/test/unit/__snapshots__/identifiers.spec.ts.snap b/test/unit/__snapshots__/identifiers.spec.ts.snap
@@ -128,6 +128,14 @@ exports[`ambient identifier must be a valid lua identifier (object literal short
 
 exports[`ambient identifier must be a valid lua identifier (object literal shorthand) ("ɥɣɎɌͼƛಠ"): diagnostics 1`] = `"main.ts(3,27): error TSTL: Invalid ambient identifier name 'ɥɣɎɌͼƛಠ'. Ambient identifiers must be valid lua identifiers."`;
 
+exports[`missing expression ("({})[]"): code 1`] = `"local ____ = ({})[____]"`;
+
+exports[`missing expression ("({})[]"): diagnostics 1`] = `"main.ts(1,6): error TS1011: An element access expression should take an argument."`;
+
+exports[`missing expression ("const x = ;"): code 1`] = `"x = ____"`;
+
+exports[`missing expression ("const x = ;"): diagnostics 1`] = `"main.ts(1,11): error TS1109: Expression expected."`;
+
 exports[`undeclared identifier must be a valid lua identifier ("$$"): code 1`] = `"foo = _____24_24_24"`;
 
 exports[`undeclared identifier must be a valid lua identifier ("$$"): diagnostics 1`] = `"main.ts(2,21): error TSTL: Invalid ambient identifier name '$$$'. Ambient identifiers must be valid lua identifiers."`;
diff --git a/test/unit/assignments.spec.ts b/test/unit/assignments.spec.ts
@@ -21,25 +21,25 @@ test.each(["const", "let"])("%s declaration top-level is global", declarationKin
 test("var declaration is disallowed", () => {
     util.testFunction`
         var foo = true;
-    `.expectToHaveDiagnostics();
+    `.expectDiagnosticsToMatchSnapshot();
 });
 
 test("var declaration in for loop is disallowed", () => {
     util.testFunction`
         for (var foo = 0;;) {}
-    `.expectToHaveDiagnostics();
+    `.expectDiagnosticsToMatchSnapshot();
 });
 
 test("var declaration in for...in loop is disallowed", () => {
     util.testFunction`
         for (var foo in {}) {}
-    `.expectToHaveDiagnostics();
+    `.expectDiagnosticsToMatchSnapshot();
 });
 
 test("var declaration in for...of loop is disallowed", () => {
     util.testFunction`
         for (var foo of []) {}
-    `.expectToHaveDiagnostics();
+    `.expectDiagnosticsToMatchSnapshot();
 });
 
 test.each(["let myvar;", "const myvar = null;", "const myvar = undefined;"])("Null assignments (%p)", declaration => {
diff --git a/test/unit/classes/__snapshots__/classes.spec.ts.snap b/test/unit/classes/__snapshots__/classes.spec.ts.snap
@@ -1,5 +1,15 @@
 // Jest Snapshot v1, https://goo.gl/fbAQLP
 
+exports[`missing declaration name: code 1`] = `
+"require(\\"lualib_bundle\\");
+____ = __TS__Class()
+____.name = \\"____\\"
+function ____.prototype.____constructor(self)
+end"
+`;
+
+exports[`missing declaration name: diagnostics 1`] = `"main.ts(2,9): error TS1211: A class declaration without the 'default' modifier must have a name."`;
+
 exports[`super without class: code 1`] = `
 "local ____exports = {}
 ____exports.__result = ____.____constructor(self)
diff --git a/test/unit/classes/classes.spec.ts b/test/unit/classes/classes.spec.ts
@@ -849,3 +849,9 @@ test("Class field override in subclass with constructors", () => {
         return (new Foo()).field + (new Bar()).field;`;
     expect(util.transpileAndExecute(code)).toBe("foobar");
 });
+
+test("missing declaration name", () => {
+    util.testModule`
+        class {}
+    `.expectDiagnosticsToMatchSnapshot();
+});
diff --git a/test/unit/functions/__snapshots__/functions.spec.ts.snap b/test/unit/functions/__snapshots__/functions.spec.ts.snap
@@ -0,0 +1,8 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`missing declaration name: code 1`] = `
+"function ____(self)
+end"
+`;
+
+exports[`missing declaration name: diagnostics 1`] = `"main.ts(2,18): error TS1003: Identifier expected."`;
diff --git a/test/unit/functions/functions.spec.ts b/test/unit/functions/functions.spec.ts
@@ -496,3 +496,9 @@ test("named function expression reference", () => {
         return y();
     `.expectToMatchJsResult();
 });
+
+test("missing declaration name", () => {
+    util.testModule`
+        function () {}
+    `.expectDiagnosticsToMatchSnapshot();
+});
diff --git a/test/unit/identifiers.spec.ts b/test/unit/identifiers.spec.ts
@@ -101,6 +101,10 @@ test.each([
     `.expectDiagnosticsToMatchSnapshot();
 });
 
+test.each(["const x = ;", "({})[]"])("missing expression (%p)", statement => {
+    util.testModule(statement).expectDiagnosticsToMatchSnapshot();
+});
+
 test.each(validTsInvalidLuaNames)(
     "ambient identifier must be a valid lua identifier (object literal shorthand) (%p)",
     name => {

Original file line number	Diff line number	Diff line change
`@@ -79,7 +79,7 @@ export function transformArrayPrototypeCall(`
`79`	`79`	`case "flatMap":`
`80`	`80`	`return transformLuaLibFunction(context, LuaLibFeature.ArrayFlatMap, node, caller, ...params);`
`81`	`81`	`default:`
`82`		`- context.diagnostics.push(unsupportedProperty(node, "array", expressionName));`
	`82`	`+ context.diagnostics.push(unsupportedProperty(expression.name, "array", expressionName));`
`83`	`83`	`}`
`84`	`84`	`}`
`85`	`85`
Original file line number	Diff line number	Diff line change
`@@ -61,6 +61,6 @@ export function transformConsoleCall(`
`61`	`61`	`);`
`62`	`62`	`return lua.createCallExpression(lua.createIdentifier("print"), [debugTracebackCall]);`
`63`	`63`	`default:`
`64`		`- context.diagnostics.push(unsupportedProperty(expression, "console", methodName));`
	`64`	`+ context.diagnostics.push(unsupportedProperty(method.name, "console", methodName));`
`65`	`65`	`}`
`66`	`66`	`}`
Original file line number	Diff line number	Diff line change
`@@ -27,6 +27,6 @@ export function transformFunctionPrototypeCall(`
`27`	`27`	`case "call":`
`28`	`28`	`return transformLuaLibFunction(context, LuaLibFeature.FunctionCall, node, caller, ...params);`
`29`	`29`	`default:`
`30`		`- context.diagnostics.push(unsupportedProperty(node, "function", expressionName));`
	`30`	`+ context.diagnostics.push(unsupportedProperty(expression.name, "function", expressionName));`
`31`	`31`	`}`
`32`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -26,7 +26,7 @@ export function transformMathProperty(`
`26`	`26`	`return lua.createNumericLiteral(Math[name], node);`
`27`	`27`
`28`	`28`	`default:`
`29`		`- context.diagnostics.push(unsupportedProperty(node, "Math", name));`
	`29`	`+ context.diagnostics.push(unsupportedProperty(node.name, "Math", name));`
`30`	`30`	`}`
`31`	`31`	`}`
`32`	`32`
`@@ -99,6 +99,6 @@ export function transformMathCall(`
`99`	`99`	`}`
`100`	`100`
`101`	`101`	`default:`
`102`		`- context.diagnostics.push(unsupportedProperty(expression, "Math", expressionName));`
	`102`	`+ context.diagnostics.push(unsupportedProperty(expression.name, "Math", expressionName));`
`103`	`103`	`}`
`104`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,7 +20,7 @@ export function transformNumberPrototypeCall(`
`20`	`20`	`? lua.createCallExpression(lua.createIdentifier("tostring"), [caller], node)`
`21`	`21`	`: transformLuaLibFunction(context, LuaLibFeature.NumberToString, node, caller, ...params);`
`22`	`22`	`default:`
`23`		`- context.diagnostics.push(unsupportedProperty(node, "number", expressionName));`
	`23`	`+ context.diagnostics.push(unsupportedProperty(expression.name, "number", expressionName));`
`24`	`24`	`}`
`25`	`25`	`}`
`26`	`26`
`@@ -37,6 +37,6 @@ export function transformNumberConstructorCall(`
`37`	`37`	`case "isFinite":`
`38`	`38`	`return transformLuaLibFunction(context, LuaLibFeature.NumberIsFinite, expression, ...parameters);`
`39`	`39`	`default:`
`40`		`- context.diagnostics.push(unsupportedProperty(expression, "Number", methodName));`
	`40`	`+ context.diagnostics.push(unsupportedProperty(method.name, "Number", methodName));`
`41`	`41`	`}`
`42`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -24,7 +24,7 @@ export function transformObjectConstructorCall(`
`24`	`24`	`case "values":`
`25`	`25`	`return transformLuaLibFunction(context, LuaLibFeature.ObjectValues, expression, ...parameters);`
`26`	`26`	`default:`
`27`		`- context.diagnostics.push(unsupportedProperty(expression, "Object", methodName));`
	`27`	`+ context.diagnostics.push(unsupportedProperty(method.name, "Object", methodName));`
`28`	`28`	`}`
`29`	`29`	`}`
`30`	`30`
Original file line number	Diff line number	Diff line change
`@@ -110,7 +110,7 @@ export function transformStringPrototypeCall(`
`110`	`110`	`case "padEnd":`
`111`	`111`	`return transformLuaLibFunction(context, LuaLibFeature.StringPadEnd, node, caller, ...params);`
`112`	`112`	`default:`
`113`		`- context.diagnostics.push(unsupportedProperty(node, "string", expressionName));`
	`113`	`+ context.diagnostics.push(unsupportedProperty(expression.name, "string", expressionName));`
`114`	`114`	`}`
`115`	`115`	`}`
`116`	`116`
`@@ -132,7 +132,7 @@ export function transformStringConstructorCall(`
`132`	`132`	`);`
`133`	`133`
`134`	`134`	`default:`
`135`		`- context.diagnostics.push(unsupportedProperty(node, "String", expressionName));`
	`135`	`+ context.diagnostics.push(unsupportedProperty(expression.name, "String", expressionName));`
`136`	`136`	`}`
`137`	`137`	`}`
`138`	`138`
`@@ -145,6 +145,6 @@ export function transformStringProperty(`
`145`	`145`	`const expression = context.transformExpression(node.expression);`
`146`	`146`	`return lua.createUnaryExpression(expression, lua.SyntaxKind.LengthOperator, node);`
`147`	`147`	`default:`
`148`		`- context.diagnostics.push(unsupportedProperty(node, "string", node.name.text));`
	`148`	`+ context.diagnostics.push(unsupportedProperty(node.name, "string", node.name.text));`
`149`	`149`	`}`
`150`	`150`	`}`
Original file line number	Diff line number	Diff line change
`@@ -20,6 +20,6 @@ export function transformSymbolConstructorCall(`
`20`	`20`	const functionIdentifier = lua.createIdentifier(`__TS__SymbolRegistry${upperMethodName}`);
`21`	`21`	`return lua.createCallExpression(functionIdentifier, parameters, expression);`
`22`	`22`	`default:`
`23`		`- context.diagnostics.push(unsupportedProperty(expression, "Symbol", methodName));`
	`23`	`+ context.diagnostics.push(unsupportedProperty(method.name, "Symbol", methodName));`
`24`	`24`	`}`
`25`	`25`	`}`