diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index b1987ec..33d3060 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -28,7 +28,8 @@ When releasing a new version: ### Bug fixes: -- Fixed non-deterministic generated code when querying graphql interfaces +- Fixed non-deterministic generated code when querying graphql interfaces. +- Fixed generated code when last component of package name is not a valid identifier (e.g. `"path/to/my-package"`). ## v0.5.0 diff --git a/generate/generate_test.go b/generate/generate_test.go index be5088e..c976244 100644 --- a/generate/generate_test.go +++ b/generate/generate_test.go @@ -171,6 +171,10 @@ func TestGenerateWithConfig(t *testing.T) { Generated: "generated.go", ContextType: "github.com/Khan/genqlient/internal/testutil.MyContext", }}, + {"CustomContextWithAlias", "", nil, &Config{ + Generated: "generated.go", + ContextType: "github.com/Khan/genqlient/internal/testutil/junk---fun.name.MyContext", + }}, {"StructReferences", "", nil, &Config{ StructReferences: true, Generated: "generated-structrefs.go", diff --git a/generate/imports.go b/generate/imports.go index f701ecb..5d6f0a2 100644 --- a/generate/imports.go +++ b/generate/imports.go @@ -2,14 +2,43 @@ package generate import ( "fmt" + "go/token" "go/types" "regexp" "strconv" "strings" + "unicode" ) +// makeIdentifier takes a string and returns a valid go identifier like it. +// +// If the string is an identifier, return the input. Otherwise, munge it to +// make a valid identifier, which at worst (if the input is entirely emoji, +// say) means coming up with one out of whole cloth. This identifier need not +// be particularly unique; the caller may add a suffix. +func makeIdentifier(candidateIdentifier string) string { + if token.IsIdentifier(candidateIdentifier) { + return candidateIdentifier + } + + var goodChars strings.Builder + for _, c := range candidateIdentifier { + // modified from token.IsIdentifier + if unicode.IsLetter(c) || c == '_' || + // digits only valid after first char + goodChars.Len() > 0 && unicode.IsDigit(c) { + goodChars.WriteRune(c) + } + } + if goodChars.Len() > 0 { + return goodChars.String() + } + + return "alias" +} + func (g *generator) addImportFor(pkgPath string) (alias string) { - pkgName := pkgPath[strings.LastIndex(pkgPath, "/")+1:] + pkgName := makeIdentifier(pkgPath[strings.LastIndex(pkgPath, "/")+1:]) alias = pkgName suffix := 2 for g.usedAliases[alias] { diff --git a/generate/imports_test.go b/generate/imports_test.go new file mode 100644 index 0000000..6b49513 --- /dev/null +++ b/generate/imports_test.go @@ -0,0 +1,42 @@ +package generate + +import ( + "go/token" + "testing" +) + +func TestMakeIdentifier(t *testing.T) { + tests := []struct { + testName string + input string + expected string + }{ + {"GoodIdentifier", "myIdent", "myIdent"}, + {"GoodIdentifierNumbers", "myIdent1234", "myIdent1234"}, + {"NumberPrefix", "1234myIdent", "myIdent"}, + {"OnlyNumbers", "1234", "alias"}, + {"Dashes", "my-ident", "myident"}, + // Note: most Go implementations won't actually allow + // this package-path, but the spec is pretty vague + // so make sure to handle it. + {"JunkAnd", "my!!\\\\\nident", "myident"}, + {"JunkOnly", "!!\\\\\n", "alias"}, + {"Accents", "née", "née"}, + {"Kanji", "日本", "日本"}, + {"EmojiAnd", "ident👍", "ident"}, + {"EmojiOnly", "👍", "alias"}, + } + + for _, test := range tests { + test := test + t.Run(test.testName, func(t *testing.T) { + actual := makeIdentifier(test.input) + if actual != test.expected { + t.Errorf("mismatch:\ngot: %s\nwant: %s", actual, test.expected) + } + if !token.IsIdentifier(actual) { + t.Errorf("not a valid identifier: %s", actual) + } + }) + } +} diff --git a/generate/testdata/snapshots/TestGenerateWithConfig-CustomContextWithAlias-testdata-queries-generated.go b/generate/testdata/snapshots/TestGenerateWithConfig-CustomContextWithAlias-testdata-queries-generated.go new file mode 100644 index 0000000..7664d61 --- /dev/null +++ b/generate/testdata/snapshots/TestGenerateWithConfig-CustomContextWithAlias-testdata-queries-generated.go @@ -0,0 +1,68 @@ +// Code generated by github.com/Khan/genqlient, DO NOT EDIT. + +package queries + +import ( + "context" + + "github.com/Khan/genqlient/graphql" + junkfunname "github.com/Khan/genqlient/internal/testutil/junk---fun.name" +) + +// Check that context_type from genqlient.yaml implements context.Context. +var _ context.Context = (junkfunname.MyContext)(nil) + +// SimpleQueryResponse is returned by SimpleQuery on success. +type SimpleQueryResponse struct { + // user looks up a user by some stuff. + // + // See UserQueryInput for what stuff is supported. + // If query is null, returns the current user. + User SimpleQueryUser `json:"user"` +} + +// GetUser returns SimpleQueryResponse.User, and is useful for accessing the field via an interface. +func (v *SimpleQueryResponse) GetUser() SimpleQueryUser { return v.User } + +// SimpleQueryUser includes the requested fields of the GraphQL type User. +// The GraphQL type's documentation follows. +// +// A User is a user! +type SimpleQueryUser struct { + // id is the user's ID. + // + // It is stable, unique, and opaque, like all good IDs. + Id string `json:"id"` +} + +// GetId returns SimpleQueryUser.Id, and is useful for accessing the field via an interface. +func (v *SimpleQueryUser) GetId() string { return v.Id } + +func SimpleQuery( + ctx junkfunname.MyContext, + client graphql.Client, +) (*SimpleQueryResponse, error) { + req := &graphql.Request{ + OpName: "SimpleQuery", + Query: ` +query SimpleQuery { + user { + id + } +} +`, + } + var err error + + var data SimpleQueryResponse + resp := &graphql.Response{Data: &data} + + err = client.MakeRequest( + ctx, + req, + resp, + ) + + return &data, err +} + diff --git a/internal/testutil/junk---fun.name/types.go b/internal/testutil/junk---fun.name/types.go new file mode 100644 index 0000000..c202859 --- /dev/null +++ b/internal/testutil/junk---fun.name/types.go @@ -0,0 +1,51 @@ +package testutil + +import ( + "context" + "time" + + "github.com/Khan/genqlient/graphql" +) + +type ID string + +type Pokemon struct { + Species string `json:"species"` + Level int `json:"level"` +} + +func (p Pokemon) Battle(q Pokemon) bool { + return p.Level > q.Level +} + +type MyContext interface { + context.Context + + MyMethod() +} + +func GetClientFromNowhere() (graphql.Client, error) { return nil, nil } +func GetClientFromContext(ctx context.Context) (graphql.Client, error) { return nil, nil } +func GetClientFromMyContext(ctx MyContext) (graphql.Client, error) { return nil, nil } + +const dateFormat = "2006-01-02" + +func MarshalDate(t *time.Time) ([]byte, error) { + // nil should never happen but we might as well check. zero-time does + // happen because omitempty doesn't consider it zero; we'd prefer to write + // null than "0001-01-01". + // + // (I mean, we're tests. Who cares! But we may as well try to match what + // prod code would want.) + if t == nil || t.IsZero() { + return []byte("null"), nil + } + return []byte(`"` + t.Format(dateFormat) + `"`), nil +} + +func UnmarshalDate(b []byte, t *time.Time) error { + // (modified from time.Time.UnmarshalJSON) + var err error + *t, err = time.Parse(`"`+dateFormat+`"`, string(b)) + return err +}