Rework core traits

[tyranron]

Requires #1028, #1047, #1052
Required for #975
Related to #1091
Eliminates #1097

This PR represents an attempt to make a fundamental rework of Juniper resolving machinery (GraphQLType, GraphQLValue, etc).

Motivation

At the moment, the core traits like GraphQLType and GraphQLValue impose the following problems to codebase:

• Polution with trait bounds. Due to trait definitions providing default methods implementations, they are obligated to have additional bounds in their definitions, which inevitably polutes all the dependent code. This is also more compicated due to a single trait having multiple responsibilities, so in contexts where we require only some single responsibility, we still need to place all the bounds for other responsibilities too.
• No polymorphism for Context and TypeInfo types, as they're defined as associative types. So, once some leaf type in schema uses a different type for Context, we're fucked, and should either make the whole schema work with a single Context type, or doesn't compile at all. The same applies to TypeInfo, which, in result, makes impossible to combine static and dynamic GraphQL schemas at all.

Solution

This PR tries to remodel the core traits machinery in the following ways:

• Make traits definitions granular: a separate trait for a separate need. This allows every piece of the codebase to work exactly with the requirements it really needs.
• Lift bounds from traits definitions. Keeping bounds only on impl blocks prevents redudant bounds pollution of the code base.
• More clever bounds on impl blocks. Require only the minimum to make impl work. For the transparent behavior the bounds should bubble-up structurally (structure implements trait if all the required fields implement trait).
• Make TypeInfo and Context as generic type parameters for all possible implementations, and require Borrow<ConcreteContextType> for the implementation needs. Not a single implemetation should put a concrete type into these type parameters, otherwise the whole polymorphism will be broken (as we have now with associative types). This allows us to fully remove the context = arguments in macros, and to use different context types in different methods of a single type. As for type info, it allows to combine dynamic and static schemas naturally.
• Impose a lifetime in FromInputValue trait, like serde::Deserialize<'de> does. This allows to pass references as arguments (and so parse unsized types) to fields in the similar degree as supported by serde deserialization.
• Decouple the responsibility of omitting orphan rules from the ScalarValue type parameter into a separate type parameter. Remove ScalarValue type parameter in places where it's not really required.

Example

Meet a shiny new Juniper:

struct Human(u32);

#[graphql::object]
impl Human {
    fn id(&self) -> u32 {
        self.0
    }

   // Yes, we now have some degree of borrowing in arguments.
   fn has_name(name: &str) -> bool { 
       name == "John"
   }
   
   fn avatar(&self, context: &FileStorage) -> Option<String> {
       Some(context.lookup_avatar(self.0)?.to_base64_str())
   }

   fn profile(&self, context: &dyn UserRepository) -> Profile {
       context.get_profile(self.0).expect("profile should always exist")
   }
}

struct Context {
    repo: Box<dyn UserRepository>,
    storage: FileStorage,
}

impl Borrow<dyn UserRepository> for Context {
    fn borrow(&self) -> &dyn UserRepository {
        &*self.repo
    }
}

impl Borrow<FileStorage> for Context {
    fn borrow(&self) -> &FileStorage {
        &self.storage
    }
}

let context = Context::init();
juniper::execute(query, None, &schema, &graphql_vars! {}, &context).await;
// Code won't compile if the type, passed as context, doesn't implement all
// the `Borrow`s required by schema fields.
// For a single context type it works naturally due to
// existing `impl<T> Borrow<T> for T` blanket impl.

Further investigation

The solution for the following additional capabilities is not figured out yet and requires further investigation:

• Polymorphism over ScalarValue. At the moment the concrete ScalarValue type in a leaf type implementation polutes all the schema the same way the Context/TypeInfo does. But we cannot just Borrow<CustomScalar> from generic like we're doing for Context. Still it would be nice to figure out the way for combining types using different ScalarValues in their implementations.
• Separate type parameter for omitting orphan rules. At the moment we hack via using a custom ScalarValue type in implementations for this purpose. But it's not the responsibility of the ScalarValue notion, and using ScalarValue for that makes to put it into places where it's not required at all (like reflect::BaseType trait). Having a separate mechanic for omitting orphan rules would be much more intuitive. However, to work this polymorphically, we need to resolve the same implications imposed by previous question.

The main problem with polymorphism in both questions is that we cannot express forall quantifier for types in Rust:

trait Foo<T> {}
impl<A, B, T> Foo<A> for Box<T> where T: Foo<B> {}

// error[E0207]: the type parameter `B` is not constrained by the impl trait, self type, or predicates
//  --> src/lib.rs:2:9
//   |
// 2 | impl<A, B, T> Foo<A> for Box<T> where T: Foo<B> {}
//   |         ^ unconstrained type parameter

Solved: See #1072 (comment)

Progress

This is a huge work, as requires an accurate re-implementation of the whole Juniper resolving and type registration machinery.

• Pointer types
  • &T (ref)
  • &mut T (mut ref)
  • Box<T>
  • Rc<T>
  • Arc<T>
  • Cow<'_, T>
• Nullable types
  • Option<T>
  • Nullable<T>
