Make uri and method attributes on HTTPServerRequest mandatory
#3542
+5
−3
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.
This adds assertion checks during initialisation of
HTTPServerRequestobject, to ensure the corresponding attributes of the object being created, won't end up beingNone. This allows the type checker to assume the value of the attribute isstrand neverNone, which I argue aids typed Tornado applications. To be clear, it is still permitted to provideNoneor omit specification for bothuriandmethodparameters to the constructor if thestart_lineparameter specifies [good] values instead.This is also motivated by the idea that per HTTP, requests always feature a method and a URI, and so the
HTTPServerRequestmodelling wasn't optimal allowingNonefor either. As for typed Tornado applications using e.g.self.request.urias part of request handling -- i.e. in code withselfbeing aRequestHandler-- these can now safely assume e.g.uriormethodarestrand notstr | None/Optional[str]which would require ad-hoc assertions alaassert self.request.uri is not None(orassert self.request.uri) in application code. That would IMO be a case of "surprising the user" -- as most everyone would expect an HTTP request to have its URI and method be "mandatory" attributes -- not allowingNonefor value.Again, because semantics of
start_lineare preserved, the initialisation of the object may omit parametersuriand/ormethodifstart_linespecifies valid values for these instead. In any case, it is the attributes of the object being constructed, that end up being effectively validated withassert-- which make the type checker (tested with MyPy 1.18.2 here) assumestrinstead ofstr | None.It's also important that this doesn't break existing Tornado applications, since the type of both attributes is effectively narrowed down, meaning that applications that would otherwise assume
str | Nonecan continue to do so safely -- "may beNone" is compatible with "won't ever beNone", obviously.