Skip to content

Remove tupleReturn annotation #1090

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 2 commits into from
Aug 20, 2021
Merged

Remove tupleReturn annotation #1090

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
0936ac9
Remove tupleReturn annotation
Perryvw Aug 15, 2021
19fc3ca
Change places tuplereturn diagnostics are added
Perryvw Aug 16, 2021
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
42 changes: 1 addition & 41 deletions src/transformation/utils/annotations.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,5 @@
import * as ts from "typescript";
import { TransformationContext } from "../context";
import { annotationDeprecated } from "./diagnostics";
import { findFirstNodeAbove, inferAssignedType } from "./typescript";

export enum AnnotationKind {
Extension = "extension",
Expand Down Expand Up @@ -118,7 +116,6 @@ export function isTupleReturnCall(context: TransformationContext, node: ts.Node)
const signature = context.checker.getResolvedSignature(node);
if (signature) {
if (getSignatureAnnotations(context, signature).has(AnnotationKind.TupleReturn)) {
context.diagnostics.push(annotationDeprecated(node, AnnotationKind.TupleReturn));
return true;
}

Expand All @@ -134,44 +131,7 @@ export function isTupleReturnCall(context: TransformationContext, node: ts.Node)
}

const type = context.checker.getTypeAtLocation(node.expression);
const result = getTypeAnnotations(type).has(AnnotationKind.TupleReturn);

if (result) {
context.diagnostics.push(annotationDeprecated(node, AnnotationKind.TupleReturn));
}

return result;
}

export function isInTupleReturnFunction(context: TransformationContext, node: ts.Node): boolean {
const declaration = findFirstNodeAbove(node, ts.isFunctionLike);
if (!declaration) {
return false;
}

let functionType: ts.Type | undefined;
if (ts.isFunctionExpression(declaration) || ts.isArrowFunction(declaration)) {
functionType = inferAssignedType(context, declaration);
} else if (ts.isMethodDeclaration(declaration) && ts.isObjectLiteralExpression(declaration.parent)) {
// Manually lookup type for object literal properties declared with method syntax
const interfaceType = inferAssignedType(context, declaration.parent);
const propertySymbol = interfaceType.getProperty(declaration.name.getText());
if (propertySymbol) {
functionType = context.checker.getTypeOfSymbolAtLocation(propertySymbol, declaration);
}
}

if (functionType === undefined) {
functionType = context.checker.getTypeAtLocation(declaration);
}

// Check all overloads for directive
const signatures = functionType.getCallSignatures();
if (signatures?.some(s => getSignatureAnnotations(context, s).has(AnnotationKind.TupleReturn))) {
return true;
}

return getTypeAnnotations(functionType).has(AnnotationKind.TupleReturn);
return getTypeAnnotations(type).has(AnnotationKind.TupleReturn);
}

