Replaces golint by revive and fix newly reported linter issues (#1946)

* replaces golint by revive and fix newly reported linter issues

* add pull request ID to CHANGELOG.md

* Update internal/matchers/temporal_matcher.go

Co-authored-by: Robert Pająk <pellared@hotmail.com>

* fix pull request issues

* explains why the linter is disabled

* Update semconv/http.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update metric/unit/unit.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* restores 'example/passthrough/go.sum' to original state

* fix after rebase

* export ErrInvalidAsyncRunner again and add nolint

* Update trace/tracestate.go

Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>

* Update sdk/metric/sdk.go

Co-authored-by: Aaron Clawson <Aaron.Clawson@gmail.com>

* Fix ContextWithoutBaggage comment

* Fix SpanEndOption comment

Co-authored-by: Robert Pająk <pellared@hotmail.com>
Co-authored-by: Tyler Yahn <MrAlias@users.noreply.github.com>
Co-authored-by: Aaron Clawson <Aaron.Clawson@gmail.com>
Co-authored-by: Tyler Yahn <codingalias@gmail.com>

.golangci.yml

@@ -7,7 +7,7 @@ linters:
   enable:
     - misspell
     - goimports
-    - golint
+    - revive
     - gofmt
 
 issues:
@@ -16,12 +16,12 @@ issues:
     - path: _test\.go
       text: "context.Context should be the first parameter of a function"
       linters:
-        - golint
+        - revive
     # Yes, they are, but it's okay in a test
     - path: _test\.go
       text: "exported func.*returns unexported type.*which can be annoying to use"
       linters:
-        - golint
+        - revive
 
 linters-settings:
   misspell:

baggage/context.go

@@ -26,7 +26,7 @@ func ContextWithBaggage(parent context.Context, b Baggage) context.Context {
 	return baggage.ContextWithList(parent, b.list)
 }
 
-// ContextWithBaggage returns a copy of parent with no baggage.
+// ContextWithoutBaggage returns a copy of parent with no baggage.
 func ContextWithoutBaggage(parent context.Context) context.Context {
 	// Delegate so any hooks for the OpenTracing bridge are handled.
 	return baggage.ContextWithList(parent, nil)

bridge/opentracing/internal/mock.go

@@ -29,6 +29,7 @@ import (
 	"go.opentelemetry.io/otel/bridge/opentracing/migration"
 )
 
+//nolint:revive // ignoring missing comments for unexported global variables in an internal package.
 var (
 	ComponentKey     = attribute.Key("component")
 	ServiceKey       = attribute.Key("service")

exporters/otlp/internal/otlpconfig/tls.go

@@ -21,6 +21,7 @@ import (
 	"io/ioutil"
 )
 
+//nolint:revive // ignoring missing comments for unexported constants in an internal package.
 const (
 	WeakCertificate = `
 -----BEGIN CERTIFICATE-----

exporters/otlp/otlphttp/driver.go

@@ -49,7 +49,7 @@ const contentTypeJSON = "application/json"
 // have our own copy to avoid handling a situation where the
 // DefaultTransport is overwritten with some different implementation
 // of http.RoundTripper or it's modified by other package.
-var ourTransport *http.Transport = &http.Transport{
+var ourTransport = &http.Transport{
 	Proxy: http.ProxyFromEnvironment,
 	DialContext: (&net.Dialer{
 		Timeout:   30 * time.Second,

internal/matchers/temporal_matcher.go

@@ -16,6 +16,7 @@ package matchers
 
 type TemporalMatcher byte
 
+//nolint:revive // ignoring missing comments for unexported constants in an internal package
 const (
 	Before TemporalMatcher = iota
 	BeforeOrSameTime

internal/metric/async.go

@@ -25,6 +25,7 @@ import (
 	"go.opentelemetry.io/otel/metric"
 )
 
+//nolint:revive // ignoring missing comments for exported error in an internal package
 var ErrInvalidAsyncRunner = errors.New("unknown async runner type")
 
 // AsyncCollector is an interface used between the MeterImpl and the

metric/unit/unit.go

@@ -16,6 +16,7 @@ package unit // import "go.opentelemetry.io/otel/metric/unit"
 
 type Unit string
 
+// Units defined by OpenTelemetry.
 const (
 	Dimensionless Unit = "1"
 	Bytes         Unit = "By"

propagation/propagation_test.go

@@ -25,7 +25,7 @@ import (
 type ctxKeyType uint
 
 var (
-	ctxKey ctxKeyType = 0
+	ctxKey ctxKeyType
 )
 
 type carrier []string

sdk/export/metric/aggregation/aggregation.go

@@ -128,6 +128,7 @@ type (
 	Kind string
 )
 
+// Kind description constants.
 const (
 	SumKind            Kind = "Sum"
 	MinMaxSumCountKind Kind = "MinMaxSumCount"
@@ -136,6 +137,7 @@ const (
 	ExactKind          Kind = "Exact"
 )
 
+// Sentinel errors for Aggregation interface.
 var (
 	ErrNegativeInput    = fmt.Errorf("negative value is out of range for this instrument")
 	ErrNaNInput         = fmt.Errorf("NaN value is an invalid input")

sdk/export/metric/metric.go

@@ -67,9 +67,9 @@ type Processor interface {
 
 	// Process is called by the SDK once per internal record,
 	// passing the export Accumulation (a Descriptor, the corresponding
-	// Labels, and the checkpointed Aggregator).  This call has no
+	// Labels, and the checkpointed Aggregator). This call has no
 	// Context argument because it is expected to perform only
-	// computation.  An SDK is not expected to call exporters from
+	// computation. An SDK is not expected to call exporters from
 	// with Process, use a controller for that (see
 	// ./controllers/{pull,push}.
 	Process(accum Accumulation) error

sdk/export/metric/metrictest/test.go

@@ -120,7 +120,7 @@ func (p *CheckpointSet) ForEach(_ export.ExportKindSelector, f func(export.Recor
 	return nil
 }
 
-// Takes a slice of []some.Aggregator and returns a slice of []export.Aggregator
+// Unslice2 takes a slice of []some.Aggregator and returns a slice of []export.Aggregator
 func Unslice2(sl interface{}) (one, two export.Aggregator) {
 	slv := reflect.ValueOf(sl)
 	if slv.Type().Kind() != reflect.Slice {

sdk/metric/aggregator/aggregatortest/test.go

@@ -77,7 +77,7 @@ func RunProfiles(t *testing.T, f func(*testing.T, Profile)) {
 	}
 }
 
-// Ensure local struct alignment prior to running tests.
+// TestMain ensures local struct alignment prior to running tests.
 func TestMain(m *testing.M) {
 	fields := []ottest.FieldOffset{
 		{
@@ -148,7 +148,7 @@ func (n *Numbers) Points() []number.Number {
 	return n.numbers
 }
 
-// Performs the same range test the SDK does on behalf of the aggregator.
+// CheckedUpdate performs the same range test the SDK does on behalf of the aggregator.
 func CheckedUpdate(t *testing.T, agg export.Aggregator, number number.Number, descriptor *metric.Descriptor) {
 	ctx := context.Background()
 

sdk/metric/controller/basic/controller.go

@@ -196,12 +196,10 @@ func (c *Controller) collect(ctx context.Context) error {
 	if c.exporter == nil {
 		return nil
 	}
+
 	// Note: this is not subject to collectTimeout.  This blocks the next
 	// collection despite collectTimeout because it holds a lock.
-	if err := c.export(ctx); err != nil {
-		return err
-	}
-	return nil
+	return c.export(ctx)
 }
 
 // checkpoint calls the Accumulator and Checkpointer interfaces to

sdk/metric/processor/basic/basic.go

@@ -119,7 +119,11 @@ type (
 var _ export.Processor = &Processor{}
 var _ export.Checkpointer = &Processor{}
 var _ export.CheckpointSet = &state{}
+
+// ErrInconsistentState is returned when the sequence of collection's starts and finishes are incorrectly balanced.
 var ErrInconsistentState = fmt.Errorf("inconsistent processor state")
+
+// ErrInvalidExportKind is returned for unknown metric.ExportKind.
 var ErrInvalidExportKind = fmt.Errorf("invalid export kind")
 
 // New returns a basic Processor that is also a Checkpointer using the provided

sdk/metric/sdk.go

@@ -147,6 +147,7 @@ var (
 	_ metric.SyncImpl      = &syncInstrument{}
 	_ metric.BoundSyncImpl = &record{}
 
+	// ErrUninitializedInstrument is returned when an instrument is used when uninitialized.
 	ErrUninitializedInstrument = fmt.Errorf("use of an uninitialized instrument")
 )
 

sdk/resource/resource.go

@@ -38,7 +38,7 @@ type Resource struct {
 var (
 	emptyResource Resource
 
-	defaultResource *Resource = func(r *Resource, err error) *Resource {
+	defaultResource = func(r *Resource, err error) *Resource {
 		if err != nil {
 			otel.Handle(err)
 		}

sdk/trace/batch_span_processor.go

@@ -24,6 +24,7 @@ import (
 	"go.opentelemetry.io/otel"
 )
 
+// Defaults for BatchSpanProcessorOptions.
 const (
 	DefaultMaxQueueSize       = 2048
 	DefaultBatchTimeout       = 5000 * time.Millisecond

sdk/trace/sampling.go

@@ -91,7 +91,7 @@ func (ts traceIDRatioSampler) Description() string {
 // always sample. Fractions < 0 are treated as zero. To respect the
 // parent trace's `SampledFlag`, the `TraceIDRatioBased` sampler should be used
 // as a delegate of a `Parent` sampler.
-//nolint:golint // golint complains about stutter of `trace.TraceIDRatioBased`
+//nolint:revive // revive complains about stutter of `trace.TraceIDRatioBased`
 func TraceIDRatioBased(fraction float64) Sampler {
 	if fraction >= 1 {
 		return AlwaysSample()

sdk/trace/sampling_test.go

@@ -174,10 +174,10 @@ func TestParentBasedDefaultDescription(t *testing.T) {
 		NeverSample().Description())
 
 	if sampler.Description() != expectedDescription {
-		t.Error(fmt.Sprintf("Sampler description should be %s, got '%s' instead",
+		t.Errorf("Sampler description should be %s, got '%s' instead",
 			expectedDescription,
 			sampler.Description(),
-		))
+		)
 	}
 
 }

sdk/trace/trace_test.go

@@ -62,7 +62,7 @@ var (
 	sid trace.SpanID
 	sc  trace.SpanContext
 
-	handler *storingHandler = &storingHandler{}
+	handler = &storingHandler{}
 
 	k1, k2, k3    attribute.Key
 	kv1, kv2, kv3 attribute.KeyValue

semconv/http.go

@@ -25,6 +25,7 @@ import (
 	"go.opentelemetry.io/otel/codes"
 )
 
+// HTTP scheme attributes.
 var (
 	HTTPSchemeHTTP  = HTTPSchemeKey.String("http")
 	HTTPSchemeHTTPS = HTTPSchemeKey.String("https")

trace/config.go

@@ -129,8 +129,8 @@ func (fn spanOptionFunc) applySpanStart(cfg *SpanConfig) {
 	fn(cfg)
 }
 
-// SpanEndOptions applies an option to a SpanConfig. These options are applicable
-// only when the span is ended.
+// SpanEndOption applies an option to a SpanConfig. These options are
+// applicable only when the span is ended.
 type SpanEndOption interface {
 	applySpanEnd(*SpanConfig)
 }

trace/trace.go

@@ -45,7 +45,7 @@ func (e errorConst) Error() string {
 }
 
 // TraceID is a unique identity of a trace.
-// nolint:golint
+// nolint:revive // revive complains about stutter of `trace.TraceID`.
 type TraceID [16]byte
 
 var nilTraceID TraceID
@@ -94,7 +94,7 @@ func (s SpanID) String() string {
 // TraceIDFromHex returns a TraceID from a hex string if it is compliant with
 // the W3C trace-context specification.  See more at
 // https://www.w3.org/TR/trace-context/#trace-id
-// nolint:golint
+// nolint:revive // revive complains about stutter of `trace.TraceIDFromHex`.
 func TraceIDFromHex(h string) (TraceID, error) {
 	t := TraceID{}
 	if len(h) != 32 {
@@ -152,7 +152,7 @@ func decodeHex(h string, b []byte) error {
 }
 
 // TraceFlags contains flags that can be set on a SpanContext
-type TraceFlags byte //nolint:golint
+type TraceFlags byte //nolint:revive // revive complains about stutter of `trace.TraceFlags`.
 
 // IsSampled returns if the sampling bit is set in the TraceFlags.
 func (tf TraceFlags) IsSampled() bool {

trace/tracestate.go

@@ -88,7 +88,7 @@ func (m member) String() string {
 // that conform to the specification. Specifically, this means that all
 // list-member's key/value pairs are valid, no duplicate list-members exist,
 // and the maximum number of list-members (32) is not exceeded.
-type TraceState struct { //nolint:golint
+type TraceState struct { //nolint:revive // revive complains about stutter of `trace.TraceState`
 	// list is the members in order.
 	list []member
 }