[red-knot] Fix panic for tuple[x[y]] string annotation#17787
Conversation
|
Nice! I've been waiting for this panic to "go away" :) |
|
So this is a bit unfortunate… the I can run |
sharkdp
requested review from
AlexWaygood,
MichaReiser,
carljm and
dcreager
as code owners
|
Going to merge this, as it seems relatively uncontroversial — the whole fix happens in a function that will eventually go away. |
| let value_ty = if builder.deferred_state.in_string_annotation() { | ||
| // Using `.expression_type` does not work in string annotations, because | ||
| // we do not store types for sub-expressions. Re-infer the type here. | ||
| builder.infer_expression(value) | ||
| } else { | ||
| builder.expression_type(value) | ||
| }; |
There was a problem hiding this comment.
oof, that seems a bit unfortunate. Does that mean we should just get rid of the expression_type() API altogether? Or have expression_type() itself check whether we're in a deferred string annotation (and fallback to infer_expression() if so)?
It feels like a bit of a footgun if we have to always make sure we're not in a deferred string annotation before calling expression_type()
There was a problem hiding this comment.
Good questions. It's certainly possible that we have other hidden problems with calling expression_type for something that might be inside a string annotation. But note that it's not common to re-enter a sub-AST and match on inner elements (and then use expression_type) after the type has already been inferred. Usually, in functions like infer_expression, we return types "back up" to the caller, i.e. to outer elements. Admittedly, I did not think too much about this exact callsite because this whole function is basically a large "To do" and will disappear eventually.
* main: [red-knot] Refactor: no mutability in call APIs (#17788) [red-knot] Fix panic for `tuple[x[y]]` string annotation (#17787) [red-knot] Implicit instance attributes in generic methods (#17769) doc: Add link to `check-typed-exception` from `S110` and `S112` (#17786) Fix module name in ASYNC110, 115, and 116 fixes (#17774) [red-knot] More informative hover-types for assignments (#17762) [syntax-errors] Use consistent message for bad starred expression usage. (#17772) red_knot_server: add auto-completion MVP Allow passing a virtual environment to `ruff analyze graph` (#17743) Bump 0.11.8 (#17766) [`flake8-use-pathlib`] Fix `PTH104`false positive when `rename` is passed a file descriptor (#17712)
3 participants
Summary
closes #17775
Test Plan
Added corpus regression test