feat: exclude declared variable when Object literal completions

[chenjigeng]

Fixes #41731

[andrewbranch]

Thanks for taking a look! Two big things to get started:

1. This needs tests.
2. I would expect zero extra work to be done if we’re not in object literal completions. Right now this is doing some amount of work for every completions request that returns any globals at all. Whatever work is going to be done should probably happen in filterObjectMembersList.

[andrewbranch]

Take a look at findAncenstor and see if that lets you write this more concisely.

[chenjigeng]

Take a look at findAncenstor and see if that lets you write this more concisely.

I used findAncenstor at first, but I found that findAncenstor didn't do much of the work because I had to filter two nodes at a time, one for property assignments and one for object literal expression nodes

[chenjigeng]

Thanks for taking a look! Two big things to get started:

1. This needs tests.
2. I would expect zero extra work to be done if we’re not in object literal completions. Right now this is doing some amount of work for every completions request that returns any globals at all. Whatever work is going to be done should probably happen in filterObjectMembersList.

1. Added
2. I find that in this case of the issue, filterObjectMembersList will not be executed and I don't know why. Can you give me a hand with that

[andrewbranch]

I find that in this case of the issue, filterObjectMembersList will not be executed and I don't know why

Oh, I totally wasn’t thinking. That’s for completions on object literal keys, not values, which could be anything.

The test looks great and answers some of the questions I was going to ask next. I think this looks good overall, just going to leave some suggestions for names and code style.

[andrewbranch]

Now that I’m thinking about this more, it’s really not specific to object literals at all. Variable declaration names should probably never show up in completions for their own initializers unless a function body boundary has been crossed:

const a = a;
const b = a && b;
const c = [{ prop: [c] }];

I suspect there wouldn’t be much performance cost to generalizing to that. You could walk up parent nodes and bail if you hit a FunctionBody or an expression that was an ArrowFunction["body"], and return the variable declaration if you hit a VariableDeclaration["initializer"].

[chenjigeng]

Now that I’m thinking about this more, it’s really not specific to object literals at all. Variable declaration names should probably never show up in completions for their own initializers unless a function body boundary has been crossed:

const a = a;
const b = a && b;
const c = [{ prop: [c] }];

I suspect there wouldn’t be much performance cost to generalizing to that. You could walk up parent nodes and bail if you hit a FunctionBody or an expression that was an ArrowFunction["body"], and return the variable declaration if you hit a VariableDeclaration["initializer"].

Yes. It Sounds like good. I can try to do it. But I think this may need many test cases.

[chenjigeng]

Now that I’m thinking about this more, it’s really not specific to object literals at all. Variable declaration names should probably never show up in completions for their own initializers unless a function body boundary has been crossed:

const a = a;
const b = a && b;
const c = [{ prop: [c] }];

I suspect there wouldn’t be much performance cost to generalizing to that. You could walk up parent nodes and bail if you hit a FunctionBody or an expression that was an ArrowFunction["body"], and return the variable declaration if you hit a VariableDeclaration["initializer"].

Yes. It Sounds like good. I can try to do it. But I think this may need many test cases.

I submit a new PR to do this (based on the current submission)�. #42087 I have modified the existing test case and added a new test case. Please help me to see if there is any problem here

[sandersn]

I'm going to close this PR since #42087 supersedes it.