-
-
Notifications
You must be signed in to change notification settings - Fork 3.1k
Fix narrowing with final type objects #20743
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
base: master
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7918,9 +7918,8 @@ def get_type_range_of_type(self, typ: Type) -> TypeRange | None: | |
|
|
||
| if isinstance(typ, UnionType): | ||
| type_ranges = [self.get_type_range_of_type(item) for item in typ.items] | ||
| is_upper_bound = any(t.is_upper_bound for t in type_ranges if t is not None) | ||
| item = make_simplified_union([t.item for t in type_ranges if t is not None]) | ||
| return TypeRange(item, is_upper_bound=is_upper_bound) | ||
| return TypeRange(item, is_upper_bound=True) | ||
| if isinstance(typ, FunctionLike) and typ.is_type_obj(): | ||
| # If a type is generic, `isinstance` can only narrow its variables to Any. | ||
| any_parameterized = fill_typevars_with_any(typ.type_object()) | ||
|
|
@@ -7937,6 +7936,8 @@ def get_type_range_of_type(self, typ: Type) -> TypeRange | None: | |
| if isinstance(typ.item, NoneType): | ||
| # except for Type[None], because "'NoneType' is not an acceptable base type" | ||
| is_upper_bound = False | ||
| if isinstance(typ.item, Instance) and typ.item.type.is_final: | ||
| is_upper_bound = False | ||
| return TypeRange(typ.item, is_upper_bound=is_upper_bound) | ||
| if isinstance(typ, AnyType): | ||
| return TypeRange(typ, is_upper_bound=False) | ||
|
|
@@ -8650,7 +8651,7 @@ def flatten_types_if_tuple(t: Type) -> list[Type]: | |
| """Flatten a nested sequence of tuples into one list of nodes.""" | ||
| t = get_proper_type(t) | ||
| if isinstance(t, UnionType): | ||
| return [b for a in t.items for b in flatten_types_if_tuple(a)] | ||
| return [UnionType.make_union([b for a in t.items for b in flatten_types_if_tuple(a)])] | ||
|
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Huh, why the recursive
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
See #20677
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
See #20675
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah thanks for the reference. However, I don't see why the
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure I understand... could you spell out the test case?
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Hmm, I'm unable to articulate that into a test case... However I noticed this seems wrong: Specifically, I think the positive branch should be
Collaborator
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Looks like that isn't relevant to this PR though! |
||
| if isinstance(t, TupleType): | ||
| return [b for a in t.items for b in flatten_types_if_tuple(a)] | ||
| elif is_named_instance(t, "builtins.tuple"): | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I added this in https://github.com/python/mypy/pull/20675/files#diff-f96a2d6138bc6cdf2a07c4d37f6071cc25c1631afc107e277a28d5b59fc0ef04R7947 but it's not right, this should always be True
See this test case I mention in the PR description: https://github.com/python/mypy/pull/20675/files#diff-e3de7a75a8a107b4f462b164cdf4945d50505c5e9f7092b753c4add0c01530bbR3021-R3037