Skip to content

feat(interface): input type cannot inherit an interface type #1254

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

Open
wants to merge 2 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
10 changes: 10 additions & 0 deletions strawberry/exceptions.py
Original file line number Diff line number Diff line change
Expand Up @@ -176,3 +176,13 @@ def __init__(self, field_name: str, argument_name: str, argument_type: str):
message = f'Argument "{argument_name}" on field "{field_name}" cannot be of type\
"{argument_type}"'
super().__init__(message)


class InvalidSuperclassInterface(Exception):
def __init__(self, input_name: str, interface_names=List[str]):
Copy link
Member

@BryceBeagle BryceBeagle Sep 17, 2021

Choose a reason for hiding this comment

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

Actually, now that I think about it, wouldn't it be better to take a type and List[type] instead? Let the Exception handle everything related to output?

Copy link
Member Author

Choose a reason for hiding this comment

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

You are right

interface_names = ", ".join(interface_names)
message = (
f"An Input class {input_name!r} cannot inherit "
f"from an Interface(s) {interface_names!r}"
)
super().__init__(message)
13 changes: 12 additions & 1 deletion strawberry/object_type.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
import dataclasses
from typing import List, Optional, Type, cast

from .exceptions import MissingFieldAnnotationError, MissingReturnAnnotationError
from .exceptions import (
InvalidSuperclassInterface,
MissingFieldAnnotationError,
MissingReturnAnnotationError,
)
from .field import StrawberryField, field
from .types.type_resolver import _get_fields
from .types.types import FederationTypeParams, TypeDefinition
Expand Down Expand Up @@ -99,6 +103,13 @@ def _process_type(
interfaces = _get_interfaces(cls)
fields = _get_fields(cls)

if is_input and interfaces:
interface_names = [interface.name for interface in interfaces]

raise InvalidSuperclassInterface(
input_name=name, interface_names=interface_names
)

cls._type_definition = TypeDefinition(
name=name,
is_input=is_input,
Expand Down
25 changes: 25 additions & 0 deletions tests/schema/test_interface.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import pytest

import strawberry
from strawberry.exceptions import InvalidSuperclassInterface


def test_query_interface():
Expand Down Expand Up @@ -161,3 +162,27 @@ def anime(self) -> Anime:

assert not result.errors
assert result.data == {"anime": {"name": "One Piece"}}


def test_input_cannot_inherit_from_interface():
Copy link
Member

Choose a reason for hiding this comment

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

Can these tests cover the actual messages of the errors as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do yo know how to catch the exception message with pytest? Or I need to try, catch, save, raise, assert?

Copy link
Member

Choose a reason for hiding this comment

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

You can use

with pytest.raises(ExceptionType) as exc:
    ...
assert exc.value == "something"

Copy link
Member

Choose a reason for hiding this comment

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

Also, note that pytest.raises takes a match argument that may be enough here

Copy link
Member Author

Choose a reason for hiding this comment

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

Let me have a look, thanks

@strawberry.interface
class SomeInterface:
some_arg: str

with pytest.raises(InvalidSuperclassInterface):

@strawberry.input
class SomeInput(SomeInterface): # this should throw an error
another_arg: str

@strawberry.interface
class SomeOtherInterface:
some_other_arg: str

with pytest.raises(InvalidSuperclassInterface):

@strawberry.input
class SomeOtherInput(
SomeInterface, SomeOtherInterface
): # this should throw an error
another_arg: str