[ty] Support empty function bodies in if TYPE_CHECKING blocks#19372
Merged
carljm merged 12 commits intoastral-sh:mainfrom Jul 16, 2025
Merged
[ty] Support empty function bodies in if TYPE_CHECKING blocks#19372carljm merged 12 commits intoastral-sh:mainfrom
if TYPE_CHECKING blocks#19372carljm merged 12 commits intoastral-sh:mainfrom
Conversation
Contributor
|
MatthewMckee4
requested review from
AlexWaygood,
carljm,
dcreager and
sharkdp
as code owners
MichaReiser
reviewed
Jul 15, 2025
| generator_functions: FxHashSet<FileScopeId>, | ||
|
|
||
| /// Set of all scopes that are inside `if TYPE_CHECKING` blocks. | ||
| function_scopes_in_type_checking: FxHashSet<FileScopeId>, |
Member
There was a problem hiding this comment.
I think it's more memory efficient if we store this on Scope instead of having another hash map
Contributor
Author
There was a problem hiding this comment.
What do you mean by that, sorry
Contributor
Author
There was a problem hiding this comment.
ah, add an attribute to the Scope struct?
MatthewMckee4
commented
Jul 15, 2025
Comment on lines
1516
to
1533
| let if_block_in_type_checking = | ||
| if let ast::Expr::Name(ast::ExprName { id, .. }) = &*node.test { | ||
| id == "TYPE_CHECKING" | ||
| } else { | ||
| false | ||
| }; | ||
|
|
||
| let else_elif_block_in_type_checking = | ||
| if let ast::Expr::UnaryOp(ast::ExprUnaryOp { op, operand, .. }) = &*node.test { | ||
| *op == ruff_python_ast::UnaryOp::Not | ||
| && if let ast::Expr::Name(ast::ExprName { id, .. }) = &**operand { | ||
| id == "TYPE_CHECKING" | ||
| } else { | ||
| false | ||
| } | ||
| } else { | ||
| false | ||
| }; |
Contributor
Author
There was a problem hiding this comment.
I'm aware this pollutes this arm a bit, should I move this to a different function outwith the SemanticIndexBuilder struct?
MatthewMckee4
changed the title
if TYPE_CHECKINGif TYPE_CHECKING blocks
Contributor
|
| Lint rule | Added | Removed | Changed |
|---|---|---|---|
invalid-return-type |
0 | 208 | 0 |
unused-ignore-comment |
18 | 0 | 0 |
| Total | 18 | 208 | 0 |
MichaReiser
previously requested changes
Jul 16, 2025
Comment on lines
+2137
to
+2139
| if self.scope().scope(self.db()).in_type_checking_block() { | ||
| return; | ||
| } |
Member
There was a problem hiding this comment.
It's unfortunate that we need to pre-compute this information in semantic index builder considering that empty function bodies in type checking blocks are rare. Ideally, we'd compute it lazily whether the function is inside a TYPE_CHECKING block but I don't see how we can do this easily because our AST doesn't allow upward traversal.
carljm
approved these changes
Jul 16, 2025
Co-authored-by: Carl Meyer <carl@oddbird.net>
Contributor
Author
|
Ah sorry let me revert that |
Contributor
|
It's all taken care of and merged already, thanks! |
Contributor
Author
|
Ah, didn't see that on my phone, thanks! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Resolves astral-sh/ty#339
Supports having a blank function body inside
if TYPE_CHECKINGblock or in the elif or else of aif not TYPE_CHECKINGblock.Test Plan
Update
function/return_type.md