Merge pull request #1946 from flant/prealloc-unparam-sqlclosecheck

Enable unparam, prealloc, sqlclosecheck linters

.golangci.yml

@@ -32,13 +32,16 @@ linters:
         - misspell
         - nakedret
         - nolintlint
+        - prealloc
         - rowserrcheck
+        - sqlclosecheck
         - staticcheck
         - structcheck
         - stylecheck
         - tparallel
         - typecheck
         - unconvert
+        - unparam
         - unused
         - varcheck
         - whitespace
@@ -51,8 +54,6 @@ linters:
         # - godot
         # - nlreturn
         # - noctx
-        # - prealloc
-        # - sqlclosecheck
         # - wrapcheck
 
         # TODO: fix linter errors before enabling (from original config)
@@ -63,7 +64,6 @@ linters:
         # - gosec
         # - lll
         # - scopelint
-        # - unparam
 
         # unused
         # - depguard

connector/atlassiancrowd/atlassiancrowd.go

@@ -128,10 +128,7 @@ func (c *crowdConnector) Login(ctx context.Context, s connector.Scopes, username
 		return connector.Identity{}, false, err
 	}
 
-	if ident, err = c.identityFromCrowdUser(user); err != nil {
+	ident = c.identityFromCrowdUser(user)
-		return connector.Identity{}, false, err
-	}
-
 	if s.Groups {
 		userGroups, err := c.getGroups(ctx, client, s.Groups, ident.Username)
 		if err != nil {
@@ -165,10 +162,7 @@ func (c *crowdConnector) Refresh(ctx context.Context, s connector.Scopes, ident
 		return ident, fmt.Errorf("crowd: get user %q: %v", data.Username, err)
 	}
 
-	newIdent, err := c.identityFromCrowdUser(user)
+	newIdent := c.identityFromCrowdUser(user)
-	if err != nil {
-		return ident, err
-	}
 	newIdent.ConnectorData = ident.ConnectorData
 
 	// If user exists, authenticate it to prolong sso session.
@@ -366,7 +360,7 @@ func (c *crowdConnector) groups(ctx context.Context, client *http.Client, userna
 }
 
 // identityFromCrowdUser converts crowdUser to Identity
-func (c *crowdConnector) identityFromCrowdUser(user crowdUser) (connector.Identity, error) {
+func (c *crowdConnector) identityFromCrowdUser(user crowdUser) connector.Identity {
 	identity := connector.Identity{
 		Username:      user.Name,
 		UserID:        user.Key,
@@ -387,7 +381,7 @@ func (c *crowdConnector) identityFromCrowdUser(user crowdUser) (connector.Identi
 		}
 	}
 
-	return identity, nil
+	return identity
 }
 
 // getGroups retrieves a list of user's groups and filters it

connector/atlassiancrowd/atlassiancrowd_test.go

@@ -71,9 +71,6 @@ func TestUserLoginFlow(t *testing.T) {
 	expectEquals(t, user.Name, "testuser")
 	expectEquals(t, user.Email, "testuser@example.com")
 
-	_, err = c.identityFromCrowdUser(user)
-	expectNil(t, err)
-
 	err = c.authenticateUser(context.Background(), newClient(), "testuser")
 	expectNil(t, err)
 
@@ -119,8 +116,7 @@ func TestIdentityFromCrowdUser(t *testing.T) {
 	expectEquals(t, user.Email, "testuser@example.com")
 
 	// Test unconfigured behaviour
-	i, err := c.identityFromCrowdUser(user)
+	i := c.identityFromCrowdUser(user)
-	expectNil(t, err)
 	expectEquals(t, i.UserID, "12345")
 	expectEquals(t, i.Username, "testuser")
 	expectEquals(t, i.Email, "testuser@example.com")
@@ -131,23 +127,19 @@ func TestIdentityFromCrowdUser(t *testing.T) {
 	expectEquals(t, i.PreferredUsername, "")
 
 	c.Config.PreferredUsernameField = "key"
-	i, err = c.identityFromCrowdUser(user)
+	i = c.identityFromCrowdUser(user)
-	expectNil(t, err)
 	expectEquals(t, i.PreferredUsername, "12345")
 
 	c.Config.PreferredUsernameField = "name"
-	i, err = c.identityFromCrowdUser(user)
+	i = c.identityFromCrowdUser(user)
-	expectNil(t, err)
 	expectEquals(t, i.PreferredUsername, "testuser")
 
 	c.Config.PreferredUsernameField = "email"
-	i, err = c.identityFromCrowdUser(user)
+	i = c.identityFromCrowdUser(user)
-	expectNil(t, err)
 	expectEquals(t, i.PreferredUsername, "testuser@example.com")
 
 	c.Config.PreferredUsernameField = "invalidstring"
-	i, err = c.identityFromCrowdUser(user)
+	i = c.identityFromCrowdUser(user)
-	expectNil(t, err)
 	expectEquals(t, i.PreferredUsername, "")
 }
 

connector/bitbucketcloud/bitbucketcloud.go

@@ -421,7 +421,6 @@ type group struct {
 }
 
 func (b *bitbucketConnector) userTeamGroups(ctx context.Context, client *http.Client, teamName string) ([]string, error) {
-	var teamGroups []string
 	apiURL := b.legacyAPIURL + "/groups/" + teamName
 
 	var response []group
@@ -429,6 +428,7 @@ func (b *bitbucketConnector) userTeamGroups(ctx context.Context, client *http.Cl
 		return nil, fmt.Errorf("get user team %q groups: %v", teamName, err)
 	}
 
+	teamGroups := make([]string, 0, len(response))
 	for _, group := range response {
 		teamGroups = append(teamGroups, teamName+"/"+group.Slug)
 	}

connector/gitea/gitea.go

@@ -72,7 +72,7 @@ type giteaConnector struct {
 	useLoginAsID bool
 }
 
-func (c *giteaConnector) oauth2Config(scopes connector.Scopes) *oauth2.Config {
+func (c *giteaConnector) oauth2Config(_ connector.Scopes) *oauth2.Config {
 	giteaEndpoint := oauth2.Endpoint{AuthURL: c.baseURL + "/login/oauth/authorize", TokenURL: c.baseURL + "/login/oauth/access_token"}
 	return &oauth2.Config{
 		ClientID:     c.clientID,

connector/ldap/ldap.go

@@ -609,7 +609,7 @@ func (c *ldapConnector) groups(ctx context.Context, user ldap.Entry) ([]string,
 		}
 	}
 
-	var groupNames []string
+	groupNames := make([]string, 0, len(groups))
 	for _, group := range groups {
 		name := getAttr(*group, c.GroupSearch.NameAttr)
 		if name == "" {

connector/saml/saml.go

@@ -488,7 +488,7 @@ func (p *provider) validateSubject(subject *subject, inResponseTo string) error
 		return fmt.Errorf("subject contained no SubjectConfirmations")
 	}
 
-	var errs []error
+	errs := make([]error, 0, len(subject.SubjectConfirmations))
 	// One of these must match our assumptions, not all.
 	for _, c := range subject.SubjectConfirmations {
 		err := func() error {

server/api.go

@@ -233,7 +233,7 @@ func (d dexAPI) ListPasswords(ctx context.Context, req *api.ListPasswordReq) (*a
 		return nil, fmt.Errorf("list passwords: %v", err)
 	}
 
-	var passwords []*api.Password
+	passwords := make([]*api.Password, 0, len(passwordList))
 	for _, password := range passwordList {
 		p := api.Password{
 			Email:    password.Email,
@@ -286,20 +286,18 @@ func (d dexAPI) ListRefresh(ctx context.Context, req *api.ListRefreshReq) (*api.
 		return nil, err
 	}
 
-	var refreshTokenRefs []*api.RefreshTokenRef
 	offlineSessions, err := d.s.GetOfflineSessions(id.UserId, id.ConnId)
 	if err != nil {
 		if err == storage.ErrNotFound {
 			// This means that this user-client pair does not have a refresh token yet.
 			// An empty list should be returned instead of an error.
-			return &api.ListRefreshResp{
+			return &api.ListRefreshResp{}, nil
-				RefreshTokens: refreshTokenRefs,
-			}, nil
 		}
 		d.logger.Errorf("api: failed to list refresh tokens %t here : %v", err == storage.ErrNotFound, err)
 		return nil, err
 	}
 
+	refreshTokenRefs := make([]*api.RefreshTokenRef, 0, len(offlineSessions.Refresh))
 	for _, session := range offlineSessions.Refresh {
 		r := api.RefreshTokenRef{
 			Id:        session.ID,

server/deviceflowhandlers.go

@@ -77,11 +77,7 @@ func (s *Server) handleDeviceCode(w http.ResponseWriter, r *http.Request) {
 		deviceCode := storage.NewDeviceCode()
 
 		// make user code
-		userCode, err := storage.NewUserCode()
+		userCode := storage.NewUserCode()
-		if err != nil {
-			s.logger.Errorf("Error generating user code: %v", err)
-			s.tokenErrHelper(w, errInvalidRequest, "", http.StatusInternalServerError)
-		}
 
 		// Generate the expire time
 		expireTime := time.Now().Add(s.deviceRequestsValidFor)

server/server.go

@@ -252,10 +252,8 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
 		}
 	}
 
-	instrumentHandlerCounter := func(handlerName string, handler http.Handler) http.HandlerFunc {
+	instrumentHandlerCounter := func(_ string, handler http.Handler) http.HandlerFunc {
-		return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+		return handler.ServeHTTP
-			handler.ServeHTTP(w, r)
-		})
 	}
 
 	if c.PrometheusRegistry != nil {
@@ -270,10 +268,10 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
 		}
 
 		instrumentHandlerCounter = func(handlerName string, handler http.Handler) http.HandlerFunc {
-			return http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
+			return func(w http.ResponseWriter, r *http.Request) {
 				m := httpsnoop.CaptureMetrics(handler, w, r)
 				requestCounter.With(prometheus.Labels{"handler": handlerName, "code": strconv.Itoa(m.Code), "method": r.Method}).Inc()
-			})
+			}
 		}
 	}
 

storage/conformance/conformance.go

@@ -285,7 +285,7 @@ func testClientCRUD(t *testing.T, s storage.Storage) {
 		t.Fatalf("create client: %v", err)
 	}
 
-	getAndCompare := func(id string, want storage.Client) {
+	getAndCompare := func(_ string, want storage.Client) {
 		gc, err := s.GetClient(id1)
 		if err != nil {
 			t.Errorf("get client: %v", err)
@@ -845,13 +845,8 @@ func testGC(t *testing.T, s storage.Storage) {
 		t.Errorf("expected storage.ErrNotFound, got %v", err)
 	}
 
-	userCode, err := storage.NewUserCode()
-	if err != nil {
-		t.Errorf("Unexpected Error: %v", err)
-	}
-
 	d := storage.DeviceRequest{
-		UserCode:     userCode,
+		UserCode:     storage.NewUserCode(),
 		DeviceCode:   storage.NewID(),
 		ClientID:     "client1",
 		ClientSecret: "secret1",
@@ -970,12 +965,8 @@ func testTimezones(t *testing.T, s storage.Storage) {
 }
 
 func testDeviceRequestCRUD(t *testing.T, s storage.Storage) {
-	userCode, err := storage.NewUserCode()
-	if err != nil {
-		panic(err)
-	}
 	d1 := storage.DeviceRequest{
-		UserCode:     userCode,
+		UserCode:     storage.NewUserCode(),
 		DeviceCode:   storage.NewID(),
 		ClientID:     "client1",
 		ClientSecret: "secret1",
@@ -988,7 +979,7 @@ func testDeviceRequestCRUD(t *testing.T, s storage.Storage) {
 	}
 
 	// Attempt to create same DeviceRequest twice.
-	err = s.CreateDeviceRequest(d1)
+	err := s.CreateDeviceRequest(d1)
 	mustBeErrAlreadyExists(t, "device request", err)
 
 	// No manual deletes for device requests, will be handled by garbage collection routines

storage/etcd/etcd.go

@@ -338,13 +338,13 @@ func (c *conn) ListPasswords() (passwords []storage.Password, err error) {
 func (c *conn) CreateOfflineSessions(s storage.OfflineSessions) error {
 	ctx, cancel := context.WithTimeout(context.Background(), defaultStorageTimeout)
 	defer cancel()
-	return c.txnCreate(ctx, keySession(offlineSessionPrefix, s.UserID, s.ConnID), fromStorageOfflineSessions(s))
+	return c.txnCreate(ctx, keySession(s.UserID, s.ConnID), fromStorageOfflineSessions(s))
 }
 
 func (c *conn) UpdateOfflineSessions(userID string, connID string, updater func(s storage.OfflineSessions) (storage.OfflineSessions, error)) error {
 	ctx, cancel := context.WithTimeout(context.Background(), defaultStorageTimeout)
 	defer cancel()
-	return c.txnUpdate(ctx, keySession(offlineSessionPrefix, userID, connID), func(currentValue []byte) ([]byte, error) {
+	return c.txnUpdate(ctx, keySession(userID, connID), func(currentValue []byte) ([]byte, error) {
 		var current OfflineSessions
 		if len(currentValue) > 0 {
 			if err := json.Unmarshal(currentValue, &current); err != nil {
@@ -363,7 +363,7 @@ func (c *conn) GetOfflineSessions(userID string, connID string) (s storage.Offli
 	ctx, cancel := context.WithTimeout(context.Background(), defaultStorageTimeout)
 	defer cancel()
 	var os OfflineSessions
-	if err = c.getKey(ctx, keySession(offlineSessionPrefix, userID, connID), &os); err != nil {
+	if err = c.getKey(ctx, keySession(userID, connID), &os); err != nil {
 		return
 	}
 	return toStorageOfflineSessions(os), nil
@@ -372,7 +372,7 @@ func (c *conn) GetOfflineSessions(userID string, connID string) (s storage.Offli
 func (c *conn) DeleteOfflineSessions(userID string, connID string) error {
 	ctx, cancel := context.WithTimeout(context.Background(), defaultStorageTimeout)
 	defer cancel()
-	return c.deleteKey(ctx, keySession(offlineSessionPrefix, userID, connID))
+	return c.deleteKey(ctx, keySession(userID, connID))
 }
 
 func (c *conn) CreateConnector(connector storage.Connector) error {
@@ -564,8 +564,8 @@ func (c *conn) txnUpdate(ctx context.Context, key string, update func(current []
 
 func keyID(prefix, id string) string       { return prefix + id }
 func keyEmail(prefix, email string) string { return prefix + strings.ToLower(email) }
-func keySession(prefix, userID, connID string) string {
+func keySession(userID, connID string) string {
-	return prefix + strings.ToLower(userID+"|"+connID)
+	return offlineSessionPrefix + strings.ToLower(userID+"|"+connID)
 }
 
 func (c *conn) CreateDeviceRequest(d storage.DeviceRequest) error {

storage/kubernetes/k8sapi/client.go

@@ -19,7 +19,7 @@ package k8sapi
 // Where possible, json tags match the cli argument names.
 // Top level config objects and all values required for proper functioning are not "omitempty".  Any truly optional piece of config is allowed to be omitted.
 
-// Config holds the information needed to build connect to remote kubernetes clusters as a given user
+// Config holds the information needed to build connect to remote kubernetes clusters as a given user.
 type Config struct {
 	// Legacy field from pkg/api/types.go TypeMeta.
 	// TODO(jlowdermilk): remove this after eliminating downstream dependencies.
@@ -51,7 +51,7 @@ type Preferences struct {
 	Extensions []NamedExtension `json:"extensions,omitempty"`
 }
 
-// Cluster contains information about how to communicate with a kubernetes cluster
+// Cluster contains information about how to communicate with a kubernetes cluster.
 type Cluster struct {
 	// Server is the address of the kubernetes cluster (https://hostname:port).
 	Server string `json:"server"`
@@ -97,7 +97,8 @@ type AuthInfo struct {
 	Extensions []NamedExtension `json:"extensions,omitempty"`
 }
 
-// Context is a tuple of references to a cluster (how do I communicate with a kubernetes cluster), a user (how do I identify myself), and a namespace (what subset of resources do I want to work with)
+// Context is a tuple of references to a cluster (how do I communicate with a kubernetes cluster),
+// a user (how do I identify myself), and a namespace (what subset of resources do I want to work with).
 type Context struct {
 	// Cluster is the name of the cluster for this context
 	Cluster string `json:"cluster"`

storage/sql/crud.go

@@ -378,6 +378,8 @@ func (c *conn) ListRefreshTokens() ([]storage.RefreshToken, error) {
 	if err != nil {
 		return nil, fmt.Errorf("query: %v", err)
 	}
+	defer rows.Close()
+
 	var tokens []storage.RefreshToken
 	for rows.Next() {
 		r, err := scanRefresh(rows)
@@ -556,6 +558,8 @@ func (c *conn) ListClients() ([]storage.Client, error) {
 	if err != nil {
 		return nil, err
 	}
+	defer rows.Close()
+
 	var clients []storage.Client
 	for rows.Next() {
 		cli, err := scanClient(rows)
@@ -652,6 +656,7 @@ func (c *conn) ListPasswords() ([]storage.Password, error) {
 	if err != nil {
 		return nil, err
 	}
+	defer rows.Close()
 
 	var passwords []storage.Password
 	for rows.Next() {
@@ -837,6 +842,8 @@ func (c *conn) ListConnectors() ([]storage.Connector, error) {
 	if err != nil {
 		return nil, err
 	}
+	defer rows.Close()
+
 	var connectors []storage.Connector
 	for rows.Next() {
 		conn, err := scanConnector(rows)

storage/storage.go

@@ -386,22 +386,19 @@ type Keys struct {
 
 // NewUserCode returns a randomized 8 character user code for the device flow.
 // No vowels are included to prevent accidental generation of words
-func NewUserCode() (string, error) {
+func NewUserCode() string {
-	code, err := randomString(8)
+	code := randomString(8)
-	if err != nil {
+	return code[:4] + "-" + code[4:]
-		return "", err
-	}
-	return code[:4] + "-" + code[4:], nil
 }
 
-func randomString(n int) (string, error) {
+func randomString(n int) string {
 	v := big.NewInt(int64(len(validUserCharacters)))
 	bytes := make([]byte, n)
 	for i := 0; i < n; i++ {
 		c, _ := rand.Int(rand.Reader, v)
 		bytes[i] = validUserCharacters[c.Int64()]
 	}
-	return string(bytes), nil
+	return string(bytes)
 }
 
 // DeviceRequest represents an OIDC device authorization request. It holds the state of a device request until the user