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

Add tests to check that genqlient handles covariance #203

Merged
merged 1 commit into from
Jun 4, 2022

Commits on Jun 2, 2022

  1. Add tests to check that genqlient handles covariance

    I didn't realize until today that implementations of GraphQL interfaces
    are actually allowed to be covariant: if the interface has a field
    `f: T`, then the implementations may have fields `f: U` where `U` is a
    subtype of `T` (for example `U` may be an implementation of the
    interface `T`, or `U` may be `T!` if `T` is non-nullable. (I thought it
    had to be `f: T` exactly.) So I figured I'd add a test and see what
    breaks.
    
    Surprisingly, and despite the fact that Go interfaces do *not* allow
    covariance, everything... worked? There's at least one place where it's
    possible we could ideally use a more specific type [1], but for now I
    just wanted to make sure we at least write something that builds and is
    vaguely reasonable. Of course I'm not sure if there's anything I've
    missed (some day I need to find a fuzzing engine that can fuzz GraphQL).
    
    [1] Specifically, the field
    `CovariantInterfaceImplementationRandomItemTopic.Next` might ideally
    have type `...NextContentTopic`, not `...NextContent`; we know it's a
    topic. This doesn't directly cause covariance problems in Go: the method
    `GetNext` still returns `...NextContent` so the interface matches. But
    that trick doesn't work for the sibling field `.Related` which is
    slice-typed: or rather, we'd need the method to copy the slice to the
    correct type. (Not to mention the implemention of any change here would
    require a bunch of plumbing because the AST doesn't quite have what we
    want.) So it's probably best to just keep this as-is for simplicity and
    consistency.
    
    Test plan: make check
    benjaminjkraft committed Jun 2, 2022
    Configuration menu
    Copy the full SHA
    0e48e3c View commit details
    Browse the repository at this point in the history