Skip to content

Created a branded type for identifier-escaped strings #16915

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.

Sign up for GitHub

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 26 commits into from
Jul 6, 2017
Merged

Created a branded type for identifier-escaped strings #16915

Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
042a78d
Created a branded type for escaped strings
weswigham Jun 30, 2017
ce4018c
Correctly double underscores WRT mapped types
weswigham Jul 3, 2017
342ec75
Add fourslash tests for other fixed issues
weswigham Jul 3, 2017
770010e
Merge branch 'master' into escaped-string-type
weswigham Jul 3, 2017
1f55366
use function call over cast
weswigham Jul 5, 2017
4411889
Update forEachEntry type accuracy
weswigham Jul 5, 2017
47f7fbd
Just use escaped names for ActiveLabel
weswigham Jul 5, 2017
7f22f39
Remove casts from getPropertyNameForPropertyNameNode
weswigham Jul 5, 2017
f88f21a
This pattern has occurred a few times, could use a helper function.
weswigham Jul 5, 2017
af30041
Remove duplicated helper
weswigham Jul 5, 2017
5d9ea3a
Remove unneeded check, use helper
weswigham Jul 5, 2017
352c13d
Identifiers list is no longer escaped strings
weswigham Jul 5, 2017
b371e7a
Extract repeated string-getting code into helper
weswigham Jul 5, 2017
ba0071e
Rename type and associated functions
weswigham Jul 5, 2017
e0c7dcc
Make getName() return UnderscoreEscapedString, add getUnescapedName()
weswigham Jul 6, 2017
50a8c04
Add list of internal symbol names to escaped string type to cut back …
weswigham Jul 6, 2017
3398d41
Remove outdated comments
weswigham Jul 6, 2017
a4bd2ab
Reassign interned values to nodes, just in case
weswigham Jul 6, 2017
0967927
Swap to string enum
weswigham Jul 6, 2017
e99b679
Add deprecated aliases to escapeIdentifier and unescapeIdentifier
weswigham Jul 6, 2017
3383d8a
Add temp var
weswigham Jul 6, 2017
5eca8a3
Remove unsafe casts
weswigham Jul 6, 2017
9424f14
Rename escaped string type as per @sandersn's suggestion, fix string …
weswigham Jul 6, 2017
23032d4
Reorganize double underscore tests
weswigham Jul 6, 2017
008532d
Remove jfreeman from TODO
weswigham Jul 6, 2017
5e6b69a
Remove unneeded parenthesis
weswigham Jul 6, 2017
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
77 changes: 39 additions & 38 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ namespace ts {
}

