Completion list for type literals in type arguments #43526

tiagovtristao · 2021-04-05T01:32:33Z

This PR aims to provide some type literal completion for type arguments, as suggested in #33302. It takes in mind intersection and union types, as well as nested type literals.

typescript-bot · 2021-04-05T01:32:46Z

This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise.

src/services/completions.ts

DanielRosenwasser · 2021-04-05T21:42:31Z

This looks like it's going in the right direction, but I haven't looked at this part of the codebase in a bit.

andrewbranch · 2021-04-07T22:23:48Z

@typescript-bot pack this

typescript-bot · 2021-04-07T22:23:50Z

Heya @andrewbranch, I've started to run the tarball bundle task on this PR at 7c0366e. You can monitor the build here.

typescript-bot · 2021-04-07T22:46:22Z

Hey @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/100473/artifacts?artifactName=tgz&fileId=DDCD906116EE5FD8D7812196CAE8F9A67FBD0EF72BBDF62B98540CF0AD70C57002&fileName=/typescript-4.3.0-insiders.20210407.tgz"
    }
}

and then running npm install.

DanielRosenwasser · 2021-04-14T00:28:24Z

Annoying that the playground build didn't work.

@typescript-bot pack this

typescript-bot · 2021-04-14T00:28:27Z

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 7c0366e. You can monitor the build here.

DanielRosenwasser

Looking at the code, it looks very clean and it seems like it does exactly what I'd expect - actually, it does more than that, specifically accounting for intersections which I hadn't thought of!

I guess one thing I do wonder about is how this interacts with symbols, and non-identifier keys (e.g. keys with spaces, keys with numbers at the beginning, fully numeric keys). Those are very niche scenarios so I won't exactly block on that, but completions shouldn't feel intrusive there.

