Conversation
| return (identifier.length >= 2 && identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ ? "_" + identifier : identifier) as __String; | ||
| } | ||
|
|
||
| export function isEscapedNameOfWellKnownSymbol(escapedName: __String) { |
There was a problem hiding this comment.
With #15473, this won't have anything to do with well-known symbols.
There was a problem hiding this comment.
But for now, it's perfectly accurate.
There was a problem hiding this comment.
can we move this to services/utilities instead?
There was a problem hiding this comment.
I can move it. I'll rename it later.
There was a problem hiding this comment.
Actually, I'll just inline it into getCompletionEntryDisplayNameForSymbol since we do similar tests in there.
| return (identifier.length >= 2 && identifier.charCodeAt(0) === CharacterCodes._ && identifier.charCodeAt(1) === CharacterCodes._ ? "_" + identifier : identifier) as __String; | ||
| } | ||
|
|
||
| export function isEscapedNameOfWellKnownSymbol(escapedName: __String) { |
There was a problem hiding this comment.
But for now, it's perfectly accurate.
| if (!uniqueNames.has(id)) { | ||
| if (symbolToOriginInfoMap && symbolToOriginInfoMap[getUniqueSymbolId(symbol, typeChecker)]) { | ||
| entry.hasAction = true; | ||
| if (!isEscapedNameOfWellKnownSymbol(symbol.escapedName)) { |
There was a problem hiding this comment.
if (isEscapedNameOfWellKnownSymbol(symbol.escapedName)) {
continue;
}to avoid the extra nesting? Likewise some of the conditions on the other nested if statements could be inverted and used with a continue to reduce the nesting a bit here.
|
|
||
| // If the symbol is for a member of an object type and is the internal name of an ES | ||
| // symbol, it is not a valid entry. Internal names for ES symbols start with "__@" | ||
| if (symbol.flags & SymbolFlags.ClassMember) { |
There was a problem hiding this comment.
I don't think internal names are specific to ClassMembers?
There was a problem hiding this comment.
ESSymbol named members are. ClassMember is a misnomer. It includes Property, Method, and Accessor, which are all of the ESSymbol-nameable members.
| // symbol, it is not a valid entry. Internal names for ES symbols start with "__@" | ||
| if (symbol.flags & SymbolFlags.ClassMember) { | ||
| const escapedName = symbol.escapedName as string; | ||
| if (escapedName.length >= 3 && |
There was a problem hiding this comment.
There's a isReservedMemberName inside checker.ts that we should just move to utilities and reuse (the negation of) here.
There was a problem hiding this comment.
isReservedMemberName explicitly does not remove ES symbols, only __call, __new, etc. since those aren't actually members. ES symbols, however, are members. Also, isReservedMemberName is already executed as part of the call to getApparentProperties on line 287.
There was a problem hiding this comment.
I wonder if SymbolObject.prototype.name should return undefined for internal names, though I'm not sure if that would have other undesirable effects.
There was a problem hiding this comment.
isReservedMemberNameexplicitly does not remove ES symbols
I misread it, OK. Would still be nice to have all these escaped name inspection snippets in one place... doesn't really matter, though.
I wonder if
SymbolObject.prototype.nameshould return undefined for internal names, though I'm not sure if that would have other undesirable effects.
Ideally we (internally) should never use .name on an internally named symbol, since we (internally) swapped everything to .__escapedName before re-implementing .name (with proper escaping) for external compatibility. Would making it return undefined on internal symbol names likely break any external consumers? If no, I'm all for it.
There was a problem hiding this comment.
I might look at the .name thing some other time.
|
@DanielRosenwasser, any other comments? |
When reporting completions for string literals we were incorrectly including the internal escaped names of ES symbols, such as
__@iteratoror__@species.Fixes #19349