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

Intellisense doesn't respect __all__ when using import * #494

Closed
letmaik opened this issue Dec 29, 2017 · 12 comments
Closed

Intellisense doesn't respect __all__ when using import * #494

letmaik opened this issue Dec 29, 2017 · 12 comments
Labels
area-intellisense LSP-related functionality: auto-complete, docstrings, navigation, refactoring, etc. bug Issue identified by VS Code Team member as probable bug

Comments

@letmaik
Copy link
Member

letmaik commented Dec 29, 2017

Environment data

VS Code version: 1.19.1
Python Extension version: 0.9.1
Python Version: 3.6.0
OS and version: Win 10

Actual behavior

# A.py
import B
B. # intellisense shows foo and bar, but accessing bar would FAIL at runtime

# B.py
from C import *

# C.py
__all__ = ['foo']
foo = 42
bar = 24

Expected behavior

With the code above intellisense should only show foo.

@brettcannon
Copy link
Member

We won't be changing this because (1) there are legitimate reasons to want to ignore __all__ and (2) use of import * outside of the REPL is generally discouraged in Python.

@letmaik
Copy link
Member Author

letmaik commented Jan 3, 2018

I agree that it is generally discouraged but in certain situations it makes sense, for example within a library to expose members of internal modules within a single public facing module. Instead of explicitly importing each name of the internal modules, a star import is quite handy and it also serves for clearly defining what the public API is made out of.
Apart from this, if someone trusts the intellisense and an __all__ is in play like in my example above, then the code will fail at runtime. I thought this should be a good reason to have a red squiggly line.

@brettcannon
Copy link
Member

Squiggle underlines gets into linting which means we rely on Pylint, flake8, or whatever linter you choose to use. So if you want to be warned that you are using something not contained in __all__ then that falls under the purview of your chosen linter.

@letmaik
Copy link
Member Author

letmaik commented Jan 8, 2018

I'm not sure if we're on the same page here. Ignore the squiggle lines for now. The main problem is that intellisense displays a lot more than what is actually possible to invoke. The members just do not exist and there will be a hard error at runtime. This is not about warnings or hiding private functions from novice users, it's about correctly computing the set of reachable symbols. Have a look at the example again to see what I mean.
If this is still something not going to be fixed (IMO it's a bug since intellisense in that case let's you construct invalid code), would you accept a PR for it?

@brettcannon brettcannon added bug Issue identified by VS Code Team member as probable bug area-intellisense LSP-related functionality: auto-complete, docstrings, navigation, refactoring, etc. awaiting 1-verification and removed closed-wontfix labels Jan 8, 2018
@brettcannon
Copy link
Member

OK, now I see where you're coming from with your example. Sorry about the misunderstanding. 😄 I've re-opened this as an issue and I have confirmed this issue does exist.

@brettcannon brettcannon reopened this Jan 8, 2018
@brettcannon brettcannon changed the title Intellisense doesn't respect __all__ Intellisense doesn't respect __all__ when using import * Jan 8, 2018
@brettcannon
Copy link
Member

And FYI this is probably going to be an issue in Jedi. @MikhailArkhipov this might be something for your meta issue on intellisense.

@letmaik
Copy link
Member Author

letmaik commented Jan 8, 2018

I found this: davidhalter/jedi#715 :/

@MikhailArkhipov
Copy link

Which is closed as 'wont fix'

@MikhailArkhipov
Copy link

MikhailArkhipov commented Apr 25, 2018

Works fine in VS engine. Won't be fixed in Jedi.

@MikhailArkhipov MikhailArkhipov added this to the May 2018 milestone Apr 25, 2018
@MikhailArkhipov MikhailArkhipov self-assigned this Apr 25, 2018
@brettcannon brettcannon removed this from the May 2018 milestone Jun 6, 2018
@MikhailArkhipov MikhailArkhipov removed their assignment Jul 10, 2018
@munael
Copy link

munael commented Jan 25, 2019

Still an issue in the Microsoft Python Language Server.

Python extension: 2018.12.1 (with the early/nightly updates setting set)
VS Code: 1.31.0 (Insiders)

Is there a more lively issue tracking this?

@brettcannon
Copy link
Member

I can verify this isn't working in the language server v0.1.78.

I've opened microsoft/python-language-server#619 to track this upstream.

@luabud
Copy link
Member

luabud commented Sep 11, 2019

Fixed upstream.

@luabud luabud closed this as completed Sep 11, 2019
@ghost ghost removed the needs upstream fix label Sep 11, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Sep 18, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-intellisense LSP-related functionality: auto-complete, docstrings, navigation, refactoring, etc. bug Issue identified by VS Code Team member as probable bug
Projects
None yet
Development

No branches or pull requests

5 participants