Skip to content

Eliminate well known symbols as a concept in the checker and rely on unique symbols #42543

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
weswigham merged 7 commits into from
Feb 22, 2021
Merged

Eliminate well known symbols as a concept in the checker and rely on unique symbols #42543

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
31dc466
Eliminate well-known symbols in the checker: 2021 edition
weswigham Jan 29, 2021
a3fc74e
Actually update the lib text to say unique symbol, too (this is unnee…
weswigham Jan 29, 2021
bd49b81
Add test showing mismatched symbol constructor type interop
weswigham Jan 29, 2021
4c808c4
Add more test cases for some other related issues this fixes
weswigham Jan 29, 2021
ebac382
Merge branch 'master' into eliminate-well-known-symbols
weswigham Feb 22, 2021
b954507
Revert computed name change
weswigham Feb 22, 2021
e99c05a
Style comments
weswigham Feb 22, 2021
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
9 changes: 3 additions & 6 deletions src/compiler/binder.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -344,12 +344,9 @@ namespace ts {
if (isSignedNumericLiteral(nameExpression)) {
return tokenToString(nameExpression.operator) + nameExpression.operand.text as __String;
}

Debug.assert(isWellKnownSymbolSyntactically(nameExpression));
return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>nameExpression).name));
}
if (isWellKnownSymbolSyntactically(name)) {
return getPropertyNameForKnownSymbolName(idText(name.name));
else {
Debug.fail("Only computed properties with literal names have declaration names");
}
}
if (isPrivateIdentifier(name)) {
// containingClass exists because private names only allowed inside classes
Expand Down
106 changes: 47 additions & 59 deletions src/compiler/checker.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -898,6 +898,7 @@ namespace ts {
// This allows users to just specify library files they want to used through --lib
// and they will not get an error from not having unrelated library files
let deferredGlobalESSymbolConstructorSymbol: Symbol | undefined;
let deferredGlobalESSymbolConstructorTypeSymbol: Symbol | undefined;
let deferredGlobalESSymbolType: ObjectType;
let deferredGlobalTypedPropertyDescriptorType: GenericType;
let deferredGlobalPromiseType: GenericType;
Expand Down Expand Up @@ -3677,11 +3678,25 @@ namespace ts {
const additionalContainers = mapDefined(container.declarations, fileSymbolIfFileSymbolExportEqualsContainer);
const reexportContainers = enclosingDeclaration && getAlternativeContainingModules(symbol, enclosingDeclaration);
const objectLiteralContainer = getVariableDeclarationOfObjectLiteral(container, meaning);
if (enclosingDeclaration && getAccessibleSymbolChain(container, enclosingDeclaration, SymbolFlags.Namespace, /*externalOnly*/ false)) {
if (enclosingDeclaration && container.flags & getQualifiedLeftMeaning(meaning) && getAccessibleSymbolChain(container, enclosingDeclaration, SymbolFlags.Namespace, /*externalOnly*/ false)) {
Comment thread
weswigham marked this conversation as resolved.
Outdated
return append(concatenate(concatenate([container], additionalContainers), reexportContainers), objectLiteralContainer); // This order expresses a preference for the real container if it is in scope
}
// we potentially a symbol which is a member of the instance side of something - look for a variable in scope with the container's type
Comment thread
weswigham marked this conversation as resolved.
Outdated
// which may be acting like a namespace
Comment thread
weswigham marked this conversation as resolved.
Outdated
const firstVariableMatch = !(container.flags & getQualifiedLeftMeaning(meaning))

@weswigham weswigham Jan 29, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'll call this out since it's a new bit compared to the older PR. This bit and the following modify our symbol container lookup to prefer, eg, Symbol.iterator to SymbolConstructor.iterator when doing printback for where the specified SymbolFlags say the first is allowable. Now, this isn't specific to symbols or Symbol in any way - other dynamic names from namespace-ish things should work similarly, too! So ArrayConstructor, Int8ArrayConsturctor.... anything with the split namespace pattern the DOM uses a lot.

&& container.flags & SymbolFlags.Type
&& getDeclaredTypeOfSymbol(container).flags & TypeFlags.Object
&& meaning === SymbolFlags.Value
? forEachSymbolTableInScope(enclosingDeclaration, t => {
return forEachEntry(t, s => {
if (s.flags & getQualifiedLeftMeaning(meaning) && getTypeOfSymbol(s) === getDeclaredTypeOfSymbol(container)) {
return s;
}
});
}) : undefined;
const res = append(append(additionalContainers, container), objectLiteralContainer);
return concatenate(res, reexportContainers);
const resWithReexports = concatenate(res, reexportContainers);
return firstVariableMatch ? [firstVariableMatch, ...resWithReexports] : resWithReexports;
Comment thread
weswigham marked this conversation as resolved.
Outdated
}
const candidates = mapDefined(symbol.declarations, d => {
if (!isAmbientModule(d) && d.parent && hasNonGlobalAugmentationExternalModuleSymbol(d.parent)) {
Expand Down Expand Up @@ -5104,8 +5119,9 @@ namespace ts {
}
}
}
context.enclosingDeclaration = saveEnclosingDeclaration;
context.enclosingDeclaration = propertySymbol.valueDeclaration || propertySymbol.declarations?.[0] || saveEnclosingDeclaration;

@weswigham weswigham Jan 29, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we don't set a specific enclosingDeclaration when serializing the name, in cases where we don't pass a contextual one (eg, error reporting), we'd print a name like [iterator] instead of [Symbol.iterator] (because symbol chain lookup bails entirely if there's no enclosing declaration provided). I think we much prefer the later, so now we actually specify an enclosing declaration for the name whenever we can.

@DanielRosenwasser DanielRosenwasser Feb 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

What's an example where propertySymbol.declarations?.[0] is useful?

@weswigham weswigham Feb 13, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

valueDeclaration is unset in two cases - one: when the symbol doesn't have a value meaning (as when it comes from an interface), two: when two value symbols merge and have differing value declarations. In both cases, we'd like a context node for the name here.

const propertyName = getPropertyNameNodeForSymbol(propertySymbol, context);
context.enclosingDeclaration = saveEnclosingDeclaration;
context.approximateLength += (symbolName(propertySymbol).length + 1);
const optionalToken = propertySymbol.flags & SymbolFlags.Optional ? factory.createToken(SyntaxKind.QuestionToken) : undefined;
if (propertySymbol.flags & (SymbolFlags.Function | SymbolFlags.Method) && !getPropertiesOfObjectType(propertyType).length && !isReadonlySymbol(propertySymbol)) {
Expand Down Expand Up @@ -5849,9 +5865,6 @@ namespace ts {
if (fromNameType) {
return fromNameType;
}
if (isKnownSymbol(symbol)) {
return factory.createComputedPropertyName(factory.createPropertyAccessExpression(factory.createIdentifier("Symbol"), (symbol.escapedName as string).substr(3)));
}
const rawName = unescapeLeadingUnderscores(symbol.escapedName);
const stringNamed = !!length(symbol.declarations) && every(symbol.declarations, isStringNamed);
return createPropertyNameNodeForIdentifierOrLiteral(rawName, stringNamed, singleQuote);
Expand Down Expand Up @@ -8704,8 +8717,18 @@ namespace ts {
return widenTypeForVariableLikeDeclaration(getTypeForVariableLikeDeclaration(declaration, /*includeOptionality*/ true), declaration, reportErrors);
}

function isGlobalSymbolConstructor(node: Node) {
const symbol = getSymbolOfNode(node);
const globalSymbol = getGlobalESSymbolConstructorTypeSymbol(/*reportErrors*/ false);
return globalSymbol && symbol && symbol === globalSymbol;
}

function widenTypeForVariableLikeDeclaration(type: Type | undefined, declaration: any, reportErrors?: boolean) {
if (type) {
// TODO: Remove the following SymbolConstructor special case when back compat with pre-3.0 libs isn't required
Comment thread
weswigham marked this conversation as resolved.
Outdated
if (type.flags & TypeFlags.ESSymbol && isGlobalSymbolConstructor(declaration.parent)) {
type = getESSymbolLikeTypeForNode(declaration);
}
if (reportErrors) {
reportErrorsFromWidening(declaration, type);
}
Expand Down Expand Up @@ -12835,6 +12858,10 @@ namespace ts {
return deferredGlobalESSymbolConstructorSymbol || (deferredGlobalESSymbolConstructorSymbol = getGlobalValueSymbol("Symbol" as __String, reportErrors));
}

function getGlobalESSymbolConstructorTypeSymbol(reportErrors: boolean) {
return deferredGlobalESSymbolConstructorTypeSymbol || (deferredGlobalESSymbolConstructorTypeSymbol = getGlobalTypeSymbol("SymbolConstructor" as __String, reportErrors));
}

function getGlobalESSymbolType(reportErrors: boolean) {
return deferredGlobalESSymbolType || (deferredGlobalESSymbolType = getGlobalType("Symbol" as __String, /*arity*/ 0, reportErrors)) || emptyObjectType;
}
Expand Down Expand Up @@ -13919,13 +13946,13 @@ namespace ts {
function getLiteralTypeFromProperty(prop: Symbol, include: TypeFlags) {
if (!(getDeclarationModifierFlagsFromSymbol(prop) & ModifierFlags.NonPublicAccessibilityModifier)) {
let type = getSymbolLinks(getLateBoundSymbol(prop)).nameType;
if (!type && !isKnownSymbol(prop)) {
if (!type) {
if (prop.escapedName === InternalSymbolName.Default) {
type = getLiteralType("default");
}
else {
const name = prop.valueDeclaration && getNameOfDeclaration(prop.valueDeclaration) as PropertyName;
type = name && getLiteralTypeFromPropertyName(name) || getLiteralType(symbolName(prop));
type = name && getLiteralTypeFromPropertyName(name) || (!isKnownSymbol(prop) ? getLiteralType(symbolName(prop)) : undefined);

@weswigham weswigham Jan 29, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Perhaps confusingly, isKnownSymbol has almost nothing to do with well-known symbols, and actually just checks if the checker Symbol has a symbol-y name (eg, it's escapedName starts with __@). It's mostly gone, since we were, intent-wise, actually using it to proxy for well-known-symboly-ness in places; however a handful of select locations remain where we actually want to check for normal symbol-ness.

@sandersn sandersn Feb 19, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

It won't be confusing once well-known symbols are gone as a concept, right? Seems fine to me.

}
}
if (type && type.flags & include) {
Expand Down Expand Up @@ -14153,11 +14180,8 @@ namespace ts {
}

function getPropertyNameFromIndex(indexType: Type, accessNode: StringLiteral | Identifier | PrivateIdentifier | ObjectBindingPattern | ArrayBindingPattern | ComputedPropertyName | NumericLiteral | IndexedAccessTypeNode | ElementAccessExpression | SyntheticExpression | undefined) {
const accessExpression = accessNode && accessNode.kind === SyntaxKind.ElementAccessExpression ? accessNode : undefined;
return isTypeUsableAsPropertyName(indexType) ?
getPropertyNameFromType(indexType) :
accessExpression && checkThatExpressionIsProperSymbolReference(accessExpression.argumentExpression, indexType, /*reportError*/ false) ?
getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>accessExpression.argumentExpression).name)) :
accessNode && isPropertyName(accessNode) ?
// late bound names are handled in the first branch, so here we only need to handle normal names
getPropertyNameForPropertyNameNode(accessNode) :
Expand Down Expand Up @@ -25151,9 +25175,6 @@ namespace ts {
!isTypeAssignableTo(links.resolvedType, stringNumberSymbolType)) {
error(node, Diagnostics.A_computed_property_name_must_be_of_type_string_number_symbol_or_any);
}
else {
checkThatExpressionIsProperSymbolReference(node.expression, links.resolvedType, /*reportError*/ true);
}
}

return links.resolvedType;
Expand Down Expand Up @@ -25214,15 +25235,15 @@ namespace ts {
// As otherwise they may not be checked until exports for the type at this position are retrieved,
// which may never occur.
for (const elem of node.properties) {
if (elem.name && isComputedPropertyName(elem.name) && !isWellKnownSymbolSyntactically(elem.name)) {
if (elem.name && isComputedPropertyName(elem.name)) {
checkComputedPropertyName(elem.name);
}
}

let offset = 0;
for (const memberDecl of node.properties) {
let member = getSymbolOfNode(memberDecl);
const computedNameType = memberDecl.name && memberDecl.name.kind === SyntaxKind.ComputedPropertyName && !isWellKnownSymbolSyntactically(memberDecl.name.expression) ?
const computedNameType = memberDecl.name && memberDecl.name.kind === SyntaxKind.ComputedPropertyName ?
checkComputedPropertyName(memberDecl.name) : undefined;
if (memberDecl.kind === SyntaxKind.PropertyAssignment ||
memberDecl.kind === SyntaxKind.ShorthandPropertyAssignment ||
Expand Down Expand Up @@ -25321,7 +25342,10 @@ namespace ts {
}

if (computedNameType && !(computedNameType.flags & TypeFlags.StringOrNumberLiteralOrUnique)) {
if (isTypeAssignableTo(computedNameType, stringNumberSymbolType)) {
if (isTypeAny(computedNameType)) {

@weswigham weswigham Jan 29, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Technically, I can push this change through without this change, if we'd prefer. However, that means when you write, eg

const obj = {
  [Symbol.whatever]: 0
};

obj would get a number index signature (since Symbol.whatever is going to be any from the error of it not existing), whereas with this it gets a string index signature (which, IMO, makes much more sense for an any computed name). It mostly only matters, in the context of unique symbols, for follow-on errors to nonexistant symbols which were previously "well known" and thus entirely unchecked (which now get an error, become any and thus produce an index signature of some kind).

@DanielRosenwasser DanielRosenwasser Feb 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

From the comment here, it sounds like this mostly to cater to the "degenerate" case of Symbol.thingThatDoesNotExist which can happen in a loosely configured JavaScript project - is that the use-case you have in mind? And

That does loosen the behavior of this by quite a bit. I think I would be willing to go the opposite way and take a break on this behavior.

@weswigham weswigham Feb 13, 2021 •
edited
Loading

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

You definitely don't wanna go the opposite way, because the numeric index signature we otherwise make makes very little sense, IMO. I actually think any producing a number index signature just feels a bit like a bug that we aught to fix.

@DanielRosenwasser DanielRosenwasser Feb 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I get that, but I think the any behavior is already pretty undesirable, and is exacerbated if you're not using --noUncheckedIndexedAccess.

@weswigham weswigham Feb 13, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

???? That flag is about accessing keys and the types accessed keys are assigned, where here the issue is solely weather or not we make a key at all, and what slot that key should go in; that seems wholly unrelated to that flag.

@DanielRosenwasser DanielRosenwasser Feb 16, 2021 •
edited
Loading

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

But

const s = {["str" as any]: "str"}

can now be indexed by a wider variety of nonsense that would never work either, like s["uh oh"]. You're allowing a much more optimistic type to be formed which you can't take back after the fact unless you add a new flag.

@DanielRosenwasser DanielRosenwasser Feb 16, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

And in fact, I think that's why the numeric index signature was chosen - because at the time we also felt the behavior it supported was more desirable even if it wasn't good.

The point of the numeric index signature isn't to support indexing with numbers - it's to support indexing with any and still getting the appropriate type out. It's a hack, but it doesn't widen the net to every possible non-numeric property.

@weswigham weswigham Feb 16, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

But the point of any is to "widen the net".

@DanielRosenwasser DanielRosenwasser Feb 17, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

At best this isn't something we need to change with this feature. I don't find the "well known symbol that doesn't exist" thing to be a use-case that should fundamentally change what happens when you use any as a computed property key.

@weswigham weswigham Feb 17, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Alright 🤷‍♂️

hasComputedStringProperty = true; // string is the closest to a catch-all index signature we have
}
else if (isTypeAssignableTo(computedNameType, stringNumberSymbolType)) {
if (isTypeAssignableTo(computedNameType, numberType)) {
hasComputedNumberProperty = true;
}
Expand Down Expand Up @@ -26879,48 +26903,6 @@ namespace ts {
return checkIndexedAccessIndexType(getFlowTypeOfAccessExpression(node, indexedAccessType.symbol, indexedAccessType, indexExpression), node);
}

function checkThatExpressionIsProperSymbolReference(expression: Expression, expressionType: Type, reportError: boolean): boolean {
if (expressionType === errorType) {
// There is already an error, so no need to report one.
return false;
}

if (!isWellKnownSymbolSyntactically(expression)) {
return false;
}

// Make sure the property type is the primitive symbol type
if ((expressionType.flags & TypeFlags.ESSymbolLike) === 0) {
if (reportError) {
error(expression, Diagnostics.A_computed_property_name_of_the_form_0_must_be_of_type_symbol, getTextOfNode(expression));
}
return false;
}

// The name is Symbol.<someName>, so make sure Symbol actually resolves to the
// global Symbol object
const leftHandSide = <Identifier>(<PropertyAccessExpression>expression).expression;
const leftHandSideSymbol = getResolvedSymbol(leftHandSide);
if (!leftHandSideSymbol) {
return false;
}

const globalESSymbol = getGlobalESSymbolConstructorSymbol(/*reportErrors*/ true);
if (!globalESSymbol) {
// Already errored when we tried to look up the symbol
return false;
}

if (leftHandSideSymbol !== globalESSymbol) {
if (reportError) {
error(leftHandSide, Diagnostics.Symbol_reference_does_not_refer_to_the_global_Symbol_constructor_object);
}
return false;
}

return true;
}

function callLikeExpressionMayHaveTypeArguments(node: CallLikeExpression): node is CallExpression | NewExpression | TaggedTemplateExpression | JsxOpeningElement {
return isCallOrNewExpression(node) || isTaggedTemplateExpression(node) || isJsxOpeningLikeElement(node);
}
Expand Down Expand Up @@ -35198,6 +35180,12 @@ namespace ts {
}
}

function getPropertyNameForKnownSymbolName(symbolName: string): __String {
const ctorType = getGlobalESSymbolConstructorSymbol(/*reportErrors*/ false);
const uniqueType = ctorType && getTypeOfPropertyOfType(getTypeOfSymbol(ctorType), escapeLeadingUnderscores(symbolName));
return uniqueType && isTypeUsableAsPropertyName(uniqueType) ? getPropertyNameFromType(uniqueType) : `__@${symbolName}` as __String;

@weswigham weswigham Jan 29, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The __@${symbolName} as __String; case is reachable in scenarios where our internals request a name for a symbol which doesn't exist in the lib. Eg, requesting Symbol.asyncIterator in a lib target where it doesn't exist. No construct can make a property symbol name in that format anymore (unique symbol names have the form __@desc@###), so it's guaranteed to result in an undefined result from getPropertyOfType, resulting in a noIterationTypes result (as you may expect when the core iteration protocol types are missing!).

@DanielRosenwasser DanielRosenwasser Feb 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Can you give a little more context on the significance? Are you saying this is handled well in the existing code, that other code needs to be careful, that there's a fundamental architectural issue, or that this now has a user-facing impact? (or all?)

@weswigham weswigham Feb 13, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

I'm saying that properties with a name like that can no longer be constructed, so when the proper name cannot be found, returning a name like that will always make a follow-up property lookup fail, which has the desired outcome.

}

/**
* Gets the *yield*, *return*, and *next* types of an `Iterable`-like or `AsyncIterable`-like
* type from its members.
Expand Down
3 changes: 1 addition & 2 deletions src/compiler/transformers/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -269,8 +269,7 @@ namespace ts {
* any such locations
*/
export function isSimpleInlineableExpression(expression: Expression) {
return !isIdentifier(expression) && isSimpleCopiableExpression(expression) ||
isWellKnownSymbolSyntactically(expression);
return !isIdentifier(expression) && isSimpleCopiableExpression(expression);

@weswigham weswigham Jan 29, 2021

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This change is pretty much single-handedly responsible for the .js emit changes in this PR. Symbol.iterator and it's ilk aren't "simple copyable expressions" since, technically, accessing it could produce side effects (like any other property access). Now... at some point, we added Identifiers to isSimpleCopiableExpression's domain (which isn't technically safe, since the identifier may get reassigned between the copied references), so maybe property accesses which only consist of Identifiers wouldn't be terribly unreasonable, either. 🤷

@DanielRosenwasser DanielRosenwasser Feb 13, 2021

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@rbuckton is probably the best judge of that.

}

export function isCompoundAssignment(kind: BinaryOperator): kind is CompoundAssignmentOperator {
Expand Down
8 changes: 1 addition & 7 deletions src/compiler/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2307,12 +2307,6 @@ namespace ts {
| CallChainRoot
;

/** @internal */
export interface WellKnownSymbolExpression extends PropertyAccessExpression {
readonly expression: Identifier & { readonly escapedText: __String & "Symbol" };
readonly name: Identifier;
}

/** @internal */
export type BindableObjectDefinePropertyCall = CallExpression & {
readonly arguments: readonly [BindableStaticNameExpression, StringLiteralLike | NumericLiteral, ObjectLiteralExpression] & Readonly<TextRange>;
Expand All @@ -2326,7 +2320,7 @@ namespace ts {

/** @internal */
export type LiteralLikeElementAccessExpression = ElementAccessExpression & Declaration & {
readonly argumentExpression: StringLiteralLike | NumericLiteral | WellKnownSymbolExpression;
readonly argumentExpression: StringLiteralLike | NumericLiteral;
};

/** @internal */
Expand Down
31 changes: 3 additions & 28 deletions src/compiler/utilities.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2244,9 +2244,7 @@ namespace ts {

/** x[0] OR x['a'] OR x[Symbol.y] */
export function isLiteralLikeElementAccess(node: Node): node is LiteralLikeElementAccessExpression {
return isElementAccessExpression(node) && (
isStringOrNumericLiteralLike(node.argumentExpression) ||
isWellKnownSymbolSyntactically(node.argumentExpression));
return isElementAccessExpression(node) && isStringOrNumericLiteralLike(node.argumentExpression);
}

/** Any series of property and element accesses. */
Expand Down Expand Up @@ -2331,9 +2329,6 @@ namespace ts {
return escapeLeadingUnderscores(name.text);
}
}
if (isElementAccessExpression(node) && isWellKnownSymbolSyntactically(node.argumentExpression)) {
return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>node.argumentExpression).name));
}
return undefined;
}

Expand Down Expand Up @@ -3142,9 +3137,6 @@ namespace ts {
* 3. The computed name is *not* expressed as a NumericLiteral.
* 4. The computed name is *not* expressed as a PlusToken or MinusToken
* immediately followed by a NumericLiteral.
* 5. The computed name is *not* expressed as `Symbol.<name>`, where `<name>`
* is a property of the Symbol constructor that denotes a built-in
* Symbol.
*/
export function hasDynamicName(declaration: Declaration): declaration is DynamicNamedDeclaration | DynamicNamedBinaryExpression {
const name = getNameOfDeclaration(declaration);
Expand All @@ -3157,17 +3149,7 @@ namespace ts {
}
const expr = isElementAccessExpression(name) ? skipParentheses(name.argumentExpression) : name.expression;
return !isStringOrNumericLiteralLike(expr) &&
!isSignedNumericLiteral(expr) &&
!isWellKnownSymbolSyntactically(expr);
}

/**
* Checks if the expression is of the form:
* Symbol.name
* where Symbol is literally the word "Symbol", and name is any identifierName
*/
export function isWellKnownSymbolSyntactically(node: Node): node is WellKnownSymbolExpression {
return isPropertyAccessExpression(node) && isESSymbolIdentifier(node.expression);
!isSignedNumericLiteral(expr);
}

export function getPropertyNameForPropertyNameNode(name: PropertyName): __String | undefined {
Expand All @@ -3180,10 +3162,7 @@ namespace ts {
return escapeLeadingUnderscores(name.text);
case SyntaxKind.ComputedPropertyName:
const nameExpression = name.expression;
if (isWellKnownSymbolSyntactically(nameExpression)) {
return getPropertyNameForKnownSymbolName(idText((<PropertyAccessExpression>nameExpression).name));
}
else if (isStringOrNumericLiteralLike(nameExpression)) {
if (isStringOrNumericLiteralLike(nameExpression)) {
return escapeLeadingUnderscores(nameExpression.text);
}
else if (isSignedNumericLiteral(nameExpression)) {
Expand Down Expand Up @@ -3221,10 +3200,6 @@ namespace ts {
return `__@${getSymbolId(symbol)}@${symbol.escapedName}` as __String;
}

export function getPropertyNameForKnownSymbolName(symbolName: string): __String {
return "__@" + symbolName as __String;
}

export function getSymbolNameForPrivateIdentifier(containingClassSymbol: Symbol, description: __String): __String {
return `__#${getSymbolId(containingClassSymbol)}@${description}` as __String;
}
Expand Down
Loading