Skip to content
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

[red-knot] fix scope inference with deferred types #13204

Merged
merged 4 commits into from
Sep 3, 2024
Merged

Conversation

carljm
Copy link
Contributor

@carljm carljm commented Sep 2, 2024

Test coverage for #13131 wasn't as good as I thought it was, because although we infer a lot of types in stubs in typeshed, we don't check typeshed, and therefore we don't do scope-level inference and pull all types for a scope. So we didn't really have good test coverage for scope-level inference in a stub. And because of this, I got the code for supporting that wrong, meaning that if we did scope-level inference with deferred types, we'd end up never populating the deferred types in the scope's TypeInference, which causes panics like #13160.

Here I both add test coverage by running the corpus tests both as .py and as .pyi (which reveals the panic), and I fix the code to support deferred types in scope inference.

This also revealed a problem with deferred types in generic functions, which effectively span two scopes. That problem will require a bit more thought, and I don't want to block this PR on it, so for now I just don't defer annotations on generic functions.

Fixes #13160.

@carljm carljm added the red-knot Multi-file analysis & type inference label Sep 2, 2024
Copy link
Contributor

github-actions bot commented Sep 2, 2024

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

✅ ecosystem check detected no linter changes.

Formatter (stable)

✅ ecosystem check detected no format changes.

Formatter (preview)

✅ ecosystem check detected no format changes.

crates/red_knot_workspace/tests/check.rs Outdated Show resolved Hide resolved
crates/red_knot_workspace/tests/check.rs Outdated Show resolved Hide resolved
@carljm carljm force-pushed the cjm/deferred-panic branch from 97a9604 to 76d2fc5 Compare September 3, 2024 17:27
@carljm carljm merged commit 29c36a5 into main Sep 3, 2024
20 checks passed
@carljm carljm deleted the cjm/deferred-panic branch September 3, 2024 18:20
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
red-knot Multi-file analysis & type inference
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[red_knot] Crash on analysing stub file with explicitly derived class
2 participants