Enable Lint Rules: early-return & indent-error-flow (jaegertracing#5526)

## Which problem is this PR solving?
-  Partial Fix for jaegertracing#5506

## Description of the changes
- Enabled  early-return & indent-error-flow in revive linter
- Simplified if-else blocks

## How was this change tested?
- `make lint` `make test`

## Checklist
- [x] I have read
https://github.com/jaegertracing/jaeger/blob/master/CONTRIBUTING_GUIDELINES.md
- [x] I have signed all commits
- [ ] I have added unit tests for the new functionality
- [x] I have run lint and test steps successfully
  - for `jaeger`: `make lint test`
  - for `jaeger-ui`: `yarn lint` and `yarn test`

Signed-off-by: FlamingSaint <raghuramkannan400@gmail.com>
Co-authored-by: Yuri Shkuro <yurishkuro@users.noreply.github.com>

.golangci.yml

@@ -204,9 +204,6 @@ linters-settings:
         # maybe enable, needs invesitgation of the impact
         - name: get-return
           disabled: true
-        # enable after cleanup
-        - name: early-return
-          disabled: true
         # investigate, could be real bugs. But didn't recent Go version changed loop variables semantics?
         - name: range-val-address
           disabled: true
@@ -223,9 +220,6 @@ linters-settings:
         - name: nested-structs
           disabled: true
         # enable after cleanup
-        - name: indent-error-flow
-          disabled: true
-        # enable after cleanup
         - name: unexported-return
           disabled: true
         # looks useful, but requires refactoring: "calls to log.Fatal only in main() or init() functions"

cmd/agent/app/reporter/grpc/flags.go

@@ -51,11 +51,11 @@ func (b *ConnBuilder) InitFromViper(v *viper.Viper) (*ConnBuilder, error) {
 		b.CollectorHostPorts = strings.Split(hostPorts, ",")
 	}
 	b.MaxRetry = uint(v.GetInt(retry))
-	if tls, err := tlsFlagsConfig.InitFromViper(v); err == nil {
-		b.TLS = tls
-	} else {
+	tls, err := tlsFlagsConfig.InitFromViper(v)
+	if err != nil {
 		return b, fmt.Errorf("failed to process TLS options: %w", err)
 	}
+	b.TLS = tls
 	b.DiscoveryMinPeers = v.GetInt(discoveryMinPeers)
 	return b, nil
 }

cmd/collector/app/flags/flags.go

@@ -239,11 +239,11 @@ func (opts *HTTPOptions) initFromViper(v *viper.Viper, logger *zap.Logger, cfg s
 	opts.IdleTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPIdleTimeout)
 	opts.ReadTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadTimeout)
 	opts.ReadHeaderTimeout = v.GetDuration(cfg.prefix + "." + flagSuffixHTTPReadHeaderTimeout)
-	if tlsOpts, err := cfg.tls.InitFromViper(v); err == nil {
-		opts.TLS = tlsOpts
-	} else {
+	tlsOpts, err := cfg.tls.InitFromViper(v)
+	if err != nil {
 		return fmt.Errorf("failed to parse HTTP TLS options: %w", err)
 	}
+	opts.TLS = tlsOpts
 	return nil
 }
 
@@ -252,11 +252,11 @@ func (opts *GRPCOptions) initFromViper(v *viper.Viper, logger *zap.Logger, cfg s
 	opts.MaxReceiveMessageLength = v.GetInt(cfg.prefix + "." + flagSuffixGRPCMaxReceiveMessageLength)
 	opts.MaxConnectionAge = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAge)
 	opts.MaxConnectionAgeGrace = v.GetDuration(cfg.prefix + "." + flagSuffixGRPCMaxConnectionAgeGrace)
