Improve typing with structured ASGI scopes and WSGI environment TypedDicts#2642
Improve typing with structured ASGI scopes and WSGI environment TypedDicts#2642MustafaNazir11 wants to merge 1 commit into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #2642 +/- ##
=========================================
Coverage 100.00% 100.00%
=========================================
Files 64 64
Lines 7976 7983 +7
Branches 1102 1103 +1
=========================================
+ Hits 7976 7983 +7 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
vytas7
left a comment
vytas7
left a comment
There was a problem hiding this comment.
Thanks for this PR, @MustafaNazir11!
Unfortunately, it requires a major overhaul before we could consider it.
If you are improving typing, please don't alter the code's behaviour itself, especially not in performance-critical code paths!
Also, the main goal of the TypedDicts was to get rid of type-ignores related to using too generic types. Unfortunately, it seems we got a slew of new ignores instead!
| asgi_info: dict[str, str] = scope['asgi'] | ||
| raw = scope['asgi'] | ||
| except KeyError: | ||
| # NOTE(kgriffs): According to the ASGI spec, "2.0" is |
There was a problem hiding this comment.
Please don't remove the notes or make unrelated changes to code.
| spec_version: str | None = asgi_info['spec_version'] | ||
| except KeyError: | ||
| spec_version = None | ||
| # Normalize into proper _ASGIVersions shape |
There was a problem hiding this comment.
Please don't make changes that can affect performance in this performance-critical path.
| # first (in contrast to one-shot lifespan events). | ||
| if scope_type == 'websocket': | ||
| await self._handle_websocket(spec_version, scope, receive, send) | ||
| await self._handle_websocket(spec_version, scope, receive, send) # type: ignore[arg-type] |
There was a problem hiding this comment.
The whole point was to get rid of type-ignores, not to add new ones!
|
|
||
| req = self._request_type( | ||
| scope, receive, first_event=first_event, options=self.req_options | ||
| scope, # type: ignore[arg-type] |
| if sys.version_info >= (3, 11): | ||
| from typing import NotRequired | ||
| from wsgiref.types import StartResponse as StartResponse | ||
| from wsgiref.types import WSGIEnvironment as WSGIEnvironment |
There was a problem hiding this comment.
Why did you remove WSGIEnvironment from here?
2 participants
Summary of Changes
This PR modernizes and significantly strengthens Falcon’s typing by replacing loosely typed dict[str, Any] usages with well-defined TypedDict structures for both ASGI scopes and WSGI environment objects.
It also updates legacy typing syntax (Union, Optional) to modern Python 3.10+ style and improves internal type safety across multiple modules.
Related Issues
Relates to #2628
Pull Request Checklist
This is just a reminder about the most common mistakes. Please make sure that you tick all appropriate boxes. Reading our contribution guide at least once will save you a few review cycles!
If an item doesn't apply to your pull request, check it anyway to make it apparent that there's nothing to do.
tox.docs/.docs/.versionadded,versionchanged, ordeprecateddirectives.docs/_newsfragments/, with the file name format{issue_number}.{fragment_type}.rst. (Runtox -e towncrier, and inspectdocs/_build/html/changes/in the browser to ensure it renders correctly.)If you have any questions to any of the points above, just submit and ask! This checklist is here to help you, not to deter you from contributing!
PR template inspired by the attrs project.