Skip to content

fix(39744): make template literals more spec compliant #45304

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

Merged
merged 3 commits into from
Oct 13, 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
89 changes: 12 additions & 77 deletions src/compiler/transformers/es2015.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4102,87 +4102,22 @@ namespace ts {
* @param node A TemplateExpression node.
*/
function visitTemplateExpression(node: TemplateExpression): Expression {
const expressions: Expression[] = [];
addTemplateHead(expressions, node);
addTemplateSpans(expressions, node);

// createAdd will check if each expression binds less closely than binary '+'.
// If it does, it wraps the expression in parentheses. Otherwise, something like
// `abc${ 1 << 2 }`
// becomes
// "abc" + 1 << 2 + ""
// which is really
// ("abc" + 1) << (2 + "")
// rather than
// "abc" + (1 << 2) + ""
const expression = reduceLeft(expressions, factory.createAdd)!;
if (nodeIsSynthesized(expression)) {
setTextRange(expression, node);
}

return expression;
}

/**
* Gets a value indicating whether we need to include the head of a TemplateExpression.
*
* @param node A TemplateExpression node.
*/
function shouldAddTemplateHead(node: TemplateExpression) {
// If this expression has an empty head literal and the first template span has a non-empty
// literal, then emitting the empty head literal is not necessary.
// `${ foo } and ${ bar }`
// can be emitted as
// foo + " and " + bar
// This is because it is only required that one of the first two operands in the emit
// output must be a string literal, so that the other operand and all following operands
// are forced into strings.
//
// If the first template span has an empty literal, then the head must still be emitted.
// `${ foo }${ bar }`
// must still be emitted as
// "" + foo + bar

// There is always atleast one templateSpan in this code path, since
// NoSubstitutionTemplateLiterals are directly emitted via emitLiteral()
Debug.assert(node.templateSpans.length !== 0);
let expression: Expression = factory.createStringLiteral(node.head.text);
for (const span of node.templateSpans) {
const args = [visitNode(span.expression, visitor, isExpression)];

const span = node.templateSpans[0];
return node.head.text.length !== 0 || span.literal.text.length === 0 || !!length(getLeadingCommentRangesOfNode(span.expression, currentSourceFile));
}
if (span.literal.text.length > 0) {
args.push(factory.createStringLiteral(span.literal.text));
}

/**
* Adds the head of a TemplateExpression to an array of expressions.
*
* @param expressions An array of expressions.
* @param node A TemplateExpression node.
*/
function addTemplateHead(expressions: Expression[], node: TemplateExpression): void {
if (!shouldAddTemplateHead(node)) {
return;
expression = factory.createCallExpression(
Copy link

@leebyron leebyron Dec 16, 2021

Choose a reason for hiding this comment

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

Curious - why create the concat call inside the loop of template spans instead of outside the loop?

In other words, why does a${x}b${y}c emit "a".concat(x, "b").concat(y, "c") instead of emitting "a".concat(x, "b", y, "c")?

Copy link
Contributor

Choose a reason for hiding this comment

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

@leebyron i believe the observable order of exceptions from ToString calls may require the chained calls; babel had the same change (please double check me)

Choose a reason for hiding this comment

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

Aha, yes thank you for this.

There's a test included in this change that would break based on that idea

class C {
    counter: number;
    constructor() {
        this.counter = 0;
    }
    get foo() {
        this.counter++;
        return {
            toString: () => this.counter++,
        };
    }
}
const c = new C;
export const output = \`\${c.foo} \${c.foo}\`;

"".concat(c.foo).concat(c.foo) would run: get foo(), toString(), get foo(), toString()

and

"".concat(c.foo, c.foo) would run: get foo(), get foo(), toString(), toString()

The first one is what you'd expect to see from a template interpolation

factory.createPropertyAccessExpression(expression, "concat"),
/*typeArguments*/ undefined,
args,
);
}

expressions.push(factory.createStringLiteral(node.head.text));
}

/**
* Visits and adds the template spans of a TemplateExpression to an array of expressions.
*
* @param expressions An array of expressions.
* @param node A TemplateExpression node.
*/
function addTemplateSpans(expressions: Expression[], node: TemplateExpression): void {
for (const span of node.templateSpans) {
expressions.push(visitNode(span.expression, visitor, isExpression));

// Only emit if the literal is non-empty.
// The binary '+' operator is left-associative, so the first string concatenation
// with the head will force the result up to this point to be a string.
// Emitting a '+ ""' has no semantic effect for middles and tails.
if (span.literal.text.length !== 0) {
expressions.push(factory.createStringLiteral(span.literal.text));
}
}
return setTextRange(expression, node);
}

/**
Expand Down
1 change: 1 addition & 0 deletions src/testRunner/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -100,6 +100,7 @@
"unittests/evaluation/optionalCall.ts",
"unittests/evaluation/objectRest.ts",
"unittests/evaluation/superInStaticInitializer.ts",
"unittests/evaluation/templateLiteral.ts",
"unittests/evaluation/updateExpressionInModule.ts",
"unittests/services/cancellableLanguageServiceOperations.ts",
"unittests/services/colorization.ts",
Expand Down
40 changes: 40 additions & 0 deletions src/testRunner/unittests/evaluation/templateLiteral.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
describe("unittests:: evaluation:: templateLiteral", () => {
it("toString() over valueOf()", () => {
const result = evaluator.evaluateTypeScript(`
class C {
toString() {
return "toString";
}
valueOf() {
return "valueOf";
}
}

export const output = \`\${new C}\`;
`);
assert.strictEqual(result.output, "toString");
});

it("correct evaluation order", () => {
const result = evaluator.evaluateTypeScript(`
class C {
counter: number;

constructor() {
this.counter = 0;
}

get foo() {
this.counter++;
return {
toString: () => this.counter++,
};
}
}

const c = new C;
export const output = \`\${c.foo} \${c.foo}\`;
`);
assert.strictEqual(result.output, "1 3");
});
});
4 changes: 2 additions & 2 deletions tests/baselines/reference/APISample_compile.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,10 +66,10 @@ function compile(fileNames, options) {
return;
}
var _a = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start), line = _a.line, character = _a.character;
console.log(diagnostic.file.fileName + " (" + (line + 1) + "," + (character + 1) + "): " + message);
console.log("".concat(diagnostic.file.fileName, " (").concat(line + 1, ",").concat(character + 1, "): ").concat(message));
});
var exitCode = emitResult.emitSkipped ? 1 : 0;
console.log("Process exiting with code '" + exitCode + "'.");
console.log("Process exiting with code '".concat(exitCode, "'."));
process.exit(exitCode);
}
exports.compile = compile;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/APISample_linter.js
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ function delint(sourceFile) {
}
function report(node, message) {
var _a = sourceFile.getLineAndCharacterOfPosition(node.getStart()), line = _a.line, character = _a.character;
console.log(sourceFile.fileName + " (" + (line + 1) + "," + (character + 1) + "): " + message);
console.log("".concat(sourceFile.fileName, " (").concat(line + 1, ",").concat(character + 1, "): ").concat(message));
}
}
exports.delint = delint;
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/APISample_parseConfig.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ function printError(error) {
if (!error) {
return;
}
console.log((error.file && error.file.fileName) + ": " + error.messageText);
console.log("".concat(error.file && error.file.fileName, ": ").concat(error.messageText));
}
function createProgram(rootFiles, compilerOptionsJson) {
var _a = ts.parseConfigFileTextToJson("tsconfig.json", compilerOptionsJson), config = _a.config, error = _a.error;
Expand Down
8 changes: 4 additions & 4 deletions tests/baselines/reference/APISample_watcher.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,10 +166,10 @@ function watch(rootFileNames, options) {
function emitFile(fileName) {
var output = services.getEmitOutput(fileName);
if (!output.emitSkipped) {
console.log("Emitting " + fileName);
console.log("Emitting ".concat(fileName));
}
else {
console.log("Emitting " + fileName + " failed");
console.log("Emitting ".concat(fileName, " failed"));
logErrors(fileName);
}
output.outputFiles.forEach(function (o) {
Expand All @@ -184,10 +184,10 @@ function watch(rootFileNames, options) {
var message = ts.flattenDiagnosticMessageText(diagnostic.messageText, "\n");
if (diagnostic.file) {
var _a = diagnostic.file.getLineAndCharacterOfPosition(diagnostic.start), line = _a.line, character = _a.character;
console.log(" Error " + diagnostic.file.fileName + " (" + (line + 1) + "," + (character + 1) + "): " + message);
console.log(" Error ".concat(diagnostic.file.fileName, " (").concat(line + 1, ",").concat(character + 1, "): ").concat(message));
}
else {
console.log(" Error: " + message);
console.log(" Error: ".concat(message));
}
});
}
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/TemplateExpression1.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,4 +2,4 @@
var v = `foo ${ a

//// [TemplateExpression1.js]
var v = "foo " + a;
var v = "foo ".concat(a);
10 changes: 5 additions & 5 deletions tests/baselines/reference/asOperator3.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,11 @@ var __makeTemplateObject = (this && this.__makeTemplateObject) || function (cook
if (Object.defineProperty) { Object.defineProperty(cooked, "raw", { value: raw }); } else { cooked.raw = raw; }
return cooked;
};
var a = "" + (123 + 456);
var b = "leading " + (123 + 456);
var c = 123 + 456 + " trailing";
var d = "Hello " + 123 + " World";
var a = "".concat(123 + 456);
var b = "leading ".concat(123 + 456);
var c = "".concat(123 + 456, " trailing");
var d = "Hello ".concat(123, " World");
var e = "Hello";
var f = 1 + (1 + " end of string");
var f = 1 + "".concat(1, " end of string");
var g = tag(__makeTemplateObject(["Hello ", " World"], ["Hello ", " World"]), 123);
var h = tag(__makeTemplateObject(["Hello"], ["Hello"]));
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ stringIndex[s].toFixed();
// @ts-check
var _a, _b;
var n = Math.random();
var s = "" + n;
var s = "".concat(n);
var numericIndex = (_a = {}, _a[n] = 1, _a);
numericIndex[n].toFixed();
var stringIndex = (_b = {}, _b[s] = 1, _b);
Expand Down
4 changes: 2 additions & 2 deletions tests/baselines/reference/classAttributeInferenceTemplate.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ var MyClass = /** @class */ (function () {
function MyClass() {
var variable = 'something';
this.property = "foo"; // Correctly inferred as `string`
this.property2 = "foo-" + variable; // Causes an error
var localProperty = "foo-" + variable; // Correctly inferred as `string`
this.property2 = "foo-".concat(variable); // Causes an error
var localProperty = "foo-".concat(variable); // Correctly inferred as `string`
}
return MyClass;
}());
2 changes: 1 addition & 1 deletion tests/baselines/reference/computedPropertyNames10_ES5.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ var v = (_a = {},
_a[a] = function () { },
_a[true] = function () { },
_a["hello bye"] = function () { },
_a["hello " + a + " bye"] = function () { },
_a["hello ".concat(a, " bye")] = function () { },
_a);
2 changes: 1 addition & 1 deletion tests/baselines/reference/computedPropertyNames11_ES5.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ var v = (_a = {},
enumerable: false,
configurable: true
}),
Object.defineProperty(_a, "hello " + a + " bye", {
Object.defineProperty(_a, "hello ".concat(a, " bye"), {
get: function () { return 0; },
enumerable: false,
configurable: true
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/computedPropertyNames12_ES5.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ var C = /** @class */ (function () {
this["hello bye"] = 0;
}
var _a, _b, _c;
_a = n, s + s, _b = s + n, +s, _c = "hello " + a + " bye";
_a = n, s + s, _b = s + n, +s, _c = "hello ".concat(a, " bye");
C[_c] = 0;
return C;
}());
2 changes: 1 addition & 1 deletion tests/baselines/reference/computedPropertyNames13_ES5.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,6 @@ var C = /** @class */ (function () {
C.prototype[a] = function () { };
C[true] = function () { };
C.prototype["hello bye"] = function () { };
C["hello " + a + " bye"] = function () { };
C["hello ".concat(a, " bye")] = function () { };
return C;
}());
2 changes: 1 addition & 1 deletion tests/baselines/reference/computedPropertyNames16_ES5.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ var C = /** @class */ (function () {
enumerable: false,
configurable: true
});
Object.defineProperty(C.prototype, "hello " + a + " bye", {
Object.defineProperty(C.prototype, "hello ".concat(a, " bye"), {
get: function () { return 0; },
enumerable: false,
configurable: true
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/computedPropertyNames4_ES5.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,5 +32,5 @@ var v = (_a = {},
_a[a] = 1,
_a[true] = 0,
_a["hello bye"] = 0,
_a["hello " + a + " bye"] = 0,
_a["hello ".concat(a, " bye")] = 0,
_a);
2 changes: 1 addition & 1 deletion tests/baselines/reference/constEnumErrors.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ var E;
var y0 = E2[1];
var name = "A";
var y1 = E2[name];
var y2 = E2["" + name];
var y2 = E2["".concat(name)];
var x = E2;
var y = [E2];
function foo(t) {
Expand Down
2 changes: 1 addition & 1 deletion tests/baselines/reference/declarationNoDanglingGenerics.js
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ exports.CKind = exports.BKind = exports.AKind = void 0;
var kindCache = {};
function register(kind) {
if (kindCache[kind]) {
throw new Error("Class with kind \"" + kind + "\" is already registered.");
throw new Error("Class with kind \"".concat(kind, "\" is already registered."));
}
kindCache[kind] = true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ var Parent = function (_a) {
};
var Child = function (_a) {
var children = _a.children, _b = _a.name, name = _b === void 0 ? "Artemis" : _b, props = __rest(_a, ["children", "name"]);
return "name: " + name + " props: " + JSON.stringify(props);
return "name: ".concat(name, " props: ").concat(JSON.stringify(props));
};
f(function (_a) {
var _1 = _a[0], _b = _a[1], _2 = _b === void 0 ? undefined : _b;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,22 +31,22 @@ var t1 = 10;
var t2 = 10;
var s;
// With TemplateTail
Math.pow(t1, -t2) + " world";
Math.pow((-t1), t2) - t1 + " world";
Math.pow((-++t1), t2) - t1 + " world";
Math.pow((-t1++), t2) - t1 + " world";
Math.pow((~t1), Math.pow(t2, --t1)) + " world";
typeof (Math.pow(t1, Math.pow(t2, t1))) + " world";
"".concat(Math.pow(t1, -t2), " world");
"".concat(Math.pow((-t1), t2) - t1, " world");
"".concat(Math.pow((-++t1), t2) - t1, " world");
"".concat(Math.pow((-t1++), t2) - t1, " world");
"".concat(Math.pow((~t1), Math.pow(t2, --t1)), " world");
"".concat(typeof (Math.pow(t1, Math.pow(t2, t1))), " world");
// TempateHead & TemplateTail are empt
Math.pow(t1, -t2) + " hello world " + Math.pow(t1, -t2);
Math.pow((-t1), t2) - t1 + " hello world " + (Math.pow((-t1), t2) - t1);
Math.pow((-++t1), t2) - t1 + " hello world " + Math.pow(t1, Math.pow((-++t1), -t1));
Math.pow((-t1++), t2) - t1 + " hello world " + Math.pow(t2, Math.pow((-t1++), -t1));
Math.pow((~t1), Math.pow(t2, --t1)) + " hello world " + Math.pow((~t1), Math.pow(t2, --t1));
typeof (Math.pow(t1, Math.pow(t2, t1))) + " hello world " + typeof (Math.pow(t1, Math.pow(t2, t1)));
"".concat(Math.pow(t1, -t2), " hello world ").concat(Math.pow(t1, -t2));
"".concat(Math.pow((-t1), t2) - t1, " hello world ").concat(Math.pow((-t1), t2) - t1);
"".concat(Math.pow((-++t1), t2) - t1, " hello world ").concat(Math.pow(t1, Math.pow((-++t1), -t1)));
"".concat(Math.pow((-t1++), t2) - t1, " hello world ").concat(Math.pow(t2, Math.pow((-t1++), -t1)));
"".concat(Math.pow((~t1), Math.pow(t2, --t1)), " hello world ").concat(Math.pow((~t1), Math.pow(t2, --t1)));
"".concat(typeof (Math.pow(t1, Math.pow(t2, t1))), " hello world ").concat(typeof (Math.pow(t1, Math.pow(t2, t1))));
// With templateHead
"hello " + (Math.pow((-t1), t2) - t1);
"hello " + (Math.pow((-++t1), t2) - t1);
"hello " + (Math.pow((-t1++), t2) - t1);
"hello " + Math.pow((~t1), Math.pow(t2, --t1));
"hello " + typeof (Math.pow(t1, Math.pow(t2, t1)));
"hello ".concat(Math.pow((-t1), t2) - t1);
"hello ".concat(Math.pow((-++t1), t2) - t1);
"hello ".concat(Math.pow((-t1++), t2) - t1);
"hello ".concat(Math.pow((~t1), Math.pow(t2, --t1)));
"hello ".concat(typeof (Math.pow(t1, Math.pow(t2, t1))));
Loading