• List types
  • Vec<T>
  • [T] (slice)
  • [T; N] (array)
  • HashSet (requires Implement HashSet support the same way as Vec #1070)
  • BTreeSet
• str
• #[graphql::Scalar] macro
• #[graphql::Enum] macro (requires Refactor #[derive(GraphQLEnum)] macro #1047)
• #[graphql::InputObject] macro (requires Refactor #[derive(GraphQLInputObject)] #1052)
• #[graphql::Object] macro
• #[graphql::Union] macro
• #[graphql::Interface] macro (requires Allow interfaces to implement other interfaces (#1000) #1028)
• #[graphql::Subscription] macro

[tyranron]

Regarding "Further investigation" section...

Polymorphism over ScalarValue.

Well, this is simple. Once we decouple the orphan rules omitting responsibility from ScalarValue type param, it worths nothing to act with the ScalarValue the same way as with Context/TypeInfo: keep it as generic everywhere and require Into<MyCustomScalarValue> in concrete impl blocks. This way the requirement will bubble-up naturally up to the place where root node is executeed, without messing up any intermediate types.

Separate type parameter for omitting orphan rules.

The main problem with polymorphism in both questions is that we cannot express forall quantifier for types in Rust:

trait Foo<T> {}
impl<A, B, T> Foo<A> for Box<T> where T: Foo<B> {}

// error[E0207]: the type parameter `B` is not constrained by the impl trait, self type, or predicates
//  --> src/lib.rs:2:9
//   |
// 2 | impl<A, B, T> Foo<A> for Box<T> where T: Foo<B> {}
//   |         ^ unconstrained type parameter

This one is trickier as hits something quite fundamental regarding the Rust type system:

1. To omit orphan rules we need to specify our type in a type parameter. Specifying it as an associated type doesn't allow to omit orphan rules, as doesn't give a dimension for parametrization. Associated type - is a concrete type, not a parameter.
2. To be able to write the impl above, we should have B as a associated type parameter, so it be a concrete type, not a "Rust, please, guess it out of nowhere" type parameter.

We cannot have 1 and 2 at the same time, as the requirements are clear contradiction.

To have them both, we need to introduce some transparent wrapper type holding our type parameter B:

struct Owned<X, By>(X, PhatomData<By>);

Now our B is constrained and we have the desired "polymorphism":

impl<A, B, T> Foo<A> for Owned<Box<T>, B> where T: Foo<B> {}

Hooray? Not yet! The problem has shifted to the place where we should or should not wrap the type. Unfortunately, we cannot do that seamlessly.

trait IntoOwned {
    type Owned;
}

struct Mine;
impl juniper::IntoOwned for u8 {  // oops! orphan rules again
    type Owned = juniper::Owned<Self, Mine>;
}

Without seamless conversion, we cannot use this implementation just like that:

#[derive(graphql::Object)]
struct Human {
    id: u8,
}

This shows very well our fundamental limitation of unconstrained type parameter, described above. Who should guess which implementation should be used here? Different crates may provide different impls for u8. Well, so we may conclude that the user code must specify the desired implementation to use.

Ok, then:

#[derive(graphql::Object)]
struct Human {
    id: Owned<u8, Mine>,
}

Hmm, not quite ergonomic. We don't want to mess with wrappers in the user code.

#[derive(graphql::Object)]
struct Human {
    #[graphql(owner = Mine)]
    id: u8,  // and we do wrapping under-the-hood in the macro expansion only
}

Naming still requires bikeshedding to be better, though.

Conclusions?

With this small ergonomic hit (in a form of additional attribute argument in places where foreign types and our types are connected) we solve the problem of "polymorphism" for a type parameter for omitting orphan rules. This way a custom type won't pollute all the schema types above it, just a field which refers it directly.

And now we're able to use ScalarValue only where it's really needed.

[tyranron]

@ilslv thoughts? ☝️

[ilslv]

@tyranron this sounds a lot like #[serde(with = "module")] attribute and looks like a big ergonomic improvement 👍

From what i can tell, there is no way we can exactly replicate #[serde(with = "module")] by providing mod with a couple of functions (maybe until we can use fn pointers inside const generics?), but I think we can require users also add a type alias for a local struct:

#[derive(graphql::Object)]
struct Human {
    #[graphql(with = "custom")]
    id: u8,  // and we do wrapping under-the-hood in the macro expansion only
}

mod custom {
    pub(super) enum Custom {}

    // functions ...
}

[tyranron]

Nota bene

At the moment we have a problem:

error[E0637]: `&` without an explicit lifetime name cannot be used here
   --> juniper/src/schema/schema.rs:140:37
    |
140 |     fn description(&self) -> Option<&str> {
    |                                     ^ explicit lifetime name needed here

Meaning that Option<&str>: ::juniper::resolve::Resolve bound at impl level is problematic.

Seems to be resolvable by quantifying all the type lifetimes into HTRB:
https://play.rust-lang.org/?version=stable&mode=debug&edition=2021&gist=1f8b36c78e61a771540b7a3324f9dd8b

[LegNeato]

Do GATs change any of the design thinking here?

[tyranron]

@LegNeato at the moment I don't see any need of them here, but it may appear eventually to solve some problems.

[elementary-particle]

Are we going to complete this rework eventually? This is a lot of work indeed, we should not leave them around.