We should also consider these to be new identifier definition locations (e.g. you can specify more than what's there without the editor getting in your way).

(see #42595 (comment) for what I mean on that last point)

typescript-bot · 2021-04-14T00:47:08Z

Hey @DanielRosenwasser, 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/100914/artifacts?artifactName=tgz&fileId=525427E0F8C0F1401335F19856E201BD4F598D66459C51B9AD42DF47448563B102&fileName=/typescript-4.3.0-insiders.20210414.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.3.0-pr-43526-7".;

andrewbranch

A couple nits, but looks great overall.

andrewbranch · 2021-04-14T17:42:55Z

src/services/completions.ts

+        return undefined;
+    }
+
+    function tryGetTypeArgumentSubType(node: Node, checker: TypeChecker): Type | undefined {


This name is a little confusing—it looks like it’s getting the constraint of a type argument, and then optionally drilling into a property of that constraint type, based on the syntax location of the cursor. I was confused about why we’d want to get a subtype of a type argument when its constraint is necessarily a supertype of it. Maybe getConstraintOfTypeArgumentProperty or something?

Agreed the naming isn't correct, and I was looking for something like what you suggested. Nice!

andrewbranch · 2021-04-14T17:44:33Z

src/services/completions.ts

+        const objectTypeLiteralCompletions = getObjectTypeLiteralInTypeArgumentCompletions(sourceFile, position, contextToken, typeChecker, compilerOptions, log, preferences);
+        if (objectTypeLiteralCompletions) {
+            return objectTypeLiteralCompletions;
+        }


Instead of putting this here, did you look at adding to getCompletionData -> tryGetGlobalSymbols? It feels similar to tryGetObjectLikeCompletionSymbols() and tryGetClassLikeCompletionSymbols() so I expected it to be another case in that big chain of logical ORs.

I did look into it initially, but wasn't sure whether it would be the best place to put it. However, I tried moving the code there, and it still works as expected, so I think it should be fine :-)

- Move main logic onto tryGetGlobalSymbols function

tiagovtristao · 2021-04-18T13:30:35Z

Looking at the code, it looks very clean and it seems like it does exactly what I'd expect - actually, it does more than that, specifically accounting for intersections which I hadn't thought of!

I guess one thing I do wonder about is how this interacts with symbols, and non-identifier keys (e.g. keys with spaces, keys with numbers at the beginning, fully numeric keys). Those are very niche scenarios so I won't exactly block on that, but completions shouldn't feel intrusive there.

We should also consider these to be new identifier definition locations (e.g. you can specify more than what's there without the editor getting in your way).

(see #42595 (comment) for what I mean on that last point)

I just added support for non-identifier keys. Good catch!
I'm not sure I fully understood what you meant by "how this interacts with symbols" but I tried 2 things: Attempting to support JS symbols as keys but found out this isn't currently supported by TS as per Allow indexing with symbols #1863; And guaranteeing that keys that share the same escaped names of available symbols are supported, to which I added extra test checks.
Marked these as new identifier definition locations as suggested

DanielRosenwasser · 2021-04-19T19:33:07Z

@typescript-bot pack this

typescript-bot · 2021-04-19T19:33:11Z

Heya @DanielRosenwasser, I've started to run the tarball bundle task on this PR at 81a48fe. You can monitor the build here.

typescript-bot · 2021-04-19T19:48:45Z

Hey @DanielRosenwasser, 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/101266/artifacts?artifactName=tgz&fileId=2550AD5ADE742CCB2683873E94B7B7CC0537B315E6B729B0BBA9F48EEFB2914802&fileName=/typescript-4.3.0-insiders.20210419.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.3.0-pr-43526-11".;

andrewbranch

Looks good to me, thanks! @DanielRosenwasser, any other edge cases you want to check?

tiagovtristao added 2 commits April 4, 2021 23:28

Completion list for type literals in type arguments

149c20d

Add tests

0a10a54

typescript-bot added the For Uncommitted Bug PR for untriaged, rejected, closed or missing bug label Apr 5, 2021

DanielRosenwasser reviewed Apr 5, 2021

View reviewed changes

src/services/completions.ts Outdated Show resolved Hide resolved

DanielRosenwasser reviewed Apr 5, 2021

View reviewed changes

src/services/completions.ts Outdated Show resolved Hide resolved

DanielRosenwasser requested a review from andrewbranch April 5, 2021 21:42

Refactor for better readability

7c0366e

DanielRosenwasser reviewed Apr 14, 2021

View reviewed changes

andrewbranch reviewed Apr 14, 2021

View reviewed changes

- Support non-identifier keys

81a48fe

- Move main logic onto tryGetGlobalSymbols function

andrewbranch approved these changes Apr 20, 2021

View reviewed changes

DanielRosenwasser merged commit 3d24b85 into microsoft:master Apr 20, 2021

DanielRosenwasser mentioned this pull request Apr 20, 2021

Completion list should include property names of constraining type when writing object type literals in type argument positions #33302

Closed

5 tasks

andrewbranch mentioned this pull request Feb 22, 2023

No string completions in type arguments where constrained to string literal union #52898

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Completion list for type literals in type arguments #43526

Completion list for type literals in type arguments #43526

tiagovtristao commented Apr 5, 2021

typescript-bot commented Apr 5, 2021

DanielRosenwasser commented Apr 5, 2021

andrewbranch commented Apr 7, 2021

typescript-bot commented Apr 7, 2021 •

edited

Loading

typescript-bot commented Apr 7, 2021

DanielRosenwasser commented Apr 14, 2021

typescript-bot commented Apr 14, 2021 •

edited

Loading

DanielRosenwasser left a comment •

edited

Loading

typescript-bot commented Apr 14, 2021 •

edited

Loading

andrewbranch left a comment

andrewbranch Apr 14, 2021

tiagovtristao Apr 18, 2021

andrewbranch Apr 14, 2021

tiagovtristao Apr 18, 2021

tiagovtristao commented Apr 18, 2021

DanielRosenwasser commented Apr 19, 2021

typescript-bot commented Apr 19, 2021 •

edited

Loading

typescript-bot commented Apr 19, 2021 •

edited

Loading

andrewbranch left a comment

Completion list for type literals in type arguments #43526

Completion list for type literals in type arguments #43526

Conversation

tiagovtristao commented Apr 5, 2021

typescript-bot commented Apr 5, 2021

DanielRosenwasser commented Apr 5, 2021

andrewbranch commented Apr 7, 2021

typescript-bot commented Apr 7, 2021 • edited Loading

typescript-bot commented Apr 7, 2021

DanielRosenwasser commented Apr 14, 2021

typescript-bot commented Apr 14, 2021 • edited Loading

DanielRosenwasser left a comment • edited Loading

Choose a reason for hiding this comment

typescript-bot commented Apr 14, 2021 • edited Loading

andrewbranch left a comment

Choose a reason for hiding this comment

andrewbranch Apr 14, 2021

Choose a reason for hiding this comment

tiagovtristao Apr 18, 2021

Choose a reason for hiding this comment

andrewbranch Apr 14, 2021

Choose a reason for hiding this comment

tiagovtristao Apr 18, 2021

Choose a reason for hiding this comment

tiagovtristao commented Apr 18, 2021

DanielRosenwasser commented Apr 19, 2021

typescript-bot commented Apr 19, 2021 • edited Loading

typescript-bot commented Apr 19, 2021 • edited Loading

andrewbranch left a comment

Choose a reason for hiding this comment

typescript-bot commented Apr 7, 2021 •

edited

Loading

typescript-bot commented Apr 14, 2021 •

edited

Loading

DanielRosenwasser left a comment •

edited

Loading

typescript-bot commented Apr 14, 2021 •

edited

Loading

typescript-bot commented Apr 19, 2021 •

edited

Loading

typescript-bot commented Apr 19, 2021 •

edited

Loading