Skip to content
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.

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

Indirect calls for imported functions #44624

Merged
merged 2 commits into from
Jun 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
20 changes: 20 additions & 0 deletions src/compiler/emitter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2473,7 +2473,17 @@ namespace ts {
}

function emitCallExpression(node: CallExpression) {
const indirectCall = getEmitFlags(node) & EmitFlags.IndirectCall;
if (indirectCall) {
writePunctuation("(");
writeLiteral("0");
writePunctuation(",");
writeSpace();
}
emitExpression(node.expression, parenthesizer.parenthesizeLeftSideOfAccess);
if (indirectCall) {
writePunctuation(")");
}
emit(node.questionDotToken);
emitTypeArguments(node, node.typeArguments);
emitExpressionList(node, node.arguments, ListFormat.CallExpressionArguments, parenthesizer.parenthesizeExpressionForDisallowedComma);
Expand All @@ -2488,7 +2498,17 @@ namespace ts {
}

function emitTaggedTemplateExpression(node: TaggedTemplateExpression) {
const indirectCall = getEmitFlags(node) & EmitFlags.IndirectCall;
if (indirectCall) {
writePunctuation("(");
writeLiteral("0");
writePunctuation(",");
writeSpace();
}
emitExpression(node.tag, parenthesizer.parenthesizeLeftSideOfAccess);
if (indirectCall) {
writePunctuation(")");
}
emitTypeArguments(node, node.typeArguments);
writeSpace();
emitExpression(node.template);
Expand Down
43 changes: 43 additions & 0 deletions src/compiler/transformers/module/module.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,8 @@ namespace ts {
const previousOnEmitNode = context.onEmitNode;
context.onSubstituteNode = onSubstituteNode;
context.onEmitNode = onEmitNode;
context.enableSubstitution(SyntaxKind.CallExpression); // Substitute calls to imported/exported symbols to avoid incorrect `this`.
context.enableSubstitution(SyntaxKind.TaggedTemplateExpression); // Substitute calls to imported/exported symbols to avoid incorrect `this`.
context.enableSubstitution(SyntaxKind.Identifier); // Substitutes expression identifiers with imported/exported symbols.
context.enableSubstitution(SyntaxKind.BinaryExpression); // Substitutes assignments to exported symbols.
context.enableSubstitution(SyntaxKind.PrefixUnaryExpression); // Substitutes updates to exported symbols.
Expand Down Expand Up @@ -1741,6 +1743,10 @@ namespace ts {
switch (node.kind) {
case SyntaxKind.Identifier:
return substituteExpressionIdentifier(node as Identifier);
case SyntaxKind.CallExpression:
return substituteCallExpression(node as CallExpression);
case SyntaxKind.TaggedTemplateExpression:
return substituteTaggedTemplateExpression(node as TaggedTemplateExpression);
case SyntaxKind.BinaryExpression:
return substituteBinaryExpression(node as BinaryExpression);
case SyntaxKind.PostfixUnaryExpression:
Expand All @@ -1751,6 +1757,43 @@ namespace ts {
return node;
}

function substituteCallExpression(node: CallExpression) {
if (isIdentifier(node.expression)) {
const expression = substituteExpressionIdentifier(node.expression);
noSubstitution[getNodeId(expression)] = true;
if (!isIdentifier(expression)) {
return addEmitFlags(
factory.updateCallExpression(node,
expression,
/*typeArguments*/ undefined,
node.arguments
),
EmitFlags.IndirectCall
);

}
}
return node;
}

function substituteTaggedTemplateExpression(node: TaggedTemplateExpression) {
if (isIdentifier(node.tag)) {
const tag = substituteExpressionIdentifier(node.tag);
noSubstitution[getNodeId(tag)] = true;
if (!isIdentifier(tag)) {
return addEmitFlags(
factory.updateTaggedTemplateExpression(node,
tag,
/*typeArguments*/ undefined,
node.template
),
EmitFlags.IndirectCall
);
}
}
return node;
}

/**
* Substitution for an Identifier expression that may contain an imported or exported
* symbol.
Expand Down
1 change: 1 addition & 0 deletions src/compiler/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6728,6 +6728,7 @@ namespace ts {
/*@internal*/ NeverApplyImportHelper = 1 << 26, // Indicates the node should never be wrapped with an import star helper (because, for example, it imports tslib itself)
/*@internal*/ IgnoreSourceNewlines = 1 << 27, // Overrides `printerOptions.preserveSourceNewlines` to print this node (and all descendants) with default whitespace.
/*@internal*/ Immutable = 1 << 28, // Indicates a node is a singleton intended to be reused in multiple locations. Any attempt to make further changes to the node will result in an error.
/*@internal*/ IndirectCall = 1 << 29, // Emit CallExpression as an indirect call: `(0, f)()`
}

export interface EmitHelperBase {
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,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",
Expand Down
84 changes: 84 additions & 0 deletions src/testRunner/unittests/evaluation/externalModules.ts
Original file line number Diff line number Diff line change
@@ -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 `.`.
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -55,8 +55,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}`
}];
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ exports.__esModule = true;
exports.a = void 0;
var func_1 = require("./func");
// hover on vextend
exports.a = func_1.vextend({
exports.a = (0, func_1.vextend)({
watch: {
data1: function (val) {
this.data2 = 1;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ exports.__esModule = true;
exports.__esModule = true;
exports.A = void 0;
var file1_1 = require("./file1");
exports.A = file1_1.styled();
exports.A = (0, file1_1.styled)();


//// [color.d.ts]
Expand Down
6 changes: 3 additions & 3 deletions tests/baselines/reference/ambientDeclarationsPatterns.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ foo(fileText);
exports.__esModule = true;
///<reference path="declarations.d.ts" />
var foobarbaz_1 = require("foobarbaz");
foobarbaz_1.foo(foobarbaz_1.baz);
(0, foobarbaz_1.foo)(foobarbaz_1.baz);
var foosball_1 = require("foosball");
foobarbaz_1.foo(foosball_1.foos);
(0, foobarbaz_1.foo)(foosball_1.foos);
// Works with relative file name
var file_text_1 = require("./file!text");
foobarbaz_1.foo(file_text_1["default"]);
(0, foobarbaz_1.foo)(file_text_1["default"]);
2 changes: 1 addition & 1 deletion tests/baselines/reference/ambientShorthand.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,4 +20,4 @@ exports.__esModule = true;
var jquery_1 = require("jquery");
var baz = require("fs");
var boom = require("jquery");
jquery_1["default"](jquery_1.bar, baz, boom);
(0, jquery_1["default"])(jquery_1.bar, baz, boom);
2 changes: 1 addition & 1 deletion tests/baselines/reference/ambientShorthand_reExport.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,4 +49,4 @@ exports.__esModule = true;
var reExportX_1 = require("./reExportX");
var $ = require("./reExportAll");
// '$' is not callable, it is an object.
reExportX_1.x($);
(0, reExportX_1.x)($);
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ define("Class", ["require", "exports", "Configurable"], function (require, expor
return _super !== null && _super.apply(this, arguments) || this;
}
return ActualClass;
}(Configurable_1.Configurable(HiddenClass)));
}((0, Configurable_1.Configurable)(HiddenClass)));
exports.ActualClass = ActualClass;
});

Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/anonClassDeclarationEmitIsAnon.js
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ var __extends = (this && this.__extends) || (function () {
exports.__esModule = true;
exports.TimestampedUser = exports.User = void 0;
var wrapClass_1 = require("./wrapClass");
exports["default"] = wrapClass_1.wrapClass(0);
exports["default"] = (0, wrapClass_1.wrapClass)(0);
// Simple class
var User = /** @class */ (function () {
function User() {
Expand All @@ -112,7 +112,7 @@ var TimestampedUser = /** @class */ (function (_super) {
return _super.call(this) || this;
}
return TimestampedUser;
}(wrapClass_1.Timestamped(User)));
}((0, wrapClass_1.Timestamped)(User)));
exports.TimestampedUser = TimestampedUser;


Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/callbackTagVariadicType.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ exports.x = void 0;
/** @type {Foo} */
var x = function () { return 1; };
exports.x = x;
var res = exports.x('a', 'b');
var res = (0, exports.x)('a', 'b');


//// [callbackTagVariadicType.d.ts]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ var Component = /** @class */ (function () {
function Component() {
}
Component.prototype.render = function () {
return _a.jsx("div", { children: null /* preserved */ }, void 0);
return (0, _a.jsx)("div", { children: null /* preserved */ }, void 0);
};
return Component;
}());
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ var Component = /** @class */ (function () {
function Component() {
}
Component.prototype.render = function () {
return _a.jsxDEV("div", { children: null /* preserved */ }, void 0, false, { fileName: _jsxFileName, lineNumber: 5, columnNumber: 15 }, this);
return (0, _a.jsxDEV)("div", { children: null /* preserved */ }, void 0, false, { fileName: _jsxFileName, lineNumber: 5, columnNumber: 15 }, this);
};
return Component;
}());
2 changes: 1 addition & 1 deletion tests/baselines/reference/commonjsSafeImport.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ exports.Foo = Foo;
"use strict";
Object.defineProperty(exports, "__esModule", { value: true });
var _10_lib_1 = require("./10_lib");
_10_lib_1.Foo();
(0, _10_lib_1.Foo)();


//// [10_lib.d.ts]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ exports.__esModule = true;
exports.y = exports.x = void 0;
var foo_1 = require("foo");
var root_1 = require("root");
exports.x = foo_1.foo();
exports.y = root_1.bar();
exports.x = (0, foo_1.foo)();
exports.y = (0, root_1.bar)();
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ exports.q = void 0;
var a_1 = require("./a");
function q() { }
exports.q = q;
q.val = a_1.f();
q.val = (0, a_1.f)();


//// [a.d.ts]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ var Point = function (x, y) { return ({ x: x, y: y }); };
exports.Point = Point;
var Rect = function (a, b) { return ({ a: a, b: b }); };
exports.Rect = Rect;
exports.Point.zero = function () { return exports.Point(0, 0); };
exports.Point.zero = function () { return (0, exports.Point)(0, 0); };


//// [declarationEmitExpandoWithGenericConstraint.d.ts]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ var get_comp_1 = require("./get-comp");
// this shouldn't need any triple-slash references - it should have a direct import to `react` and that's it
// This issue (#35343) _only_ reproduces in the test harness when the file in question is in a subfolder
exports.obj = {
comp: get_comp_1.getComp()
comp: (0, get_comp_1.getComp)()
};
//// [some-other-file.js]
"use strict";
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ Object.defineProperty(exports, "__esModule", { value: true });
exports.bar = void 0;
var utils_1 = require("./utils");
Object.defineProperty(exports, "bar", { enumerable: true, get: function () { return utils_1.bar; } });
utils_1.foo();
(0, utils_1.foo)();
var obj;


Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ export const a: import("typescript-fsa").A;
exports.__esModule = true;
exports.a = void 0;
var typescript_fsa_1 = require("typescript-fsa");
exports.a = typescript_fsa_1.getA();
exports.a = (0, typescript_fsa_1.getA)();


//// [index.d.ts]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ export const a: import("typescript-fsa").A;
exports.__esModule = true;
exports.a = void 0;
var typescript_fsa_1 = require("typescript-fsa");
exports.a = typescript_fsa_1.getA();
exports.a = (0, typescript_fsa_1.getA)();


//// [index.d.ts]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ var ParentThing = /** @class */ (function () {
return ParentThing;
}());
exports.ParentThing = ParentThing;
child1_1.child1(ParentThing.prototype);
(0, child1_1.child1)(ParentThing.prototype);
//// [child1.js]
"use strict";
exports.__esModule = true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ exports.createNamed = createNamed;
exports.__esModule = true;
exports.Value = void 0;
var b_1 = require("./b");
exports.Value = b_1.createNamed();
exports.Value = (0, b_1.createNamed)();


//// [b.d.ts]
Expand Down
Loading