-
-
Notifications
You must be signed in to change notification settings - Fork 577
Fix untyped decorator #3867
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
base: main
Are you sure you want to change the base?
Fix untyped decorator #3867
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
release type: patch | ||
|
||
This release fixes a long standing typing issue where mypy would | ||
return the following error: | ||
|
||
```text | ||
main:14: error: Untyped decorator makes function "e" untyped [misc] (diff) | ||
``` | ||
|
||
When using the following code: | ||
|
||
```python | ||
import strawberry | ||
|
||
|
||
@strawberry.type | ||
class Query: | ||
@strawberry.field(description="Get the last user") | ||
def last_user_v2(self) -> str: | ||
return "Hello" | ||
``` |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -475,7 +475,7 @@ def field( | |
directives: Optional[Sequence[object]] = (), | ||
extensions: Optional[list[FieldExtension]] = None, | ||
graphql_type: Optional[Any] = None, | ||
) -> Any: ... | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. thought: Almost sure the reason we have
Without Not sure if there's a way around that, though :( There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Random showerthought: I feel like there should be a way, given that dataclasses use a similar notation and get away with it: https://docs.python.org/3/library/dataclasses.html#dataclasses.field There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yeah, I was taking a look at this, https://peps.python.org/pep-0681/ and it seems that they also use Any 🤷♂️ There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I would say we can close this PR =P There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @bellini666 the issue is still there though 🤔 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
The original issue mentioned mypy (only a later comment mentions pyright). It looks like it reproduces in the mypy playground: https://mypy-play.net/?mypy=latest&python=3.12&flags=strict&gist=ced03d8cb75457ab2ed0d89c2d82b23d There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @DoctorJohn can you open an issue on mypy's side? :D |
||
) -> StrawberryField: ... | ||
|
||
|
||
@overload | ||
|
@@ -515,23 +515,23 @@ def field( | |
|
||
|
||
def field( | ||
resolver: Optional[_RESOLVER_TYPE[Any]] = None, | ||
resolver=None, | ||
*, | ||
name: Optional[str] = None, | ||
is_subscription: bool = False, | ||
description: Optional[str] = None, | ||
permission_classes: Optional[list[type[BasePermission]]] = None, | ||
deprecation_reason: Optional[str] = None, | ||
default: Any = dataclasses.MISSING, | ||
default_factory: Union[Callable[..., object], object] = dataclasses.MISSING, | ||
metadata: Optional[Mapping[Any, Any]] = None, | ||
directives: Optional[Sequence[object]] = (), | ||
extensions: Optional[list[FieldExtension]] = None, | ||
graphql_type: Optional[Any] = None, | ||
name=None, | ||
is_subscription=False, | ||
description=None, | ||
permission_classes=None, | ||
deprecation_reason=None, | ||
default=dataclasses.MISSING, | ||
default_factory=dataclasses.MISSING, | ||
metadata=None, | ||
directives=(), | ||
extensions=None, | ||
graphql_type=None, | ||
# This init parameter is used by PyRight to determine whether this field | ||
# is added in the constructor or not. It is not used to change | ||
# any behavior at the moment. | ||
init: Literal[True, False, None] = None, | ||
init=None, | ||
) -> Any: | ||
"""Annotates a method or property as a GraphQL field. | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
from inline_snapshot import snapshot | ||
|
||
from .utils.marks import requires_mypy, requires_pyright, skip_on_windows | ||
from .utils.typecheck import typecheck | ||
|
||
pytestmark = [skip_on_windows, requires_pyright, requires_mypy] | ||
|
||
|
||
CODE = """ | ||
import strawberry | ||
|
||
|
||
@strawberry.type | ||
class Query: | ||
@strawberry.field(description="Get the last user") | ||
def last_user_v2(self) -> str: | ||
return "Hello" | ||
|
||
@strawberry.federation.type | ||
class FederatedQuery: | ||
@strawberry.federation.field(description="Get the last user") | ||
def last_user_v2(self) -> str: | ||
return "Hello" | ||
""" | ||
|
||
|
||
def test(): | ||
results = typecheck(CODE) | ||
|
||
assert results.pyright == snapshot([]) | ||
assert results.mypy == snapshot([]) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
style: Consider adding a more descriptive title for this release, e.g. 'Fix mypy typing issues with @strawberry.field decorator arguments'