-	if tlsOpts, err := cfg.tls.InitFromViper(v); err == nil {
-		opts.TLS = tlsOpts
-	} else {
+	tlsOpts, err := cfg.tls.InitFromViper(v)
+	if err != nil {
 		return fmt.Errorf("failed to parse gRPC TLS options: %w", err)
 	}
+	opts.TLS = tlsOpts
 	opts.Tenancy = tenancy.InitFromViper(v)
 
 	return nil
@@ -289,11 +289,11 @@ func (cOpts *CollectorOptions) InitFromViper(v *viper.Viper, logger *zap.Logger)
 
 	cOpts.Zipkin.KeepAlive = v.GetBool(flagZipkinKeepAliveEnabled)
 	cOpts.Zipkin.HTTPHostPort = ports.FormatHostPort(v.GetString(flagZipkinHTTPHostPort))
-	if tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v); err == nil {
-		cOpts.Zipkin.TLS = tlsZipkin
-	} else {
+	tlsZipkin, err := tlsZipkinFlagsConfig.InitFromViper(v)
+	if err != nil {
 		return cOpts, fmt.Errorf("failed to parse Zipkin TLS options: %w", err)
 	}
+	cOpts.Zipkin.TLS = tlsZipkin
 	cOpts.Zipkin.CORS = corsZipkinFlags.InitFromViper(v)
 
 	return cOpts, nil

cmd/collector/app/span_processor_test.go

@@ -607,11 +607,10 @@ func TestStartDynQueueSizeUpdater(t *testing.T) {
 
 	// we wait up to 50 milliseconds
 	for i := 0; i < 5; i++ {
-		if p.queue.Capacity() == 100 {
-			time.Sleep(10 * time.Millisecond)
-		} else {
+		if p.queue.Capacity() != 100 {
 			break
 		}
+		time.Sleep(10 * time.Millisecond)
 	}
 
 	assert.EqualValues(t, 104857, p.queue.Capacity())

cmd/es-rollover/app/init/action.go

@@ -62,17 +62,16 @@ func (c Action) Do() error {
 		return err
 	}
 	if c.Config.UseILM {
-		if version >= ilmVersionSupport {
-			policyExist, err := c.ILMClient.Exists(c.Config.ILMPolicyName)
-			if err != nil {
-				return err
-			}
-			if !policyExist {
-				return fmt.Errorf("ILM policy %s doesn't exist in Elasticsearch. Please create it and re-run init", c.Config.ILMPolicyName)
-			}
-		} else {
+		if version < ilmVersionSupport {
 			return fmt.Errorf("ILM is supported only for ES version 7+")
 		}
+		policyExist, err := c.ILMClient.Exists(c.Config.ILMPolicyName)
+		if err != nil {
+			return err
+		}
+		if !policyExist {
+			return fmt.Errorf("ILM policy %s doesn't exist in Elasticsearch. Please create it and re-run init", c.Config.ILMPolicyName)
+		}
 	}
 	rolloverIndices := app.RolloverIndices(c.Config.Archive, c.Config.SkipDependencies, c.Config.AdaptiveSampling, c.Config.IndexPrefix)
 	for _, indexName := range rolloverIndices {

cmd/ingester/app/consumer/offset/concurrent_list.go

@@ -76,11 +76,10 @@ func getHighestContiguous(offsets []int64) int64 {
 
 	highestContiguous := minOffset
 	for {
-		if _, ok := offsetSet[highestContiguous+1]; ok {
-			highestContiguous++
-		} else {
+		if _, ok := offsetSet[highestContiguous+1]; !ok {
 			break
 		}
+		highestContiguous++
 	}
 
 	return highestContiguous

cmd/internal/flags/service.go

@@ -84,16 +84,16 @@ func (s *Service) Start(v *viper.Viper) error {
 	sFlags := new(SharedFlags).InitFromViper(v)
 	newProdConfig := zap.NewProductionConfig()
 	newProdConfig.Sampling = nil
-	if logger, err := sFlags.NewLogger(newProdConfig); err == nil {
-		s.Logger = logger
-		grpcZap.ReplaceGrpcLoggerV2(logger.WithOptions(
-			// grpclog is not consistent with the depth of call tree before it's dispatched to zap,
-			// but Skip(2) still shows grpclog as caller, while Skip(3) shows actual grpc packages.
-			zap.AddCallerSkip(3),
-		))
-	} else {
+	logger, err := sFlags.NewLogger(newProdConfig)
+	if err != nil {
 		return fmt.Errorf("cannot create logger: %w", err)
 	}
+	s.Logger = logger
+	grpcZap.ReplaceGrpcLoggerV2(logger.WithOptions(
+		// grpclog is not consistent with the depth of call tree before it's dispatched to zap,
+		// but Skip(2) still shows grpclog as caller, while Skip(3) shows actual grpc packages.
+		zap.AddCallerSkip(3),
+	))
 
 	metricsBuilder := new(metricsbuilder.Builder).InitFromViper(v)
 	metricsFactory, err := metricsBuilder.CreateMetricsFactory("")

cmd/jaeger/internal/extension/jaegerstorage/config.go

@@ -36,7 +36,6 @@ func (cfg *Config) Validate() error {
 	emptyCfg := createDefaultConfig().(*Config)
 	if reflect.DeepEqual(*cfg, *emptyCfg) {
 		return fmt.Errorf("%s: no storage type present in config", ID)
-	} else {
-		return nil
 	}
+	return nil
 }

cmd/query/app/flags.go

@@ -122,16 +122,16 @@ func AddFlags(flagSet *flag.FlagSet) {
 func (qOpts *QueryOptions) InitFromViper(v *viper.Viper, logger *zap.Logger) (*QueryOptions, error) {
 	qOpts.HTTPHostPort = v.GetString(queryHTTPHostPort)
 	qOpts.GRPCHostPort = v.GetString(queryGRPCHostPort)
-	if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil {
-		qOpts.TLSGRPC = tlsGrpc
-	} else {
+	tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v)
+	if err != nil {
 		return qOpts, fmt.Errorf("failed to process gRPC TLS options: %w", err)
 	}
-	if tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v); err == nil {
-		qOpts.TLSHTTP = tlsHTTP
-	} else {
+	qOpts.TLSGRPC = tlsGrpc
+	tlsHTTP, err := tlsHTTPFlagsConfig.InitFromViper(v)
+	if err != nil {
 		return qOpts, fmt.Errorf("failed to process HTTP TLS options: %w", err)
 	}
+	qOpts.TLSHTTP = tlsHTTP
 	qOpts.BasePath = v.GetString(queryBasePath)
 	qOpts.StaticAssets.Path = v.GetString(queryStaticFiles)
 	qOpts.StaticAssets.LogAccess = v.GetBool(queryLogStaticAssetsAccess)

cmd/query/app/query_parser.go

@@ -165,11 +165,11 @@ func (p *queryParser) parseTraceQueryParams(r *http.Request) (*traceQueryParamet
 
 	var traceIDs []model.TraceID
 	for _, id := range r.Form[traceIDParam] {
-		if traceID, err := model.TraceIDFromString(id); err == nil {
-			traceIDs = append(traceIDs, traceID)
-		} else {
+		traceID, err := model.TraceIDFromString(id)
+		if err != nil {
 			return nil, fmt.Errorf("cannot parse traceID param: %w", err)
 		}
+		traceIDs = append(traceIDs, traceID)
 	}
 
 	traceQuery := &traceQueryParameters{
@@ -360,11 +360,11 @@ func parseSpanKinds(r *http.Request, paramName string, defaultSpanKinds []string
 func mapSpanKindsToOpenTelemetry(spanKinds []string) ([]string, error) {
 	otelSpanKinds := make([]string, len(spanKinds))
 	for i, spanKind := range spanKinds {
-		if v, ok := jaegerToOtelSpanKind[spanKind]; ok {
-			otelSpanKinds[i] = v
-		} else {
+		v, ok := jaegerToOtelSpanKind[spanKind]
+		if !ok {
 			return otelSpanKinds, fmt.Errorf("unsupported span kind: '%s'", spanKind)
 		}
+		otelSpanKinds[i] = v
 	}
 	return otelSpanKinds, nil
 }
@@ -385,11 +385,10 @@ func (*queryParser) parseTags(simpleTags []string, jsonTags []string) (map[strin
 	retMe := make(map[string]string)
 	for _, tag := range simpleTags {
 		keyAndValue := strings.Split(tag, ":")
-		if l := len(keyAndValue); l > 1 {
-			retMe[keyAndValue[0]] = strings.Join(keyAndValue[1:], ":")
-		} else {
+		if l := len(keyAndValue); l <= 1 {
 			return nil, fmt.Errorf("malformed 'tag' parameter, expecting key:value, received: %s", tag)
 		}
+		retMe[keyAndValue[0]] = strings.Join(keyAndValue[1:], ":")
 	}
 	for _, tags := range jsonTags {
 		var fromJSON map[string]string

cmd/remote-storage/app/flags.go

@@ -54,11 +54,11 @@ func AddFlags(flagSet *flag.FlagSet) {
 // InitFromViper initializes Options with properties from CLI flags.
 func (o *Options) InitFromViper(v *viper.Viper, logger *zap.Logger) (*Options, error) {
 	o.GRPCHostPort = v.GetString(flagGRPCHostPort)
-	if tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v); err == nil {
-		o.TLSGRPC = tlsGrpc
-	} else {
+	tlsGrpc, err := tlsGRPCFlagsConfig.InitFromViper(v)
+	if err != nil {
 		return o, fmt.Errorf("failed to process gRPC TLS options: %w", err)
 	}
+	o.TLSGRPC = tlsGrpc
 	o.Tenancy = tenancy.InitFromViper(v)
 	return o, nil
 }

pkg/es/client/index_client.go

@@ -244,12 +244,10 @@ func (i IndicesClient) version() (uint, error) {
 // CreateTemplate an ES index template
 func (i IndicesClient) CreateTemplate(template, name string) error {
 	endpointFmt := "_template/%s"
-	if v, err := i.version(); err == nil {
-		if v >= 8 {
-			endpointFmt = "_index_template/%s"
-		}
-	} else {
+	if v, err := i.version(); err != nil {
 		return err
+	} else if v >= 8 {
+		endpointFmt = "_index_template/%s"
 	}
 	_, err := i.request(elasticRequest{
 		endpoint: fmt.Sprintf(endpointFmt, name),

pkg/fswatcher/fswatcher.go

@@ -86,11 +86,11 @@ func (w *FSWatcher) setupWatchedPaths(filepaths []string) error {
 		if p == "" {
 			continue
 		}
-		if h, err := hashFile(p); err == nil {
-			w.fileHashContentMap[p] = h
-		} else {
+		h, err := hashFile(p)
+		if err != nil {
 			return err
 		}
+		w.fileHashContentMap[p] = h
 		dir := path.Dir(p)
 		if _, ok := uniqueDirs[dir]; !ok {
 			if err := w.watcher.Add(dir); err != nil {

pkg/queue/bounded_queue_test.go

@@ -80,11 +80,10 @@ func helper(t *testing.T, startConsumers func(q *BoundedQueue, consumerFn func(i
 	q.StartLengthReporting(time.Millisecond, gauge)
 	for i := 0; i < 1000; i++ {
 		_, g := mFact.Snapshot()
-		if g["size"] == 0 {
-			time.Sleep(time.Millisecond)
-		} else {
+		if g["size"] != 0 {
 			break
 		}
+		time.Sleep(time.Millisecond)
 	}
 
 	c, g := mFact.Snapshot()

plugin/storage/cassandra/factory.go

@@ -155,11 +155,11 @@ func (f *Factory) Initialize(metricsFactory metrics.Factory, logger *zap.Logger)
 	f.primarySession = primarySession
 
 	if f.archiveConfig != nil {
-		if archiveSession, err := f.archiveConfig.NewSession(logger); err == nil {
-			f.archiveSession = archiveSession
-		} else {
+		archiveSession, err := f.archiveConfig.NewSession(logger)
+		if err != nil {
 			return err
 		}
+		f.archiveSession = archiveSession
 	} else {
 		logger.Info("Cassandra archive storage configuration is empty, skipping")
 	}

plugin/storage/grpc/shared/grpc_handler.go

@@ -156,9 +156,8 @@ func (s *GRPCHandler) Close(ctx context.Context, r *storage_v1.CloseWriterReques
 		}
 
 		return &storage_v1.CloseWriterResponse{}, nil
-	} else {
-		return nil, status.Error(codes.Unimplemented, "span writer does not support graceful shutdown")
 	}
+	return nil, status.Error(codes.Unimplemented, "span writer does not support graceful shutdown")
 }
 
 // GetTrace takes a traceID and streams a Trace associated with that traceID