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

Display type aliases in auto-import suggestions #606

Closed
cdce8p opened this issue Nov 12, 2020 · 9 comments
Closed

Display type aliases in auto-import suggestions #606

cdce8p opened this issue Nov 12, 2020 · 9 comments
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version

Comments

@cdce8p
Copy link

cdce8p commented Nov 12, 2020

I really like the change to the auto-import suggestions (#163) introduced with the last release 2020.11.1.
However it doesn't seem like type aliases get indexed. I've already set python.analysis.indexing to true.
Maybe this is a feature worth considering.

@jakebailey
Copy link
Member

Can you clarify what you mean? Are you saying that the names themselves should appear as symbols in the workspace symbol search? We don't include these in document symbols (the outline) or workspace symbols (but I don't think we did before either).

@jakebailey jakebailey added the waiting for user response Requires more information from user label Nov 12, 2020
@github-actions github-actions bot removed the triage label Nov 12, 2020
@cdce8p
Copy link
Author

cdce8p commented Nov 12, 2020

@jakebailey Given two files:

# const.py
MyAlias = list[int]
MyConst = 42

class MyClass:
    ...


# main.py
var: [Alias]

If I start writing My for Alias I will only get the suggestion for MyClass, although MyAlias would be equally valid.

--
As a side note, even MyConst isn't suggested. It seems only constants with all capital letters are indexed. If I change it to MY_CONST, it works.

Screen Shot 2020-11-13 at 00 15 39

@jakebailey
Copy link
Member

Ah, my mistake. "type aliases", I took it to mean an import alias in import X as Y as that's part of what #163 was about.

With indexing enabled, you should get the completion for that (or, by opening up the file so it's loaded). Is it correct that you're saying it's missing?

@cdce8p
Copy link
Author

cdce8p commented Nov 12, 2020

Yeah, at least I though so. After some further investigation I found that MyAlias would not get indexed whereas MY_ALIAS would. This is the same behavior as with constants. However, I believe type aliases commonly use CamelCase. https://www.python.org/dev/peps/pep-0484/#type-aliases

@jakebailey
Copy link
Member

There's some heuristics to indexing to prevent it from taking too long in library code, but if this is all within user code, we should be doing a better job.

@heejaechang
Copy link
Contributor

@cdce8p yes, you are right on the heuristics. since python doesn't have a formal way (like export in typescript) to specify which variables should be accessible from others, including every variable in the auto-import causes too much noise in completion. (easily cause hundreds of more entries to be inserted into completion).

so we have this heuristics

but that being said, if we can come up with better heuristics, we can try that.

FYI @savannahostrowski @judej

@erictraut
Copy link
Contributor

I think it makes sense to include capitalized (camel-case) variables defined at the module level. These are likely type aliases.

Also, I noticed that your current heuristic uses a reg-ex that is incorrect because it doesn't allow numbers or underscores. See symbolNameUtils.isConstantName for a better approach.

@jakebailey jakebailey added needs investigation Could be an issue - needs investigation and removed waiting for user response Requires more information from user labels Nov 13, 2020
@heejaechang
Copy link
Contributor

@erictraut existing regExp is negative check, so it disallows any variable that contains non capital char, so it will allow number and such that is legal to identifier. It doesn't care about space and such since name already passed identifier name check.

but sure, I can change it to isConstantName if that is prefered.

@jakebailey jakebailey added fixed in next version (main) A fix has been implemented and will appear in an upcoming version and removed needs investigation Could be an issue - needs investigation labels Nov 16, 2020
@jakebailey
Copy link
Member

This issue has been fixed in version 2020.11.2, which we've just released. You can find the changelog here: https://github.com/microsoft/pylance-release/blob/master/CHANGELOG.md#2020112-18-november-2020

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fixed in next version (main) A fix has been implemented and will appear in an upcoming version
Projects
None yet
Development

No branches or pull requests

4 participants