@semanticNonNull/@catch: default levels to [0] instead of null

[martinbonnin]

Because this is bumping the version to 0.2, it's a new file but you can see the diff in this commit

Not anymore because there have been additional changes, you'll have to read the full new text, sorry!

[captbaritone]

Unrelated to the main change in this PR, but...

I'm guessing you considered this already, but it would be possible to let regular GraphQL schema validation enforce this invariant for us by making a separate directive for the case where you specify a field, and then have that directive require a field be specified, and removing the argument from the field version.

As it stands we need ad-hoc validation that you do specify a field when used on a type, and that you don't specify a field when used on a field.

directive @semanticNonNull(level: Int = 0) repeatable on FIELD_DEFINITION
directive @semanticNonNullField(field: String!, level: Int = 0) repeatable on OBJECT

That said, I can see that having two directives is also cumbersome.

[martinbonnin]

Makes sense. The less custom validation I have to do, the happier I am. @semanticNonNullField(field: String!) feels like a mouthful of "fields" though. Other suggestions:

extend type User @semanticNonNullField(name: "friends", level: 0)
extend type User @addSemanticNonNull(forField: "friends", level: 0)

Also thinking about this, we could also remove the repeatable from the regular @semanticNonNull:

# pass all your levels all at once
directive @semanticNonNull(levels: [Int] = [0]) on FIELD_DEFINITION
# this ones needs to be repeatable for different fields. Which means we need validation that different directives for the same field do not conflict but 🤷 
directive @semanticNonNullField(name: String, levels: [Int] = [0]) repeatable on OBJECT | INTERFACE

[captbaritone]

Yeah, if we make levels plural, then we can remove the repeatable for the FIELD_DEFINITION variant. @addSemanticNonNull seem fine!

So the remaining validation we need to do:

1. The field/level(s) being referenced exist and are currently nullable
2. For @addSemanticNonNull, validate that the field exists
3. Validate type compatibility with interfaces

In theory you could end up with a type getting marked as semantically non null in multiple ways. Probably that's okay? As long as it does not confuse tools:

type User @addSemanticNonNull(forField: "name") {
  name: String @semanticNonNull
}

Or

type User {
  friends: [User] @semanticNonNull(levels: [1,1,1,])
}

[martinbonnin]

In theory you could end up with a type getting marked as semantically non null in multiple ways.

More validation to be done but I think it's ok.

[BoD]

Name nit: I find the word add to be a bit off for a directive because in a way all directives add something. semanticNonNullField works better for me but not a strong opinion.

[martinbonnin]

Agree directives are declarative and shouldn't start with a verb. I settled with @semanticNonNullField(name: String, levels: [Int]) in ba09b50, let me know what you think.

Also nitpick but it should really be @semanticNonNullPosition(field: String, levels: [Int]) but the spec uses field instead of position (or something else) in a lot of places so I kept Field for consistency even if it's a bit wrong.

[BoD]

Name nit: I find the word add to be a bit off for a directive because in a way all directives add something. semanticNonNullField works better for me but not a strong opinion.

[captbaritone]

One optional minor nit, but otherwise looks good!

[martinbonnin]

Something I realized by making @catch non repeatable, we lose the possibility to have different CatchTo for different levels:

{
  user {
    # this is not possible
    # throw if the list errors but recover from item errors
    friends @catch(to: THROW, levels: [0]) @catch(to: NULL, levels: [1])
  }
}

Maybe not too bad but maybe the levels in @catch should behave differently than in @semanticNonNull