Skip to content

override http.HTTPMessage methods to only use str for the header type #11114

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.

Sign up for GitHub

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

Merged
merged 5 commits into from
Dec 19, 2023
Merged

override http.HTTPMessage methods to only use str for the header type #11114

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
f10dd55
HTTPMessage str override
Avasam Dec 6, 2023
5ad636a
Update stdlib/http/client.pyi
Avasam Dec 7, 2023
cf667cf
Use Any trick on `HttpMessage.__getitem__`
Avasam Dec 7, 2023
df10900
Make __getitem__ return potentially None
Avasam Dec 19, 2023
8973977
new override type: ignore needed
Avasam Dec 19, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions stdlib/http/client.pyi
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,24 @@ responses: dict[int, str]

class HTTPMessage(email.message.Message):
def getallmatchingheaders(self, name: str) -> list[str]: ... # undocumented
# override below all of Message's methods that use `_HeaderType` / `_HeaderTypeParam` with `str`
# `HTTPMessage` breaks the Liskov substitution principle by only intending for `str` headers
# This is easier than making `Message` generic
def __getitem__(self, name: str) -> str | None: ...
def __setitem__(self, name: str, val: str) -> None: ... # type: ignore[override]
def values(self) -> list[str]: ...
def items(self) -> list[tuple[str, str]]: ...
@overload
def get(self, name: str, failobj: None = None) -> str | None: ...
@overload
def get(self, name: str, failobj: _T) -> str | _T: ...
@overload
def get_all(self, name: str, failobj: None = None) -> list[str] | None: ...
@overload
def get_all(self, name: str, failobj: _T) -> list[str] | _T: ...
Comment on lines +115 to +116
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally, we'd be able to have an overload like this, but I don't think it's possible:

@overload
def get_all(self, name: str, failobj: list[<empty>]) -> list[str]: ...

Copy link
Collaborator Author

@Avasam Avasam Dec 7, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure what such an overload would be trying to do. Consumer code is the one passing the failobj and immediately receiving the return type. get_all does nothing with failobj.
If it only supports a list[_HeaderType] it would be its responsibility to pass a value of the appropriate type to failobj, which a typechecker checks for.

def foo(message: HttpMessage) -> list[str] | SentinelType:
	return message.get_all("foo", sentinel_value)  # Ok
	
def foo(message: HttpMessage) -> list[str]:
	return message.get_all("foo", [])  # Fails pyright `reportUnknownArgumentType` but by no fault of the definition
	
def foo(message: HttpMessage) -> list[str]:
	return message.get_all("foo", list[str]())  # ok
	
def foo(message: HttpMessage) -> list[str]:
	return message.get_all("foo", sentinel_value)  # Fails type-checking

If we still really wanna restrict it, we can bind _T to list[_HeaderType]

def replace_header(self, _name: str, _value: str) -> None: ... # type: ignore[override]
def set_raw(self, name: str, value: str) -> None: ... # type: ignore[override]
def raw_items(self) -> Iterator[tuple[str, str]]: ...

def parse_headers(fp: io.BufferedIOBase, _class: Callable[[], email.message.Message] = ...) -> HTTPMessage: ...

Expand Down