Reject operation or argument names that are Go keywords (#195)
GraphQL is pretty restrictive about its identifiers, so for the most part we can and do safely use GraphQL identifiers in the Go we generate with attention only to conflicts with other such identifiers. But we do need to check one thing, which is that the identifier isn't a Go keyword. (If it is, the generated code will almost certainly fail to compile, but often with a confusing error message.) In this commit I add such checks. The most likely place to run into trouble here is argument names, which are often one word and are used as-is. Operation names, if unexported, can also be keywords, although in practice they're usually multiword. Field names are always exported, thus safe. Generated type names are always prefixed, camel-cased, with the operation name, so they always contain an uppercase letter (even if the operation name is lowercase), but type-names specified by `typename` may collide, so we check those. In theory we could check type-names specified by `bind`, but these must be defined by the user, so their code will already fail to compile, so I didn't bother. I think that's all the places to consider, although it's hard to be sure. In summary, we check argument names, operation names, and user-specified type names. Test plan: make check
This commit is contained in:
@@ -34,6 +34,8 @@ When releasing a new version:
|
||||
|
||||
### Bug fixes:
|
||||
|
||||
- genqlient now explicitly rejects argument, operation, and type names which are Go keywords, rather than failing with an opaque error.
|
||||
|
||||
## v0.4.0
|
||||
|
||||
Version 0.4.0 adds several new configuration options, as well as additional methods to simplify the use of interfaces.
|
||||
|
||||
@@ -164,6 +164,10 @@ func (g *generator) convertArguments(
|
||||
name := "__" + operation.Name + "Input"
|
||||
fields := make([]*goStructField, len(operation.VariableDefinitions))
|
||||
for i, arg := range operation.VariableDefinitions {
|
||||
if goKeywords[arg.Variable] {
|
||||
return nil, errorf(arg.Position, "variable name must not be a go keyword")
|
||||
}
|
||||
|
||||
_, options, err := g.parsePrecedingComment(arg, nil, arg.Position, queryOptions)
|
||||
if err != nil {
|
||||
return nil, err
|
||||
@@ -317,6 +321,9 @@ func (g *generator) convertDefinition(
|
||||
// Determine the name to use for this type.
|
||||
var name string
|
||||
if options.TypeName != "" {
|
||||
if goKeywords[options.TypeName] {
|
||||
return nil, errorf(pos, "typename option must not be a go keyword")
|
||||
}
|
||||
// If the user specified a name, use it!
|
||||
name = options.TypeName
|
||||
if namePrefix != nil && namePrefix.head == name && namePrefix.tail == nil {
|
||||
|
||||
@@ -232,6 +232,8 @@ func (g *generator) preprocessQueryDocument(doc *ast.QueryDocument) {
|
||||
func (g *generator) addOperation(op *ast.OperationDefinition) error {
|
||||
if op.Name == "" {
|
||||
return errorf(op.Position, "operations must have operation-names")
|
||||
} else if goKeywords[op.Name] {
|
||||
return errorf(op.Position, "operation name must not be a go keyword")
|
||||
}
|
||||
|
||||
queryDoc := &ast.QueryDocument{
|
||||
|
||||
@@ -0,0 +1,3 @@
|
||||
query KeywordArgumentName($type: String) {
|
||||
f(type: $type)
|
||||
}
|
||||
@@ -0,0 +1 @@
|
||||
type Query { f(type: String): String }
|
||||
@@ -0,0 +1 @@
|
||||
query type { f }
|
||||
@@ -0,0 +1 @@
|
||||
type Query { f: String }
|
||||
@@ -0,0 +1,6 @@
|
||||
query KeywordTypeName {
|
||||
# @genqlient(typename: "type")
|
||||
f {
|
||||
g
|
||||
}
|
||||
}
|
||||
@@ -0,0 +1,2 @@
|
||||
type Query { f: T }
|
||||
type T { g: String }
|
||||
@@ -0,0 +1 @@
|
||||
testdata/errors/KeywordArgumentName.graphql:1: variable name must not be a go keyword
|
||||
@@ -0,0 +1 @@
|
||||
testdata/errors/KeywordOperationName.graphql:1: operation name must not be a go keyword
|
||||
@@ -0,0 +1 @@
|
||||
testdata/errors/KeywordTypeName.schema.graphql:1: typename option must not be a go keyword
|
||||
@@ -46,3 +46,32 @@ func goConstName(s string) string {
|
||||
return ret
|
||||
}, s)
|
||||
}
|
||||
|
||||
// https://go.dev/ref/spec#Keywords
|
||||
var goKeywords = map[string]bool{
|
||||
"break": true,
|
||||
"default": true,
|
||||
"func": true,
|
||||
"interface": true,
|
||||
"select": true,
|
||||
"case": true,
|
||||
"defer": true,
|
||||
"go": true,
|
||||
"map": true,
|
||||
"struct": true,
|
||||
"chan": true,
|
||||
"else": true,
|
||||
"goto": true,
|
||||
"package": true,
|
||||
"switch": true,
|
||||
"const": true,
|
||||
"fallthrough": true,
|
||||
"if": true,
|
||||
"range": true,
|
||||
"type": true,
|
||||
"continue": true,
|
||||
"for": true,
|
||||
"import": true,
|
||||
"return": true,
|
||||
"var": true,
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user