Skip to content

Add arguments to completion list #4249#4347

Merged
mhegazy merged 2 commits into
microsoft:masterfrom
Schmavery:master
Aug 18, 2015
Merged

Add arguments to completion list #4249#4347
mhegazy merged 2 commits into
microsoft:masterfrom
Schmavery:master

Conversation

@Schmavery

Copy link
Copy Markdown
Contributor

Here's my attempt to fix issue #4249.

@msftclas

Copy link
Copy Markdown

Hi @Schmavery, I'm your friendly neighborhood Microsoft Pull Request Bot (You can call me MSBOT). Thanks for your contribution!
You've already signed the contribution license agreement. Thanks!
We will now validate the agreement and then real humans will evaluate your PR.

TTYL, MSBOT;

Comment thread src/compiler/checker.ts Outdated

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.

This isn't quite right. Arrow functions shouldn't automatically get an arguments in the completion list, nor does it get them in call signatures, etc.

Add the following to utilities.ts

function introducesArgumentsExoticObject(node: Node) {
    switch (decl.kind) {
        SyntaxKind.MethodDeclaration:
        SyntaxKind.MethodSignature:
        SyntaxKind.Constructor:
        SyntaxKind.GetAccessor:
        SyntaxKind.SetAccessor:
        SyntaxKind.FunctionDeclaration:
        SyntaxKind.FunctionExpression:
            return true;
    }
    return false;
}

Then use that here. I'd add a test like

function f() {
    let g = () => /*shouldHaveArguments*/
}

let g = () => /*shouldNotHaveArguments*/

mhegazy added a commit that referenced this pull request Aug 18, 2015
Add `arguments` to completion list #4249
@mhegazy
mhegazy merged commit 387da2f into microsoft:master Aug 18, 2015
@mhegazy

mhegazy commented Aug 18, 2015

Copy link
Copy Markdown
Contributor

Thanks!

@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants