Add gocritic

Signed-off-by: m.nabokikh <maksim.nabokikh@flant.com>

.golangci.yml

@@ -19,6 +19,7 @@ linters:
   - goimports
   - golint
   - gosimple
+  - gocritic
   - govet
   - ineffassign
   - interfacer
@@ -40,6 +41,5 @@ linters:
   # - gocyclo
   # - lll
   # - goconst
-  # - gocritic
   # - errcheck
   # - dupl

connector/authproxy/authproxy.go

@@ -34,7 +34,7 @@ func (m *callback) LoginURL(s connector.Scopes, callbackURL, state string) (stri
 	if err != nil {
 		return "", fmt.Errorf("failed to parse callbackURL %q: %v", callbackURL, err)
 	}
-	u.Path = u.Path + m.pathSuffix
+	u.Path += m.pathSuffix
 	v := u.Query()
 	v.Set("state", state)
 	u.RawQuery = v.Encode()

connector/github/github.go

@@ -334,11 +334,12 @@ func (c *githubConnector) Refresh(ctx context.Context, s connector.Scopes, ident
 
 // getGroups retrieves GitHub orgs and teams a user is in, if any.
 func (c *githubConnector) getGroups(ctx context.Context, client *http.Client, groupScope bool, userLogin string) ([]string, error) {
-	if len(c.orgs) > 0 {
+	switch {
+	case len(c.orgs) > 0:
 		return c.groupsForOrgs(ctx, client, userLogin)
-	} else if c.org != "" {
+	case c.org != "":
 		return c.teamsForOrg(ctx, client, c.org)
-	} else if groupScope && c.loadAllGroups {
+	case groupScope && c.loadAllGroups:
 		return c.userGroups(ctx, client)
 	}
 	return nil, nil

connector/ldap/ldap.go

@@ -188,7 +188,7 @@ func parseScope(s string) (int, bool) {
 // See "Config.GroupSearch.UserMatchers" comments for the details
 func (c *ldapConnector) userMatchers() []UserMatcher {
 	if len(c.GroupSearch.UserMatchers) > 0 && c.GroupSearch.UserMatchers[0].UserAttr != "" {
-		return c.GroupSearch.UserMatchers[:]
+		return c.GroupSearch.UserMatchers
 	}
 
 	return []UserMatcher{
@@ -245,9 +245,9 @@ func (c *Config) openConnector(logger log.Logger) (*ldapConnector, error) {
 	if host, _, err = net.SplitHostPort(c.Host); err != nil {
 		host = c.Host
 		if c.InsecureNoSSL {
-			c.Host = c.Host + ":389"
+			c.Host += ":389"
 		} else {
-			c.Host = c.Host + ":636"
+			c.Host += ":636"
 		}
 	}
 

server/deviceflowhandlers_test.go

@@ -24,7 +24,7 @@ func TestDeviceVerificationURI(t *testing.T) {
 	defer cancel()
 	// Setup a dex server.
 	httpServer, s := newTestServer(ctx, t, func(c *Config) {
-		c.Issuer = c.Issuer + "/non-root-path"
+		c.Issuer += "/non-root-path"
 		c.Now = now
 	})
 	defer httpServer.Close()
@@ -76,7 +76,7 @@ func TestHandleDeviceCode(t *testing.T) {
 
 			// Setup a dex server.
 			httpServer, s := newTestServer(ctx, t, func(c *Config) {
-				c.Issuer = c.Issuer + "/non-root-path"
+				c.Issuer += "/non-root-path"
 				c.Now = now
 			})
 			defer httpServer.Close()
@@ -506,7 +506,7 @@ func TestDeviceTokenResponse(t *testing.T) {
 
 			// Setup a dex server.
 			httpServer, s := newTestServer(ctx, t, func(c *Config) {
-				c.Issuer = c.Issuer + "/non-root-path"
+				c.Issuer += "/non-root-path"
 				c.Now = now
 			})
 			defer httpServer.Close()
@@ -637,7 +637,7 @@ func TestVerifyCodeResponse(t *testing.T) {
 
 			// Setup a dex server.
 			httpServer, s := newTestServer(ctx, t, func(c *Config) {
-				c.Issuer = c.Issuer + "/non-root-path"
+				c.Issuer += "/non-root-path"
 				c.Now = now
 			})
 			defer httpServer.Close()

server/handlers.go

@@ -505,7 +505,7 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
 
 	email := claims.Email
 	if !claims.EmailVerified {
-		email = email + " (unverified)"
+		email += " (unverified)"
 	}
 
 	s.logger.Infof("login successful: connector %q, username=%q, preferred_username=%q, email=%q, groups=%q",
@@ -518,7 +518,8 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
 	}
 
 	// Try to retrieve an existing OfflineSession object for the corresponding user.
-	if session, err := s.storage.GetOfflineSessions(identity.UserID, authReq.ConnectorID); err != nil {
+	session, err := s.storage.GetOfflineSessions(identity.UserID, authReq.ConnectorID)
+	if err != nil {
 		if err != storage.ErrNotFound {
 			s.logger.Errorf("failed to get offline session: %v", err)
 			return "", err
@@ -536,7 +537,10 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
 			s.logger.Errorf("failed to create offline session: %v", err)
 			return "", err
 		}
-	} else {
+
+		return returnURL, nil
+	}
+
 	// Update existing OfflineSession obj with new RefreshTokenRef.
 	if err := s.storage.UpdateOfflineSessions(session.UserID, session.ConnID, func(old storage.OfflineSessions) (storage.OfflineSessions, error) {
 		if len(identity.ConnectorData) > 0 {
@@ -547,7 +551,6 @@ func (s *Server) finalizeLogin(identity connector.Identity, authReq storage.Auth
 		s.logger.Errorf("failed to update offline session: %v", err)
 		return "", err
 	}
-	}
 
 	return returnURL, nil
 }
@@ -1017,15 +1020,18 @@ func (s *Server) handleRefreshToken(w http.ResponseWriter, r *http.Request, clie
 	}
 
 	var connectorData []byte
-	if session, err := s.storage.GetOfflineSessions(refresh.Claims.UserID, refresh.ConnectorID); err != nil {
+
+	session, err := s.storage.GetOfflineSessions(refresh.Claims.UserID, refresh.ConnectorID)
+	switch {
+	case err != nil:
 		if err != storage.ErrNotFound {
 			s.logger.Errorf("failed to get offline session: %v", err)
 			return
 		}
-	} else if len(refresh.ConnectorData) > 0 {
+	case len(refresh.ConnectorData) > 0:
 		// Use the old connector data if it exists, should be deleted once used
 		connectorData = refresh.ConnectorData
-	} else {
+	default:
 		connectorData = session.ConnectorData
 	}
 

server/server.go

@@ -305,7 +305,7 @@ func newServer(ctx context.Context, c Config, rotationStrategy rotationStrategy)
 		}
 		r.Handle(path.Join(issuerURL.Path, p), instrumentHandlerCounter(p, handler))
 	}
-	r.NotFoundHandler = http.HandlerFunc(http.NotFound)
+	r.NotFoundHandler = http.NotFoundHandler()
 
 	discoveryHandler, err := s.discoveryHandler()
 	if err != nil {

server/server_test.go

@@ -177,7 +177,7 @@ func TestDiscovery(t *testing.T) {
 	defer cancel()
 
 	httpServer, _ := newTestServer(ctx, t, func(c *Config) {
-		c.Issuer = c.Issuer + "/non-root-path"
+		c.Issuer += "/non-root-path"
 	})
 	defer httpServer.Close()
 
@@ -504,7 +504,7 @@ func TestOAuth2CodeFlow(t *testing.T) {
 
 			// Setup a dex server.
 			httpServer, s := newTestServer(ctx, t, func(c *Config) {
-				c.Issuer = c.Issuer + "/non-root-path"
+				c.Issuer += "/non-root-path"
 				c.Now = now
 				c.IDTokensValidFor = idTokensValidFor
 			})
@@ -766,7 +766,7 @@ func TestCrossClientScopes(t *testing.T) {
 	defer cancel()
 
 	httpServer, s := newTestServer(ctx, t, func(c *Config) {
-		c.Issuer = c.Issuer + "/non-root-path"
+		c.Issuer += "/non-root-path"
 	})
 	defer httpServer.Close()
 
@@ -889,7 +889,7 @@ func TestCrossClientScopesWithAzpInAudienceByDefault(t *testing.T) {
 	defer cancel()
 
 	httpServer, s := newTestServer(ctx, t, func(c *Config) {
-		c.Issuer = c.Issuer + "/non-root-path"
+		c.Issuer += "/non-root-path"
 	})
 	defer httpServer.Close()
 
@@ -1180,7 +1180,7 @@ type oauth2Client struct {
 // that only valid refresh tokens can be used to refresh an expired token.
 func TestRefreshTokenFlow(t *testing.T) {
 	state := "state"
-	now := func() time.Time { return time.Now() }
+	now := time.Now
 	ctx, cancel := context.WithCancel(context.Background())
 	defer cancel()
 
@@ -1300,7 +1300,7 @@ func TestOAuth2DeviceFlow(t *testing.T) {
 
 			// Setup a dex server.
 			httpServer, s := newTestServer(ctx, t, func(c *Config) {
-				c.Issuer = c.Issuer + "/non-root-path"
+				c.Issuer += "/non-root-path"
 				c.Now = now
 				c.IDTokensValidFor = idTokensValidFor
 			})

storage/conformance/conformance.go

@@ -763,11 +763,9 @@ func testGC(t *testing.T, s storage.Storage) {
 		result, err := s.GarbageCollect(expiry.Add(-time.Hour).In(tz))
 		if err != nil {
 			t.Errorf("garbage collection failed: %v", err)
-		} else {
-			if result.AuthCodes != 0 || result.AuthRequests != 0 {
+		} else if result.AuthCodes != 0 || result.AuthRequests != 0 {
 			t.Errorf("expected no garbage collection results, got %#v", result)
 		}
-		}
 		if _, err := s.GetAuthCode(c.ID); err != nil {
 			t.Errorf("expected to be able to get auth code after GC: %v", err)
 		}
@@ -815,11 +813,9 @@ func testGC(t *testing.T, s storage.Storage) {
 		result, err := s.GarbageCollect(expiry.Add(-time.Hour).In(tz))
 		if err != nil {
 			t.Errorf("garbage collection failed: %v", err)
-		} else {
-			if result.AuthCodes != 0 || result.AuthRequests != 0 {
+		} else if result.AuthCodes != 0 || result.AuthRequests != 0 {
 			t.Errorf("expected no garbage collection results, got %#v", result)
 		}
-		}
 		if _, err := s.GetAuthRequest(a.ID); err != nil {
 			t.Errorf("expected to be able to get auth request after GC: %v", err)
 		}
@@ -859,11 +855,9 @@ func testGC(t *testing.T, s storage.Storage) {
 		result, err := s.GarbageCollect(expiry.Add(-time.Hour).In(tz))
 		if err != nil {
 			t.Errorf("garbage collection failed: %v", err)
-		} else {
-			if result.DeviceRequests != 0 {
+		} else if result.DeviceRequests != 0 {
 			t.Errorf("expected no device garbage collection results, got %#v", result)
 		}
-		}
 		if _, err := s.GetDeviceRequest(d.UserCode); err != nil {
 			t.Errorf("expected to be able to get auth request after GC: %v", err)
 		}
@@ -897,11 +891,9 @@ func testGC(t *testing.T, s storage.Storage) {
 		result, err := s.GarbageCollect(expiry.Add(-time.Hour).In(tz))
 		if err != nil {
 			t.Errorf("garbage collection failed: %v", err)
-		} else {
-			if result.DeviceTokens != 0 {
+		} else if result.DeviceTokens != 0 {
 			t.Errorf("expected no device token garbage collection results, got %#v", result)
 		}
-		}
 		if _, err := s.GetDeviceToken(dt.DeviceCode); err != nil {
 			t.Errorf("expected to be able to get device token after GC: %v", err)
 		}

storage/kubernetes/k8sapi/client.go

@@ -24,7 +24,7 @@ type Config struct {
 	// Legacy field from pkg/api/types.go TypeMeta.
 	// TODO(jlowdermilk): remove this after eliminating downstream dependencies.
 	Kind string `json:"kind,omitempty"`
-	// DEPRECATED: APIVersion is the preferred api version for communicating with the kubernetes cluster (v1, v2, etc).
+	// Deprecated: APIVersion is the preferred api version for communicating with the kubernetes cluster (v1, v2, etc).
 	// Because a cluster can run multiple API groups and potentially multiple versions of each, it no longer makes sense to specify
 	// a single value for the cluster version.
 	// This field isn't really needed anyway, so we are deprecating it without replacement.

storage/sql/config.go

@@ -289,16 +289,19 @@ func (s *MySQL) open(logger log.Logger) (*conn, error) {
 			cfg.Addr = s.Host
 		}
 	}
-	if s.SSL.CAFile != "" || s.SSL.CertFile != "" || s.SSL.KeyFile != "" {
+
+	switch {
+	case s.SSL.CAFile != "" || s.SSL.CertFile != "" || s.SSL.KeyFile != "":
 		if err := s.makeTLSConfig(); err != nil {
 			return nil, fmt.Errorf("failed to make TLS config: %v", err)
 		}
 		cfg.TLSConfig = mysqlSSLCustom
-	} else if s.SSL.Mode == "" {
+	case s.SSL.Mode == "":
 		cfg.TLSConfig = mysqlSSLTrue
-	} else {
+	default:
 		cfg.TLSConfig = s.SSL.Mode
 	}
+
 	for k, v := range s.params {
 		cfg.Params[k] = v
 	}