Skip to content

Conversation

@PranavSenthilnathan
Copy link
Member

@PranavSenthilnathan PranavSenthilnathan commented Oct 25, 2023

Improves completion to now show the qualified name of the completion.

const o = { A: "a" } as const;
enum e { B = "b" };
type t = { [o.A]: 1, [e.B]: 2 };
declare const x: t;
x.
// Old: completes to x[o]| and x[e]|
// New: completes to x[o.A]| and x[e.B]|
// ('|' denotes cursor position)

If the user sets includeCompletionsWithInsertText to false, we will only use the literal completion for the property (x.a and x.b in the example above).

Fixes #56096

@typescript-bot typescript-bot added the For Milestone Bug PRs that fix a bug with a specific milestone label Oct 25, 2023
@gabritto
Copy link
Member

@typescript-bot pack this

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 31, 2023

Heya @gabritto, I've started to run the tarball bundle task on this PR at 825f71b. You can monitor the build here.

@typescript-bot
Copy link
Collaborator

typescript-bot commented Oct 31, 2023

Hey @gabritto, 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/158414/artifacts?artifactName=tgz&fileId=2A45EB82B73EAB8E3099A28A01A1A3B9CAFBCE7179BC1A2BDC4B6A53301F59AA02&fileName=/typescript-5.3.0-insiders.20231031.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/[email protected]".;

node = typeChecker.symbolToEntityName(nameSymbol, /*meaning*/ undefined!, contextToken, NodeBuilderFlags.UseAliasDefinedOutsideCurrentScope);
}
else {
// Object literals assigned as const
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's this case for, exactly? I think I don't understand the comment here and how we know this from knowing that nameSymbol is a transient symbol.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This scenario goes into this branch:

const x = { a: "foo" } as const;
const y = { [x.a]: 0 };
y.|

It's because the symbol a is associated with the object and not x so walking up the parent symbols will just go to __object - so symbolToEntityName (the other branch) won't give a full access expression to it.

That being said, I don't know if isTransientSymbol is the right check here - are there transient symbols that won't run into this issue? and are there non-transient symbols that will run into this issue? I can be safe an always choose the else branch (get rid of the whole if).

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if all transient symbols run into this problem with symbolToEntityName, but I think I was confused by the comment because it sorta implied that if the symbol is transient, then we have that object literal situation, which I think is not always the case.
@weswigham might know the details of symbolToEntityName and symbolToExpression.

@gabritto
Copy link
Member

Improvement suggestion:
My understanding is that, with this PR, if we can't find an already-imported way to offer the computed property, we won't offer it in this new way and will default to the old way instead. Now, the old way already supports auto-importing symbols when possible. Could we make the new way also auto-import symbols?

Example:

// @filename: ex.ts
export type T = {
    [Sym.sym]: number;
}
export namespace Sym {
    export const sym = Symbol("unique!");
}
// @filename: index.ts
import { T } from "./ex";

declare const obj: T;
obj.|

Since we don't have Sym imported from "ex.ts", we will default to offering a completion for Sym and auto-importing it.
We could, I imagine, suggest the new completion for Sym.sym and auto-import Sym.

}

/**
* An accessible symbol is a local in the same block or any enclosing blocks, up to and including the global scope (note imports are global).
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if this comment is 100% accurate, but this is my understanding of what this method does. Open to change this if it's wrong.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

For Milestone Bug PRs that fix a bug with a specific milestone

Projects

Status: Waiting on reviewers

Development

Successfully merging this pull request may close these issues.

Intellisense provides different suggestions on autocomplete when using indexed access or dotted access

5 participants