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.

Sign up for GitHub

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

Jump to bottom

Clean-up lookup functions in semanal.py #4157

Open
ilevkivskyi opened this issue Oct 25, 2017 · 4 comments
Open

Clean-up lookup functions in semanal.py #4157

ilevkivskyi opened this issue Oct 25, 2017 · 4 comments
Labels
priority-1-normal refactoring Changing mypy's internals semantic-analyzer Problems that happen during semantic analysis

Comments

@ilevkivskyi
Copy link
Member

ilevkivskyi commented Oct 25, 2017 •
edited
Loading

Second pass of semantic analysis has 6 lookup functions:

  • lookup
  • lookup_qualified
  • lookup_fully_qualified
  • lookup_fully_qualified_or_none
  • lookup_type_node

plus 3 related helpers:

  • builtin_type
  • named_type
  • named_type_or_none

I think 9 lookup functions is too much and can cause confusions (this happened to me several times). I think we can leave only 3 lookup functions plus 2 helpers.
@ilevkivskyi ilevkivskyi added refactoring Changing mypy's internals priority-1-normal labels Oct 25, 2017
@ilevkivskyi
Copy link
Member Author

I think we need to fix this soon. Already three lookup functions and two lookup helpers sneaked in the plugin API (which is I believe too many).

@ilevkivskyi ilevkivskyi mentioned this issue Apr 2, 2019
Open
@ilevkivskyi ilevkivskyi added the semantic-analyzer Problems that happen during semantic analysis label Apr 2, 2019
@ilevkivskyi
Copy link
Member Author

(Added the new analyzer label. Although it is not specific to the new analyzer, it is a great opportunity to clean things up.)

@Michael0x2a Michael0x2a mentioned this issue Apr 12, 2019
Closed
This was referenced Sep 27, 2021
Merged
Closed
@97littleleaf11
Copy link
Collaborator

There also exist several lookup funcs in checker.py. I just have a glanced at them and it seems they share the same logic with those in other files. Shall we extract them to lookup.py?

@ilevkivskyi
Copy link
Member Author

Yeah, I think moving everything to one place should be good.

@97littleleaf11 97littleleaf11 mentioned this issue Oct 4, 2021
Merged
JukkaL pushed a commit that referenced this issue Oct 5, 2021
@97littleleaf11
Merge lookup_fully_qualified and lookup_fully_qualified_or_none (#11205)
d1d4316 
Partially solves #4157
JukkaL pushed a commit that referenced this issue Oct 8, 2021
@97littleleaf11
Remove redundant lookup funcs in fixup.py (#11253)
15f2385 
Removes the redundant lookup funcs in fixup.py. Related to #4157.

* Removes lookup_qualified_stnode: uncessary nested call to lookup_fully_qualified.
* Removes lookup_qualified: inconsistency return types with other lookup_qualified 
  funcs. Let the callers extract the SymbolNode from the SymbolTableNode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
priority-1-normal refactoring Changing mypy's internals semantic-analyzer Problems that happen during semantic analysis
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@JukkaL @97littleleaf11 @ilevkivskyi