Enable method signature completion for object literals by gabritto · Pull Request #48168 · microsoft/TypeScript · GitHub

This PR enables signature completions for methods in object literals. In the following code:

interface IFoo {
    bar(x: number): void;
}

const obj: IFoo = {
    |
}

we now offer as completion: bar(x: number): void { }:

interface IFoo {
    bar(x: number): void;
}

const obj: IFoo = {
    bar(x: number): void {
        |  
    },
}

Design decisions

This signature completion is only offered for object literal expressions, and for methods (properties that are functions).
For an object literal method, we now offer two completion entries:
- A regular one, whose insertText is simply the name of the method (e.g. bar in the example above), which is the old behavior
- A signature one, whose insertText is the method declaration (e.g. bar(x: number): void {} in the example above)
  The regular completion entry should appear before its corresponding signature one
No support for methods that have overloads, as there's no (non-hacky, generalizable) way to have overloads declared inside object literals.
No support for accessors. This could be added later, if we think it's important, but for now it wasn't completely clear to me when/how we should offer accessor signatures, so I left it out.
No support for JS, because that would require us being able to handle JSDoc annotations when creating the completion signature. If we decide to add this later, we could probably share a good chunk of the implementation and make it so that both this and class member completions can work in JS.
For now, we always generate method declarations (e.g. bar(x: number): void {}). If there's a need for it, we can make this configurable later, and generate other forms of declaration (e.g. bar: (x: number): void => {})
We don't add async (and similarly don't support the asterisk modifier for generators), see discussions in await completions should auto-add an async modifier to containing method #48093.

Things that I have to fix in this PR: for some reason, the entries are shown in the wrong order in vscode. I'm investigating that to see what's the best way to address it.

Edit: updated the sort text on the method signature completion entries so that they show up lower than the normal entries. They now have a sortText that is one lower than the normal entries would have.

Looking good at a first glance—I’ll try to give it a try soon.

TypeScript Bot (@typescript-bot) pack this

Heya Ryan Cavanaugh (@RyanCavanaugh), I've started to run the tarball bundle task on this PR at 814561f. You can monitor the build here.

Hey Ryan Cavanaugh (@RyanCavanaugh), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/121521/artifacts?artifactName=tgz&fileId=DA6CBC84DA5E5CF30D3454243FDA8980270092508288925B1CD311BF1EE7888902&fileName=/typescript-4.7.0-insiders.20220309.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.7.0-pr-48168-4".;

I realized I put the feature behind a new flag, so it is not set by default. Going to remove that part of the code in a bit so that people can use this more easily.

TypeScript Bot (@typescript-bot) pack this

Heya Gabriela Araujo Britto (@gabritto), I've started to run the tarball bundle task on this PR at b032aa5. You can monitor the build here.

TypeScript Bot (@typescript-bot) pack this

Heya Andrew Branch (@andrewbranch), I've started to run the tarball bundle task on this PR at 411bc18. You can monitor the build here.

Hey Andrew Branch (@andrewbranch), I've packed this into an installable tgz. You can install it for testing by referencing it in your package.json like so:

{
    "devDependencies": {
        "typescript": "https://typescript.visualstudio.com/cf7ac146-d525-443c-b23c-0d58337efebc/_apis/build/builds/122534/artifacts?artifactName=tgz&fileId=60AB0E85050006EA44CE384DB33288D1AEF05E4219B1A590F13CE020D289F31B02&fileName=/typescript-4.7.0-insiders.20220323.tgz"
    }
}

and then running npm install.

There is also a playground for this build and an npm module you can use via "typescript": "npm:@typescript-deploys/pr-build@4.7.0-pr-48168-33".;

If you close the detail pane of the completions widget, the displayParts of the name entry gets inlined into the right side of the label and you can’t really tell these apart anymore:

I think we discussed in an editor sync putting the signature, or something like the first line of insertText, in the left side of the label. Quick hack:

No strong opinion on whether it should end in { or { … } or no braces, but I think having something very similar to what’s about to get inserted left-aligned in the label makes it more clear. (I think this change should apply to class method snippet completions too.)

Another side note is that VS Code and the LSP spec support another newer label scheme with three parts: one for the main part of the name (bar in this case), one rendered right next to it but perhaps with different font treatment (the rest of the method signature in this case) and one right-aligned bit. If we already supported that format, it would be the clear way to go as it was originally designed specifically with names, signatures, and module names/specifiers in mind for the three label parts respectively. We don’t need to adopt it right now, but its design does reinforce that the name and the rest of the signature should be rendered together on the left side of the label. That will make for a less intrusive change when we someday align more with the LSP spec.

Another side note is that VS Code and the LSP spec support another newer label scheme with three parts: one for the main part of the name (bar in this case), one rendered right next to it but perhaps with different font treatment (the rest of the method signature in this case) and one right-aligned bit.

I think I want to try and add support for it after this PR. It's doable to do it, right? We need a new field in CompletionEntry, and to convert between our completion entry field and the vscode entry label.details field in the vscode extension.

Yes, I think it should be possible.

Ok, so the main thing I’m wondering is if we can consolidate a few of the separate steps here. My understanding of the process is something like this:

When processing symbols in tryGetObjectLikeCompletionSymbols, you do a fast check via isObjectLiteralMethodSymbol and as long as that passes, you push the symbol and an origin (with no info other than kind) onto the symbols array.
After pushing all the symbols on, you update their SortText—I can’t quite tell why this can’t be done in (1).
At the very end of the completions process, in createCompletionEntry, you call into this function here for any symbols that got their origin set accordingly in (1). This function does all the hard work and introduces some extra conditions which might result in us dropping the entry altogether.

I could definitely be missing something, but I don’t see much of a reason for these three steps to be separate. It seems like you could go as far as creating the MethodDeclaration where you currently are only doing step (1), and then store that on the origin. That way, if you can’t create the MethodDeclaration, you don’t need to bother with setting SortText. In other words, by the time you push a symbol and an origin onto those arrays, you know for sure it will be able to turn into a CompletionEntry because most of the work to do it is already done. It seems like this would consolidate the implementation a little. Does that make sense, or what did I miss?

That made sense. I updated the code so that (3) is now done in (1).

About (2), we can't do that when pushing the method snippet symbols, because we need all of the object completions to have the transformed format. Still, I moved the functions that set/transform sort texts to avoid doing some checks.

Makes sense about (2). Looks much better now without having to create the Set of seen symbols 👍

* upstream/main: (473 commits) Correct node used for isDefinition calculation (microsoft#48499) fix(48405): emit dummy members from a mapped type (microsoft#48481) CFA for dependent parameters typed by generic constraints (microsoft#48411) No contextual typing from return types for boolean literals (microsoft#48380) fix(47733): omit JSDoc comment template suggestion on node with existing JSDoc (microsoft#47748) Ensure that we copy empty NodeArrays during transform (microsoft#48490) Add a new compiler option `moduleSuffixes` to expand the node module resolver's search algorithm (microsoft#48189) feat(27615): Add missing member fix should work for type literals (microsoft#47212) Add label details to completion entry (microsoft#48429) Enable method signature completion for object literals (microsoft#48168) Fix string literal completions when a partially-typed string fixes inference to a type parameter (microsoft#48410) fix(48445): show errors on type-only import/export specifiers in JavaScript files (microsoft#48449) Fix newline inserted in empty block at end of formatting range (microsoft#48463) Prevent looking up symbol for as const from triggering an error (microsoft#48464) Revise accessor resolution logic and error reporting (microsoft#48459) fix(48166): skip checking module.exports in a truthiness call expression (microsoft#48337) LEGO: Merge pull request 48450 LEGO: Merge pull request 48436 fix(48031): show circularity error for self referential get accessor annotations (microsoft#48050) Revert "Fix contextual discrimination for omitted members (microsoft#43937)" (microsoft#48426) ...

Gabriela Araujo Britto (gabritto) added 13 commits February 23, 2022 12:30

skeleton of new feature

41913b7

working prototype

d5038ce

refactor print and format code into its own function

fca74c1

minor changes; don't support overloads

249c3ba

have two completion entries

ef15f81

get rid of accessor support

1f26a52

add snippet support

1c28090

add formatting

e628590

add trailing comma

515fc73

add sourcedisplay

0229a7d

support auto-imports via completion details

9892dc8

add user preference option and fix ordering of entries

8550bba

cleanup

78c7d9e

TypeScript Bot (typescript-bot) assigned Gabriela Araujo Britto (gabritto) Mar 7, 2022

TypeScript Bot (typescript-bot) added Author: Team For Milestone Bug PRs that fix a bug with a specific milestone labels Mar 7, 2022

Gabriela Araujo Britto (gabritto) added 2 commits March 7, 2022 16:26

don't return code actions for no import fixes

1f67d97

Merge branch 'main' into gabritto/issue46590

6c96290

Gabriela Araujo Britto (gabritto) marked this pull request as draft March 8, 2022 00:52

Gabriela Araujo Britto (gabritto) requested review from Daniel Rosenwasser (DanielRosenwasser), Andrew Casey (amcasey) and Andrew Branch (andrewbranch) March 8, 2022 01:07

make sortText lower priority for snippets

814561f

Gabriela Araujo Britto (gabritto) marked this pull request as ready for review March 9, 2022 00:57

Merge branch 'main' into gabritto/issue46590

b032aa5

Andrew Branch (andrewbranch) reviewed Mar 23, 2022

View reviewed changes

Comment thread src/services/completions.ts Outdated

Gabriela Araujo Britto (gabritto) added 3 commits March 23, 2022 12:46

re-add flag

08a55c3

fix tests

6ebe361

rename sort text helper

411bc18

Andrew Branch (andrewbranch) reviewed Mar 23, 2022

View reviewed changes

Comment thread src/compiler/types.ts

Andrew Branch (andrewbranch) reviewed Mar 23, 2022

View reviewed changes

Comment thread tests/cases/fourslash/completionsObjectLiteralMethod3.ts

fix test and code for union case

1ebdf3d

Gabriela Araujo Britto (gabritto) mentioned this pull request Mar 24, 2022

[typescript-language-features] Add includeCompletionsWithObjectLiteralMethodSnippets preference microsoft/vscode#145941

Merged

Gabriela Araujo Britto (gabritto) added 2 commits March 24, 2022 11:25

add new flag to protocol type

5477408

fix spaces

af7de76

Gabriela Araujo Britto (gabritto) mentioned this pull request Mar 25, 2022

Add label details to completion entry #48429

Merged

Andrew Branch (andrewbranch) reviewed Mar 29, 2022

View reviewed changes

Gabriela Araujo Britto (gabritto) added 4 commits March 29, 2022 13:06

CR: minor fixes

1b884c4

CR: more fixes

3820f32

CR: restructure main flow

e7a51e6

minor fix

203aab0

Andrew Branch (andrewbranch) approved these changes Mar 30, 2022

View reviewed changes

Gabriela Araujo Britto (gabritto) merged commit e25f04a into main Mar 30, 2022

Gabriela Araujo Britto (gabritto) deleted the gabritto/issue46590 branch March 30, 2022 00:19

Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 22, 2025

                       return undefined;
                   }
+                  function getEntryForObjectLiteralMethodCompletion(

Enable method signature completion for object literals by gabritto · Pull Request #48168 · microsoft/TypeScript · GitHub

Uh oh!

Enable method signature completion for object literals - #48168

Enable method signature completion for object literals#48168Gabriela Araujo Britto (gabritto) merged 38 commits intomainmicrosoft/TypeScript:mainfrom gabritto/issue46590microsoft/TypeScript:gabritto/issue46590Copy head branch name to clipboard

Conversation

Gabriela Araujo Britto (gabritto) commented Mar 7, 2022

Design decisions

Uh oh!

Gabriela Araujo Britto (gabritto) commented Mar 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andrew Branch (andrewbranch) commented Mar 8, 2022

Uh oh!

Ryan Cavanaugh (RyanCavanaugh) commented Mar 9, 2022

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 9, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Gabriela Araujo Britto (gabritto) commented Mar 9, 2022

Uh oh!

Uh oh!

Gabriela Araujo Britto (gabritto) commented Mar 23, 2022

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andrew Branch (andrewbranch) commented Mar 23, 2022

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

TypeScript Bot (typescript-bot) commented Mar 23, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Andrew Branch (andrewbranch) commented Mar 23, 2022

Uh oh!

Uh oh!

Gabriela Araujo Britto (gabritto) commented Mar 24, 2022

Uh oh!

Andrew Branch (andrewbranch) commented Mar 24, 2022

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Andrew Branch (andrewbranch) Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

Gabriela Araujo Britto (gabritto) Mar 29, 2022

Choose a reason for hiding this comment

Uh oh!

Andrew Branch (andrewbranch) Mar 30, 2022

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Enable method signature completion for object literals#48168
Gabriela Araujo Britto (gabritto) merged 38 commits into
mainfrom
gabritto/issue46590

Gabriela Araujo Britto (gabritto) commented Mar 8, 2022 •
edited
Loading

TypeScript Bot (typescript-bot) commented Mar 9, 2022 •
edited
Loading

TypeScript Bot (typescript-bot) commented Mar 9, 2022 •
edited
Loading

TypeScript Bot (typescript-bot) commented Mar 23, 2022 •
edited
Loading

TypeScript Bot (typescript-bot) commented Mar 23, 2022 •
edited
Loading

TypeScript Bot (typescript-bot) commented Mar 23, 2022 •
edited
Loading