Skip to content

[red-knot] Understand typing.Protocol and typing_extensions.Protocol as equivalent #17446

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
Merged
AlexWaygood merged 2 commits into from
Apr 17, 2025
Merged

[red-knot] Understand typing.Protocol and typing_extensions.Protocol as equivalent #17446

AlexWaygood merged 2 commits into from
Apr 17, 2025

Conversation

@AlexWaygood
Copy link
Member

@AlexWaygood AlexWaygood commented Apr 17, 2025

Summary

We currently have special-casing that avoids false positives for typing.Protocol, but the special-casing doesn't currently recognise typing_extensions.Protocol as also being a valid class base, etc. Due to various bugfixes and performance optimisations having been backported via typing_extensions, typing_extensions.Protocol is a different symbol to typing.Protocol on most Python versions: it would be wrong for us to infer Literal[True] as the result of the expression typing_extensions.Protocol is typing.Protocol, for example. Nonetheless, they should be treated identically by a type checker in terms of their semantics. This PR implements that.

Test Plan

Existing mdtests have been updated to reflect that we no longer issue false-positive diagnostics on classes inheriting from typing_extensions.Protocol.

@AlexWaygood AlexWaygood added the ty Multi-file analysis & type inference label Apr 17, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Apr 17, 2025 •
edited
Loading

mypy_primer results

Changes were detected when running on open source projects 
dacite (https://github.com/konradhalas/dacite)
- error[lint:invalid-base] /tmp/mypy_primer/projects/dacite/dacite/data.py:8:12: Invalid class base with type `typing.Protocol | _SpecialForm` (all bases must be a class, `Any`, `Unknown` or `Todo`)
- error[lint:invalid-return-type] /tmp/mypy_primer/projects/dacite/dacite/data.py:11:48: Function can implicitly return `None`, which is not assignable to return type `bool`
- Found 158 diagnostics
+ Found 156 diagnostics

AlexWaygood
AlexWaygood commented Apr 17, 2025
View reviewed changes
@AlexWaygood AlexWaygood marked this pull request as draft April 17, 2025 12:15
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Apr 17, 2025

mypy_primer results

Changes were detected when running on open source projects

dacite (https://github.com/konradhalas/dacite)
+ error[lint:invalid-base] /tmp/mypy_primer/projects/dacite/dacite/data.py:8:12: Invalid class base with type `typing.Protocol | typing_extensions.Protocol` (all bases must be a class, `Any`, `Unknown` or `Todo`)
- error[lint:invalid-base] /tmp/mypy_primer/projects/dacite/dacite/data.py:8:12: Invalid class base with type `typing.Protocol | _SpecialForm` (all bases must be a class, `Any`, `Unknown` or `Todo`)

This shows the cost of treating the symbols typing_extensions.Protocol and typing.Protocol as inhabiting distinct types: if they were understood by us as inhabiting exactly the same type, then we wouldn't infer a union type for the Protocol symbol here, so we wouldn't emit that diagnostic. I think instead of my current approach, I'll switch to treating them as inhabiting exactly the same type, but rework some of the logic around KnownInstanceType so that we no longer assume a type is a singleton type if it's a KnownInstanceType. This doesn't necessarily hold true if the symbol could originate from either typing or typing_extensions.

@AlexWaygood AlexWaygood force-pushed the alex/typing-extensions-protocol branch from 2954b58 to 91aa212 Compare April 17, 2025 12:51
@AlexWaygood
Copy link
Member Author

AlexWaygood commented Apr 17, 2025 •
edited
Loading

The mypy_primer results with the new approach look much better!

@AlexWaygood AlexWaygood marked this pull request as ready for review April 17, 2025 12:53
carljm
carljm approved these changes Apr 17, 2025
View reviewed changes
Copy link
Contributor

@carljm carljm left a comment

Choose a reason for hiding this comment

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

Looks great!

@AlexWaygood AlexWaygood merged commit 9965cee into main Apr 17, 2025
23 checks passed
@AlexWaygood AlexWaygood deleted the alex/typing-extensions-protocol branch April 17, 2025 20:54
dcreager added a commit that referenced this pull request Apr 18, 2025
@dcreager
Merge branch 'main' into dcreager/subscript-types
0c489ca 
* main:
  [red-knot] class bases are not affected by __future__.annotations (#17456)
  [red-knot] Add support for overloaded functions (#17366)
  [`pyupgrade`] Add fix safety section to docs (`UP036`) (#17444)
  [red-knot] more type-narrowing in match statements (#17302)
  [red-knot] Add some narrowing for assignment expressions (#17448)
  [red-knot] Understand `typing.Protocol` and `typing_extensions.Protocol` as equivalent (#17446)
  Server: Use `min` instead of `max` to limit the number of threads (#17421)
dcreager added a commit that referenced this pull request Apr 18, 2025
@dcreager
Merge branch 'main' into dcreager/legacy-typevar-instance
fd91822 
* main: (123 commits)
  [red-knot] Handle explicit class specialization in type expressions (#17434)
  [red-knot] allow assignment expression in call compare narrowing (#17461)
  [red-knot] fix building unions with literals and AlwaysTruthy/AlwaysFalsy (#17451)
  [red-knot] Type narrowing for assertions (take 2) (#17345)
  [red-knot] class bases are not affected by __future__.annotations (#17456)
  [red-knot] Add support for overloaded functions (#17366)
  [`pyupgrade`] Add fix safety section to docs (`UP036`) (#17444)
  [red-knot] more type-narrowing in match statements (#17302)
  [red-knot] Add some narrowing for assignment expressions (#17448)
  [red-knot] Understand `typing.Protocol` and `typing_extensions.Protocol` as equivalent (#17446)
  Server: Use `min` instead of `max` to limit the number of threads (#17421)
  [red-knot] Detect version-related syntax errors (#16379)
  [`pyflakes`] Add fix safety section (`F841`) (#17410)
  [red-knot] Add `KnownFunction` variants for `is_protocol`, `get_protocol_members` and `runtime_checkable` (#17450)
  Bump 0.11.6 (#17449)
  Auto generate `visit_source_order` (#17180)
  [red-knot] Initial tests for protocols (#17436)
  [red-knot] Dataclasses: synthesize `__init__` with proper signature (#17428)
  [red-knot] Dataclasses: support `order=True` (#17406)
  [red-knot] Super-basic generic inference at call sites (#17301)
  ...
@carljm carljm mentioned this pull request Apr 24, 2025
Closed
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@carljm carljm carljm approved these changes

@sharkdp sharkdp Awaiting requested review from sharkdp

@dcreager dcreager Awaiting requested review from dcreager

Assignees

No one assigned

Labels

ty Multi-file analysis & type inference

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@AlexWaygood @carljm