interface ActiveLabel {
name: string;
name: UnderscoreEscapedString;
breakTarget: FlowLabel;
continueTarget: FlowLabel;
referenced: boolean;
Expand Down Expand Up @@ -132,8 +132,8 @@ namespace ts {
let inStrictMode: boolean;

let symbolCount = 0;
let Symbol: { new (flags: SymbolFlags, name: string): Symbol };
let classifiableNames: Map<string>;
let Symbol: { new (flags: SymbolFlags, name: UnderscoreEscapedString): Symbol };
let classifiableNames: UnderscoreEscapedMap<UnderscoreEscapedString>;

const unreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
const reportedUnreachableFlow: FlowNode = { flags: FlowFlags.Unreachable };
Expand All @@ -147,7 +147,7 @@ namespace ts {
options = opts;
languageVersion = getEmitScriptTarget(options);
inStrictMode = bindInStrictMode(file, opts);
classifiableNames = createMap<string>();
classifiableNames = createUnderscoreEscapedMap<UnderscoreEscapedString>();
symbolCount = 0;
skipTransformFlagAggregation = file.isDeclarationFile;

Expand Down Expand Up @@ -191,7 +191,7 @@ namespace ts {
}
}

function createSymbol(flags: SymbolFlags, name: string): Symbol {
function createSymbol(flags: SymbolFlags, name: UnderscoreEscapedString): Symbol {
symbolCount++;
return new Symbol(flags, name);
}
Expand All @@ -207,11 +207,11 @@ namespace ts {
symbol.declarations.push(node);

if (symbolFlags & SymbolFlags.HasExports && !symbol.exports) {
symbol.exports = createMap<Symbol>();
symbol.exports = createSymbolTable();
}

if (symbolFlags & SymbolFlags.HasMembers && !symbol.members) {
symbol.members = createMap<Symbol>();
symbol.members = createSymbolTable();
}

if (symbolFlags & SymbolFlags.Value) {
Expand All @@ -226,23 +226,24 @@ namespace ts {

// Should not be called on a declaration with a computed property name,
// unless it is a well known Symbol.
function getDeclarationName(node: Declaration): string {
function getDeclarationName(node: Declaration): UnderscoreEscapedString {
const name = getNameOfDeclaration(node);
if (name) {
if (isAmbientModule(node)) {
return isGlobalScopeAugmentation(<ModuleDeclaration>node) ? "__global" : `"${(<LiteralExpression>name).text}"`;
const moduleName = getTextOfIdentifierOrLiteral(<Identifier | LiteralExpression>name);
return (isGlobalScopeAugmentation(<ModuleDeclaration>node) ? "__global" : `"${moduleName}"`) as UnderscoreEscapedString;
}
if (name.kind === SyntaxKind.ComputedPropertyName) {
const nameExpression = (<ComputedPropertyName>name).expression;
// treat computed property names where expression is string/numeric literal as just string/numeric literal
if (isStringOrNumericLiteral(nameExpression)) {
return nameExpression.text;
return escapeLeadingUnderscores(nameExpression.text);
}

Debug.assert(isWellKnownSymbolSyntactically(nameExpression));
return getPropertyNameForKnownSymbolName((<PropertyAccessExpression>nameExpression).name.text);
return getPropertyNameForKnownSymbolName(unescapeLeadingUnderscores((<PropertyAccessExpression>nameExpression).name.text));
}
return (<Identifier | LiteralExpression>name).text;
return getEscapedTextOfIdentifierOrLiteral(<Identifier | LiteralExpression>name);
}
switch (node.kind) {
case SyntaxKind.Constructor:
Expand All @@ -258,7 +259,7 @@ namespace ts {
case SyntaxKind.ExportDeclaration:
return "__export";
case SyntaxKind.ExportAssignment:
return (<ExportAssignment>node).isExportEquals ? "export=" : "default";
return ((<ExportAssignment>node).isExportEquals ? "export=" : "default");
Copy link
Member

Choose a reason for hiding this comment

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

looks like some extra parens got left in after their associated casts were removed.

case SyntaxKind.BinaryExpression:
if (getSpecialPropertyAssignmentKind(node as BinaryExpression) === SpecialPropertyAssignmentKind.ModuleExports) {
// module.exports = ...
Expand All @@ -269,19 +270,19 @@ namespace ts {

case SyntaxKind.FunctionDeclaration:
case SyntaxKind.ClassDeclaration:
return hasModifier(node, ModifierFlags.Default) ? "default" : undefined;
return (hasModifier(node, ModifierFlags.Default) ? "default" : undefined);
case SyntaxKind.JSDocFunctionType:
return isJSDocConstructSignature(node) ? "__new" : "__call";
return (isJSDocConstructSignature(node) ? "__new" : "__call");
case SyntaxKind.Parameter:
// Parameters with names are handled at the top of this function. Parameters
// without names can only come from JSDocFunctionTypes.
Debug.assert(node.parent.kind === SyntaxKind.JSDocFunctionType);
const functionType = <JSDocFunctionType>node.parent;
const index = indexOf(functionType.parameters, node);
return "arg" + index;
return "arg" + index as UnderscoreEscapedString;
case SyntaxKind.JSDocTypedefTag:
const parentNode = node.parent && node.parent.parent;
let nameFromParentNode: string;
let nameFromParentNode: UnderscoreEscapedString;
if (parentNode && parentNode.kind === SyntaxKind.VariableStatement) {
if ((<VariableStatement>parentNode).declarationList.declarations.length > 0) {
const nameIdentifier = (<VariableStatement>parentNode).declarationList.declarations[0].name;
Expand All @@ -295,7 +296,7 @@ namespace ts {
}

function getDisplayName(node: Declaration): string {
return (node as NamedDeclaration).name ? declarationNameToString((node as NamedDeclaration).name) : getDeclarationName(node);
return (node as NamedDeclaration).name ? declarationNameToString((node as NamedDeclaration).name) : unescapeLeadingUnderscores(getDeclarationName(node));
}

/**
Expand Down Expand Up @@ -481,7 +482,7 @@ namespace ts {
if (containerFlags & ContainerFlags.IsContainer) {
container = blockScopeContainer = node;
if (containerFlags & ContainerFlags.HasLocals) {
container.locals = createMap<Symbol>();
container.locals = createSymbolTable();
}
addToContainerChain(container);
}
Expand Down Expand Up @@ -1006,7 +1007,7 @@ namespace ts {
currentFlow = unreachableFlow;
}

function findActiveLabel(name: string) {
function findActiveLabel(name: UnderscoreEscapedString) {
if (activeLabels) {
for (const label of activeLabels) {
if (label.name === name) {
Expand Down Expand Up @@ -1169,7 +1170,7 @@ namespace ts {
bindEach(node.statements);
}

function pushActiveLabel(name: string, breakTarget: FlowLabel, continueTarget: FlowLabel): ActiveLabel {
function pushActiveLabel(name: UnderscoreEscapedString, breakTarget: FlowLabel, continueTarget: FlowLabel): ActiveLabel {
const activeLabel = {
name,
breakTarget,
Expand Down Expand Up @@ -1647,7 +1648,7 @@ namespace ts {

const typeLiteralSymbol = createSymbol(SymbolFlags.TypeLiteral, "__type");
addDeclarationToSymbol(typeLiteralSymbol, node, SymbolFlags.TypeLiteral);
typeLiteralSymbol.members = createMap<Symbol>();
typeLiteralSymbol.members = createSymbolTable();
typeLiteralSymbol.members.set(symbol.name, symbol);
}

Expand All @@ -1658,14 +1659,14 @@ namespace ts {
}

if (inStrictMode) {
const seen = createMap<ElementKind>();
const seen = createUnderscoreEscapedMap<ElementKind>();

for (const prop of node.properties) {
if (prop.kind === SyntaxKind.SpreadAssignment || prop.name.kind !== SyntaxKind.Identifier) {
continue;
}

const identifier = <Identifier>prop.name;
const identifier = prop.name;

// ECMA-262 11.1.5 Object Initializer
// If previous is not undefined then throw a SyntaxError exception if any of the following conditions are true
Expand Down Expand Up @@ -1704,7 +1705,7 @@ namespace ts {
return declareSymbolAndAddToSymbolTable(node, symbolFlags, symbolExcludes);
}

function bindAnonymousDeclaration(node: Declaration, symbolFlags: SymbolFlags, name: string) {
function bindAnonymousDeclaration(node: Declaration, symbolFlags: SymbolFlags, name: UnderscoreEscapedString) {
const symbol = createSymbol(symbolFlags, name);
addDeclarationToSymbol(symbol, node, symbolFlags);
}
Expand All @@ -1722,7 +1723,7 @@ namespace ts {
// falls through
default:
if (!blockScopeContainer.locals) {
blockScopeContainer.locals = createMap<Symbol>();
blockScopeContainer.locals = createSymbolTable();
addToContainerChain(blockScopeContainer);
}
declareSymbol(blockScopeContainer.locals, /*parent*/ undefined, node, symbolFlags, symbolExcludes);
Expand Down Expand Up @@ -1803,7 +1804,7 @@ namespace ts {
// otherwise report generic error message.
const span = getErrorSpanForNode(file, name);
file.bindDiagnostics.push(createFileDiagnostic(file, span.start, span.length,
getStrictModeEvalOrArgumentsMessage(contextNode), identifier.text));
getStrictModeEvalOrArgumentsMessage(contextNode), unescapeLeadingUnderscores(identifier.text)));
}
}
}
Expand Down Expand Up @@ -1895,8 +1896,8 @@ namespace ts {
file.bindDiagnostics.push(createFileDiagnostic(file, span.start, span.length, message, arg0, arg1, arg2));
}

function getDestructuringParameterName(node: Declaration) {
return "__" + indexOf((<SignatureDeclaration>node.parent).parameters, node);
function getDestructuringParameterName(node: Declaration): UnderscoreEscapedString {
return "__" + indexOf((<SignatureDeclaration>node.parent).parameters, node) as UnderscoreEscapedString;
}

function bind(node: Node): void {
Expand Down Expand Up @@ -2212,7 +2213,7 @@ namespace ts {
}

function bindSourceFileAsExternalModule() {
bindAnonymousDeclaration(file, SymbolFlags.ValueModule, `"${removeFileExtension(file.fileName)}"`);
bindAnonymousDeclaration(file, SymbolFlags.ValueModule, `"${removeFileExtension(file.fileName)}"` as UnderscoreEscapedString);
}

function bindExportAssignment(node: ExportAssignment | BinaryExpression) {
Expand Down Expand Up @@ -2256,7 +2257,7 @@ namespace ts {
}
}

file.symbol.globalExports = file.symbol.globalExports || createMap<Symbol>();
file.symbol.globalExports = file.symbol.globalExports || createSymbolTable();
declareSymbol(file.symbol.globalExports, file.symbol, node, SymbolFlags.Alias, SymbolFlags.AliasExcludes);
}

Expand Down Expand Up @@ -2341,7 +2342,7 @@ namespace ts {
case SyntaxKind.FunctionDeclaration:
case SyntaxKind.FunctionExpression:
// Declare a 'member' if the container is an ES5 class or ES6 constructor
container.symbol.members = container.symbol.members || createMap<Symbol>();
container.symbol.members = container.symbol.members || createSymbolTable();
// It's acceptable for multiple 'this' assignments of the same identifier to occur
declareSymbol(container.symbol.members, container.symbol, node, SymbolFlags.Property, SymbolFlags.PropertyExcludes & ~SymbolFlags.Property);
break;
Expand Down Expand Up @@ -2403,11 +2404,11 @@ namespace ts {
}
}

function lookupSymbolForName(name: string) {
function lookupSymbolForName(name: UnderscoreEscapedString) {
return (container.symbol && container.symbol.exports && container.symbol.exports.get(name)) || (container.locals && container.locals.get(name));
}

function bindPropertyAssignment(functionName: string, propertyAccessExpression: PropertyAccessExpression, isPrototypeProperty: boolean) {
function bindPropertyAssignment(functionName: UnderscoreEscapedString, propertyAccessExpression: PropertyAccessExpression, isPrototypeProperty: boolean) {
let targetSymbol = lookupSymbolForName(functionName);

if (targetSymbol && isDeclarationOfFunctionOrClassExpression(targetSymbol)) {
Expand All @@ -2420,8 +2421,8 @@ namespace ts {

// Set up the members collection if it doesn't exist already
const symbolTable = isPrototypeProperty ?
(targetSymbol.members || (targetSymbol.members = createMap<Symbol>())) :
(targetSymbol.exports || (targetSymbol.exports = createMap<Symbol>()));
(targetSymbol.members || (targetSymbol.members = createSymbolTable())) :
(targetSymbol.exports || (targetSymbol.exports = createSymbolTable()));

// Declare the method/property
declareSymbol(symbolTable, targetSymbol, propertyAccessExpression, SymbolFlags.Property, SymbolFlags.PropertyExcludes);
Expand Down Expand Up @@ -2459,13 +2460,13 @@ namespace ts {
// Note: we check for this here because this class may be merging into a module. The
// module might have an exported variable called 'prototype'. We can't allow that as
// that would clash with the built-in 'prototype' for the class.
const prototypeSymbol = createSymbol(SymbolFlags.Property | SymbolFlags.Prototype, "prototype");
const prototypeSymbol = createSymbol(SymbolFlags.Property | SymbolFlags.Prototype, "prototype" as UnderscoreEscapedString);
const symbolExport = symbol.exports.get(prototypeSymbol.name);
if (symbolExport) {
if (node.name) {
node.name.parent = node;
}
file.bindDiagnostics.push(createDiagnosticForNode(symbolExport.declarations[0], Diagnostics.Duplicate_identifier_0, prototypeSymbol.name));
file.bindDiagnostics.push(createDiagnosticForNode(symbolExport.declarations[0], Diagnostics.Duplicate_identifier_0, unescapeLeadingUnderscores(prototypeSymbol.name)));
}
symbol.exports.set(prototypeSymbol.name, prototypeSymbol);
prototypeSymbol.parent = symbol;
Expand Down
Loading