Add completions from the 'this' type - #21231
Conversation
Sorry, something went wrong.
| return node && tryGetThisTypeAt(node); | ||
| }, | ||
| isMemberSymbol: symbol => | ||
| symbol.flags & SymbolFlags.ClassMember |
There was a problem hiding this comment.
Asked Nathan Shively-Sanders (@sandersn) and Wesley Wigham (@weswigham) and they didn't know why these are properties, but if I try to change undefined to a variable I can't build the compiler.
Sorry, something went wrong.
There was a problem hiding this comment.
seems that this should be in utilities instead on the checker. nothing here is about the checker internal state, but rather how we mark symbols.
Sorry, something went wrong.
|
I think this one could use a second review. Sheetal Nandi (@sheetalkamat) Wesley Wigham (@weswigham) Nathan Shively-Sanders (@sandersn) Ron Buckton (@rbuckton) Mohamed Hegazy (@mhegazy) |
Sorry, something went wrong.
|
|
||
| function getInsertTextAndReplacementSpan(): { insertText?: string, replacementSpan?: TextSpan } { | ||
| if (kind === CompletionKind.Global) { | ||
| if (typeChecker.isMemberSymbol(symbol)) { |
There was a problem hiding this comment.
seems kinda weird that we add the symbol, then later on ask the checker if they were the ones we added.. i wounder if we can do something different here..
Sorry, something went wrong.
There was a problem hiding this comment.
alas.. i could not come up with a better idea here myself. ignore my previous rant.
Sorry, something went wrong.
There was a problem hiding this comment.
We could use SymbolOriginInfoMap with a union type { type: "this-type" } | { type: "export", ... }, would that be a good idea?
Sorry, something went wrong.
There was a problem hiding this comment.
yeah.. that is not bad..
Sorry, something went wrong.
| hasAction: trueOrUndefined(needsConvertPropertyAccess || origin !== undefined), | ||
| hasAction: trueOrUndefined(origin !== undefined), | ||
| isRecommended: trueOrUndefined(isRecommendedCompletionMatch(symbol, recommendedCompletion, typeChecker)), | ||
| ...getInsertTextAndReplacementSpan(), |
There was a problem hiding this comment.
can we optimize this a bit instead of generating an extra object allocation on every entry.
Sorry, something went wrong.
| let insertText: string | undefined; | ||
| let replacementSpan: TextSpan | undefined; | ||
| if (kind === CompletionKind.Global && origin && origin.type === "this-type") { | ||
| insertText = needsConvertPropertyAccess ? `this["${name}"]` : `this.${name}`; |
There was a problem hiding this comment.
do you not need replacement span eg. if refering to property x and you want to replace it with this.x ?
Sorry, something went wrong.
There was a problem hiding this comment.
The default replacement span should already be the identifier itself. We only need to set replacementSpan if we are replacing something else, too, such as replacing .x with ["x"].
Sorry, something went wrong.
Fixes #21202