Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add semantic token modifiers to callable variables. #5841

Open
heejaechang opened this issue May 2, 2024 · 4 comments
Open

Add semantic token modifiers to callable variables. #5841

heejaechang opened this issue May 2, 2024 · 4 comments
Assignees
Labels
enhancement New feature or request

Comments

@heejaechang
Copy link
Contributor

this is related to #5785

basically, user could have something like this

func: Callable[...] = ...

it is a variable, but it can be used like function

also, user could have something like this

class Foo():
     def __call__(self): ...

func = Foo()
func()

here func is a variable again but it can be used like function

we could add semantic token modifier to these variables so users who want to distinguish them can use editor.semanticTokenColorCustomizations to do so.

@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label May 2, 2024
@heejaechang heejaechang added the enhancement New feature or request label May 2, 2024
@erictraut
Copy link
Contributor

erictraut commented May 2, 2024

If my understanding is correct, today's semantic token provider generates tokens based on semantic categories only, not on type information. What you're proposing would require the token provider to perform full type analysis of every identifier at every point in the code flow since the type of an identifier can change based on narrowing, assignments, etc. It also means that the color of an identifier might now vary from one place in the code flow to the next. And it would require you to decide what to do with types that involve unions between, for example, a callable type and a non-callable type.

@heejaechang
Copy link
Contributor Author

how we will implement or how much perf cost it will have, we need to dig in but before that, we probably want to see how many people will upvote this.

@StellaHuang95 StellaHuang95 removed the needs repro Issue has not been reproduced yet label May 2, 2024
@heejaechang
Copy link
Contributor Author

I briefly looked, and we get decls for all identifiers (basically names), but we only get the type of the decl, not at each specific position, which will cause us to do the narrowing and other things Eric mentioned above.

So, if we do it based on the declared type, there won't be any extra cost. However, if we want it to be precise and correct at each position where identifiers are used, there will be extra performance cost.

That said, since the semantic token provider and checker (the one that produces diagnostics) will share the same cache, and since the checker will call getType on every identifier anyway, in reality, the overall cost should be the same. It would be just a matter of who does it first and gets the blame.

For the same variable/parameter being colored differently if we decide to do it per each position, since we just mark the token with extra data and people who want it need to opt in, it might be okay?

But I am not here to draw conclusions but to provide more context so people can discuss with more information in hand.

@maflAT
Copy link

maflAT commented May 17, 2024

This is related to #5612
Currently the same symbol can be treated either as a function or a variable depending on how it is first declared.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

4 participants