export function isLuaIteratorType(context: TransformationContext, node: ts.Node): boolean {
Expand Down
6 changes: 5 additions & 1 deletion src/transformation/visitors/call.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import * as ts from "typescript";
import * as lua from "../../LuaAST";
import { transformBuiltinCallExpression } from "../builtins";
import { FunctionVisitor, TransformationContext } from "../context";
import { AnnotationKind, getTypeAnnotations } from "../utils/annotations";
import { AnnotationKind, getTypeAnnotations, isTupleReturnCall } from "../utils/annotations";
import { validateAssignment } from "../utils/assignment-validation";
import { ContextType, getDeclarationContextType } from "../utils/function-context";
import { createUnpackCall, wrapInTable } from "../utils/lua-ast";
Expand Down Expand Up @@ -249,6 +249,10 @@ export const transformCallExpression: FunctionVisitor<ts.CallExpression> = (node
return wrapResultInTable ? wrapInTable(builtinResult) : builtinResult;
}

if (isTupleReturnCall(context, node)) {
context.diagnostics.push(annotationRemoved(node, AnnotationKind.TupleReturn));
}

if (isOperatorMapping(context, node)) {
return transformOperatorMappingExpression(context, node);
}
Expand Down
3 changes: 1 addition & 2 deletions src/transformation/visitors/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import * as ts from "typescript";
import * as lua from "../../LuaAST";
import { FunctionVisitor } from "../context";
import { isInTupleReturnFunction } from "../utils/annotations";
import { createUnpackCall } from "../utils/lua-ast";
import { findScope, ScopeType } from "../utils/scope";
import { transformScopeBlock } from "./block";
Expand Down Expand Up @@ -89,7 +88,7 @@ export const transformTryStatement: FunctionVisitor<ts.TryStatement> = (statemen
returnValues.push(lua.createBooleanLiteral(true));
}

if (isInTupleReturnFunction(context, statement) || isInMultiReturnFunction(context, statement)) {
if (isInMultiReturnFunction(context, statement)) {
returnValues.push(createUnpackCall(context, lua.cloneIdentifier(returnValueIdentifier)));
} else {
returnValues.push(lua.cloneIdentifier(returnValueIdentifier));
Expand Down
10 changes: 10 additions & 0 deletions src/transformation/visitors/function.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,8 @@ import * as ts from "typescript";
import * as lua from "../../LuaAST";
import { assert } from "../../utils";
import { FunctionVisitor, TransformationContext } from "../context";
import { AnnotationKind, getNodeAnnotations } from "../utils/annotations";
import { annotationRemoved } from "../utils/diagnostics";
import { createDefaultExportStringLiteral, hasDefaultExportModifier } from "../utils/export";
import { ContextType, getFunctionContextType } from "../utils/function-context";
import {
Expand Down Expand Up @@ -220,6 +222,10 @@ export function transformFunctionLikeDeclaration(
return lua.createNilLiteral();
}

if (getNodeAnnotations(node).has(AnnotationKind.TupleReturn)) {
context.diagnostics.push(annotationRemoved(node, AnnotationKind.TupleReturn));
}

const [functionExpression, functionScope] = transformFunctionToExpression(context, node);

// Handle named function expressions which reference themselves
Expand Down Expand Up @@ -250,6 +256,10 @@ export function transformFunctionLikeDeclaration(
}

export const transformFunctionDeclaration: FunctionVisitor<ts.FunctionDeclaration> = (node, context) => {
if (getNodeAnnotations(node).has(AnnotationKind.TupleReturn)) {
context.diagnostics.push(annotationRemoved(node, AnnotationKind.TupleReturn));
}

// Don't transform functions without body (overload declarations)
if (node.body === undefined) {
return undefined;
Expand Down
8 changes: 2 additions & 6 deletions src/transformation/visitors/language-extensions/multi.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { TransformationContext } from "../../context";
import { findFirstNodeAbove } from "../../utils/typescript";
import { isIterableExpression } from "./iterable";
import { invalidMultiFunctionUse } from "../../utils/diagnostics";
import { isTupleReturnCall } from "../../utils/annotations";

export function isMultiReturnType(type: ts.Type): boolean {
return extensions.isExtensionType(type, extensions.ExtensionKind.MultiType);
Expand All @@ -25,10 +24,7 @@ export function returnsMultiType(context: TransformationContext, node: ts.CallEx
}

export function isMultiReturnCall(context: TransformationContext, expression: ts.Expression) {
return (
(ts.isCallExpression(expression) && returnsMultiType(context, expression)) ||
isTupleReturnCall(context, expression)
);
return ts.isCallExpression(expression) && returnsMultiType(context, expression);
}

export function isMultiFunctionNode(context: TransformationContext, node: ts.Node): boolean {
Expand All @@ -47,7 +43,7 @@ export function isInMultiReturnFunction(context: TransformationContext, node: ts
}

export function shouldMultiReturnCallBeWrapped(context: TransformationContext, node: ts.CallExpression) {
if (!returnsMultiType(context, node) && !isTupleReturnCall(context, node)) {
if (!returnsMultiType(context, node)) {
return false;
}

Expand Down
27 changes: 1 addition & 26 deletions src/transformation/visitors/return.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,19 +1,16 @@
import * as ts from "typescript";
import * as lua from "../../LuaAST";
import { FunctionVisitor, TransformationContext } from "../context";
import { isInTupleReturnFunction } from "../utils/annotations";
import { validateAssignment } from "../utils/assignment-validation";
import { createUnpackCall, wrapInTable } from "../utils/lua-ast";
import { ScopeType, walkScopesUp } from "../utils/scope";
import { isArrayType } from "../utils/typescript";
import { transformArguments } from "./call";
import {
returnsMultiType,
shouldMultiReturnCallBeWrapped,
isMultiFunctionCall,
isMultiReturnType,
isInMultiReturnFunction,
isMultiReturnCall,
canBeMultiReturnType,
} from "./language-extensions/multi";
import { invalidMultiFunctionReturnType } from "../utils/diagnostics";
Expand Down Expand Up @@ -50,29 +47,7 @@ function transformExpressionsInReturn(
return [createUnpackCall(context, context.transformExpression(node), node)];
}

if (!isInTupleReturnFunction(context, node)) {
return [context.transformExpression(node)];
}

let results: lua.Expression[];

// Parent function is a TupleReturn function
if (ts.isArrayLiteralExpression(node)) {
// If return expression is an array literal, leave out brackets.
results = node.elements.map(e => context.transformExpression(e));
} else if (!isMultiReturnCall(context, node) && isArrayType(context, expressionType)) {
// If return expression is an array-type and not another TupleReturn call, unpack it
results = [createUnpackCall(context, context.transformExpression(node), node)];
} else {
results = [context.transformExpression(node)];
}

// Wrap tupleReturn results when returning inside try/catch
if (insideTryCatch) {
results = [wrapInTable(...results)];
}

return results;
return [context.transformExpression(node)];
}

export function transformExpressionBodyToReturnStatement(
Expand Down
40 changes: 40 additions & 0 deletions test/unit/annotations/__snapshots__/deprecated.spec.ts.snap
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,46 @@ __TS__ClassExtends(ClassB, ClassA)"

exports[`pureAbstract removed: diagnostics 1`] = `"main.ts(4,22): error TSTL: '@pureAbstract' has been removed and will no longer have any effect.See https://typescripttolua.github.io/docs/advanced/compiler-annotations#pureabstract for more information."`;

exports[`tuplereturn lambda: code 1`] = `
"local ____exports = {}
function ____exports.__main(self)
local function f()
return {3, 4}
end
end
return ____exports"
`;

exports[`tuplereturn lambda: diagnostics 1`] = `"main.ts(2,39): error TSTL: '@tupleReturn' has been removed and will no longer have any effect.See https://typescripttolua.github.io/docs/advanced/compiler-annotations#tuplereturn for more information."`;

exports[`tuplereturn removed on function declaration: code 1`] = `
"local ____exports = {}
function ____exports.__main(self)
local function tuple(self)
return {3, 5, 1}
end
end
return ____exports"
`;

exports[`tuplereturn removed on function declaration: diagnostics 1`] = `"main.ts(3,9): error TSTL: '@tupleReturn' has been removed and will no longer have any effect.See https://typescripttolua.github.io/docs/advanced/compiler-annotations#tuplereturn for more information."`;

exports[`tuplereturn removed: code 1`] = `
"local ____exports = {}
function ____exports.__main(self)
local function tuple(self)
return {3, 5, 1}
end
return tuple(nil)[3]
end
return ____exports"
`;

exports[`tuplereturn removed: diagnostics 1`] = `
"main.ts(3,9): error TSTL: '@tupleReturn' has been removed and will no longer have any effect.See https://typescripttolua.github.io/docs/advanced/compiler-annotations#tuplereturn for more information.
main.ts(4,16): error TSTL: '@tupleReturn' has been removed and will no longer have any effect.See https://typescripttolua.github.io/docs/advanced/compiler-annotations#tuplereturn for more information."
`;

exports[`vararg removed: code 1`] = `
"function foo(self, ...)
end
Expand Down
21 changes: 21 additions & 0 deletions test/unit/annotations/deprecated.spec.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,27 @@ test("luaiterator removed", () => {
`.expectDiagnosticsToMatchSnapshot([annotationRemoved.code]);
});

test("tuplereturn removed", () => {
util.testFunction`
/** @tupleReturn */
function tuple(): [number, number, number] { return [3, 5, 1]; }
return tuple()[2];
`.expectDiagnosticsToMatchSnapshot([annotationRemoved.code, annotationRemoved.code]); // One annotation on the function, one on the call
});

test("tuplereturn removed on function declaration", () => {
util.testFunction`
/** @tupleReturn */
function tuple(): [number, number, number] { return [3, 5, 1]; }
`.expectDiagnosticsToMatchSnapshot([annotationRemoved.code]);
});

test("tuplereturn lambda", () => {
util.testFunction`
const f = /** @tupleReturn */ () => [3, 4];
`.expectDiagnosticsToMatchSnapshot([annotationRemoved.code]);
Copy link
Member

@lolleko lolleko Aug 15, 2021

Choose a reason for hiding this comment

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

Add a test for just:

        /** @tupleReturn */
         function tuple(): [number, number, number] { return [3, 5, 1]; };

(without the call). This should also result in a diagnostic

});

const tableLibClass = `
/** @luaTable */
declare class Table<K extends {} = {}, V = any> {
Expand Down
Loading