refactor: fix gocritic lint issues (#3113)

alexandear · web-flow · commit 1a7c6090e09c · 2024-05-31T13:29:07.000-04:00
diff --git a/.golangci.yml b/.golangci.yml
@@ -3,6 +3,15 @@ run:
   timeout: 5m
 
 linters-settings:
+  gocritic:
+    enabled-checks:
+      - emptyStringTest
+      - equalFold
+      - httpNoBody
+      - nilValReturn
+      - paramTypeCombine
+      - preferFprint
+      - yodaStyleExpr
   errcheck:
     exclude-functions:
       - (io.Writer).Write
@@ -69,6 +78,10 @@ issues:
       linters:
         - dupl
         - errcheck
+    # It's autogenerated code.
+    - path: codegen/testserver/.*/resolver\.go
+      linters:
+        - gocritic
     # Disable revive.use-any for backwards compatibility
     - path: graphql/map.go
       text: "use-any: since GO 1.18 'interface{}' can be replaced by 'any'"
diff --git a/_examples/chat/resolvers.go b/_examples/chat/resolvers.go
@@ -62,7 +62,7 @@ type Chatroom struct {
 
 type mutationResolver struct{ *resolver }
 
-func (r *mutationResolver) Post(ctx context.Context, text string, username string, roomName string) (*Message, error) {
+func (r *mutationResolver) Post(ctx context.Context, text, username, roomName string) (*Message, error) {
 	room := r.getRoom(roomName)
 
 	message := &Message{
diff --git a/_examples/scalars/model/model.go b/_examples/scalars/model/model.go
@@ -25,7 +25,7 @@ func (b Banned) MarshalGQL(w io.Writer) {
 func (b *Banned) UnmarshalGQL(v any) error {
 	switch v := v.(type) {
 	case string:
-		*b = strings.ToLower(v) == "true"
+		*b = Banned(strings.EqualFold(v, "true"))
 		return nil
 	case bool:
 		*b = Banned(v)
diff --git a/client/client.go b/client/client.go
@@ -111,7 +111,7 @@ var boundaryRegex = regexp.MustCompile(`multipart/form-data; ?boundary=.*`)
 func (p *Client) newRequest(query string, options ...Option) (*http.Request, error) {
 	bd := &Request{
 		Query: query,
-		HTTP:  httptest.NewRequest(http.MethodPost, "/", nil),
+		HTTP:  httptest.NewRequest(http.MethodPost, "/", http.NoBody),
 	}
 	bd.HTTP.Header.Set("Content-Type", "application/json")
 
@@ -128,7 +128,7 @@ func (p *Client) newRequest(query string, options ...Option) (*http.Request, err
 	switch {
 	case boundaryRegex.MatchString(contentType):
 		break
-	case "application/json" == contentType:
+	case contentType == "application/json":
 		requestBody, err := json.Marshal(bd)
 		if err != nil {
 			return nil, fmt.Errorf("encode: %w", err)
@@ -146,7 +146,7 @@ func (p *Client) SetCustomDecodeConfig(dc *mapstructure.DecoderConfig) {
 	p.dc = dc
 }
 
-func unpack(data any, into any, customDc *mapstructure.DecoderConfig) error {
+func unpack(data, into any, customDc *mapstructure.DecoderConfig) error {
 	dc := &mapstructure.DecoderConfig{
 		TagName:     "json",
 		ErrorUnused: true,
diff --git a/client/client_test.go b/client/client_test.go
@@ -182,7 +182,7 @@ func TestSetCustomDecodeConfig(t *testing.T) {
 	h := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
 		w.WriteHeader(http.StatusOK)
 		w.Header().Set("Content-Type", "application/json")
-		w.Write([]byte(fmt.Sprintf(`{"data": {"created_at":"%s"}}`, now.Format(time.RFC3339))))
+		fmt.Fprintf(w, `{"data": {"created_at":"%s"}}`, now.Format(time.RFC3339))
 	})
 
 	dc := &mapstructure.DecoderConfig{
diff --git a/client/options.go b/client/options.go
@@ -36,7 +36,7 @@ func Path(url string) Option {
 }
 
 // AddHeader adds a header to the outgoing request. This is useful for setting expected Authentication headers for example.
-func AddHeader(key string, value string) Option {
+func AddHeader(key, value string) Option {
 	return func(bd *Request) {
 		bd.HTTP.Header.Add(key, value)
 	}
diff --git a/codegen/config/binder.go b/codegen/config/binder.go
@@ -61,7 +61,7 @@ func (b *Binder) FindTypeFromName(name string) (types.Type, error) {
 	return b.FindType(pkgName, typeName)
 }
 
-func (b *Binder) FindType(pkgName string, typeName string) (types.Type, error) {
+func (b *Binder) FindType(pkgName, typeName string) (types.Type, error) {
 	if pkgName == "" {
 		if typeName == "map[string]interface{}" {
 			return MapType, nil
@@ -123,7 +123,7 @@ func (b *Binder) DefaultUserObject(name string) (types.Type, error) {
 	return obj.Type(), nil
 }
 
-func (b *Binder) FindObject(pkgName string, typeName string) (types.Object, error) {
+func (b *Binder) FindObject(pkgName, typeName string) (types.Object, error) {
 	if pkgName == "" {
 		return nil, errors.New("package cannot be nil")
 	}
@@ -349,7 +349,7 @@ func isIntf(t types.Type) bool {
 
 func unwrapOmittable(t types.Type) (types.Type, bool) {
 	if t == nil {
-		return t, false
+		return nil, false
 	}
 	named, ok := t.(*types.Named)
 	if !ok {
diff --git a/codegen/config/config.go b/codegen/config/config.go
@@ -644,7 +644,7 @@ func (tm TypeMap) ReferencedPackages() []string {
 	return pkgs
 }
 
-func (tm TypeMap) Add(name string, goType string) {
+func (tm TypeMap) Add(name, goType string) {
 	modelCfg := tm[name]
 	modelCfg.Model = append(modelCfg.Model, goType)
 	tm[name] = modelCfg
diff --git a/codegen/templates/templates.go b/codegen/templates/templates.go
@@ -185,7 +185,7 @@ func parseTemplates(cfg Options, t *template.Template) (*template.Template, erro
 	return t, nil
 }
 
-func center(width int, pad string, s string) string {
+func center(width int, pad, s string) string {
 	if len(s)+2 > width {
 		return s
 	}
diff --git a/graphql/bool.go b/graphql/bool.go
@@ -15,7 +15,7 @@ func MarshalBoolean(b bool) Marshaler {
 func UnmarshalBoolean(v any) (bool, error) {
 	switch v := v.(type) {
 	case string:
-		return strings.ToLower(v) == "true", nil
+		return strings.EqualFold(v, "true"), nil
 	case int:
 		return v != 0, nil
 	case bool:
diff --git a/graphql/context_field.go b/graphql/context_field.go
@@ -98,7 +98,7 @@ func WithFieldContext(ctx context.Context, rc *FieldContext) context.Context {
 	return context.WithValue(ctx, resolverCtx, rc)
 }
 
-func equalPath(a ast.Path, b ast.Path) bool {
+func equalPath(a, b ast.Path) bool {
 	if len(a) != len(b) {
 		return false
 	}
diff --git a/graphql/executable_schema.go b/graphql/executable_schema.go
@@ -116,7 +116,7 @@ func instanceOf(val string, satisfies []string) bool {
 	return false
 }
 
-func getOrCreateAndAppendField(c *[]CollectedField, name string, alias string, objectDefinition *ast.Definition, creator func() CollectedField) *CollectedField {
+func getOrCreateAndAppendField(c *[]CollectedField, name, alias string, objectDefinition *ast.Definition, creator func() CollectedField) *CollectedField {
 	for i, cf := range *c {
 		if cf.Name == name && cf.Alias == alias {
 			if cf.ObjectDefinition == objectDefinition {
diff --git a/graphql/float.go b/graphql/float.go
@@ -12,7 +12,7 @@ import (
 
 func MarshalFloat(f float64) Marshaler {
 	return WriterFunc(func(w io.Writer) {
-		io.WriteString(w, fmt.Sprintf("%g", f))
+		fmt.Fprintf(w, "%g", f)
 	})
 }
 
@@ -38,7 +38,7 @@ func MarshalFloatContext(f float64) ContextMarshaler {
 		if math.IsInf(f, 0) || math.IsNaN(f) {
 			return errors.New("cannot marshal infinite no NaN float values")
 		}
-		io.WriteString(w, fmt.Sprintf("%g", f))
+		fmt.Fprintf(w, "%g", f)
 		return nil
 	})
 }
diff --git a/graphql/handler/extension/complexity_test.go b/graphql/handler/extension/complexity_test.go
@@ -108,7 +108,7 @@ func TestFixedComplexity(t *testing.T) {
 	})
 }
 
-func doRequest(handler http.Handler, method string, target string, body string) *httptest.ResponseRecorder {
+func doRequest(handler http.Handler, method, target, body string) *httptest.ResponseRecorder {
 	r := httptest.NewRequest(method, target, strings.NewReader(body))
 	r.Header.Set("Content-Type", "application/json")
 	w := httptest.NewRecorder()
diff --git a/graphql/handler/server_test.go b/graphql/handler/server_test.go
@@ -186,15 +186,15 @@ func TestRecover(t *testing.T) {
 }
 
 func get(handler http.Handler, target string) *httptest.ResponseRecorder {
-	r := httptest.NewRequest("GET", target, nil)
+	r := httptest.NewRequest("GET", target, http.NoBody)
 	w := httptest.NewRecorder()
 
 	handler.ServeHTTP(w, r)
 	return w
 }
 
 func post(handler http.Handler, target, contentType string) *httptest.ResponseRecorder {
-	r := httptest.NewRequest("POST", target, nil)
+	r := httptest.NewRequest("POST", target, http.NoBody)
 	r.Header.Set("Content-Type", contentType)
 	w := httptest.NewRecorder()
 
diff --git a/graphql/handler/transport/http_graphql_test.go b/graphql/handler/transport/http_graphql_test.go
@@ -97,7 +97,7 @@ func TestGRAPHQL(t *testing.T) {
 	})
 }
 
-func doGraphqlRequest(handler http.Handler, method string, target string, body string) *httptest.ResponseRecorder {
+func doGraphqlRequest(handler http.Handler, method, target, body string) *httptest.ResponseRecorder {
 	r := httptest.NewRequest(method, target, strings.NewReader(body))
 	r.Header.Set("Content-Type", "application/graphql")
 	w := httptest.NewRecorder()
diff --git a/graphql/handler/transport/http_post_test.go b/graphql/handler/transport/http_post_test.go
@@ -98,7 +98,7 @@ func TestPOST(t *testing.T) {
 	})
 }
 
-func doRequest(handler http.Handler, method string, target string, body string, contentType string) *httptest.ResponseRecorder {
+func doRequest(handler http.Handler, method, target, body, contentType string) *httptest.ResponseRecorder {
 	r := httptest.NewRequest(method, target, strings.NewReader(body))
 	r.Header.Set("Content-Type", contentType)
 	w := httptest.NewRecorder()
diff --git a/graphql/playground/helper_test.go b/graphql/playground/helper_test.go
@@ -16,7 +16,7 @@ import (
 
 func testResourceIntegrity(t *testing.T, handler func(title, endpoint string) http.HandlerFunc) {
 	recorder := httptest.NewRecorder()
-	request := httptest.NewRequest(http.MethodGet, "http://localhost:8080/", nil)
+	request := httptest.NewRequest(http.MethodGet, "http://localhost:8080/", http.NoBody)
 	handler("example.org API", "/query").ServeHTTP(recorder, request)
 
 	res := recorder.Result()
@@ -40,7 +40,7 @@ func testResourceIntegrity(t *testing.T, handler func(title, endpoint string) ht
 	assertNodesIntegrity(t, baseUrl, doc, "link", "href", "integrity")
 }
 
-func assertNodesIntegrity(t *testing.T, baseUrl string, doc *goquery.Document, selector string, urlAttrKey, integrityAttrKey string) {
+func assertNodesIntegrity(t *testing.T, baseUrl string, doc *goquery.Document, selector, urlAttrKey, integrityAttrKey string) {
 	selection := doc.Find(selector)
 	for _, node := range selection.Nodes {
 		var url string
@@ -53,11 +53,11 @@ func assertNodesIntegrity(t *testing.T, baseUrl string, doc *goquery.Document, s
 			}
 		}
 
-		if len(integrity) != 0 {
+		if integrity != "" {
 			assert.NotEmpty(t, url)
 		}
 
-		if len(url) != 0 && len(integrity) != 0 {
+		if url != "" && integrity != "" {
 			resp, err := http.Get(baseUrl + url)
 			require.NoError(t, err)
 			hasher := sha256.New()
diff --git a/graphql/playground/playground.go b/graphql/playground/playground.go
@@ -85,14 +85,14 @@ var page = template.Must(template.New("graphiql").Parse(`<!DOCTYPE html>
 `))
 
 // Handler responsible for setting up the playground
-func Handler(title string, endpoint string) http.HandlerFunc {
+func Handler(title, endpoint string) http.HandlerFunc {
 	return HandlerWithHeaders(title, endpoint, nil, nil)
 }
 
 // HandlerWithHeaders sets up the playground.
 // fetcherHeaders are used by the playground's fetcher instance and will not be visible in the UI.
 // uiHeaders are default headers that will show up in the UI headers editor.
-func HandlerWithHeaders(title string, endpoint string, fetcherHeaders map[string]string, uiHeaders map[string]string) http.HandlerFunc {
+func HandlerWithHeaders(title, endpoint string, fetcherHeaders, uiHeaders map[string]string) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
 		w.Header().Add("Content-Type", "text/html; charset=UTF-8")
 		err := page.Execute(w, map[string]any{
diff --git a/graphql/playground/playground_test.go b/graphql/playground/playground_test.go
@@ -10,7 +10,7 @@ import (
 
 func TestHandler_createsAbsoluteURLs(t *testing.T) {
 	rec := httptest.NewRecorder()
-	req := httptest.NewRequest(http.MethodGet, "https://example.org/query", nil)
+	req := httptest.NewRequest(http.MethodGet, "https://example.org/query", http.NoBody)
 	h := Handler("example.org API", "https://example.org/query")
 	h.ServeHTTP(rec, req)
 
@@ -43,7 +43,7 @@ func TestHandler_createsAbsoluteURLs(t *testing.T) {
 
 func TestHandler_createsRelativeURLs(t *testing.T) {
 	rec := httptest.NewRecorder()
-	req := httptest.NewRequest(http.MethodGet, "http://localhost:8080/query", nil)
+	req := httptest.NewRequest(http.MethodGet, "http://localhost:8080/query", http.NoBody)
 	h := Handler("example.org API", "/customquery")
 	h.ServeHTTP(rec, req)
 
diff --git a/handler/handler.go b/handler/handler.go
@@ -244,12 +244,12 @@ func (a apqAdapter) Add(ctx context.Context, key string, value any) {
 }
 
 type PersistedQueryCache interface {
-	Add(ctx context.Context, hash string, query string)
+	Add(ctx context.Context, hash, query string)
 	Get(ctx context.Context, hash string) (string, bool)
 }
 
 // Deprecated: use playground.Handler instead
-func Playground(title string, endpoint string) http.HandlerFunc {
+func Playground(title, endpoint string) http.HandlerFunc {
 	return playground.Handler(title, endpoint)
 }
 
diff --git a/plugin/modelgen/models.go b/plugin/modelgen/models.go
@@ -622,7 +622,7 @@ func removeDuplicateTags(t string) string {
 		key := kv[0]
 		value := strings.Join(kv[1:], ":")
 		processed[key] = true
-		if len(returnTags) > 0 {
+		if returnTags != "" {
 			returnTags = " " + returnTags
 		}
 
diff --git a/plugin/resolvergen/resolver.go b/plugin/resolvergen/resolver.go
@@ -269,7 +269,7 @@ func (r *Resolver) Implementation() string {
 	return r.ImplementationStr
 }
 
-func gqlToResolverName(base string, gqlname, filenameTmpl string) string {
+func gqlToResolverName(base, gqlname, filenameTmpl string) string {
 	gqlname = filepath.Base(gqlname)
 	ext := filepath.Ext(gqlname)
 	if filenameTmpl == "" {
diff --git a/plugin/stubgen/stubs.go b/plugin/stubgen/stubs.go
@@ -15,7 +15,7 @@ import (
 //go:embed stubs.gotpl
 var stubsTemplate string
 
-func New(filename string, typename string) plugin.Plugin {
+func New(filename, typename string) plugin.Plugin {
 	return &Plugin{filename: filename, typeName: typename}
 }
 

Original file line number	Diff line number	Diff line change
`@@ -36,7 +36,7 @@ func Path(url string) Option {`
`36`	`36`	`}`
`37`	`37`
`38`	`38`	`// AddHeader adds a header to the outgoing request. This is useful for setting expected Authentication headers for example.`
`39`		`-func AddHeader(key string, value string) Option {`
	`39`	`+func AddHeader(key, value string) Option {`
`40`	`40`	`return func(bd *Request) {`
`41`	`41`	`bd.HTTP.Header.Add(key, value)`
`42`	`42`	`}`
Original file line number	Diff line number	Diff line change
`@@ -61,7 +61,7 @@ func (b *Binder) FindTypeFromName(name string) (types.Type, error) {`
`61`	`61`	`return b.FindType(pkgName, typeName)`
`62`	`62`	`}`
`63`	`63`
`64`		`-func (b *Binder) FindType(pkgName string, typeName string) (types.Type, error) {`
	`64`	`+func (b *Binder) FindType(pkgName, typeName string) (types.Type, error) {`
`65`	`65`	`if pkgName == "" {`
`66`	`66`	`if typeName == "map[string]interface{}" {`
`67`	`67`	`return MapType, nil`
`@@ -123,7 +123,7 @@ func (b *Binder) DefaultUserObject(name string) (types.Type, error) {`
`123`	`123`	`return obj.Type(), nil`
`124`	`124`	`}`
`125`	`125`
`126`		`-func (b *Binder) FindObject(pkgName string, typeName string) (types.Object, error) {`
	`126`	`+func (b *Binder) FindObject(pkgName, typeName string) (types.Object, error) {`
`127`	`127`	`if pkgName == "" {`
`128`	`128`	`return nil, errors.New("package cannot be nil")`
`129`	`129`	`}`
`@@ -349,7 +349,7 @@ func isIntf(t types.Type) bool {`
`349`	`349`
`350`	`350`	`func unwrapOmittable(t types.Type) (types.Type, bool) {`
`351`	`351`	`if t == nil {`
`352`		`- return t, false`
	`352`	`+ return nil, false`
`353`	`353`	`}`
`354`	`354`	`named, ok := t.(*types.Named)`
`355`	`355`	`if !ok {`
Original file line number	Diff line number	Diff line change
`@@ -644,7 +644,7 @@ func (tm TypeMap) ReferencedPackages() []string {`
`644`	`644`	`return pkgs`
`645`	`645`	`}`
`646`	`646`
`647`		`-func (tm TypeMap) Add(name string, goType string) {`
	`647`	`+func (tm TypeMap) Add(name, goType string) {`
`648`	`648`	`modelCfg := tm[name]`
`649`	`649`	`modelCfg.Model = append(modelCfg.Model, goType)`
`650`	`650`	`tm[name] = modelCfg`
Original file line number	Diff line number	Diff line change
`@@ -185,7 +185,7 @@ func parseTemplates(cfg Options, t template.Template) (template.Template, erro`
`185`	`185`	`return t, nil`
`186`	`186`	`}`
`187`	`187`
`188`		`-func center(width int, pad string, s string) string {`
	`188`	`+func center(width int, pad, s string) string {`
`189`	`189`	`if len(s)+2 > width {`
`190`	`190`	`return s`
`191`	`191`	`}`
Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ func WithFieldContext(ctx context.Context, rc *FieldContext) context.Context {`
`98`	`98`	`return context.WithValue(ctx, resolverCtx, rc)`
`99`	`99`	`}`
`100`	`100`
`101`		`-func equalPath(a ast.Path, b ast.Path) bool {`
	`101`	`+func equalPath(a, b ast.Path) bool {`
`102`	`102`	`if len(a) != len(b) {`
`103`	`103`	`return false`
`104`	`104`	`}`
Original file line number	Diff line number	Diff line change
`@@ -116,7 +116,7 @@ func instanceOf(val string, satisfies []string) bool {`
`116`	`116`	`return false`
`117`	`117`	`}`
`118`	`118`
`119`		`-func getOrCreateAndAppendField(c []CollectedField, name string, alias string, objectDefinition ast.Definition, creator func() CollectedField) *CollectedField {`
	`119`	`+func getOrCreateAndAppendField(c []CollectedField, name, alias string, objectDefinition ast.Definition, creator func() CollectedField) *CollectedField {`
`120`	`120`	`for i, cf := range *c {`
`121`	`121`	`if cf.Name == name && cf.Alias == alias {`
`122`	`122`	`if cf.ObjectDefinition == objectDefinition {`
Original file line number	Diff line number	Diff line change
`@@ -12,7 +12,7 @@ import (`
`12`	`12`
`13`	`13`	`func MarshalFloat(f float64) Marshaler {`
`14`	`14`	`return WriterFunc(func(w io.Writer) {`
`15`		`- io.WriteString(w, fmt.Sprintf("%g", f))`
	`15`	`+ fmt.Fprintf(w, "%g", f)`
`16`	`16`	`})`
`17`	`17`	`}`
`18`	`18`
`@@ -38,7 +38,7 @@ func MarshalFloatContext(f float64) ContextMarshaler {`
`38`	`38`	`if math.IsInf(f, 0) \|\| math.IsNaN(f) {`
`39`	`39`	`return errors.New("cannot marshal infinite no NaN float values")`
`40`	`40`	`}`
`41`		`- io.WriteString(w, fmt.Sprintf("%g", f))`
	`41`	`+ fmt.Fprintf(w, "%g", f)`
`42`	`42`	`return nil`
`43`	`43`	`})`
`44`	`44`	`}`
Original file line number	Diff line number	Diff line change
`@@ -108,7 +108,7 @@ func TestFixedComplexity(t *testing.T) {`
`108`	`108`	`})`
`109`	`109`	`}`
`110`	`110`
`111`		`-func doRequest(handler http.Handler, method string, target string, body string) *httptest.ResponseRecorder {`
	`111`	`+func doRequest(handler http.Handler, method, target, body string) *httptest.ResponseRecorder {`
`112`	`112`	`r := httptest.NewRequest(method, target, strings.NewReader(body))`
`113`	`113`	`r.Header.Set("Content-Type", "application/json")`
`114`	`114`	`w := httptest.NewRecorder()`
Original file line number	Diff line number	Diff line change
`@@ -186,15 +186,15 @@ func TestRecover(t *testing.T) {`
`186`	`186`	`}`
`187`	`187`
`188`	`188`	`func get(handler http.Handler, target string) *httptest.ResponseRecorder {`
`189`		`- r := httptest.NewRequest("GET", target, nil)`
	`189`	`+ r := httptest.NewRequest("GET", target, http.NoBody)`
`190`	`190`	`w := httptest.NewRecorder()`
`191`	`191`
`192`	`192`	`handler.ServeHTTP(w, r)`
`193`	`193`	`return w`
`194`	`194`	`}`
`195`	`195`
`196`	`196`	`func post(handler http.Handler, target, contentType string) *httptest.ResponseRecorder {`
`197`		`- r := httptest.NewRequest("POST", target, nil)`
	`197`	`+ r := httptest.NewRequest("POST", target, http.NoBody)`
`198`	`198`	`r.Header.Set("Content-Type", contentType)`
`199`	`199`	`w := httptest.NewRecorder()`
`200`	`200`
Original file line number	Diff line number	Diff line change
`@@ -97,7 +97,7 @@ func TestGRAPHQL(t *testing.T) {`
`97`	`97`	`})`
`98`	`98`	`}`
`99`	`99`
`100`		`-func doGraphqlRequest(handler http.Handler, method string, target string, body string) *httptest.ResponseRecorder {`
	`100`	`+func doGraphqlRequest(handler http.Handler, method, target, body string) *httptest.ResponseRecorder {`
`101`	`101`	`r := httptest.NewRequest(method, target, strings.NewReader(body))`
`102`	`102`	`r.Header.Set("Content-Type", "application/graphql")`
`103`	`103`	`w := httptest.NewRecorder()`