Skip to content

Commit 4e6b2f3

Browse files
weswighamweswigham
authored
Created a branded type for identifier-escaped strings (#16915)
* Created a branded type for escaped strings Then flowed it throughout the compiler, finding and fixing a handful of bugs relating to underscore-prefixed identifiers in the process. Includes a test for two cases noticed - diagnostics from conflicting symbols from export *'s, and enum with underscore prefixed member emit. * Correctly double underscores WRT mapped types * Add fourslash tests for other fixed issues * use function call over cast * Update forEachEntry type accuracy * Just use escaped names for ActiveLabel * Remove casts from getPropertyNameForPropertyNameNode * This pattern has occurred a few times, could use a helper function. * Remove duplicated helper * Remove unneeded check, use helper * Identifiers list is no longer escaped strings * Extract repeated string-getting code into helper * Rename type and associated functions * Make getName() return UnderscoreEscapedString, add getUnescapedName() * Add list of internal symbol names to escaped string type to cut back on casting * Remove outdated comments * Reassign interned values to nodes, just in case * Swap to string enum * Add deprecated aliases to escapeIdentifier and unescapeIdentifier * Add temp var * Remove unsafe casts * Rename escaped string type as per @sandersn's suggestion, fix string enum usages * Reorganize double underscore tests * Remove jfreeman from TODO * Remove unneeded parenthesis
1 parent ad291d9 commit 4e6b2f3

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

63 files changed

+1206
-514
lines changed

‎src/compiler/binder.ts

Lines changed: 54 additions & 53 deletions
Large diffs are not rendered by default.

‎src/compiler/checker.ts

Lines changed: 250 additions & 251 deletions
Large diffs are not rendered by default.

‎src/compiler/commandLineParser.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1047,7 +1047,7 @@ namespace ts {
10471047
errors.push(createDiagnosticForNodeInSourceFile(sourceFile, element.name, Diagnostics.String_literal_with_double_quotes_expected));
10481048
}
10491049

1050-
const keyText = getTextOfPropertyName(element.name);
1050+
const keyText = unescapeLeadingUnderscores(getTextOfPropertyName(element.name));
10511051
const option = knownOptions ? knownOptions.get(keyText) : undefined;
10521052
if (extraKeyDiagnosticMessage && !option) {
10531053
errors.push(createDiagnosticForNodeInSourceFile(sourceFile, element.name, extraKeyDiagnosticMessage, keyText));

‎src/compiler/core.ts

Lines changed: 35 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,22 @@ namespace ts {
4747
return new MapCtr<T>();
4848
}
4949

50+
/** Create a new escaped identifier map. */
51+
export function createUnderscoreEscapedMap<T>(): UnderscoreEscapedMap<T> {
52+
return new MapCtr<T>() as UnderscoreEscapedMap<T>;
53+
}
54+
55+
/* @internal */
56+
export function createSymbolTable(symbols?: Symbol[]): SymbolTable {
57+
const result = createMap<Symbol>() as SymbolTable;
58+
if (symbols) {
59+
for (const symbol of symbols) {
60+
result.set(symbol.name, symbol);
61+
}
62+
}
63+
return result;
64+
}
65+
5066
export function createMapFromTemplate<T>(template?: MapLike<T>): Map<T> {
5167
const map: Map<T> = new MapCtr<T>();
5268

@@ -1000,11 +1016,13 @@ namespace ts {
10001016
* Calls `callback` for each entry in the map, returning the first truthy result.
10011017
* Use `map.forEach` instead for normal iteration.
10021018
*/
1003-
export function forEachEntry<T, U>(map: Map<T>, callback: (value: T, key: string) => U | undefined): U | undefined {
1019+
export function forEachEntry<T, U>(map: UnderscoreEscapedMap<T>, callback: (value: T, key: __String) => U | undefined): U | undefined;
1020+
export function forEachEntry<T, U>(map: Map<T>, callback: (value: T, key: string) => U | undefined): U | undefined;
1021+
export function forEachEntry<T, U>(map: UnderscoreEscapedMap<T> | Map<T>, callback: (value: T, key: (string & __String)) => U | undefined): U | undefined {
10041022
const iterator = map.entries();
10051023
for (let { value: pair, done } = iterator.next(); !done; { value: pair, done } = iterator.next()) {
10061024
const [key, value] = pair;
1007-
const result = callback(value, key);
1025+
const result = callback(value, key as (string & __String));
10081026
if (result) {
10091027
return result;
10101028
}
@@ -1013,10 +1031,12 @@ namespace ts {
10131031
}
10141032

10151033
/** `forEachEntry` for just keys. */
1016-
export function forEachKey<T>(map: Map<{}>, callback: (key: string) => T | undefined): T | undefined {
1034+
export function forEachKey<T>(map: UnderscoreEscapedMap<{}>, callback: (key: __String) => T | undefined): T | undefined;
1035+
export function forEachKey<T>(map: Map<{}>, callback: (key: string) => T | undefined): T | undefined;
1036+
export function forEachKey<T>(map: UnderscoreEscapedMap<{}> | Map<{}>, callback: (key: string & __String) => T | undefined): T | undefined {
10171037
const iterator = map.keys();
10181038
for (let { value: key, done } = iterator.next(); !done; { value: key, done } = iterator.next()) {
1019-
const result = callback(key);
1039+
const result = callback(key as string & __String);
10201040
if (result) {
10211041
return result;
10221042
}
@@ -1025,9 +1045,11 @@ namespace ts {
10251045
}
10261046

10271047
/** Copy entries from `source` to `target`. */
1028-
export function copyEntries<T>(source: Map<T>, target: Map<T>): void {
1029-
source.forEach((value, key) => {
1030-
target.set(key, value);
1048+
export function copyEntries<T>(source: UnderscoreEscapedMap<T>, target: UnderscoreEscapedMap<T>): void;
1049+
export function copyEntries<T>(source: Map<T>, target: Map<T>): void;
1050+
export function copyEntries<T, U extends UnderscoreEscapedMap<T> | Map<T>>(source: U, target: U): void {
1051+
(source as Map<T>).forEach((value, key) => {
1052+
(target as Map<T>).set(key, value);
10311053
});
10321054
}
10331055

@@ -1099,9 +1121,11 @@ namespace ts {
10991121
return arrayToMap<T, true>(array, makeKey, () => true);
11001122
}
11011123

1102-
export function cloneMap<T>(map: Map<T>) {
1124+
export function cloneMap(map: SymbolTable): SymbolTable;
1125+
export function cloneMap<T>(map: Map<T>): Map<T>;
1126+
export function cloneMap<T>(map: Map<T> | SymbolTable): Map<T> | SymbolTable {
11031127
const clone = createMap<T>();
1104-
copyEntries(map, clone);
1128+
copyEntries(map as Map<T>, clone);
11051129
return clone;
11061130
}
11071131

@@ -2272,13 +2296,13 @@ namespace ts {
22722296
getTokenConstructor(): new <TKind extends SyntaxKind>(kind: TKind, pos?: number, end?: number) => Token<TKind>;
22732297
getIdentifierConstructor(): new (kind: SyntaxKind.Identifier, pos?: number, end?: number) => Identifier;
22742298
getSourceFileConstructor(): new (kind: SyntaxKind.SourceFile, pos?: number, end?: number) => SourceFile;
2275-
getSymbolConstructor(): new (flags: SymbolFlags, name: string) => Symbol;
2299+
getSymbolConstructor(): new (flags: SymbolFlags, name: __String) => Symbol;
22762300
getTypeConstructor(): new (checker: TypeChecker, flags: TypeFlags) => Type;
22772301
getSignatureConstructor(): new (checker: TypeChecker) => Signature;
22782302
getSourceMapSourceConstructor(): new (fileName: string, text: string, skipTrivia?: (pos: number) => number) => SourceMapSource;
22792303
}
22802304

2281-
function Symbol(this: Symbol, flags: SymbolFlags, name: string) {
2305+
function Symbol(this: Symbol, flags: SymbolFlags, name: __String) {
22822306
this.flags = flags;
22832307
this.name = name;
22842308
this.declarations = undefined;

‎src/compiler/emitter.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2761,7 +2761,7 @@ namespace ts {
27612761
return generateName(node);
27622762
}
27632763
else if (isIdentifier(node) && (nodeIsSynthesized(node) || !node.parent)) {
2764-
return unescapeIdentifier(node.text);
2764+
return unescapeLeadingUnderscores(node.text);
27652765
}
27662766
else if (node.kind === SyntaxKind.StringLiteral && (<StringLiteral>node).textSourceNode) {
27672767
return getTextOfNode((<StringLiteral>node).textSourceNode, includeTrivia);
@@ -2818,13 +2818,13 @@ namespace ts {
28182818
// Auto, Loop, and Unique names are cached based on their unique
28192819
// autoGenerateId.
28202820
const autoGenerateId = name.autoGenerateId;
2821-
return autoGeneratedIdToGeneratedName[autoGenerateId] || (autoGeneratedIdToGeneratedName[autoGenerateId] = unescapeIdentifier(makeName(name)));
2821+
return autoGeneratedIdToGeneratedName[autoGenerateId] || (autoGeneratedIdToGeneratedName[autoGenerateId] = makeName(name));
28222822
}
28232823
}
28242824

28252825
function generateNameCached(node: Node) {
28262826
const nodeId = getNodeId(node);
2827-
return nodeIdToGeneratedName[nodeId] || (nodeIdToGeneratedName[nodeId] = unescapeIdentifier(generateNameForNode(node)));
2827+
return nodeIdToGeneratedName[nodeId] || (nodeIdToGeneratedName[nodeId] = generateNameForNode(node));
28282828
}
28292829

28302830
/**
@@ -2843,7 +2843,7 @@ namespace ts {
28432843
function isUniqueLocalName(name: string, container: Node): boolean {
28442844
for (let node = container; isNodeDescendantOf(node, container); node = node.nextContainer) {
28452845
if (node.locals) {
2846-
const local = node.locals.get(name);
2846+
const local = node.locals.get(escapeLeadingUnderscores(name));
28472847
// We conservatively include alias symbols to cover cases where they're emitted as locals
28482848
if (local && local.flags & (SymbolFlags.Value | SymbolFlags.ExportValue | SymbolFlags.Alias)) {
28492849
return false;
@@ -2918,7 +2918,7 @@ namespace ts {
29182918
function generateNameForImportOrExportDeclaration(node: ImportDeclaration | ExportDeclaration) {
29192919
const expr = getExternalModuleName(node);
29202920
const baseName = expr.kind === SyntaxKind.StringLiteral ?
2921-
escapeIdentifier(makeIdentifierFromModuleName((<LiteralExpression>expr).text)) : "module";
2921+
makeIdentifierFromModuleName((<LiteralExpression>expr).text) : "module";
29222922
return makeUniqueName(baseName);
29232923
}
29242924

@@ -2981,7 +2981,7 @@ namespace ts {
29812981
case GeneratedIdentifierKind.Loop:
29822982
return makeTempVariableName(TempFlags._i);
29832983
case GeneratedIdentifierKind.Unique:
2984-
return makeUniqueName(unescapeIdentifier(name.text));
2984+
return makeUniqueName(unescapeLeadingUnderscores(name.text));
29852985
}
29862986

29872987
Debug.fail("Unsupported GeneratedIdentifierKind.");

‎src/compiler/factory.ts

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ namespace ts {
9999
}
100100

101101
function createLiteralFromNode(sourceNode: StringLiteral | NumericLiteral | Identifier): StringLiteral {
102-
const node = createStringLiteral(sourceNode.text);
102+
const node = createStringLiteral(getTextOfIdentifierOrLiteral(sourceNode));
103103
node.textSourceNode = sourceNode;
104104
return node;
105105
}
@@ -112,7 +112,7 @@ namespace ts {
112112
export function createIdentifier(text: string, typeArguments: TypeNode[]): Identifier;
113113
export function createIdentifier(text: string, typeArguments?: TypeNode[]): Identifier {
114114
const node = <Identifier>createSynthesizedNode(SyntaxKind.Identifier);
115-
node.text = escapeIdentifier(text);
115+
node.text = escapeLeadingUnderscores(text);
116116
node.originalKeywordKind = text ? stringToToken(text) : SyntaxKind.Unknown;
117117
node.autoGenerateKind = GeneratedIdentifierKind.None;
118118
node.autoGenerateId = 0;
@@ -124,7 +124,7 @@ namespace ts {
124124

125125
export function updateIdentifier(node: Identifier, typeArguments: NodeArray<TypeNode> | undefined): Identifier {
126126
return node.typeArguments !== typeArguments
127-
? updateNode(createIdentifier(node.text, typeArguments), node)
127+
? updateNode(createIdentifier(unescapeLeadingUnderscores(node.text), typeArguments), node)
128128
: node;
129129
}
130130

@@ -2645,12 +2645,12 @@ namespace ts {
26452645
function createJsxFactoryExpressionFromEntityName(jsxFactory: EntityName, parent: JsxOpeningLikeElement): Expression {
26462646
if (isQualifiedName(jsxFactory)) {
26472647
const left = createJsxFactoryExpressionFromEntityName(jsxFactory.left, parent);
2648-
const right = createIdentifier(jsxFactory.right.text);
2648+
const right = createIdentifier(unescapeLeadingUnderscores(jsxFactory.right.text));
26492649
right.text = jsxFactory.right.text;
26502650
return createPropertyAccess(left, right);
26512651
}
26522652
else {
2653-
return createReactNamespace(jsxFactory.text, parent);
2653+
return createReactNamespace(unescapeLeadingUnderscores(jsxFactory.text), parent);
26542654
}
26552655
}
26562656

‎src/compiler/parser.ts

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -1178,12 +1178,11 @@ namespace ts {
11781178
}
11791179

11801180
const result = createNode(kind, scanner.getStartPos());
1181-
(<Identifier>result).text = "";
1181+
(<Identifier>result).text = "" as __String;
11821182
return finishNode(result);
11831183
}
11841184

11851185
function internIdentifier(text: string): string {
1186-
text = escapeIdentifier(text);
11871186
let identifier = identifiers.get(text);
11881187
if (identifier === undefined) {
11891188
identifiers.set(text, identifier = text);
@@ -1203,7 +1202,7 @@ namespace ts {
12031202
if (token() !== SyntaxKind.Identifier) {
12041203
node.originalKeywordKind = token();
12051204
}
1206-
node.text = internIdentifier(scanner.getTokenValue());
1205+
node.text = escapeLeadingUnderscores(internIdentifier(scanner.getTokenValue()));
12071206
nextToken();
12081207
return finishNode(node);
12091208
}
@@ -1227,7 +1226,9 @@ namespace ts {
12271226

12281227
function parsePropertyNameWorker(allowComputedPropertyNames: boolean): PropertyName {
12291228
if (token() === SyntaxKind.StringLiteral || token() === SyntaxKind.NumericLiteral) {
1230-
return <StringLiteral | NumericLiteral>parseLiteralNode(/*internName*/ true);
1229+
const node = <StringLiteral | NumericLiteral>parseLiteralNode();
1230+
node.text = internIdentifier(node.text);
1231+
return node;
12311232
}
12321233
if (allowComputedPropertyNames && token() === SyntaxKind.OpenBracketToken) {
12331234
return parseComputedPropertyName();
@@ -2049,26 +2050,26 @@ namespace ts {
20492050
return finishNode(span);
20502051
}
20512052

2052-
function parseLiteralNode(internName?: boolean): LiteralExpression {
2053-
return <LiteralExpression>parseLiteralLikeNode(token(), internName);
2053+
function parseLiteralNode(): LiteralExpression {
2054+
return <LiteralExpression>parseLiteralLikeNode(token());
20542055
}
20552056

20562057
function parseTemplateHead(): TemplateHead {
2057-
const fragment = parseLiteralLikeNode(token(), /*internName*/ false);
2058+
const fragment = parseLiteralLikeNode(token());
20582059
Debug.assert(fragment.kind === SyntaxKind.TemplateHead, "Template head has wrong token kind");
20592060
return <TemplateHead>fragment;
20602061
}
20612062

20622063
function parseTemplateMiddleOrTemplateTail(): TemplateMiddle | TemplateTail {
2063-
const fragment = parseLiteralLikeNode(token(), /*internName*/ false);
2064+
const fragment = parseLiteralLikeNode(token());
20642065
Debug.assert(fragment.kind === SyntaxKind.TemplateMiddle || fragment.kind === SyntaxKind.TemplateTail, "Template fragment has wrong token kind");
20652066
return <TemplateMiddle | TemplateTail>fragment;
20662067
}
20672068

2068-
function parseLiteralLikeNode(kind: SyntaxKind, internName: boolean): LiteralLikeNode {
2069+
function parseLiteralLikeNode(kind: SyntaxKind): LiteralLikeNode {
20692070
const node = <LiteralExpression>createNode(kind);
20702071
const text = scanner.getTokenValue();
2071-
node.text = internName ? internIdentifier(text) : text;
2072+
node.text = text;
20722073

20732074
if (scanner.hasExtendedUnicodeEscape()) {
20742075
node.hasExtendedUnicodeEscape = true;
@@ -5624,7 +5625,8 @@ namespace ts {
56245625
node.flags |= NodeFlags.GlobalAugmentation;
56255626
}
56265627
else {
5627-
node.name = <StringLiteral>parseLiteralNode(/*internName*/ true);
5628+
node.name = <StringLiteral>parseLiteralNode();
5629+
node.name.text = internIdentifier(node.name.text);
56285630
}
56295631

56305632
if (token() === SyntaxKind.OpenBraceToken) {
@@ -5768,7 +5770,7 @@ namespace ts {
57685770
function parseModuleSpecifier(): Expression {
57695771
if (token() === SyntaxKind.StringLiteral) {
57705772
const result = parseLiteralNode();
5771-
internIdentifier((<LiteralExpression>result).text);
5773+
result.text = internIdentifier(result.text);
57725774
return result;
57735775
}
57745776
else {
@@ -6990,7 +6992,7 @@ namespace ts {
69906992
const pos = scanner.getTokenPos();
69916993
const end = scanner.getTextPos();
69926994
const result = <Identifier>createNode(SyntaxKind.Identifier, pos);
6993-
result.text = content.substring(pos, end);
6995+
result.text = escapeLeadingUnderscores(content.substring(pos, end));
69946996
finishNode(result, end);
69956997

69966998
nextJSDocToken();

‎src/compiler/program.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -405,7 +405,7 @@ namespace ts {
405405
let commonSourceDirectory: string;
406406
let diagnosticsProducingTypeChecker: TypeChecker;
407407
let noDiagnosticsTypeChecker: TypeChecker;
408-
let classifiableNames: Map<string>;
408+
let classifiableNames: UnderscoreEscapedMap<__String>;
409409
let modifiedFilePaths: Path[] | undefined;
410410

411411
const cachedSemanticDiagnosticsForFile: DiagnosticCache = {};
@@ -580,7 +580,7 @@ namespace ts {
580580
if (!classifiableNames) {
581581
// Initialize a checker so that all our files are bound.
582582
getTypeChecker();
583-
classifiableNames = createMap<string>();
583+
classifiableNames = createUnderscoreEscapedMap<__String>();
584584

585585
for (const sourceFile of files) {
586586
copyEntries(sourceFile.classifiableNames, classifiableNames);

‎src/compiler/transformers/destructuring.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -411,11 +411,11 @@ namespace ts {
411411
}
412412
else if (isStringOrNumericLiteral(propertyName)) {
413413
const argumentExpression = getSynthesizedClone(propertyName);
414-
argumentExpression.text = unescapeIdentifier(argumentExpression.text);
414+
argumentExpression.text = argumentExpression.text;
415415
return createElementAccess(value, argumentExpression);
416416
}
417417
else {
418-
const name = createIdentifier(unescapeIdentifier(propertyName.text));
418+
const name = createIdentifier(unescapeLeadingUnderscores(propertyName.text));
419419
return createPropertyAccess(value, name);
420420
}
421421
}

‎src/compiler/transformers/es2015.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -661,7 +661,7 @@ namespace ts {
661661
// - break/continue is non-labeled and located in non-converted loop/switch statement
662662
const jump = node.kind === SyntaxKind.BreakStatement ? Jump.Break : Jump.Continue;
663663
const canUseBreakOrContinue =
664-
(node.label && convertedLoopState.labels && convertedLoopState.labels.get(node.label.text)) ||
664+
(node.label && convertedLoopState.labels && convertedLoopState.labels.get(unescapeLeadingUnderscores(node.label.text))) ||
665665
(!node.label && (convertedLoopState.allowedNonLabeledJumps & jump));
666666

667667
if (!canUseBreakOrContinue) {
@@ -680,11 +680,11 @@ namespace ts {
680680
else {
681681
if (node.kind === SyntaxKind.BreakStatement) {
682682
labelMarker = `break-${node.label.text}`;
683-
setLabeledJump(convertedLoopState, /*isBreak*/ true, node.label.text, labelMarker);
683+
setLabeledJump(convertedLoopState, /*isBreak*/ true, unescapeLeadingUnderscores(node.label.text), labelMarker);
684684
}
685685
else {
686686
labelMarker = `continue-${node.label.text}`;
687-
setLabeledJump(convertedLoopState, /*isBreak*/ false, node.label.text, labelMarker);
687+
setLabeledJump(convertedLoopState, /*isBreak*/ false, unescapeLeadingUnderscores(node.label.text), labelMarker);
688688
}
689689
}
690690
let returnExpression: Expression = createLiteral(labelMarker);
@@ -2236,11 +2236,11 @@ namespace ts {
22362236
}
22372237

22382238
function recordLabel(node: LabeledStatement) {
2239-
convertedLoopState.labels.set(node.label.text, node.label.text);
2239+
convertedLoopState.labels.set(unescapeLeadingUnderscores(node.label.text), unescapeLeadingUnderscores(node.label.text));
22402240
}
22412241

22422242
function resetLabel(node: LabeledStatement) {
2243-
convertedLoopState.labels.set(node.label.text, undefined);
2243+
convertedLoopState.labels.set(unescapeLeadingUnderscores(node.label.text), undefined);
22442244
}
22452245

22462246
function visitLabeledStatement(node: LabeledStatement): VisitResult<Statement> {
@@ -3053,7 +3053,7 @@ namespace ts {
30533053
else {
30543054
loopParameters.push(createParameter(/*decorators*/ undefined, /*modifiers*/ undefined, /*dotDotDotToken*/ undefined, name));
30553055
if (resolver.getNodeCheckFlags(decl) & NodeCheckFlags.NeedsLoopOutParameter) {
3056-
const outParamName = createUniqueName("out_" + unescapeIdentifier(name.text));
3056+
const outParamName = createUniqueName("out_" + unescapeLeadingUnderscores(name.text));
30573057
loopOutParameters.push({ originalName: name, outParamName });
30583058
}
30593059
}

0 commit comments

Comments
 (0)