GODRIVER-1066 Use golangci-lint to replace and expand linters. (mongo…

…db#731)

.evergreen/config.yml

@@ -134,14 +134,10 @@ functions:
         script: |
           ${PREPARE_SHELL}
 
-          # Install any go tools that we need. Do this in a temp directory because running "go get" inside the driver
-          # repo will update the driver's go.mod/go.sum files. This puts the contents of go.mod/go.sum out of sync with
-          # the vendor directory and tests fail to compile due to inconsistent vendoring.
-          cd $(mktemp -d)
-          go get -u golang.org/x/lint/golint
-          go get -u github.com/kisielk/errcheck
-          go get -u github.com/walle/lll/...
-          cd -
+          # Install any go tools that we need. Use "go install" instead of "go get" to prevent
+          # modifying the go.mod file.
+          go install github.com/golangci/golangci-lint/cmd/golangci-lint@latest
+          go install github.com/walle/lll/...@latest
 
           # initialize submodules
           git submodule init
@@ -893,27 +889,13 @@ tasks:
         vars:
           targets: check-fmt
 
-  - name: sa-errcheck
-    tags: ["static-analysis"]
-    commands:
-      - func: run-make
-        vars:
-          targets: errcheck
-
   - name: sa-lint
     tags: ["static-analysis"]
     commands:
       - func: run-make
         vars:
           targets: lint
 
-  - name: sa-vet
-    tags: ["static-analysis"]
-    commands:
-      - func: run-make
-        vars:
-          targets: vet
-
   - name: perf
     tags: ["performance"]
     exec_timeout_secs: 7200

.golangci.yml

@@ -0,0 +1,64 @@
+run:
+  timeout: 5m
+
+linters:
+  disable-all: true
+  # TODO(GODRIVER-2156): Enable all commented-out linters.
+  enable:
+    # - deadcode
+    - errcheck
+    # - errorlint
+    - goimports
+    # - gosimple
+    - govet
+    - ineffassign
+    # - misspell
+    # - nakedret
+    - revive
+    - staticcheck
+    # - structcheck
+    - typecheck
+    # - unused
+    # - unconvert
+    # - unparam
+    # - varcheck
+
+linters-settings:
+  errcheck:
+    exclude: .errcheck-excludes
+  govet:
+    disable:
+      - cgocall
+      - composites
+  staticcheck:
+    checks: [
+      "all",
+      "-SA1019", # Disable deprecation warnings for now.
+      "-SA1012", # Disable "do not pass a nil Context" to allow testing nil contexts in tests.
+    ]
+
+issues:
+  exclude:
+    # Ignore capitalization warning for this weird field name.
+    - "var-naming: struct field CqCssWxW should be CqCSSWxW"
+    # Ignore warnings for common "wiremessage.Read..." usage because the safest way to use that API
+    # is by assigning possibly unused returned byte buffers.
+    - "SA4006: this value of `wm` is never used"
+    - "SA4006: this value of `rem` is never used"
+    - "ineffectual assignment to wm"
+    - "ineffectual assignment to rem"
+  skip-dirs-use-default: false
+  skip-dirs:
+    - (^|/)vendor($|/)
+    - (^|/)testdata($|/)
+    - (^|/)data($|/)
+    - (^|/)etc($|/)
+  exclude-rules:
+    - path: examples/
+      linters:
+        - revive
+        - errcheck
+    - path: x/mongo/driver/operation/
+      linters:
+        - staticcheck # TODO: Re-enable once GODRIVER-2154 is done.
+        - ineffassign # TODO: Re-enable once GODRIVER-2154 is done.

Makefile

@@ -17,7 +17,7 @@ ATLAS_URIS = "$(ATLAS_FREE)" "$(ATLAS_REPLSET)" "$(ATLAS_SHARD)" "$(ATLAS_TLS11)
 TEST_TIMEOUT = 1800
 
 .PHONY: default
-default: check-env check-fmt vet build-examples lint errcheck test-cover test-race
+default: check-env check-fmt build-examples lint test-cover test-race
 
 .PHONY: check-env
 check-env:
@@ -59,16 +59,7 @@ fmt:
 
 .PHONY: lint
 lint:
-	golint $(PKGS) | ./etc/lintscreen.pl .lint-allowlist
-
-.PHONY: lint-add-allowlist
-lint-add-allowlist:
-	golint $(PKGS) | ./etc/lintscreen.pl -u .lint-allowlist
-	sort .lint-allowlist -o .lint-allowlist
-
-.PHONY: errcheck
-errcheck:
-	errcheck -exclude .errcheck-excludes ./bson/... ./mongo/... ./x/...
+	golangci-lint run --config .golangci.yml ./...
 
 .PHONY: test
 test:
@@ -126,11 +117,6 @@ update-server-selection-tests:
 update-notices:
 	etc/generate-notices.pl > THIRD-PARTY-NOTICES
 
-.PHONY: vet
-vet:
-	go vet $(BUILD_TAGS) -cgocall=false -composites=false -unusedstringmethods="Error" $(PKGS)
-
-
 # Evergreen specific targets
 .PHONY: evg-test
 evg-test:

benchmark/single.go

@@ -62,6 +62,9 @@ func SingleRunCommand(ctx context.Context, tm TimerManager, iters int) error {
 		}
 		// read the document and then throw it away to prevent
 		out, err := doc.MarshalBSON()
+		if err != nil {
+			return err
+		}
 		if len(out) == 0 {
 			return errors.New("output of command is empty")
 		}

bson/bson_corpus_spec_test.go

@@ -160,6 +160,7 @@ func normalizeCanonicalDouble(t *testing.T, key string, cEJ string) string {
 	// Parse the float contained by the map.
 	expectedString := cEJMap[key]["$numberDouble"]
 	expectedFloat, err := strconv.ParseFloat(expectedString, 64)
+	require.NoError(t, err)
 
 	// Normalize the string
 	return fmt.Sprintf(`{"%s":{"$numberDouble":"%s"}}`, key, formatDouble(expectedFloat))

bson/bson_test.go

@@ -133,12 +133,12 @@ func (kb keyBool) MarshalKey() (string, error) {
 	return fmt.Sprintf("%v", kb), nil
 }
 
-func (kb keyBool) UnmarshalKey(key string) error {
+func (kb *keyBool) UnmarshalKey(key string) error {
 	switch key {
 	case "true":
-		kb = true
+		*kb = true
 	case "false":
-		kb = false
+		*kb = false
 	default:
 		return fmt.Errorf("invalid bool value %v", key)
 	}
@@ -169,12 +169,12 @@ func TestMapCodec(t *testing.T) {
 		}
 	})
 	t.Run("keys implements keyMarshaler and keyUnmarshaler", func(t *testing.T) {
-		mapObj := map[keyBool]int{keyBool(false): 1}
+		mapObj := map[keyBool]int{keyBool(true): 1}
 
 		doc, err := Marshal(mapObj)
 		assert.Nil(t, err, "Marshal error: %v", err)
 		idx, want := bsoncore.AppendDocumentStart(nil)
-		want = bsoncore.AppendInt32Element(want, "false", 1)
+		want = bsoncore.AppendInt32Element(want, "true", 1)
 		want, _ = bsoncore.AppendDocumentEnd(want, idx)
 		assert.Equal(t, want, doc, "expected result %v, got %v", string(want), string(doc))
 

bson/bsonrw/extjson_writer.go

@@ -10,7 +10,6 @@ import (
 	"bytes"
 	"encoding/base64"
 	"fmt"
-	"go.mongodb.org/mongo-driver/bson/primitive"
 	"io"
 	"math"
 	"sort"
@@ -19,6 +18,8 @@ import (
 	"sync"
 	"time"
 	"unicode/utf8"
+
+	"go.mongodb.org/mongo-driver/bson/primitive"
 )
 
 var ejvwPool = sync.Pool{

bson/primitive/objectid.go

@@ -35,7 +35,7 @@ var objectIDCounter = readRandomUint32()
 var processUnique = processUniqueBytes()
 
 var _ encoding.TextMarshaler = ObjectID{}
-var _ encoding.TextUnmarshaler = ObjectID{}
+var _ encoding.TextUnmarshaler = &ObjectID{}
 
 // NewObjectID generates a new ObjectID.
 func NewObjectID() ObjectID {
@@ -105,9 +105,13 @@ func (id ObjectID) MarshalText() ([]byte, error) {
 
 // UnmarshalText populates the byte slice with the ObjectID. Implementing this allows us to use ObjectID
 // as a map key when unmarshalling JSON. See https://pkg.go.dev/encoding#TextUnmarshaler
-func (id ObjectID) UnmarshalText(b []byte) error {
-	id, err := ObjectIDFromHex(string(b))
-	return err
+func (id *ObjectID) UnmarshalText(b []byte) error {
+	oid, err := ObjectIDFromHex(string(b))
+	if err != nil {
+		return err
+	}
+	*id = oid
+	return nil
 }
 
 // MarshalJSON returns the ObjectID as a string

bson/primitive/objectid_test.go

@@ -233,3 +233,20 @@ func TestObjectID_UnmarshalJSON(t *testing.T) {
 		})
 	}
 }
+
+func TestObjectID_MarshalText(t *testing.T) {
+	oid := ObjectID{0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xA, 0xB}
+	b, err := oid.MarshalText()
+	assert.Nil(t, err, "MarshalText error: %v", err)
+	want := "000102030405060708090a0b"
+	got := string(b)
+	assert.Equal(t, want, got, "want %v, got %v", want, got)
+}
+
+func TestObjectID_UnmarshalText(t *testing.T) {
+	var oid ObjectID
+	err := oid.UnmarshalText([]byte("000102030405060708090a0b"))
+	assert.Nil(t, err, "UnmarshalText error: %v", err)
+	want := ObjectID{0x0, 0x1, 0x2, 0x3, 0x4, 0x5, 0x6, 0x7, 0x8, 0x9, 0xA, 0xB}
+	assert.Equal(t, want, oid, "want %v, got %v", want, oid)
+}