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

Enable reporting unknown types #333

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions changes/333.internal.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Enable reporting unknown types for basedpyright.
12 changes: 6 additions & 6 deletions docs/conf.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
from pathlib import Path

from packaging.version import parse as parse_version
from typing_extensions import override
from typing_extensions import Any, override

if sys.version_info >= (3, 11):
from tomllib import load as toml_parse
Expand Down Expand Up @@ -110,7 +110,7 @@
autodoc_member_order = "bysource"

# Default options for all autodoc directives
autodoc_default_options = {
autodoc_default_options: dict[str, Any] = {
Copy link
Contributor

Choose a reason for hiding this comment

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

I don't see how these kind of type annotations are better

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, the thing is, pyright would otherwise mark this as dict[str, Unknown] and with Unknown being banned, if a wildcard type is indeed what we want, it must be declared explicitly like this. That's also one of the reasons why these rules might be a bit too strict.

"members": True,
"undoc-members": True,
"show-inheritance": True,
Expand Down Expand Up @@ -198,10 +198,10 @@ def override_towncrier_draft_format() -> None:
from docutils import statemachine
from sphinx.util.nodes import nodes

orig_f = sphinxcontrib.towncrier.ext._nodes_from_document_markup_source # pyright: ignore[reportPrivateUsage]
orig_f = sphinxcontrib.towncrier.ext._nodes_from_document_markup_source # pyright: ignore[reportPrivateUsage,reportUnknownMemberType,reportUnknownVariableType]

def override_f(
state: statemachine.State, # pyright: ignore[reportMissingTypeArgument] # arg not specified in orig_f either
state: statemachine.State, # pyright: ignore[reportMissingTypeArgument,reportUnknownParameterType] # arg not specified in orig_f either
markup_source: str,
) -> list[nodes.Node]:
markup_source = markup_source.replace("## Version Unreleased changes", "## Unreleased changes")
Expand All @@ -212,9 +212,9 @@ def override_f(
markup_source = markup_source[:-3]

markup_source = markup_source.rstrip(" \n")
markup_source = m2r2.M2R()(markup_source)
markup_source = m2r2.M2R()(markup_source) # pyright: ignore[reportUnknownVariableType]

return orig_f(state, markup_source)
return orig_f(state, markup_source) # pyright: ignore[reportUnknownArgumentType]

sphinxcontrib.towncrier.ext._nodes_from_document_markup_source = override_f # pyright: ignore[reportPrivateUsage]

Expand Down
18 changes: 9 additions & 9 deletions docs/extensions/attributetable.py
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ def visit_attributetabletitle_node(self: HTML5Translator, node: AttributeTableTi


def visit_attributetablebadge_node(self: HTML5Translator, node: AttributeTableBadge) -> None:
attributes = {
attributes: dict[str, Any] = {
"class": "py-attribute-table-badge",
"title": node["badge-type"],
}
Expand Down Expand Up @@ -153,7 +153,7 @@ def run(self) -> list[AttributeTablePlaceholder]:
def build_lookup_table(env: BuildEnvironment) -> dict[str, list[str]]:
# Given an environment, load up a lookup table of
# full-class-name: objects
result = {}
result: dict[str, list[str]] = {}
Copy link
Contributor

Choose a reason for hiding this comment

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

But for these ones it's interesting to have type checking

Copy link
Member Author

Choose a reason for hiding this comment

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

I agree, in some cases, being forced to explicitly declare the annotations can be very helpful to the type checker, it is nice that this would now be a requirement, where types can't be easily inferred.

domain = env.domains["py"]

ignored = {
Expand Down Expand Up @@ -285,11 +285,11 @@ def class_results_to_node(key: str, elements: Sequence[TableElement]) -> Attribu

def setup(app: Sphinx) -> dict[str, Any]:
app.add_directive("attributetable", PyAttributeTable)
app.add_node(AttributeTable, html=(visit_attributetable_node, depart_attributetable_node))
app.add_node(AttributeTableColumn, html=(visit_attributetablecolumn_node, depart_attributetablecolumn_node))
app.add_node(AttributeTableTitle, html=(visit_attributetabletitle_node, depart_attributetabletitle_node))
app.add_node(AttributeTableBadge, html=(visit_attributetablebadge_node, depart_attributetablebadge_node))
app.add_node(AttributeTableItem, html=(visit_attributetable_item_node, depart_attributetable_item_node))
app.add_node(AttributeTablePlaceholder)
_ = app.connect("doctree-resolved", process_attributetable)
app.add_node(AttributeTable, html=(visit_attributetable_node, depart_attributetable_node)) # pyright: ignore[reportUnknownMemberType]
app.add_node(AttributeTableColumn, html=(visit_attributetablecolumn_node, depart_attributetablecolumn_node)) # pyright: ignore[reportUnknownMemberType]
app.add_node(AttributeTableTitle, html=(visit_attributetabletitle_node, depart_attributetabletitle_node)) # pyright: ignore[reportUnknownMemberType]
app.add_node(AttributeTableBadge, html=(visit_attributetablebadge_node, depart_attributetablebadge_node)) # pyright: ignore[reportUnknownMemberType]
app.add_node(AttributeTableItem, html=(visit_attributetable_item_node, depart_attributetable_item_node)) # pyright: ignore[reportUnknownMemberType]
app.add_node(AttributeTablePlaceholder) # pyright: ignore[reportUnknownMemberType]
_ = app.connect("doctree-resolved", process_attributetable) # pyright: ignore[reportUnknownMemberType]
return {"parallel_read_safe": True}
4 changes: 4 additions & 0 deletions mcproto/auth/account.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,10 @@ class Account:

__slots__ = ("access_token", "username", "uuid")

username: str
uuid: McUUID
access_token: str

def __init__(self, username: str, uuid: McUUID, access_token: str) -> None:
self.username = username
self.uuid = uuid
Expand Down
6 changes: 3 additions & 3 deletions mcproto/auth/microsoft/xbox.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
from typing import NamedTuple

import httpx
from typing_extensions import override
from typing_extensions import Any, override

__all__ = [
"XSTSErrorType",
Expand Down Expand Up @@ -67,7 +67,7 @@ def __init__(self, exc: httpx.HTTPStatusError):
@property
def msg(self) -> str:
"""Produce a message for this error."""
msg_parts = []
msg_parts: list[str] = []
Copy link
Contributor

Choose a reason for hiding this comment

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

I already have things like that all over the place in more-packets

if self.err_type is not XSTSErrorType.UNKNOWN:
msg_parts.append(f"{self.err_type.name}: {self.err_type.value!r}")
else:
Expand Down Expand Up @@ -96,7 +96,7 @@ async def xbox_auth(client: httpx.AsyncClient, microsoft_access_token: str, bedr
See :func:`~mcproto.auth.microsoft.oauth.full_microsoft_oauth` for info on ``microsoft_access_token``.
"""
# Obtain XBL token
payload = {
payload: dict[str, Any] = {
"Properties": {
"AuthMethod": "RPS",
"SiteName": "user.auth.xboxlive.com",
Expand Down
2 changes: 1 addition & 1 deletion mcproto/auth/msa.py
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def __init__(self, exc: httpx.HTTPStatusError):
@property
def msg(self) -> str:
"""Produce a message for this error."""
msg_parts = []
msg_parts: list[str] = []
msg_parts.append(f"HTTP {self.code} from {self.url}:")
msg_parts.append(f"type={self.err_type.name!r}")

Expand Down
8 changes: 4 additions & 4 deletions mcproto/auth/yggdrasil.py
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
from uuid import uuid4

import httpx
from typing_extensions import Self, override
from typing_extensions import Any, Self, override

from mcproto.auth.account import Account
from mcproto.types.uuid import UUID as McUUID # noqa: N811
Expand Down Expand Up @@ -89,7 +89,7 @@ def __init__(self, exc: httpx.HTTPStatusError):
@property
def msg(self) -> str:
"""Produce a message for this error."""
msg_parts = []
msg_parts: list[str] = []
msg_parts.append(f"HTTP {self.code} from {self.url}:")
msg_parts.append(f"type={self.err_type.name!r}")

Expand Down Expand Up @@ -126,7 +126,7 @@ async def refresh(self, client: httpx.AsyncClient) -> None:
having to go through a complete re-login. This can happen after some time period, or
for example when someone else logs in to this minecraft account elsewhere.
"""
payload = {
payload: dict[str, Any] = {
"accessToken": self.access_token,
"clientToken": self.client_token,
"selectedProfile": {"id": str(self.uuid), "name": self.username},
Expand Down Expand Up @@ -196,7 +196,7 @@ async def authenticate(cls, client: httpx.AsyncClient, login: str, password: str
# Any random string, we use a random v4 uuid, needs to remain same in further communications
client_token = str(uuid4())

payload = {
payload: dict[str, Any] = {
"agent": {
"name": "Minecraft",
"version": 1,
Expand Down
2 changes: 1 addition & 1 deletion mcproto/buffer.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ class Buffer(BaseSyncWriter, BaseSyncReader, bytearray):

__slots__ = ("pos",)

def __init__(self, *args, **kwargs):
def __init__(self, *args: object, **kwargs: object):
super().__init__(*args, **kwargs)
self.pos = 0

Expand Down
6 changes: 3 additions & 3 deletions mcproto/connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ def __enter__(self) -> Self:
raise IOError("Connection already closed.")
return self

def __exit__(self, *a, **kw) -> None:
def __exit__(self, *a: object, **kw: object) -> None:
self.close()


Expand Down Expand Up @@ -260,7 +260,7 @@ async def __aenter__(self) -> Self:
raise IOError("Connection already closed.")
return self

async def __aexit__(self, *a, **kw) -> None:
async def __aexit__(self, *a: object, **kw: object) -> None:
await self.close()


Expand Down Expand Up @@ -367,7 +367,7 @@ def socket(self) -> socket.socket:
"""Obtain the underlying socket behind the :class:`~asyncio.Transport`."""
# TODO: This should also have pyright: ignore[reportPrivateUsage]
# See: https://github.com/DetachHead/basedpyright/issues/494
return self.writer.transport._sock # pyright: ignore[reportAttributeAccessIssue]
return self.writer.transport._sock # pyright: ignore[reportAttributeAccessIssue,reportUnknownMemberType,reportUnknownVariableType]


class UDPSyncConnection(SyncConnection, Generic[T_SOCK]):
Expand Down
2 changes: 1 addition & 1 deletion mcproto/multiplayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ def __init__(self, exc: httpx.HTTPStatusError):
@property
def msg(self) -> str:
"""Produce a message for this error."""
msg_parts = []
msg_parts: list[str] = []
msg_parts.append(f"HTTP {self.code} from {self.url}:")
msg_parts.append(f"type={self.err_type.name!r}")

Expand Down
2 changes: 1 addition & 1 deletion mcproto/packets/packet.py
Original file line number Diff line number Diff line change
Expand Up @@ -128,7 +128,7 @@ def from_packet_class(cls, packet_class: type[Packet], buffer: Buffer, message:
@property
def msg(self) -> str:
"""Produce a message for this error."""
msg_parts = []
msg_parts: list[str] = []

if self.direction is PacketDirection.CLIENTBOUND:
msg_parts.append("Clientbound")
Expand Down
13 changes: 13 additions & 0 deletions mcproto/types/nbt.py
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,13 @@ def from_object(data: FromObjectType, schema: FromObjectSchema, name: str = "")
:param name: The name of the NBT tag.
:return: The NBT tag created from the python object.
"""
# TODO: There are a lot of isinstance checks for dict/list, however, the FromObjectType/FromObjectSchema
# type alias declares a Sequence/Mapping type for these collections. The isinstance checks here should
# probably be replaced with these types (they should support runtime comparison).

# TODO: Consider splitting this function up into smaller functions, that each parse a specific type of schema
# i.e. _from_object_dict, _from_object_list, _from_object_tag, ...

# Case 0 : schema is an object with a `to_nbt` method (could be a subclass of NBTag for all we know, as long
# as the data is an instance of the schema it will work)
if isinstance(schema, type) and hasattr(schema, "to_nbt") and isinstance(data, schema):
Expand All @@ -339,23 +346,28 @@ def from_object(data: FromObjectType, schema: FromObjectSchema, name: str = "")
if isinstance(schema, type) and issubclass(schema, NBTag):
if schema in (CompoundNBT, ListNBT):
raise ValueError("Use a list or a dictionary in the schema to create a CompoundNBT or a ListNBT.")

# Check if the data contains the name (if it is a dictionary)
if isinstance(data, dict):
if len(data) != 1:
raise ValueError("Expected a dictionary with a single key-value pair.")
# We also check if the name isn't already set
if name:
raise ValueError("The name is already set.")

key, value = next(iter(data.items()))
# Recursive call to go to the next part
return NBTag.from_object(value, schema, name=key)

# Else we check if the data can be a payload for the tag
if not isinstance(data, (bytes, str, int, float, list)):
raise TypeError(f"Expected one of (bytes, str, int, float, list), but found {type(data).__name__}.")

# Check if the data is a list of integers
if isinstance(data, list) and not all(isinstance(item, int) for item in data):
raise TypeError("Expected a list of integers, but a non-integer element was found.")
data = cast(Union[bytes, str, int, float, "list[int]"], data)

# Create the tag with the data and the name
return schema(data, name=name) # pyright: ignore[reportCallIssue] # The schema is a subclass of NBTag

Expand Down Expand Up @@ -385,6 +397,7 @@ def from_object(data: FromObjectType, schema: FromObjectSchema, name: str = "")
# as there are only dicts, or only lists in the schema
if not isinstance(data, list):
raise TypeError(f"Expected a list, but found {type(data).__name__}.")

if len(schema) == 1:
# We have two cases here, either the schema supports an unknown number of elements of a single type ...
children_schema = schema[0]
Expand Down
7 changes: 0 additions & 7 deletions pyproject.toml
Original file line number Diff line number Diff line change
Expand Up @@ -103,11 +103,6 @@ reportImplicitStringConcatenation = false
reportUnreachable = "hint"
reportUnusedParameter = "hint"
reportUnannotatedClassAttribute = false
reportUnknownArgumentType = false # consider enabling
reportUnknownVariableType = false # consider enabling
reportUnknownMemberType = false # consider enabling
reportUnknownParameterType = false # consider enabling
reportUnknownLambdaType = false # consider enabling
reportMissingTypeStubs = "information" # consider bumping to warning/error
reportUninitializedInstanceVariable = false # until https://github.com/DetachHead/basedpyright/issues/491
reportMissingParameterType = false # ruff's flake8-annotations (ANN) already covers this + gives us more control
Expand Down Expand Up @@ -149,8 +144,6 @@ ignore = [
"D416", # Section name should end with a colon
"D417", # Missing argument descrition in the docstring

"ANN002", # Missing type annotation for *args
"ANN003", # Missing type annotation for **kwargs
"ANN204", # Missing return type annotation for special method
"ANN401", # Dynamically typed expressions (typing.Any) disallowed

Expand Down
2 changes: 1 addition & 1 deletion tests/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ class be of/return the same class.
_mock_sealed: bool
_extract_mock_name: Callable[[], str]

def _get_child_mock(self, **kwargs) -> T_Mock:
def _get_child_mock(self, **kwargs: object) -> T_Mock:
"""Make :attr:`.child_mock_type`` instances instead of instances of the same class.

By default, this method creates a new mock instance of the same original class, and passes
Expand Down
4 changes: 2 additions & 2 deletions tests/mcproto/protocol/helpers.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
class WriteFunctionMock(Mock):
"""Mock write function, storing the written data."""

def __init__(self, *a, **kw):
def __init__(self, *a: object, **kw: object):
super().__init__(*a, **kw)
self.combined_data = bytearray()

Expand Down Expand Up @@ -40,7 +40,7 @@ class WriteFunctionAsyncMock(WriteFunctionMock, AsyncMock): # pyright: ignore[r
class ReadFunctionMock(Mock):
"""Mock read function, giving pre-defined data."""

def __init__(self, *a, combined_data: bytearray | None = None, **kw):
def __init__(self, *a: object, combined_data: bytearray | None = None, **kw: object):
super().__init__(*a, **kw)
if combined_data is None:
combined_data = bytearray()
Expand Down
6 changes: 3 additions & 3 deletions tests/mcproto/test_connection.py
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ class MockSocket(CustomMockMixin[MagicMock], MagicMock): # pyright: ignore[repo

spec_set = socket.socket

def __init__(self, *args, read_data: bytearray | None = None, **kwargs) -> None:
def __init__(self, *args: object, read_data: bytearray | None = None, **kwargs: object) -> None:
super().__init__(*args, **kwargs)
self.mock_add_spec(["_recv", "_send", "_closed"])
self._recv = ReadFunctionMock(combined_data=read_data)
Expand Down Expand Up @@ -60,7 +60,7 @@ class MockStreamWriter(CustomMockMixin[MagicMock], MagicMock): # pyright: ignor

spec_set = asyncio.StreamWriter

def __init__(self, *args, **kwargs):
def __init__(self, *args: object, **kwargs: object):
super().__init__(*args, **kwargs)
self.mock_add_spec(["_white", "_closed"])
self._write = WriteFunctionMock()
Expand All @@ -86,7 +86,7 @@ class MockStreamReader(CustomMockMixin[MagicMock], MagicMock): # pyright: ignor

spec_set = asyncio.StreamReader

def __init__(self, *args, read_data: bytearray | None = None, **kwargs) -> None:
def __init__(self, *args: object, read_data: bytearray | None = None, **kwargs: object) -> None:
super().__init__(*args, **kwargs)
self.mock_add_spec(["_read"])
self._read = ReadFunctionAsyncMock(combined_data=read_data)
Expand Down
2 changes: 1 addition & 1 deletion tests/mcproto/test_multiplayer.py
Original file line number Diff line number Diff line change
Expand Up @@ -89,7 +89,7 @@ async def test_join_request_invalid(
("172.17.0.1"),
],
)
async def test_join_check_valid(client_ip, httpx_mock: HTTPXMock):
async def test_join_check_valid(client_ip: str | None, httpx_mock: HTTPXMock):
"""Test making a join check, getting back a valid response."""
client_username = "ItsDrike"
server_hash = "-745fc7fdb2d6ae7c4b20e2987770def8f3dd1105"
Expand Down
33 changes: 19 additions & 14 deletions tests/mcproto/types/test_chat.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
from __future__ import annotations

from typing import cast

import pytest

from mcproto.types.chat import ChatMessage, RawChatMessage, RawChatMessageDict
Expand All @@ -8,20 +10,23 @@

@pytest.mark.parametrize(
("raw", "expected_dict"),
[
(
{"text": "A Minecraft Server"},
{"text": "A Minecraft Server"},
),
(
"A Minecraft Server",
{"text": "A Minecraft Server"},
),
(
[{"text": "hello", "bold": True}, {"text": "there"}],
{"extra": [{"text": "hello", "bold": True}, {"text": "there"}]},
),
],
cast(
"list[tuple[RawChatMessage, RawChatMessageDict]]",
[
(
{"text": "A Minecraft Server"},
{"text": "A Minecraft Server"},
),
(
"A Minecraft Server",
{"text": "A Minecraft Server"},
),
(
[{"text": "hello", "bold": True}, {"text": "there"}],
{"extra": [{"text": "hello", "bold": True}, {"text": "there"}]},
),
],
),
)
def test_as_dict(raw: RawChatMessage, expected_dict: RawChatMessageDict):
"""Test converting raw ChatMessage input into dict produces expected dict."""
Expand Down
Loading
Loading