From bcba99107cf8ecdf1957d9d3639e6ca2ed6d93d2 Mon Sep 17 00:00:00 2001 From: Zerio Date: Sat, 21 Oct 2023 02:51:49 +0200 Subject: [PATCH 01/13] Cleanup code / make more readable --- src/transformation/visitors/call.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index 679ae6dc1..a0c97ea74 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -262,8 +262,10 @@ export const transformCallExpression: FunctionVisitor = (node let callPath: lua.Expression; let parameters: lua.Expression[]; + const isContextualCall = isContextualCallExpression(context, signature); - if (!isContextualCall) { + + if (isContextualCall) { [callPath, parameters] = transformCallAndArguments(context, calledExpression, node.arguments, signature); } else { // if is optionalContinuation, context will be handled by transformOptionalChain. @@ -287,10 +289,12 @@ export const transformCallExpression: FunctionVisitor = (node function isContextualCallExpression(context: TransformationContext, signature: ts.Signature | undefined): boolean { const declaration = signature?.getDeclaration(); - if (!declaration) { - return !context.options.noImplicitSelf; + + if (declaration) { + return getDeclarationContextType(context, declaration) === ContextType.Void; } - return getDeclarationContextType(context, declaration) !== ContextType.Void; + + return context.options.noImplicitSelf !== undefined ? context.options.noImplicitSelf : false; } export function getCalledExpression(node: ts.CallExpression): ts.Expression { From 80ee20166b99f74464e3f79b4396fccd6a45fc3b Mon Sep 17 00:00:00 2001 From: Zerio Date: Sat, 21 Oct 2023 02:56:24 +0200 Subject: [PATCH 02/13] Further readability fix --- src/transformation/visitors/call.ts | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index a0c97ea74..fabbff1b3 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -288,10 +288,12 @@ export const transformCallExpression: FunctionVisitor = (node }; function isContextualCallExpression(context: TransformationContext, signature: ts.Signature | undefined): boolean { - const declaration = signature?.getDeclaration(); + if (signature) { + const declaration = signature.getDeclaration(); - if (declaration) { - return getDeclarationContextType(context, declaration) === ContextType.Void; + if (declaration) { + return getDeclarationContextType(context, declaration) === ContextType.Void; + } } return context.options.noImplicitSelf !== undefined ? context.options.noImplicitSelf : false; From 7614dd99750f16c5d354d936acf8cfbc8fe80bac Mon Sep 17 00:00:00 2001 From: Zerio Date: Sat, 21 Oct 2023 03:10:02 +0200 Subject: [PATCH 03/13] Fix noSelfInFile check for call visitor --- src/transformation/visitors/call.ts | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index fabbff1b3..f04543fdb 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -15,6 +15,7 @@ import { transformInPrecedingStatementScope } from "../utils/preceding-statement import { getOptionalContinuationData, transformOptionalChain } from "./optional-chaining"; import { transformImportExpression } from "./modules/import"; import { transformLanguageExtensionCallExpression } from "./language-extensions/call-extension"; +import { AnnotationKind, getFileAnnotations } from "../utils/annotations"; export function validateArguments( context: TransformationContext, @@ -245,6 +246,7 @@ export const transformCallExpression: FunctionVisitor = (node } const signature = context.checker.getResolvedSignature(node); + const srcFile = node.getSourceFile(); // Handle super calls properly if (calledExpression.kind === ts.SyntaxKind.SuperKeyword) { @@ -263,7 +265,7 @@ export const transformCallExpression: FunctionVisitor = (node let callPath: lua.Expression; let parameters: lua.Expression[]; - const isContextualCall = isContextualCallExpression(context, signature); + const isContextualCall = isContextualCallExpression(context, signature, srcFile); if (isContextualCall) { [callPath, parameters] = transformCallAndArguments(context, calledExpression, node.arguments, signature); @@ -287,7 +289,11 @@ export const transformCallExpression: FunctionVisitor = (node return wrapResultInTable ? wrapInTable(callExpression) : callExpression; }; -function isContextualCallExpression(context: TransformationContext, signature: ts.Signature | undefined): boolean { +function isContextualCallExpression( + context: TransformationContext, + signature?: ts.Signature, + srcFile?: ts.SourceFile +): boolean { if (signature) { const declaration = signature.getDeclaration(); @@ -296,7 +302,11 @@ function isContextualCallExpression(context: TransformationContext, signature: t } } - return context.options.noImplicitSelf !== undefined ? context.options.noImplicitSelf : false; + if (srcFile && getFileAnnotations(srcFile).has(AnnotationKind.NoSelfInFile)) { + return true; + } + + return context.options.noImplicitSelf ?? false; } export function getCalledExpression(node: ts.CallExpression): ts.Expression { From f3d73a1398c619d58ae2b7b634e2c30d1e245f83 Mon Sep 17 00:00:00 2001 From: Zerio Date: Sun, 22 Oct 2023 02:49:19 +0200 Subject: [PATCH 04/13] Fix/further rewrite of `isContextualCallExpression` --- src/transformation/visitors/call.ts | 24 ++++++++++++++++-------- 1 file changed, 16 insertions(+), 8 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index 9cadda4db..153e007c4 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -15,8 +15,8 @@ import { transformInPrecedingStatementScope } from "../utils/preceding-statement import { getOptionalContinuationData, transformOptionalChain } from "./optional-chaining"; import { transformImportExpression } from "./modules/import"; import { transformLanguageExtensionCallExpression } from "./language-extensions/call-extension"; -import { AnnotationKind, getFileAnnotations } from "../utils/annotations"; import { getCustomNameFromSymbol } from "./identifier"; +import { AnnotationKind, getFileAnnotations } from "../utils/annotations"; export function validateArguments( context: TransformationContext, @@ -251,7 +251,6 @@ export const transformCallExpression: FunctionVisitor = (node } const signature = context.checker.getResolvedSignature(node); - const srcFile = node.getSourceFile(); // Handle super calls properly if (calledExpression.kind === ts.SyntaxKind.SuperKeyword) { @@ -270,9 +269,9 @@ export const transformCallExpression: FunctionVisitor = (node let callPath: lua.Expression; let parameters: lua.Expression[]; - const isContextualCall = isContextualCallExpression(context, signature, srcFile); + const isContextualCall = isContextualCallExpression(context, signature, node.getSourceFile()); - if (isContextualCall) { + if (!isContextualCall) { [callPath, parameters] = transformCallAndArguments(context, calledExpression, node.arguments, signature); } else { // if is optionalContinuation, context will be handled by transformOptionalChain. @@ -299,19 +298,28 @@ function isContextualCallExpression( signature?: ts.Signature, srcFile?: ts.SourceFile ): boolean { + const hasNoSelfInFile = srcFile && getFileAnnotations(srcFile).has(AnnotationKind.NoSelfInFile); + if (signature) { const declaration = signature.getDeclaration(); if (declaration) { - return getDeclarationContextType(context, declaration) === ContextType.Void; + const contextTypeCheck = getDeclarationContextType(context, declaration) !== ContextType.Void; + + // respect implicit self over noSelfInFile + if (hasNoSelfInFile && contextTypeCheck !== true) { + return false; + } + + return contextTypeCheck; } } - if (srcFile && getFileAnnotations(srcFile).has(AnnotationKind.NoSelfInFile)) { - return true; + if (hasNoSelfInFile) { + return false; } - return context.options.noImplicitSelf ?? false; + return !context.options.noImplicitSelf; } export function getCalledExpression(node: ts.CallExpression): ts.Expression { From b775bf3a32121be7560f7fa76271f20b82bcf376 Mon Sep 17 00:00:00 2001 From: Zerio Date: Sun, 22 Oct 2023 03:05:00 +0200 Subject: [PATCH 05/13] Further readability improvements --- src/transformation/visitors/call.ts | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index 153e007c4..5a8f72ad9 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -271,7 +271,7 @@ export const transformCallExpression: FunctionVisitor = (node const isContextualCall = isContextualCallExpression(context, signature, node.getSourceFile()); - if (!isContextualCall) { + if (isContextualCall) { [callPath, parameters] = transformCallAndArguments(context, calledExpression, node.arguments, signature); } else { // if is optionalContinuation, context will be handled by transformOptionalChain. @@ -287,7 +287,7 @@ export const transformCallExpression: FunctionVisitor = (node } const callExpression = lua.createCallExpression(callPath, parameters, node); - if (optionalContinuation && isContextualCall) { + if (optionalContinuation && !isContextualCall) { optionalContinuation.contextualCall = callExpression; } return wrapResultInTable ? wrapInTable(callExpression) : callExpression; @@ -304,11 +304,11 @@ function isContextualCallExpression( const declaration = signature.getDeclaration(); if (declaration) { - const contextTypeCheck = getDeclarationContextType(context, declaration) !== ContextType.Void; + const contextTypeCheck = getDeclarationContextType(context, declaration) === ContextType.Void; // respect implicit self over noSelfInFile - if (hasNoSelfInFile && contextTypeCheck !== true) { - return false; + if (hasNoSelfInFile && contextTypeCheck === true) { + return true; } return contextTypeCheck; @@ -316,10 +316,10 @@ function isContextualCallExpression( } if (hasNoSelfInFile) { - return false; + return true; } - return !context.options.noImplicitSelf; + return context.options.noImplicitSelf ?? false; } export function getCalledExpression(node: ts.CallExpression): ts.Expression { From 6cdddb2926f85b3d300c68fda8d91f412d847e9d Mon Sep 17 00:00:00 2001 From: Zerio Date: Sun, 22 Oct 2023 14:39:21 +0200 Subject: [PATCH 06/13] Change back returns in `isContextualCallExpression` --- src/transformation/visitors/call.ts | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index 5a8f72ad9..1f14085e5 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -271,7 +271,7 @@ export const transformCallExpression: FunctionVisitor = (node const isContextualCall = isContextualCallExpression(context, signature, node.getSourceFile()); - if (isContextualCall) { + if (!isContextualCall) { [callPath, parameters] = transformCallAndArguments(context, calledExpression, node.arguments, signature); } else { // if is optionalContinuation, context will be handled by transformOptionalChain. @@ -287,7 +287,7 @@ export const transformCallExpression: FunctionVisitor = (node } const callExpression = lua.createCallExpression(callPath, parameters, node); - if (optionalContinuation && !isContextualCall) { + if (optionalContinuation && isContextualCall) { optionalContinuation.contextualCall = callExpression; } return wrapResultInTable ? wrapInTable(callExpression) : callExpression; @@ -304,11 +304,12 @@ function isContextualCallExpression( const declaration = signature.getDeclaration(); if (declaration) { - const contextTypeCheck = getDeclarationContextType(context, declaration) === ContextType.Void; + const contextTypeCheck = getDeclarationContextType(context, declaration) !== ContextType.Void; // respect implicit self over noSelfInFile - if (hasNoSelfInFile && contextTypeCheck === true) { - return true; + // look at "explicit this parameter respected over @noSelf" test + if (hasNoSelfInFile && contextTypeCheck !== true) { + return false; } return contextTypeCheck; @@ -316,10 +317,10 @@ function isContextualCallExpression( } if (hasNoSelfInFile) { - return true; + return false; } - return context.options.noImplicitSelf ?? false; + return !context.options.noImplicitSelf; } export function getCalledExpression(node: ts.CallExpression): ts.Expression { From 58bb3f45598d8eae3e1466233f5aeb80b2052d0f Mon Sep 17 00:00:00 2001 From: Zerio Date: Sun, 22 Oct 2023 14:45:29 +0200 Subject: [PATCH 07/13] Add testcase for noSelfInFile over noImplicitSelf --- test/unit/functions/noSelfAnnotation.spec.ts | 27 ++++++++++++++++++++ 1 file changed, 27 insertions(+) diff --git a/test/unit/functions/noSelfAnnotation.spec.ts b/test/unit/functions/noSelfAnnotation.spec.ts index 1125f9cda..65198a433 100644 --- a/test/unit/functions/noSelfAnnotation.spec.ts +++ b/test/unit/functions/noSelfAnnotation.spec.ts @@ -62,3 +62,30 @@ test("explicit this parameter respected over @noSelf", () => { export const result = foo(1); `.expectToMatchJsResult(); }); + +test("respect noSelfInFile over noImplicitSelf", () => { + const result = util.testModule` + /** @noSelfInFile **/ + + const funcByValue: Record = { + hello: () => 1 + }; + + const func = funcByValue["hello"]; + export const result = func(1); + ` + .expectToMatchJsResult() + .getLuaResult(); + + expect(result.transpiledFiles).not.toHaveLength(0); + + const mainFile = result.transpiledFiles.find(f => f.outPath === "main.lua"); + expect(mainFile).toBeDefined(); + + // avoid ts error "not defined", even though toBeDefined is being checked above + if (!mainFile) return; + + expect(mainFile.lua).toBeDefined(); + expect(mainFile.lua).toContain("func(1)"); + expect(mainFile.lua).not.toContain("_G"); +}); From 634cce65fcfcc682a9c14f780dd6ce91f9a6328b Mon Sep 17 00:00:00 2001 From: Zerio Date: Sun, 22 Oct 2023 19:19:59 +0200 Subject: [PATCH 08/13] Simplify testcase --- test/unit/functions/noSelfAnnotation.spec.ts | 7 +------ 1 file changed, 1 insertion(+), 6 deletions(-) diff --git a/test/unit/functions/noSelfAnnotation.spec.ts b/test/unit/functions/noSelfAnnotation.spec.ts index 65198a433..bf5add022 100644 --- a/test/unit/functions/noSelfAnnotation.spec.ts +++ b/test/unit/functions/noSelfAnnotation.spec.ts @@ -66,12 +66,7 @@ test("explicit this parameter respected over @noSelf", () => { test("respect noSelfInFile over noImplicitSelf", () => { const result = util.testModule` /** @noSelfInFile **/ - - const funcByValue: Record = { - hello: () => 1 - }; - - const func = funcByValue["hello"]; + const func: Function = () => 1; export const result = func(1); ` .expectToMatchJsResult() From 36900b44b692458cc87c6f6a6c095248b8b3cb97 Mon Sep 17 00:00:00 2001 From: Zerio Date: Fri, 10 Nov 2023 20:45:18 +0100 Subject: [PATCH 09/13] Improve NoSelfInFile check --- src/transformation/visitors/call.ts | 18 ++++++++++++++---- 1 file changed, 14 insertions(+), 4 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index 1f14085e5..a89d3614f 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -269,7 +269,7 @@ export const transformCallExpression: FunctionVisitor = (node let callPath: lua.Expression; let parameters: lua.Expression[]; - const isContextualCall = isContextualCallExpression(context, signature, node.getSourceFile()); + const isContextualCall = isContextualCallExpression(context, calledExpression, signature); if (!isContextualCall) { [callPath, parameters] = transformCallAndArguments(context, calledExpression, node.arguments, signature); @@ -295,10 +295,20 @@ export const transformCallExpression: FunctionVisitor = (node function isContextualCallExpression( context: TransformationContext, - signature?: ts.Signature, - srcFile?: ts.SourceFile + calledExpression: ts.Expression, + signature?: ts.Signature ): boolean { - const hasNoSelfInFile = srcFile && getFileAnnotations(srcFile).has(AnnotationKind.NoSelfInFile); + const symbol = context.checker.getSymbolAtLocation(calledExpression); + let hasNoSelfInFile = false; + + if (symbol?.declarations) { + for (const declaration of symbol.declarations) { + const srcFile = declaration.getSourceFile(); + + hasNoSelfInFile = getFileAnnotations(srcFile).has(AnnotationKind.NoSelfInFile); + break; + } + } if (signature) { const declaration = signature.getDeclaration(); From 8074909c55ec2b30e02809981090e6b8ee384a69 Mon Sep 17 00:00:00 2001 From: Zerio Date: Fri, 10 Nov 2023 20:58:09 +0100 Subject: [PATCH 10/13] Further hasnoselfinfile fix --- src/transformation/visitors/call.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index a89d3614f..ee5059655 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -302,12 +302,9 @@ function isContextualCallExpression( let hasNoSelfInFile = false; if (symbol?.declarations) { - for (const declaration of symbol.declarations) { - const srcFile = declaration.getSourceFile(); - - hasNoSelfInFile = getFileAnnotations(srcFile).has(AnnotationKind.NoSelfInFile); - break; - } + hasNoSelfInFile = + symbol.declarations.some(d => getFileAnnotations(d.getSourceFile()).has(AnnotationKind.NoSelfInFile)) ?? + false; } if (signature) { From 8b581e0cabf419324bfb56564cb42d4af3f736bf Mon Sep 17 00:00:00 2001 From: Zerio Date: Mon, 13 Nov 2023 09:55:33 +0000 Subject: [PATCH 11/13] wrap declaration statement again --- src/transformation/visitors/call.ts | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/transformation/visitors/call.ts b/src/transformation/visitors/call.ts index ee5059655..0840d8616 100644 --- a/src/transformation/visitors/call.ts +++ b/src/transformation/visitors/call.ts @@ -307,20 +307,18 @@ function isContextualCallExpression( false; } - if (signature) { - const declaration = signature.getDeclaration(); + const declaration = signature?.getDeclaration(); - if (declaration) { - const contextTypeCheck = getDeclarationContextType(context, declaration) !== ContextType.Void; + if (declaration) { + const contextTypeCheck = getDeclarationContextType(context, declaration) !== ContextType.Void; - // respect implicit self over noSelfInFile - // look at "explicit this parameter respected over @noSelf" test - if (hasNoSelfInFile && contextTypeCheck !== true) { - return false; - } - - return contextTypeCheck; + // respect implicit self over noSelfInFile + // look at "explicit this parameter respected over @noSelf" test + if (hasNoSelfInFile && contextTypeCheck !== true) { + return false; } + + return contextTypeCheck; } if (hasNoSelfInFile) { From d9430ec11268bcdf8e89993aec5d7399f576738c Mon Sep 17 00:00:00 2001 From: Zerio Date: Sun, 19 Nov 2023 17:01:03 +0100 Subject: [PATCH 12/13] Add noSelfInFile check for func declared in other file --- test/unit/functions/noSelfAnnotation.spec.ts | 25 +++++++++++++++++++ .../noSelfAnnotationRespect/functions.ts | 6 +++++ .../functions/noSelfAnnotationRespect/main.ts | 4 +++ .../noSelfAnnotationRespect/tsconfig.json | 12 +++++++++ 4 files changed, 47 insertions(+) create mode 100644 test/unit/functions/noSelfAnnotationRespect/functions.ts create mode 100644 test/unit/functions/noSelfAnnotationRespect/main.ts create mode 100644 test/unit/functions/noSelfAnnotationRespect/tsconfig.json diff --git a/test/unit/functions/noSelfAnnotation.spec.ts b/test/unit/functions/noSelfAnnotation.spec.ts index bf5add022..0ef19035f 100644 --- a/test/unit/functions/noSelfAnnotation.spec.ts +++ b/test/unit/functions/noSelfAnnotation.spec.ts @@ -1,3 +1,4 @@ +import path = require("path"); import * as util from "../../util"; const methodHolders = ["class", "interface"]; @@ -84,3 +85,27 @@ test("respect noSelfInFile over noImplicitSelf", () => { expect(mainFile.lua).toContain("func(1)"); expect(mainFile.lua).not.toContain("_G"); }); + +const projectPath = path.resolve(__dirname, "noSelfAnnotationRespect"); + +const projectFile = util + .testProject(path.join(projectPath, "tsconfig.json")) + .setMainFileName(path.join(projectPath, "main.ts")); + +test("respect noSelfInFile over noImplicitSelf (func declared in other file)", () => { + const result = projectFile.getLuaResult(); + + expect(result.transpiledFiles).not.toHaveLength(0); + + console.log(result); + console.log(result.transpiledFiles); + const mainFile = result.transpiledFiles.find(f => f.outPath.includes("main.lua")); + expect(mainFile).toBeDefined(); + + // avoid ts error "not defined", even though toBeDefined is being checked above + if (!mainFile) return; + + expect(mainFile.lua).toBeDefined(); + expect(mainFile.lua).toContain("func(1)"); + expect(mainFile.lua).not.toContain("_G"); +}); diff --git a/test/unit/functions/noSelfAnnotationRespect/functions.ts b/test/unit/functions/noSelfAnnotationRespect/functions.ts new file mode 100644 index 000000000..3a17bcb25 --- /dev/null +++ b/test/unit/functions/noSelfAnnotationRespect/functions.ts @@ -0,0 +1,6 @@ +/* eslint-disable spaced-comment */ +/* eslint-disable @typescript-eslint/ban-types */ + +/** @noSelfInFile **/ +export const func: Function = () => 1; +export const result = func(1); diff --git a/test/unit/functions/noSelfAnnotationRespect/main.ts b/test/unit/functions/noSelfAnnotationRespect/main.ts new file mode 100644 index 000000000..517cd1766 --- /dev/null +++ b/test/unit/functions/noSelfAnnotationRespect/main.ts @@ -0,0 +1,4 @@ +import { func, result } from "./functions"; + +export const result1 = result; +export const result2 = func(1); diff --git a/test/unit/functions/noSelfAnnotationRespect/tsconfig.json b/test/unit/functions/noSelfAnnotationRespect/tsconfig.json new file mode 100644 index 000000000..935b64af6 --- /dev/null +++ b/test/unit/functions/noSelfAnnotationRespect/tsconfig.json @@ -0,0 +1,12 @@ +{ + "compilerOptions": { + "strict": true, + "moduleResolution": "Node", + "noUnusedLocals": true, + "noUnusedParameters": true, + "target": "esnext", + "lib": ["esnext"], + "types": [], + "rootDir": "." + } +} From 969e3d36958c93fd82a9d59b8cf3338a862bb388 Mon Sep 17 00:00:00 2001 From: Zerio Date: Sun, 19 Nov 2023 17:03:09 +0100 Subject: [PATCH 13/13] Remove accidentally commited debug logs --- test/unit/functions/noSelfAnnotation.spec.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/test/unit/functions/noSelfAnnotation.spec.ts b/test/unit/functions/noSelfAnnotation.spec.ts index 0ef19035f..06b1ff66a 100644 --- a/test/unit/functions/noSelfAnnotation.spec.ts +++ b/test/unit/functions/noSelfAnnotation.spec.ts @@ -97,8 +97,6 @@ test("respect noSelfInFile over noImplicitSelf (func declared in other file)", ( expect(result.transpiledFiles).not.toHaveLength(0); - console.log(result); - console.log(result.transpiledFiles); const mainFile = result.transpiledFiles.find(f => f.outPath.includes("main.lua")); expect(mainFile).toBeDefined();