Provide string completions for in keyword checks#60272
Provide string completions for in keyword checks#60272jakebailey merged 2 commits intomicrosoft:mainfrom
in keyword checks#60272Conversation
|
The TypeScript team hasn't accepted the linked issue #52736. If you can get it accepted, this PR will have a better chance of being reviewed. |
typescript-bot
added
the
For Uncommitted Bug
| }); | ||
| verify.completions({ | ||
| marker: "7", | ||
| exact: ["bar", "foo"], |
There was a problem hiding this comment.
This includes a completion from private bar = 0 but apparently that kind of a check is valid 🤷♂️ TS playground
There was a problem hiding this comment.
That is cursed and I hate it
src/services/stringCompletions.ts
Outdated
| if ((parent as BinaryExpression).operatorToken.kind === SyntaxKind.InKeyword) { | ||
| const { left, right } = parent as BinaryExpression; | ||
| const leftOrRight = findAncestor(node, n => n.parent.kind === SyntaxKind.BinaryExpression); | ||
| const type = typeChecker.getTypeAtLocation(leftOrRight === right ? left : right); |
There was a problem hiding this comment.
What exactly is the purpose of getting the type on the right side? Where is that tested?
There was a problem hiding this comment.
This code has to handle both:
key in objobj in key
Those new completions are trying to suggest available keys. So to do that we obtain "the other" type and based on it we compute properties (one line below this) based on which those are computed.
There was a problem hiding this comment.
What language construct is obj in key supposed to correspond to though? Isn't that just someone writing a construct backwards?
There was a problem hiding this comment.
That's a new construct coming in the next edition of JabaScript.
You are, of course, right - this was nonsense. I'm sometimes a dumdum when producing code in short 5min spans of time I have at hand.
I just pushed out a commit removing this bit.
| }); | ||
| verify.completions({ | ||
| marker: "7", | ||
| exact: ["bar", "foo"], |
There was a problem hiding this comment.
That is cursed and I hate it
| return { | ||
| kind: StringLiteralCompletionKind.Properties, | ||
| symbols: properties.filter(prop => !prop.valueDeclaration || !isPrivateIdentifierClassElementDeclaration(prop.valueDeclaration)), | ||
| hasIndexSignature: false, |
There was a problem hiding this comment.
Does this mean that we will aggressively complete in this position? Does this need to return true if any type has a string/number index signature?
There was a problem hiding this comment.
This is only used to return value for isNewIdentifierLocation and I don't quite understand what this is meant to represent. IIRC it, at times, works inconsistently anyway. I'd love to be able to tell what should be returned for it here but I simply just don't know ;p
DanielRosenwasser
left a comment
DanielRosenwasser
left a comment
There was a problem hiding this comment.
Probably fine, but I'd take my last comment into account.
4 participants
closes #52736