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

Use lookup_fully_qualified for SemanticAnalyzer.named_type #11332

Merged
merged 2 commits into from
Oct 15, 2021

Conversation

97littleleaf11
Copy link
Collaborator

@97littleleaf11 97littleleaf11 commented Oct 14, 2021

Description

Related #6578

This PR replaces lookup_qualified with lookup_fully_qualified for SemanticAnalyzer.named_type. Now it's consistant with the implementation of checker.named_type. This change also allows calling named_type without dunders.

Test Plan

Shouldn't affect any tests.

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.

This looks great! The old behavior was clearly inconsistent.

It looks like this will be a breaking change to some plugins? If that's the case, we'll need to add a comment to #6617 about the change, and explain what needs to be done to support the new behavior.

mypy/semanal.py Outdated

def str_type(self) -> Instance:
return self.named_type('__builtins__.str')
return self.named_type('builtins.str')

def named_type(self, qualified_name: str, args: Optional[List[Type]] = None) -> Instance:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Rename qualified_name to fullname?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

It looks like this will be a breaking change to some plugins?

Yes. I think we should notify that we are using full name now and we support direct call on builtins module instead of __builtins__.

@JukkaL JukkaL merged commit 9bd6517 into python:master Oct 15, 2021
@JukkaL
Copy link
Collaborator

JukkaL commented Oct 15, 2021

Can we close #6578 now?

@97littleleaf11 97littleleaf11 deleted the use-fully-qualified branch October 20, 2021 11:07
freundTech added a commit to freundTech/mypy that referenced this pull request Oct 20, 2021
JukkaL pushed a commit that referenced this pull request Nov 4, 2021
Related to #6578.

builtin_type could be totally replaced with named_type after #11332. 
Also renames other builtin_type for consistency since all of them are 
actually calling named_type.
PrettyWood added a commit to PrettyWood/pydantic that referenced this pull request Dec 24, 2021
Due to python/mypy#11332, mypy would crash
because `__builtins__` is not part of `ctx.api` modules, `builtins` is
samuelcolvin pushed a commit to pydantic/pydantic that referenced this pull request Dec 29, 2021
* build(deps): bump mypy from 0.920 to 0.930

* fix: avoid mypy plugin crash

Due to python/mypy#11332, mypy would crash
because `__builtins__` is not part of `ctx.api` modules, `builtins` is

* fix tests
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
)

Related to python#6578

This PR replaces lookup_qualified with lookup_fully_qualified for SemanticAnalyzer.named_type. 
Now it's consistant with the implementation of checker.named_type. This change also allows 
calling named_type without dunders.
tushar-deepsource pushed a commit to DeepSourceCorp/mypy that referenced this pull request Jan 20, 2022
Related to python#6578.

builtin_type could be totally replaced with named_type after python#11332. 
Also renames other builtin_type for consistency since all of them are 
actually calling named_type.
squarebridges pushed a commit to nicejobinc/pydantic that referenced this pull request Jun 24, 2022
* build(deps): bump mypy from 0.920 to 0.930

* fix: avoid mypy plugin crash

Due to python/mypy#11332, mypy would crash
because `__builtins__` is not part of `ctx.api` modules, `builtins` is

* fix tests
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.

2 participants