Skip to content

Some tweaks for backtick strings #45099

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 1 commit into from
Aug 5, 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
2 changes: 2 additions & 0 deletions src/compiler/transformers/taggedTemplate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,8 @@ namespace ts {
// Examples: `\n` is converted to "\\n", a template string with a newline to "\n".
let text = node.rawText;
if (text === undefined) {
Debug.assertIsDefined(currentSourceFile,
"Template literal node is missing 'rawText' and does not have a source file. Possibly bad transform.");
text = getSourceTextOfNodeFromSourceFile(currentSourceFile, node);

// text contains the original source, it will also contain quotes ("`"), dolar signs and braces ("${" and "}"),
Expand Down
9 changes: 5 additions & 4 deletions src/compiler/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -670,7 +670,7 @@ namespace ts {
const escapeText = flags & GetLiteralTextFlags.NeverAsciiEscape || (getEmitFlags(node) & EmitFlags.NoAsciiEscaping) ? escapeString :
escapeNonAsciiString;

const rawText = (node as TemplateLiteralLikeNode).rawText || escapeTemplateSubstitution(escapeText(node.text, CharacterCodes.backtick));
const rawText = (node as TemplateLiteralLikeNode).rawText ?? escapeTemplateSubstitution(escapeText(node.text, CharacterCodes.backtick));
switch (node.kind) {
case SyntaxKind.NoSubstitutionTemplateLiteral:
return "`" + rawText + "`";
Expand Down Expand Up @@ -3847,8 +3847,8 @@ namespace ts {
// There is no reason for this other than that JSON.stringify does not handle it either.
const doubleQuoteEscapedCharsRegExp = /[\\\"\u0000-\u001f\t\v\f\b\r\n\u2028\u2029\u0085]/g;
const singleQuoteEscapedCharsRegExp = /[\\\'\u0000-\u001f\t\v\f\b\r\n\u2028\u2029\u0085]/g;
// Template strings should be preserved as much as possible
const backtickQuoteEscapedCharsRegExp = /[\\`]/g;
// Template strings preserve simple LF newlines, still encode CRLF (or CR)
const backtickQuoteEscapedCharsRegExp = /\r\n|[\\\`\u0000-\u001f\t\v\f\b\r\u2028\u2029\u0085]/g;
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you add a test for this change? I'm unclear on why we wouldn't want to preserve these. backtickQuoteEsacpedCharsRegExp is only used when the quote character is CharacterCodes.backtick, which mean's we're preserving the raw text of the template string as much as possible, so it seems like adding extra escapes would break anyone using the raw strings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

  • I'll see if I can add a test for this.

  • Re the reason, I listed two reasons in the commit log:

    • Not escaping things like a TAB or a FF character can get mangled by editor style settings
    • Not escaping a CRLF newline and instead having the template literal have a CRLF will be interpreted as a regular newline, and result in a string with just LF. (It's even more likely to get mangled by your editor, but even if not you'll get that problem.)

Copy link
Contributor

Choose a reason for hiding this comment

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

  • Not escaping things like a TAB or a FF character can get mangled by editor style settings

Yeah, I can see that being a possible issue, however I'm still not sure "<TAB>foo" should become `\tfoo` since it is still semantically different between the raw and cooked forms. There are only two ways to get to this code path: 1) Via a user-defined transformation, in which case we would want to preserve what was written, or 2) via one of our code fixes. If we want to normalize TAB or FF in the code fix, that's fine, but I'm concerned whether this would affect anyone using custom transforms. This is a small enough corner case its probably reasonable however, and we can address it if there's feedback.

  • Not escaping a CRLF newline and instead having the template literal have a CRLF will be interpreted as a regular newline, and result in a string with just LF. (It's even more likely to get mangled by your editor, but even if not you'll get that problem.)

StringLiteral can't contain a raw CRLF unless its part of a LineContinuation, which becomes an empty code sequence, per NOTE 2 of https://tc39.es/ecma262/#sec-literals-string-literals and in https://tc39.es/ecma262/#sec-static-semantics-sv, so that wouldn't matter in a StringLiteralTemplateLiteral conversion. The escape sequence \r\n in a string literal should remain as the escape sequence \r\n in the template literal, but we shouldn't just convert a raw CRLF into the escape sequence as that could impact custom transforms (which I could see taking a reliance on the CRLF->LF normalization of template literals).

So, -1 on converting CRLF into the escaped form, +1 on converting other characters (at least until someone raises an issue).

const escapedCharsMap = new Map(getEntries({
"\t": "\\t",
"\v": "\\v",
Expand All @@ -3862,7 +3862,8 @@ namespace ts {
"\`": "\\\`",
"\u2028": "\\u2028", // lineSeparator
"\u2029": "\\u2029", // paragraphSeparator
"\u0085": "\\u0085" // nextLine
"\u0085": "\\u0085", // nextLine
"\r\n": "\\r\\n", // special case for CRLFs in backticks
}));

function encodeUtf16EscapeSequence(charCode: number): string {
Expand Down
14 changes: 14 additions & 0 deletions tests/baselines/reference/templateLiteralsInTypes.errors.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
tests/cases/compiler/templateLiteralsInTypes.ts(3,1): error TS2554: Expected 2 arguments, but got 1.
tests/cases/compiler/templateLiteralsInTypes.ts(3,8): error TS2339: Property 'foo' does not exist on type '`${string}:\t${number}\r\n`'.


==== tests/cases/compiler/templateLiteralsInTypes.ts (2 errors) ====
const f = (hdr: string, val: number) => `${hdr}:\t${val}\r\n` as `${string}:\t${number}\r\n`;

f("x").foo;
~~~~~~
!!! error TS2554: Expected 2 arguments, but got 1.
!!! related TS6210 tests/cases/compiler/templateLiteralsInTypes.ts:1:25: An argument for 'val' was not provided.
~~~
!!! error TS2339: Property 'foo' does not exist on type '`${string}:\t${number}\r\n`'.

14 changes: 14 additions & 0 deletions tests/baselines/reference/templateLiteralsInTypes.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
//// [templateLiteralsInTypes.ts]
const f = (hdr: string, val: number) => `${hdr}:\t${val}\r\n` as `${string}:\t${number}\r\n`;

f("x").foo;


//// [templateLiteralsInTypes.js]
"use strict";
var f = function (hdr, val) { return hdr + ":\t" + val + "\r\n"; };
f("x").foo;


//// [templateLiteralsInTypes.d.ts]
declare const f: (hdr: string, val: number) => `${string}:\t${number}\r\n`;
11 changes: 11 additions & 0 deletions tests/baselines/reference/templateLiteralsInTypes.symbols
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
=== tests/cases/compiler/templateLiteralsInTypes.ts ===
const f = (hdr: string, val: number) => `${hdr}:\t${val}\r\n` as `${string}:\t${number}\r\n`;
>f : Symbol(f, Decl(templateLiteralsInTypes.ts, 0, 5))
>hdr : Symbol(hdr, Decl(templateLiteralsInTypes.ts, 0, 11))
>val : Symbol(val, Decl(templateLiteralsInTypes.ts, 0, 23))
>hdr : Symbol(hdr, Decl(templateLiteralsInTypes.ts, 0, 11))
>val : Symbol(val, Decl(templateLiteralsInTypes.ts, 0, 23))

f("x").foo;
>f : Symbol(f, Decl(templateLiteralsInTypes.ts, 0, 5))

18 changes: 18 additions & 0 deletions tests/baselines/reference/templateLiteralsInTypes.types
Original file line number Diff line number Diff line change
@@ -0,0 +1,18 @@
=== tests/cases/compiler/templateLiteralsInTypes.ts ===
const f = (hdr: string, val: number) => `${hdr}:\t${val}\r\n` as `${string}:\t${number}\r\n`;
>f : (hdr: string, val: number) => `${string}:\t${number}\r\n`
>(hdr: string, val: number) => `${hdr}:\t${val}\r\n` as `${string}:\t${number}\r\n` : (hdr: string, val: number) => `${string}:\t${number}\r\n`
>hdr : string
>val : number
>`${hdr}:\t${val}\r\n` as `${string}:\t${number}\r\n` : `${string}:\t${number}\r\n`
>`${hdr}:\t${val}\r\n` : `${string}:\t${number}\r\n`
>hdr : string
>val : number

f("x").foo;
>f("x").foo : any
>f("x") : `${string}:\t${number}\r\n`
>f : (hdr: string, val: number) => `${string}:\t${number}\r\n`
>"x" : "x"
>foo : any

6 changes: 6 additions & 0 deletions tests/cases/compiler/templateLiteralsInTypes.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
// @strict: true
// @declaration: true

const f = (hdr: string, val: number) => `${hdr}:\t${val}\r\n` as `${string}:\t${number}\r\n`;

f("x").foo;