Use already enabled revive linter and add depguard (open-telemetry#2883)

* Refactor golangci-lint conf Order settings alphabetically. * Add revive settings to golangci conf * Check blank imports * Check bool-literal-in-expr * Check constant-logical-expr * Check context-as-argument * Check context-key-type * Check deep-exit * Check defer * Check dot-imports * Check duplicated-imports * Check early-return * Check empty-block * Check empty-lines * Check error-naming * Check error-return * Check error-strings * Check errorf * Stop ignoring context first arg in tests * Check exported comments * Check flag-parameter * Check identical branches * Check if-return * Check increment-decrement * Check indent-error-flow * Check deny list of go imports * Check import shadowing * Check package comments * Check range * Check range val in closure * Check range val address * Check redefines builtin id * Check string-format * Check struct tag * Check superfluous else * Check time equal * Check var naming * Check var declaration * Check unconditional recursion * Check unexported return * Check unhandled errors * Check unnecessary stmt * Check unnecessary break * Check waitgroup by value * Exclude deep-exit check in example*_test.go files
tiger0105 · May 19, 2022 · 1f5b159 · 1f5b159
1 parent c5809aa
commit 1f5b159
Show file tree

Hide file tree

Showing 113 changed files with 767 additions and 449 deletions.
diff --git a/.golangci.yml b/.golangci.yml
@@ -10,12 +10,13 @@ linters:
   # Specifically enable linters we want to use.
   enable:
     - deadcode
+    - depguard
     - errcheck
+    - godot
     - gofmt
     - goimports
     - gosimple
     - govet
-    - godot
     - ineffassign
     - misspell
     - revive
@@ -25,30 +26,221 @@ linters:
     - unused
     - varcheck
 
-
 issues:
+  # Maximum issues count per one linter.
+  # Set to 0 to disable.
+  # Default: 50
+  # Setting to unlimited so the linter only is run once to debug all issues.
+  max-issues-per-linter: 0
+  # Maximum count of issues with the same text.
+  # Set to 0 to disable.
+  # Default: 3
+  # Setting to unlimited so the linter only is run once to debug all issues.
+  max-same-issues: 0
+  # Excluding configuration per-path, per-linter, per-text and per-source.
   exclude-rules:
-    # helpers in tests often (rightfully) pass a *testing.T as their first argument
-    - path: _test\.go
-      text: "context.Context should be the first parameter of a function"
+    # TODO: Having appropriate comments for exported objects helps development,
+    # even for objects in internal packages. Appropriate comments for all
+    # exported objects should be added and this exclusion removed.
+    - path: '.*internal/.*'
+      text: "exported (method|function|type|const) (.+) should have comment or be unexported"
       linters:
         - revive
-    # Yes, they are, but it's okay in a test
+    # 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:
         - revive
+    # Example test functions should be treated like main.
+    - path: example.*_test\.go
+      text: "calls to (.+) only in main[(][)] or init[(][)] functions"
+      linters:
+        - revive
+  include:
+    # revive exported should have comment or be unexported.
+    - EXC0012
+    # revive package comment should be of the form ...
+    - EXC0013
 
 linters-settings:
-  misspell:
-    locale: US
-    ignore-words:
-      - cancelled
-  goimports:
-    local-prefixes: go.opentelemetry.io
+  depguard:
+    # Check the list against standard lib.
+    # Default: false
+    include-go-root: true
+    # A list of packages for the list type specified.
+    # Default: []
+    packages:
+      - "crypto/md5"
+      - "crypto/sha1"
+      - "crypto/**/pkix"
+    ignore-file-rules:
+      - "**/*_test.go"
+    additional-guards:
+      # Do not allow testing packages in non-test files.
+      - list-type: denylist
+        include-go-root: true
+        packages:
+          - testing
+          - github.com/stretchr/testify
+        ignore-file-rules:
+          - "**/*_test.go"
+          - "**/*test/*.go"
+          - "**/internal/matchers/*.go"
   godot:
     exclude:
       # Exclude sentence fragments for lists.
       - '^[ ]*[-•]'
       # Exclude sentences prefixing a list.
       - ':$'
+  goimports:
+    local-prefixes: go.opentelemetry.io
+  misspell:
+    locale: US
+    ignore-words:
+      - cancelled
+  revive:
+    # Sets the default failure confidence.
+    # This means that linting errors with less than 0.8 confidence will be ignored.
+    # Default: 0.8
+    confidence: 0.01
+    rules:
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#blank-imports
+      - name: blank-imports
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#bool-literal-in-expr
+      - name: bool-literal-in-expr
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#constant-logical-expr
+      - name: constant-logical-expr
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#context-as-argument
+      - name: context-as-argument
+        disabled: false
+        arguments:
+          allowTypesBefore: "*testing.T"
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#context-keys-type
+      - name: context-keys-type
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#deep-exit
+      - name: deep-exit
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#defer
+      - name: defer
+        disabled: false
+        arguments:
+          - ["call-chain", "loop"]
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#dot-imports
+      - name: dot-imports
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#duplicated-imports
+      - name: duplicated-imports
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#early-return
+      - name: early-return
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#empty-block
+      - name: empty-block
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#empty-lines
+      - name: empty-lines
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#error-naming
+      - name: error-naming
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#error-return
+      - name: error-return
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#error-strings
+      - name: error-strings
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#errorf
+      - name: errorf
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#exported
+      - name: exported
+        disabled: false
+        arguments:
+          - "sayRepetitiveInsteadOfStutters"
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#flag-parameter
+      - name: flag-parameter
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#identical-branches
+      - name: identical-branches
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#if-return
+      - name: if-return
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#increment-decrement
+      - name: increment-decrement
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#indent-error-flow
+      - name: indent-error-flow
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#import-shadowing
+      - name: import-shadowing
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#package-comments
+      - name: package-comments
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#range
+      - name: range
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#range-val-in-closure
+      - name: range-val-in-closure
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#range-val-address
+      - name: range-val-address
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#redefines-builtin-id
+      - name: redefines-builtin-id
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#string-format
+      - name: string-format
+        disabled: false
+        arguments:
+          - - panic
+            - '/^[^\n]*$/'
+            - must not contain line breaks
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#struct-tag
+      - name: struct-tag
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#superfluous-else
+      - name: superfluous-else
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#time-equal
+      - name: time-equal
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#var-naming
+      - name: var-naming
+        disabled: false
+        arguments:
+          - ["ID"] # AllowList
+          - ["Otel", "Aws", "Gcp"] # DenyList
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#var-declaration
+      - name: var-declaration
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unconditional-recursion
+      - name: unconditional-recursion
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unexported-return
+      - name: unexported-return
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unhandled-error
+      - name: unhandled-error
+        disabled: false
+        arguments:
+          - "fmt.Fprint"
+          - "fmt.Fprintf"
+          - "fmt.Fprintln"
+          - "fmt.Print"
+          - "fmt.Printf"
+          - "fmt.Println"
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#unnecessary-stmt
+      - name: unnecessary-stmt
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#useless-break
+      - name: useless-break
+        disabled: false
+      # https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#waitgroup-by-value
+      - name: waitgroup-by-value
+        disabled: false
diff --git a/attribute/encoder.go b/attribute/encoder.go
@@ -133,9 +133,9 @@ func copyAndEscape(buf *bytes.Buffer, val string) {
 	for _, ch := range val {
 		switch ch {
 		case '=', ',', escapeChar:
-			buf.WriteRune(escapeChar)
+			_, _ = buf.WriteRune(escapeChar)
 		}
-		buf.WriteRune(ch)
+		_, _ = buf.WriteRune(ch)
 	}
 }
 

diff --git a/attribute/iterator_test.go b/attribute/iterator_test.go
@@ -56,7 +56,6 @@ func TestEmptyIterator(t *testing.T) {
 }
 
 func TestMergedIterator(t *testing.T) {
-
 	type inputs struct {
 		name   string
 		keys1  []string

diff --git a/attribute/set.go b/attribute/set.go
@@ -71,8 +71,8 @@ func EmptySet() *Set {
 	return emptySet
 }
 
-// reflect abbreviates reflect.ValueOf.
-func (d Distinct) reflect() reflect.Value {
+// reflectValue abbreviates reflect.ValueOf(d).
+func (d Distinct) reflectValue() reflect.Value {
 	return reflect.ValueOf(d.iface)
 }
 
@@ -86,15 +86,15 @@ func (l *Set) Len() int {
 	if l == nil || !l.equivalent.Valid() {
 		return 0
 	}
-	return l.equivalent.reflect().Len()
+	return l.equivalent.reflectValue().Len()
 }
 
 // Get returns the KeyValue at ordered position idx in this set.
 func (l *Set) Get(idx int) (KeyValue, bool) {
 	if l == nil {
 		return KeyValue{}, false
 	}
-	value := l.equivalent.reflect()
+	value := l.equivalent.reflectValue()
 
 	if idx >= 0 && idx < value.Len() {
 		// Note: The Go compiler successfully avoids an allocation for
@@ -110,7 +110,7 @@ func (l *Set) Value(k Key) (Value, bool) {
 	if l == nil {
 		return Value{}, false
 	}
-	rValue := l.equivalent.reflect()
+	rValue := l.equivalent.reflectValue()
 	vlen := rValue.Len()
 
 	idx := sort.Search(vlen, func(idx int) bool {

diff --git a/attribute/value.go b/attribute/value.go
@@ -25,7 +25,7 @@ import (
 //go:generate stringer -type=Type
 
 // Type describes the type of the data Value holds.
-type Type int
+type Type int // nolint: revive  // redefines builtin Type.
 
 // Value represents the value part in key-value pairs.
 type Value struct {

diff --git a/baggage/baggage.go b/baggage/baggage.go
@@ -68,6 +68,9 @@ type Property struct {
 	hasData bool
 }
 
+// NewKeyProperty returns a new Property for key.
+//
+// If key is invalid, an error will be returned.
 func NewKeyProperty(key string) (Property, error) {
 	if !keyRe.MatchString(key) {
 		return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key)
@@ -77,6 +80,9 @@ func NewKeyProperty(key string) (Property, error) {
 	return p, nil
 }
 
+// NewKeyValueProperty returns a new Property for key with value.
+//
+// If key or value are invalid, an error will be returned.
 func NewKeyValueProperty(key, value string) (Property, error) {
 	if !keyRe.MatchString(key) {
 		return newInvalidProperty(), fmt.Errorf("%w: %q", errInvalidKey, key)

diff --git a/baggage/baggage_test.go b/baggage/baggage_test.go
@@ -612,7 +612,7 @@ func TestBaggageMembers(t *testing.T) {
 		},
 	}
 
-	baggage := Baggage{list: baggage.List{
+	bag := Baggage{list: baggage.List{
 		"foo": {
 			Value: "1",
 			Properties: []baggage.Property{
@@ -626,13 +626,13 @@ func TestBaggageMembers(t *testing.T) {
 		},
 	}}
 
-	assert.ElementsMatch(t, members, baggage.Members())
+	assert.ElementsMatch(t, members, bag.Members())
 }
 
 func TestBaggageMember(t *testing.T) {
-	baggage := Baggage{list: baggage.List{"foo": {Value: "1"}}}
-	assert.Equal(t, Member{key: "foo", value: "1"}, baggage.Member("foo"))
-	assert.Equal(t, Member{}, baggage.Member("bar"))
+	bag := Baggage{list: baggage.List{"foo": {Value: "1"}}}
+	assert.Equal(t, Member{key: "foo", value: "1"}, bag.Member("foo"))
+	assert.Equal(t, Member{}, bag.Member("bar"))
 }
 
 func TestMemberKey(t *testing.T) {

diff --git a/bridge/opencensus/aggregation_test.go b/bridge/opencensus/aggregation_test.go
@@ -276,7 +276,7 @@ func TestHistogramAggregation(t *testing.T) {
 	if output.Kind() != aggregation.HistogramKind {
 		t.Errorf("recordAggregationsFromPoints(%v) = %v, want %v", input, output.Kind(), aggregation.HistogramKind)
 	}
-	if end != now {
+	if !end.Equal(now) {
 		t.Errorf("recordAggregationsFromPoints(%v).end() = %v, want %v", input, end, now)
 	}
 	distAgg, ok := output.(aggregation.Histogram)

diff --git a/bridge/opencensus/exporter.go b/bridge/opencensus/exporter.go
@@ -37,7 +37,7 @@ import (
 	"go.opentelemetry.io/otel/sdk/resource"
 )
 
-var errConversion = errors.New("Unable to convert from OpenCensus to OpenTelemetry")
+var errConversion = errors.New("unable to convert from OpenCensus to OpenTelemetry")
 
 // NewMetricExporter returns an OpenCensus exporter that exports to an
 // OpenTelemetry exporter.

diff --git a/bridge/opencensus/test/bridge_test.go b/bridge/opencensus/test/bridge_test.go
@@ -137,7 +137,6 @@ func TestToFromContext(t *testing.T) {
 		// Get the opentelemetry span using the OpenCensus FromContext, and end it
 		otSpan2 := octrace.FromContext(ctx)
 		defer otSpan2.End()
-
 	}()
 
 	spans := sr.Ended()