Reject the use of both typename and bind (#172)

In #133, Craig added support for a new use of typename, where it applies
to a scalar and means that genqlient should generate a named type, e.g.
`# @genqlient(typename: "MyString")` on a node of type string will
generate and use `type MyString string`.  But this gets a bit confusing
if you mix it with `bind`; should
`typename: "MyString", bind: "int32"` generate `type MyString int32`, or
should one override the other, or what?  Of course in practice you're
not likely to write that all in one place, but you could via a global
binding, or a `for` directive, and in that case probably it was a
mistake.  In #138, we looked at making them work together correctly, but
it added complexity and got even more confusing.

So instead, here, we just ban it; we can always add it back if it proves
useful.  (Or, you can make the `typename` win over a global binding by
locally unbinding it via `bind: "-"`.)  This required changes in
surprisingly many places; I already knew the directive-validation code
was due for a refactor but that will happen some other day.  The tests
show that it works, in any case.

Interestingly, this problem actually could have arisen for a struct
binding already, before #133.  But all the same reasons it's confusing
seem to apply, so I just banned it there too.  This is technically a
breaking change although I doubt anyone will hit it.

Test plan: make check
This commit is contained in:
Ben Kraft
2022-02-09 16:54:31 -08:00
committed by GitHub
parent 8aa56aacf1
commit 67f2575cae
13 changed files with 81 additions and 0 deletions
+21
View File
@@ -213,6 +213,10 @@ func (dir *genqlientDirective) validate(node interface{}, schema *ast.Schema) er
if fieldDir.Omitempty != nil && field.Type.NonNull {
return errorf(fieldDir.pos, "omitempty may only be used on optional arguments")
}
if fieldDir.TypeName != "" && fieldDir.Bind != "" && fieldDir.Bind != "-" {
return errorf(fieldDir.pos, "typename and bind may not be used together")
}
}
}
@@ -255,6 +259,10 @@ func (dir *genqlientDirective) validate(node interface{}, schema *ast.Schema) er
return errorf(dir.pos, "for is only applicable to operations and arguments")
}
if dir.TypeName != "" && dir.Bind != "" && dir.Bind != "-" {
return errorf(dir.pos, "typename and bind may not be used together")
}
return nil
case *ast.Field:
if dir.Omitempty != nil {
@@ -278,6 +286,10 @@ func (dir *genqlientDirective) validate(node interface{}, schema *ast.Schema) er
return errorf(dir.pos, "for is only applicable to operations and arguments")
}
if dir.TypeName != "" && dir.Bind != "" && dir.Bind != "-" {
return errorf(dir.pos, "typename and bind may not be used together")
}
return nil
default:
return errorf(dir.pos, "invalid @genqlient directive location: %T", node)
@@ -490,6 +502,15 @@ func (g *generator) parsePrecedingComment(
if queryOptions != nil {
// If we are part of an operation/fragment, merge its options in.
directive.mergeOperationDirective(node, parentIfInputField, queryOptions)
// TODO(benkraft): Really we should do all the validation after
// merging, probably? But this is the only check that can fail only
// after merging, and it's a bit tricky because the "does not apply"
// checks may need to happen before merging so we know where the
// directive "is".
if directive.TypeName != "" && directive.Bind != "" && directive.Bind != "-" {
return "", nil, errorf(directive.pos, "typename and bind may not be used together")
}
}
reverse(commentLines)