-
Notifications
You must be signed in to change notification settings - Fork 1.7k
[ty] Type inference for @asynccontextmanager
#21876
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
Conversation
| if yield_ty.is_divergent() { | ||
| return Some(yield_ty); | ||
| } |
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.
This took me a while to figure out. Looks related to astral-sh/ty#1821 / astral-sh/ty#1821. If we don't do this, we end up with unions of the actual type that we want to have and Divergent
Diagnostic diff on typing conformance testsNo changes detected when running ty on typing conformance tests ✅ |
|
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-argument-type |
2 | 0 | 0 |
invalid-return-type |
1 | 0 | 1 |
invalid-assignment |
1 | 0 | 0 |
possibly-missing-attribute |
1 | 0 | 0 |
unsupported-base |
0 | 1 | 0 |
| Total | 5 | 1 | 1 |
810e257 to
f02359c
Compare
|
|
||
| @asynccontextmanager | ||
| async def clone(self: Self, checkout_dir: Path) -> AsyncIterator[Path]: | ||
| async def clone(self: Self, checkout_dir: Path) -> AsyncIterator[str]: |
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.
Oh look, we found a bug.
|
f02359c to
9bf36f0
Compare
carljm
left a comment
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.
This looks like a reasonable temporary patch for a common case to me!
dcreager
left a comment
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.
👍 thank you!
|
Merging despite the failing benchmark run (timeout). Benchmarks were running on a previous version of this PR and I only made a completely trivial change since then. |
* origin/main: (33 commits) [ty] Simplify union lower bounds and intersection upper bounds in constraint sets (#21871) [ty] Collapse `never` paths in constraint set BDDs (#21880) Fix leading comment formatting for lambdas with multiple parameters (#21879) [ty] Type inference for `@asynccontextmanager` (#21876) Fix comment placement in lambda parameters (#21868) [`pylint`] Detect subclasses of builtin exceptions (`PLW0133`) (#21382) Fix stack overflow with recursive generic protocols (depth limit) (#21858) New diagnostics for unused range suppressions (#21783) [ty] Use default settings in completion tests [ty] Infer type variables within generic unions (#21862) [ty] Fix overload filtering to prefer more "precise" match (#21859) [ty] Stabilize auto-import [ty] Fix reveal-type E2E test (#21865) [ty] Use concise message for LSP clients not supporting related diagnostic information (#21850) Include more details in Tokens 'offset is inside token' panic message (#21860) apply range suppressions to filter diagnostics (#21623) [ty] followup: add-import action for `reveal_type` too (#21668) [ty] Enrich function argument auto-complete suggestions with annotated types [ty] Add autocomplete suggestions for function arguments [`flake8-bugbear`] Accept immutable slice default arguments (`B008`) (#21823) ...
Summary
This PR adds special handling for
asynccontextmanagercalls as a temporary solution for astral-sh/ty#1804. We will be able to remove this soon once we have support for generic protocols in the solver.closes astral-sh/ty#1804
Ecosystem
These look like true positives
Known problems or true positives, just caused by the new type for
sessionJust a more concrete type
Good
Test Plan