Bump golangci-lint and (max) Go versions (#219)

Lint is failing with some inscrutable panic (on a commit with no code
changes). Let's try bumping the version in case they fixed it.

Additionally, the new version's github action uses Go 1.19, which means
it pulls in gofmt updates to match the new [doc-comment formatting rules][1].
So I added Go 1.19 to our list of versions to test (fixes #216)
and updated some of our doc-comments to format better in the
new world (mostly using the new link syntax).

[1]: https://go.dev/doc/comment

Test plan: make lint
This commit is contained in:
Ben Kraft
2022-08-15 15:05:36 -07:00
committed by GitHub
parent c0510ff54a
commit f600b6e5d3
16 changed files with 396 additions and 196 deletions
+11 -7
View File
@@ -16,10 +16,11 @@ var cfgFilenames = []string{".genqlient.yml", ".genqlient.yaml", "genqlient.yml"
// Config represents genqlient's configuration, generally read from
// genqlient.yaml.
//
// Callers must call ValidateAndFillDefaults before using the config.
// Callers must call [Config.ValidateAndFillDefaults] before using the config.
type Config struct {
// The following fields are documented at:
// https://github.com/Khan/genqlient/blob/main/docs/genqlient.yaml
// The following fields are documented in the [genqlient.yaml docs].
//
// [genqlient.yaml docs]: https://github.com/Khan/genqlient/blob/main/docs/genqlient.yaml
Schema StringList `yaml:"schema"`
Operations StringList `yaml:"operations"`
Generated string `yaml:"generated"`
@@ -46,8 +47,9 @@ type Config struct {
}
// A TypeBinding represents a Go type to which genqlient will bind a particular
// GraphQL type, and is documented further at:
// https://github.com/Khan/genqlient/blob/main/docs/genqlient.yaml
// GraphQL type, and is documented further in the [genqlient.yaml docs].
//
// [genqlient.yaml docs]: https://github.com/Khan/genqlient/blob/main/docs/genqlient.yaml
type TypeBinding struct {
Type string `yaml:"type"`
ExpectExactFields string `yaml:"expect_exact_fields"`
@@ -56,8 +58,10 @@ type TypeBinding struct {
}
// A PackageBinding represents a Go package for which genqlient will
// automatically generate TypeBindings, and is documented further at:
// https://github.com/Khan/genqlient/blob/main/docs/genqlient.yaml
// automatically generate [TypeBinding] values, and is documented further in
// the [genqlient.yaml docs].
//
// [genqlient.yaml docs]: https://github.com/Khan/genqlient/blob/main/docs/genqlient.yaml
type PackageBinding struct {
Package string `yaml:"package"`
}
+13 -10
View File
@@ -424,12 +424,12 @@ func (g *generator) convertDefinition(
goName := upperFirst(field.Name)
// Several of the arguments don't really make sense here:
// (note field.Type is necessarily a scalar, input, or enum)
// - namePrefix is ignored for input types and enums (see
// names.go) and for scalars (they use client-specified
// names)
// - selectionSet is ignored for input types, because we
// just use all fields of the type; and it's nonexistent
// for scalars and enums, our only other possible types
// - namePrefix is ignored for input types and enums (see
// names.go) and for scalars (they use client-specified
// names)
// - selectionSet is ignored for input types, because we
// just use all fields of the type; and it's nonexistent
// for scalars and enums, our only other possible types
// TODO(benkraft): Can we refactor to avoid passing the values that
// will be ignored? We know field.Type is a scalar, enum, or input
// type. But plumbing that is a bit tricky in practice.
@@ -650,12 +650,14 @@ func (g *generator) convertSelectionSet(
// of the given type", which is true when the given type is or implements
// the fragment's type. This is distinct from the rules for when a fragment
// spread is legal, which is true when the fragment would be active for *any*
// of the concrete types the spread-context could have (see
// https://spec.graphql.org/draft/#sec-Fragment-Spreads or docs/DESIGN.md).
// of the concrete types the spread-context could have (see the [GraphQL spec]
// or docs/DESIGN.md).
//
// containingTypedef is as described in convertInlineFragment, below.
// fragmentTypedef is the definition of the fragment's type-condition, i.e. the
// definition of MyType in a fragment `on MyType`.
//
// [GraphQL spec]: https://spec.graphql.org/draft/#sec-Fragment-Spreads
func fragmentMatches(containingTypedef, fragmentTypedef *ast.Definition) bool {
if containingTypedef.Name == fragmentTypedef.Name {
return true
@@ -663,10 +665,11 @@ func fragmentMatches(containingTypedef, fragmentTypedef *ast.Definition) bool {
for _, iface := range containingTypedef.Interfaces {
// Note we don't need to recurse into the interfaces here, because in
// GraphQL types must list all the interfaces they implement, including
// all types those interfaces implement [1]. Actually, at present
// all types those interfaces implement ([spec]). Actually, at present
// gqlparser doesn't even support interfaces implementing other
// interfaces, but our code would handle that too.
// [1] https://spec.graphql.org/draft/#sec-Interfaces.Interfaces-Implementing-Interfaces
//
// [spec]: https://spec.graphql.org/draft/#sec-Interfaces.Interfaces-Implementing-Interfaces
if iface == fragmentTypedef.Name {
return true
}
+3 -3
View File
@@ -313,9 +313,9 @@ func (g *generator) addOperation(op *ast.OperationDefinition) error {
// Generate is the main programmatic entrypoint to genqlient, and generates and
// returns Go source code based on the given configuration.
//
// See Config for more on creating a configuration. The return value is a map
// from filename to the generated file-content (e.g. Go source). Callers who
// don't want to manage reading and writing the files should call Main.
// See [Config] for more on creating a configuration. The return value is a
// map from filename to the generated file-content (e.g. Go source). Callers
// who don't want to manage reading and writing the files should call [Main].
func Generate(config *Config) (map[string][]byte, error) {
// Step 1: Read in the schema and operations from the files defined by the
// config (and validate the operations against the schema). This is all
+2
View File
@@ -107,8 +107,10 @@ func setString(optionName string, dst *string, v *ast.Value, pos *ast.Position)
//
// If there are multiple genqlient directives are applied to the same node,
// e.g.
//
// # @genqlient(...)
// # @genqlient(...)
//
// add will be called several times. In this case, conflicts between the
// options are an error.
func (dir *genqlientDirective) add(graphQLDirective *ast.Directive, pos *ast.Position) error {
+12 -8
View File
@@ -29,20 +29,24 @@ var _sliceOrMapPrefixRegexp = regexp.MustCompile(`^(\*|\[\d*\]|map\[string\])*`)
//
// Ideally, we want to allow a reference to basically an arbitrary symbol.
// But that's very hard, because it might be quite complicated, like
//
// struct{ F []map[mypkg.K]otherpkg.V }
//
// Now in practice, using an unnamed struct is not a great idea, but we do
// want to allow as much as we can that encoding/json knows how to work
// with, since you would reasonably expect us to accept, say,
// map[string][]interface{}. So we allow:
// - any named type (mypkg.T)
// - any predeclared basic type (string, int, etc.)
// - interface{}
// - for any allowed type T, *T, []T, [N]T, and map[string]T
// - any named type (mypkg.T)
// - any predeclared basic type (string, int, etc.)
// - interface{}
// - for any allowed type T, *T, []T, [N]T, and map[string]T
//
// which effectively excludes:
// - unnamed struct types
// - map[K]V where K is a named type wrapping string
// - any nonstandard spelling of those (interface {/* hi */},
// map[ string ]T)
// - unnamed struct types
// - map[K]V where K is a named type wrapping string
// - any nonstandard spelling of those (interface {/* hi */},
// map[ string ]T)
//
// (This is documented in docs/genqlient.yaml)
func (g *generator) ref(fullyQualifiedName string) (qualifiedName string, err error) {
errorMsg := `invalid type-name "%v" (%v); expected a builtin, ` +
+7 -3
View File
@@ -1,6 +1,8 @@
// Package generate provides programmatic access to genqlient's functionality,
// and documentation of its configuration options. For general usage
// documentation, see github.com/Khan/genqlient.
// documentation, see the project [GitHub].
//
// [GitHub]: https://github.com/Khan/genqlient
package generate
import (
@@ -62,8 +64,10 @@ See https://github.com/Khan/genqlient for full documentation.
}
// Main is the command-line entrypoint to genqlient; it's equivalent to calling
// `go run github.com/Khan/genqlient`. For lower-level control over
// genqlient's operation, see Generate.
//
// go run github.com/Khan/genqlient
//
// For lower-level control over genqlient's operation, see [Generate].
func Main() {
exitIfError := func(err error) {
if err != nil {
+3 -1
View File
@@ -81,9 +81,11 @@ package generate
// can collide.
// All cases seem fairly rare in practice; eventually we'll likely allow users
// the ability to specify their own names, which they could use to avoid this
// (see https://github.com/Khan/genqlient/issues/12).
// (see [issue #12]).
// TODO(benkraft): We should probably at least try to detect it and bail.
//
// [issue #12]: https://github.com/Khan/genqlient/issues/12
//
// To implement all of the above, as we traverse the operation (and schema) in
// convert.go, we keep track of a list of parts to prefix to our type-names.
// The list always ends with a field, not a type; and we extend it when
+3 -2
View File
@@ -1,8 +1,9 @@
package generate
// StringList provides yaml unmarshaler to accept both `string` and `[]string` as a valid type.
// Sourced from:
// https://github.com/99designs/gqlgen/blob/1a0b19feff6f02d2af6631c9d847bc243f8ede39/codegen/config/config.go#L302-L329
// Sourced from [gqlgen].
//
// [gqlgen]: https://github.com/99designs/gqlgen/blob/1a0b19feff6f02d2af6631c9d847bc243f8ede39/codegen/config/config.go#L302-L329
type StringList []string
func (a *StringList) UnmarshalYAML(unmarshal func(interface{}) error) error {
+17 -10
View File
@@ -186,11 +186,11 @@ func (field *goStructField) Selector() string {
}
// unmarshaler returns:
// - the name of the function to use to unmarshal this field
// - true if this is a fully-qualified name (false if it is a package-local
// unqualified name)
// - true if we need to generate an unmarshaler at all, false if the default
// behavior will suffice
// - the name of the function to use to unmarshal this field
// - true if this is a fully-qualified name (false if it is a package-local
// unqualified name)
// - true if we need to generate an unmarshaler at all, false if the default
// behavior will suffice
func (field *goStructField) unmarshaler() (qualifiedName string, needsImport bool, needsUnmarshaler bool) {
switch typ := field.GoType.Unwrap().(type) {
case *goOpaqueType:
@@ -214,9 +214,9 @@ func (field *goStructField) Unmarshaler(g *generator) (string, error) {
}
// marshaler returns:
// - the fully-qualified name of the function to use to marshal this field
// - true if we need to generate an marshaler at all, false if the default
// behavior will suffice
// - the fully-qualified name of the function to use to marshal this field
// - true if we need to generate an marshaler at all, false if the default
// behavior will suffice
func (field *goStructField) marshaler() (qualifiedName string, needsImport bool, needsMarshaler bool) {
switch typ := field.GoType.Unwrap().(type) {
case *goOpaqueType:
@@ -273,17 +273,21 @@ type selector struct {
// and the paths to reach them (via those embeds), but with different
// visibility rules for conflicting fields than Go.
//
// (Before you read further, now's a good time to review Go's rules:
// https://golang.org/ref/spec#Selectors. Done? Good.)
// (Before you read further, now's a good time to review [Go's rules].
// Done? Good.)
//
// To illustrate the need, consider the following query:
//
// fragment A on T { id }
// fragment B on T { id }
// query Q { t { ...A ...B } }
//
// We generate types:
//
// type A struct { Id string `json:"id"` }
// type B struct { Id string `json:"id"` }
// type QT struct { A; B }
//
// According to Go's embedding rules, QT has no field Id: since QT.A.Id and
// QT.B.Id are at equal depth, neither wins and gets promoted. (Go's JSON
// library uses similar logic to decide which field to write to JSON, except
@@ -307,7 +311,10 @@ type selector struct {
// practice, hopefully, they all match, but validating that is even more work
// for a fairly rare case.) This function returns, for each JSON-name, the Go
// field we want to use. In the example above, it would return:
//
// []selector{{<goStructField for QT.A.Id>, "A.Id"}}
//
// [Go's rules]: https://golang.org/ref/spec#Selectors
func (typ *goStructType) FlattenedFields() ([]*selector, error) {
seenJSONNames := map[string]bool{}
retval := make([]*selector, 0, len(typ.Fields))