fix receiver on calls of imported and exported functions (#35877)

* fix receiver of imported and exported functions

fixes: #35420

* Rebase against master and clean up substitution flow

* Add evaluator tests

* Fix evaluator tests

Co-authored-by: Ron Buckton <ron.buckton@microsoft.com>

src/compiler/transformers/module/module.ts

@@ -1,5 +1,6 @@
 /*@internal*/
 namespace ts {
+
     export function transformModule(context: TransformationContext) {
         interface AsynchronousDependencies {
             aliasedModuleNames: Expression[];
@@ -34,6 +35,8 @@ namespace ts {
         context.onEmitNode = onEmitNode;
         context.enableSubstitution(SyntaxKind.Identifier); // Substitutes expression identifiers with imported/exported symbols.
         context.enableSubstitution(SyntaxKind.BinaryExpression); // Substitutes assignments to exported symbols.
+        context.enableSubstitution(SyntaxKind.CallExpression); // Substitutes expression identifiers with imported/exported symbols.
+        context.enableSubstitution(SyntaxKind.TaggedTemplateExpression); // Substitutes expression identifiers with imported/exported symbols.
         context.enableSubstitution(SyntaxKind.PrefixUnaryExpression); // Substitutes updates to exported symbols.
         context.enableSubstitution(SyntaxKind.PostfixUnaryExpression); // Substitutes updates to exported symbols.
         context.enableSubstitution(SyntaxKind.ShorthandPropertyAssignment); // Substitutes shorthand property assignments for imported/exported symbols.
@@ -46,6 +49,7 @@ namespace ts {
         let currentModuleInfo: ExternalModuleInfo; // The ExternalModuleInfo for the current file.
         let noSubstitution: boolean[]; // Set of nodes for which substitution rules should be ignored.
         let needUMDDynamicImportHelper: boolean;
+        let bindingReferenceCache: ESMap<Node, Identifier | SourceFile | ImportClause | ImportSpecifier | undefined> | undefined;
 
         return chainBundle(context, transformSourceFile);
 
@@ -1742,6 +1746,10 @@ namespace ts {
                     return substituteExpressionIdentifier(<Identifier>node);
                 case SyntaxKind.BinaryExpression:
                     return substituteBinaryExpression(<BinaryExpression>node);
+                case SyntaxKind.CallExpression:
+                    return substituteCallExpression(<CallExpression>node);
+                case SyntaxKind.TaggedTemplateExpression:
+                    return substituteTaggedTemplateExpression(<TaggedTemplateExpression>node);
                 case SyntaxKind.PostfixUnaryExpression:
                 case SyntaxKind.PrefixUnaryExpression:
                     return substituteUnaryExpression(<PrefixUnaryExpression | PostfixUnaryExpression>node);
@@ -1750,6 +1758,86 @@ namespace ts {
             return node;
         }
 
+        /**
+         * For an Identifier, gets the import or export binding that it references.
+         * @returns One of the following:
+         * - An `Identifier` if node references an external helpers module (i.e., `tslib`).
+         * - A `SourceFile` if the node references an export in the file.
+         * - An `ImportClause` or `ImportSpecifier` if the node references an import binding.
+         * - Otherwise, `undefined`.
+         */
+        function getImportOrExportBindingReferenceWorker(node: Identifier): Identifier | SourceFile | ImportClause | ImportSpecifier | undefined {
+            if (getEmitFlags(node) & EmitFlags.HelperName) {
+                const externalHelpersModuleName = getExternalHelpersModuleName(currentSourceFile);
+                if (externalHelpersModuleName) {
+                    return externalHelpersModuleName;
+                }
+            }
+            else if (!(isGeneratedIdentifier(node) && !(node.autoGenerateFlags & GeneratedIdentifierFlags.AllowNameSubstitution)) && !isLocalName(node)) {
+                const exportContainer = resolver.getReferencedExportContainer(node, isExportName(node));
+                if (exportContainer?.kind === SyntaxKind.SourceFile) {
+                    return exportContainer;
+                }
+                const importDeclaration = resolver.getReferencedImportDeclaration(node);
+                if (importDeclaration && (isImportClause(importDeclaration) || isImportSpecifier(importDeclaration))) {
+                    return importDeclaration;
+                }
+            }
+            return undefined;
+        }
+
+        /**
+         * For an Identifier, gets the import or export binding that it references.
+         * @param removeEntry When `false`, the result is cached to avoid recomputing the result in a later substitution.
+         * When `true`, any cached result for the node is removed.
+         * @returns One of the following:
+         * - An `Identifier` if node references an external helpers module (i.e., `tslib`).
+         * - A `SourceFile` if the node references an export in the file.
+         * - An `ImportClause` or `ImportSpecifier` if the node references an import binding.
+         * - Otherwise, `undefined`.
+         */
+        function getImportOrExportBindingReference(node: Identifier, removeEntry: boolean): Identifier | SourceFile | ImportClause | ImportSpecifier | undefined {
+            let result = bindingReferenceCache?.get(node);
+            if (!result && !bindingReferenceCache?.has(node)) {
+                result = getImportOrExportBindingReferenceWorker(node);
+                if (!removeEntry) {
+                    bindingReferenceCache ||= new Map();
+                    bindingReferenceCache.set(node, result);
+                }
+            }
+            else if (removeEntry) {
+                bindingReferenceCache?.delete(node);
+            }
+            return result;
+        }
+
+        function substituteCallExpression(node: CallExpression) {
+            if (isIdentifier(node.expression) && getImportOrExportBindingReference(node.expression, /*removeEntry*/ false)) {
+                return isCallChain(node) ?
+                    factory.updateCallChain(node,
+                        setTextRange(factory.createComma(factory.createNumericLiteral(0), node.expression), node.expression),
+                        node.questionDotToken,
+                        /*typeArguments*/ undefined,
+                        node.arguments) :
+                    factory.updateCallExpression(node,
+                        setTextRange(factory.createComma(factory.createNumericLiteral(0), node.expression), node.expression),
+                        /*typeArguments*/ undefined,
+                        node.arguments);
+            }
+            return node;
+        }
+
+        function substituteTaggedTemplateExpression(node: TaggedTemplateExpression) {
+            if (isIdentifier(node.tag) && getImportOrExportBindingReference(node.tag, /*removeEntry*/ false)) {
+                return factory.updateTaggedTemplateExpression(
+                    node,
+                    setTextRange(factory.createComma(factory.createNumericLiteral(0), node.tag), node.tag),
+                    /*typeArguments*/ undefined,
+                    node.template);
+            }
+            return node;
+        }
+
         /**
          * Substitution for an Identifier expression that may contain an imported or exported
          * symbol.
@@ -1757,18 +1845,11 @@ namespace ts {
          * @param node The node to substitute.
          */
         function substituteExpressionIdentifier(node: Identifier): Expression {
-            if (getEmitFlags(node) & EmitFlags.HelperName) {
-                const externalHelpersModuleName = getExternalHelpersModuleName(currentSourceFile);
-                if (externalHelpersModuleName) {
-                    return factory.createPropertyAccessExpression(externalHelpersModuleName, node);
-                }
-
-                return node;
-            }
-
-            if (!(isGeneratedIdentifier(node) && !(node.autoGenerateFlags & GeneratedIdentifierFlags.AllowNameSubstitution)) && !isLocalName(node)) {
-                const exportContainer = resolver.getReferencedExportContainer(node, isExportName(node));
-                if (exportContainer && exportContainer.kind === SyntaxKind.SourceFile) {
+            const result = getImportOrExportBindingReference(node, /*removeEntry*/ true);
+            switch (result?.kind) {
+                case SyntaxKind.Identifier: // tslib import
+                    return factory.createPropertyAccessExpression(result, node);
+                case SyntaxKind.SourceFile: // top-level export
                     return setTextRange(
                         factory.createPropertyAccessExpression(
                             factory.createIdentifier("exports"),
@@ -1776,32 +1857,26 @@ namespace ts {
                         ),
                         /*location*/ node
                     );
-                }
-
-                const importDeclaration = resolver.getReferencedImportDeclaration(node);
-                if (importDeclaration) {
-                    if (isImportClause(importDeclaration)) {
-                        return setTextRange(
-                            factory.createPropertyAccessExpression(
-                                factory.getGeneratedNameForNode(importDeclaration.parent),
-                                factory.createIdentifier("default")
-                            ),
-                            /*location*/ node
-                        );
-                    }
-                    else if (isImportSpecifier(importDeclaration)) {
-                        const name = importDeclaration.propertyName || importDeclaration.name;
-                        return setTextRange(
-                            factory.createPropertyAccessExpression(
-                                factory.getGeneratedNameForNode(importDeclaration.parent?.parent?.parent || importDeclaration),
-                                factory.cloneNode(name)
-                            ),
-                            /*location*/ node
-                        );
-                    }
-                }
+                case SyntaxKind.ImportClause:
+                    return setTextRange(
+                        factory.createPropertyAccessExpression(
+                            factory.getGeneratedNameForNode(result.parent),
+                            factory.createIdentifier("default")
+                        ),
+                        /*location*/ node
+                    );
+                case SyntaxKind.ImportSpecifier:
+                    const name = result.propertyName || result.name;
+                    return setTextRange(
+                        factory.createPropertyAccessExpression(
+                            factory.getGeneratedNameForNode(result.parent?.parent?.parent || result),
+                            factory.cloneNode(name)
+                        ),
+                        /*location*/ node
+                    );
+                default:
+                    return node;
             }
-            return node;
         }
 
         /**

src/harness/evaluatorImpl.ts

@@ -4,23 +4,6 @@ namespace evaluator {
     const sourceFile = vpath.combine(vfs.srcFolder, "source.ts");
     const sourceFileJs = vpath.combine(vfs.srcFolder, "source.js");
 
-    function compile(sourceText: string, options?: ts.CompilerOptions) {
-        const fs = vfs.createFromFileSystem(Harness.IO, /*ignoreCase*/ false);
-        fs.writeFileSync(sourceFile, sourceText);
-        const compilerOptions: ts.CompilerOptions = {
-            target: ts.ScriptTarget.ES5,
-            module: ts.ModuleKind.CommonJS,
-            lib: ["lib.esnext.d.ts", "lib.dom.d.ts"],
-            ...options
-        };
-        const host = new fakes.CompilerHost(fs, compilerOptions);
-        return compiler.compileFiles(host, [sourceFile], compilerOptions);
-    }
-
-    function noRequire(id: string) {
-        throw new Error(`Module '${id}' could not be found.`);
-    }
-
     // Define a custom "Symbol" constructor to attach missing built-in symbols without
     // modifying the global "Symbol" constructor
     const FakeSymbol: SymbolConstructor = ((description?: string) => Symbol(description)) as any;
@@ -32,8 +15,17 @@ namespace evaluator {
     // Add "asyncIterator" if missing
     if (!ts.hasProperty(FakeSymbol, "asyncIterator")) Object.defineProperty(FakeSymbol, "asyncIterator", { value: Symbol.for("Symbol.asyncIterator"), configurable: true });
 
-    export function evaluateTypeScript(sourceText: string, options?: ts.CompilerOptions, globals?: Record<string, any>) {
-        const result = compile(sourceText, options);
+    export function evaluateTypeScript(source: string | { files: vfs.FileSet, rootFiles: string[], main: string }, options?: ts.CompilerOptions, globals?: Record<string, any>) {
+        if (typeof source === "string") source = { files: { [sourceFile]: source }, rootFiles: [sourceFile], main: sourceFile };
+        const fs = vfs.createFromFileSystem(Harness.IO, /*ignoreCase*/ false, { files: source.files });
+        const compilerOptions: ts.CompilerOptions = {
+            target: ts.ScriptTarget.ES5,
+            module: ts.ModuleKind.CommonJS,
+            lib: ["lib.esnext.d.ts", "lib.dom.d.ts"],
+            ...options
+        };
+        const host = new fakes.CompilerHost(fs, compilerOptions);
+        const result = compiler.compileFiles(host, source.rootFiles, compilerOptions);
         if (ts.some(result.diagnostics)) {
             assert.ok(/*value*/ false, "Syntax error in evaluation source text:\n" + ts.formatDiagnostics(result.diagnostics, {
                 getCanonicalFileName: file => file,
@@ -42,29 +34,100 @@ namespace evaluator {
             }));
         }
 
-        const output = result.getOutput(sourceFile, "js")!;
+        const output = result.getOutput(source.main, "js")!;
         assert.isDefined(output);
 
-        return evaluateJavaScript(output.text, globals, output.file);
+        globals = { Symbol: FakeSymbol, ...globals };
+        return createLoader(fs, globals)(output.file);
+    }
+
+    function createLoader(fs: vfs.FileSystem, globals: Record<string, any>) {
+        interface Module {
+            exports: any;
+        }
+
+        const moduleCache = new ts.Map<string, Module>();
+        return load;
+
+        function evaluate(text: string, file: string, module: Module) {
+            const globalNames: string[] = [];
+            const globalArgs: any[] = [];
+            for (const name in globals) {
+                if (ts.hasProperty(globals, name)) {
+                    globalNames.push(name);
+                    globalArgs.push(globals[name]);
+                }
+            }
+            const base = vpath.dirname(file);
+            const localRequire = (id: string) => requireModule(id, base);
+            const evaluateText = `(function (module, exports, require, __dirname, __filename, ${globalNames.join(", ")}) { ${text} })`;
+            // eslint-disable-next-line no-eval
+            const evaluateThunk = (void 0, eval)(evaluateText) as (module: any, exports: any, require: (id: string) => any, dirname: string, filename: string, ...globalArgs: any[]) => void;
+            evaluateThunk.call(globals, module, module.exports, localRequire, vpath.dirname(file), file, FakeSymbol, ...globalArgs);
+        }
+
+        function loadModule(file: string): Module {
+            if (!ts.isExternalModuleNameRelative(file)) throw new Error(`Module '${file}' could not be found.`);
+            let module = moduleCache.get(file);
+            if (module) return module;
+            moduleCache.set(file, module = { exports: {} });
+            try {
+                const sourceText = fs.readFileSync(file, "utf8");
+                evaluate(sourceText, file, module);
+                return module;
+            }
+            catch (e) {
+                moduleCache.delete(file);
+                throw e;
+            }
+        }
+
+        function isFile(file: string) {
+            return fs.existsSync(file) && fs.statSync(file).isFile();
+        }
+
+        function loadAsFile(file: string): Module | undefined {
+            if (isFile(file)) return loadModule(file);
+            if (isFile(file + ".js")) return loadModule(file + ".js");
+            return undefined;
+        }
+
+        function loadIndex(dir: string): Module | undefined {
+            const indexFile = vpath.resolve(dir, "index.js");
+            if (isFile(indexFile)) return loadModule(indexFile);
+            return undefined;
+        }
+
+        function loadAsDirectory(dir: string): Module | undefined {
+            const packageFile = vpath.resolve(dir, "package.json");
+            if (isFile(packageFile)) {
+                const text = fs.readFileSync(packageFile, "utf8");
+                const json = JSON.parse(text);
+                if (json.main) {
+                    const main = vpath.resolve(dir, json.main);
+                    const result = loadAsFile(main) || loadIndex(main);
+                    if (result === undefined) throw new Error("Module not found");
+                }
+            }
+            return loadIndex(dir);
+        }
+
+        function requireModule(id: string, base: string) {
+            if (!ts.isExternalModuleNameRelative(id)) throw new Error(`Module '${id}' could not be found.`);
+            const file = vpath.resolve(base, id);
+            const module = loadAsFile(file) || loadAsDirectory(file);
+            if (!module) throw new Error(`Module '${id}' could not be found.`);
+            return module.exports;
+        }
+
+        function load(file: string) {
+            return requireModule(file, fs.cwd());
+        }
     }
 
     export function evaluateJavaScript(sourceText: string, globals?: Record<string, any>, sourceFile = sourceFileJs) {
         globals = { Symbol: FakeSymbol, ...globals };
-
-        const globalNames: string[] = [];
-        const globalArgs: any[] = [];
-        for (const name in globals) {
-            if (ts.hasProperty(globals, name)) {
-                globalNames.push(name);
-                globalArgs.push(globals[name]);
-            }
-        }
-
-        const evaluateText = `(function (module, exports, require, __dirname, __filename, ${globalNames.join(", ")}) { ${sourceText} })`;
-        // eslint-disable-next-line no-eval
-        const evaluateThunk = (void 0, eval)(evaluateText) as (module: any, exports: any, require: (id: string) => any, dirname: string, filename: string, ...globalArgs: any[]) => void;
-        const module: { exports: any; } = { exports: {} };
-        evaluateThunk.call(globals, module, module.exports, noRequire, vpath.dirname(sourceFile), sourceFile, FakeSymbol, ...globalArgs);
-        return module.exports;
+        const fs = new vfs.FileSystem(/*ignoreCase*/ false, { files: { [sourceFile]: sourceText } });
+        return createLoader(fs, globals)(sourceFile);
     }
 }

src/testRunner/tsconfig.json

@@ -92,6 +92,7 @@
         "unittests/evaluation/asyncGenerator.ts",
         "unittests/evaluation/awaiter.ts",
         "unittests/evaluation/destructuring.ts",
+        "unittests/evaluation/externalModules.ts",
         "unittests/evaluation/forAwaitOf.ts",
         "unittests/evaluation/forOf.ts",
         "unittests/evaluation/optionalCall.ts",

src/testRunner/unittests/evaluation/externalModules.ts

@@ -0,0 +1,84 @@
+describe("unittests:: evaluation:: externalModules", () => {
+    // https://github.com/microsoft/TypeScript/issues/35420
+    it("Correct 'this' in function exported from external module", async () => {
+        const result = evaluator.evaluateTypeScript({
+            files: {
+                "/.src/output.ts": `
+                    export const output: any[] = [];
+                `,
+                "/.src/other.ts": `
+                    import { output } from "./output";
+                    export function f(this: any, expected) {
+                        output.push(this === expected);
+                    }
+
+                    // 0
+                    f(undefined);
+                `,
+                "/.src/main.ts": `
+                    export { output } from "./output";
+                    import { output } from "./output";
+                    import { f } from "./other";
+                    import * as other from "./other";
+
+                    // 1
+                    f(undefined);
+
+                    // 2
+                    const obj = {};
+                    f.call(obj, obj);
+
+                    // 3
+                    other.f(other);
+                `
+            },
+            rootFiles: ["/.src/main.ts"],
+            main: "/.src/main.ts"
+        });
+        assert.equal(result.output[0], true); // `f(undefined)` inside module. Existing behavior is correct.
+        assert.equal(result.output[1], true); // `f(undefined)` from import. New behavior to match first case.
+        assert.equal(result.output[2], true); // `f.call(obj, obj)`. Behavior of `.call` (or `.apply`, etc.) should not be affected.
+        assert.equal(result.output[3], true); // `other.f(other)`. `this` is still namespace because it is left of `.`.
+    });
+
+    it("Correct 'this' in function expression exported from external module", async () => {
+        const result = evaluator.evaluateTypeScript({
+            files: {
+                "/.src/output.ts": `
+                    export const output: any[] = [];
+                `,
+                "/.src/other.ts": `
+                    import { output } from "./output";
+                    export const f = function(this: any, expected) {
+                        output.push(this === expected);
+                    }
+
+                    // 0
+                    f(undefined);
+                `,
+                "/.src/main.ts": `
+                    export { output } from "./output";
+                    import { output } from "./output";
+                    import { f } from "./other";
+                    import * as other from "./other";
+
+                    // 1
+                    f(undefined);
+
+                    // 2
+                    const obj = {};
+                    f.call(obj, obj);
+
+                    // 3
+                    other.f(other);
+                `
+            },
+            rootFiles: ["/.src/main.ts"],
+            main: "/.src/main.ts"
+        });
+        assert.equal(result.output[0], true); // `f(undefined)` inside module. Existing behavior is incorrect.
+        assert.equal(result.output[1], true); // `f(undefined)` from import. New behavior to match first case.
+        assert.equal(result.output[2], true); // `f.call(obj, obj)`. Behavior of `.call` (or `.apply`, etc.) should not be affected.
+        assert.equal(result.output[3], true); // `other.f(other)`. `this` is still namespace because it is left of `.`.
+    });
+});

src/testRunner/unittests/tsserver/projectReferenceCompileOnSave.ts

@@ -238,8 +238,8 @@ exports.fn3 = fn3;`;
                     content: `"use strict";
 exports.__esModule = true;${appendJsText === changeJs ? "\nexports.fn3 = void 0;" : ""}
 var fns_1 = require("../decls/fns");
-fns_1.fn1();
-fns_1.fn2();
+(0, fns_1.fn1)();
+(0, fns_1.fn2)();
 ${appendJs}`
                 }]
             };