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

Implements module attribute suggestions #7971

Merged
merged 3 commits into from
Nov 23, 2019

Conversation

theodoretliu
Copy link
Contributor

Addresses the remainder of issue #824

Copy link
Collaborator

@JukkaL JukkaL left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! This will be a nice usability improvement. I'm mostly worried about potential performance impact, otherwise looks good.

module_members = None

if isinstance(base, RefExpr) and isinstance(base.node, MypyFile):
module_members = [key for key in base.node.names.keys() if not base.node.names.get(key).module_hidden]
Copy link
Collaborator

Choose a reason for hiding this comment

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

This can be expensive to calculate, in case the module has many attributes. Instead of passing the members, what about passing a SymbolTable object, and calculating the members only later if we generate an error message?

@@ -126,7 +126,7 @@ import m
if int():
from m import foobaz # E:5: Module 'm' has no attribute 'foobaz'; maybe "foobar"?
(1).x # E:2: "int" has no attribute "x"
(m.foobaz()) # E:2: Module has no attribute "foobaz"
(m.foobaz()) # E:2: Module has no attribute "foobaz"; maybe "foobar"?
Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe add a test case where there are multiple possible suggestions?

@JukkaL
Copy link
Collaborator

JukkaL commented Nov 19, 2019

Also, self check seems to be failing (Travis CI).

@theodoretliu
Copy link
Contributor Author

Hey, I addressed the comments! I wasn't sure what you meant by the Union type though

@python python deleted a comment from JukkaL Nov 22, 2019
@ilevkivskyi
Copy link
Member

(GitHub did something very weird and I have no idea how to undo this.)

@theodoretliu
Copy link
Contributor Author

Hm, I never received an email for that comment either

@ilevkivskyi
Copy link
Member

Anyway, I think Jukka meant that you can use the same argument (say extra_info) instead of two separate ones like original_type and module_symbol_table, and adjust the logic in the method. I don't think this is really important. I think however it makes sense in any case update the docstrings for analyze_member_access() and has_no_attr() to mention the args you added.

Copy link
Member

@ilevkivskyi ilevkivskyi left a comment

Choose a reason for hiding this comment

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

Thanks, LGTM now!

@ilevkivskyi ilevkivskyi merged commit fffb4b4 into python:master Nov 23, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants