Check for and fix suspicious append result assignments. (mongodb#983)

.golangci.yml

@@ -8,6 +8,7 @@ linters:
     - deadcode
     - errcheck
     # - errorlint
+    - gocritic
     - goimports
     - gosimple
     - gosec
@@ -29,6 +30,10 @@ linters:
 linters-settings:
   errcheck:
     exclude: .errcheck-excludes
+  gocritic:
+    enabled-checks:
+      # Detects suspicious append result assignments. E.g. "b := append(a, 1, 2, 3)"
+      - appendAssign
   govet:
     disable:
       - cgocall

bson/primitive/decimal_test.go

@@ -98,18 +98,20 @@ var bigIntTestCases = []bigIntTestCase{
 
 func TestDecimal128_BigInt(t *testing.T) {
 	for _, c := range bigIntTestCases {
-		switch c.remark {
-		case "NaN", "Infinity", "-Infinity":
-			d128 := NewDecimal128(c.h, c.l)
-			_, _, err := d128.BigInt()
-			require.Error(t, err, "case %s", c.s)
-		case "":
-			d128 := NewDecimal128(c.h, c.l)
-			bi, e, err := d128.BigInt()
-			require.NoError(t, err, "case %s", c.s)
-			require.Equal(t, 0, c.bi.Cmp(bi), "case %s e:%s a:%s", c.s, c.bi.String(), bi.String())
-			require.Equal(t, c.exp, e, "case %s", c.s, d128.String())
-		}
+		t.Run(c.s, func(t *testing.T) {
+			switch c.remark {
+			case "NaN", "Infinity", "-Infinity":
+				d128 := NewDecimal128(c.h, c.l)
+				_, _, err := d128.BigInt()
+				require.Error(t, err, "case %s", c.s)
+			case "":
+				d128 := NewDecimal128(c.h, c.l)
+				bi, e, err := d128.BigInt()
+				require.NoError(t, err, "case %s", c.s)
+				require.Equal(t, 0, c.bi.Cmp(bi), "case %s e:%s a:%s", c.s, c.bi.String(), bi.String())
+				require.Equal(t, c.exp, e, "case %s", c.s, d128.String())
+			}
+		})
 	}
 }
 
@@ -131,27 +133,30 @@ func TestParseDecimal128FromBigInt(t *testing.T) {
 }
 
 func TestParseDecimal128(t *testing.T) {
-	cases := append(bigIntTestCases,
-		[]bigIntTestCase{
-			{s: "-0001231.453454000000565600000000E-21", h: 0xafe6000003faa269, l: 0x81cfeceaabdb1800},
-			{s: "12345E+21", h: 0x306a000000000000, l: 12345},
-			{s: "0.10000000000000000000000000000000000000000001", remark: "parse fail"},
-			{s: ".125e1", h: 0x303c000000000000, l: 125},
-			{s: ".125", h: 0x303a000000000000, l: 125},
-			{s: "", remark: "parse fail"},
-		}...)
-	for _, c := range cases {
-		switch c.remark {
-		case "overflow", "parse fail":
-			_, err := ParseDecimal128(c.s)
-			require.Error(t, err)
-		case "", "rounding", "subnormal", "clamped", "NaN", "Infinity", "-Infinity":
-			d128, err := ParseDecimal128(c.s)
-			require.NoError(t, err)
+	cases := make([]bigIntTestCase, 0, len(bigIntTestCases))
+	cases = append(cases, bigIntTestCases...)
+	cases = append(cases,
+		bigIntTestCase{s: "-0001231.453454000000565600000000E-21", h: 0xafe6000003faa269, l: 0x81cfeceaabdb1800},
+		bigIntTestCase{s: "12345E+21", h: 0x306a000000000000, l: 12345},
+		bigIntTestCase{s: "0.10000000000000000000000000000000000000000001", remark: "parse fail"},
+		bigIntTestCase{s: ".125e1", h: 0x303c000000000000, l: 125},
+		bigIntTestCase{s: ".125", h: 0x303a000000000000, l: 125},
+		bigIntTestCase{s: "", remark: "parse fail"})
 
-			require.Equal(t, c.h, d128.h, "case %s", c.s, d128.l)
-			require.Equal(t, c.l, d128.l, "case %s", c.s, d128.h)
-		}
+	for _, c := range cases {
+		t.Run(c.s, func(t *testing.T) {
+			switch c.remark {
+			case "overflow", "parse fail":
+				_, err := ParseDecimal128(c.s)
+				require.Error(t, err)
+			case "", "rounding", "subnormal", "clamped", "NaN", "Infinity", "-Infinity":
+				d128, err := ParseDecimal128(c.s)
+				require.NoError(t, err)
+
+				require.Equal(t, c.h, d128.h, "case %s", c.s, d128.l)
+				require.Equal(t, c.l, d128.l, "case %s", c.s, d128.h)
+			}
+		})
 	}
 }
 
@@ -187,28 +192,31 @@ func TestDecimal128_JSON(t *testing.T) {
 		assert.Equal(t, want.l, got.l, "expected l: %v got: %v", want.l, got.l)
 	})
 	t.Run("unmarshal", func(t *testing.T) {
-		cases := append(bigIntTestCases,
-			[]bigIntTestCase{
-				{s: "-0001231.453454000000565600000000E-21", h: 0xafe6000003faa269, l: 0x81cfeceaabdb1800},
-				{s: "12345E+21", h: 0x306a000000000000, l: 12345},
-				{s: "0.10000000000000000000000000000000000000000001", remark: "parse fail"},
-				{s: ".125e1", h: 0x303c000000000000, l: 125},
-				{s: ".125", h: 0x303a000000000000, l: 125},
-			}...)
-		for _, c := range cases {
-			input := fmt.Sprintf(`{"foo": %q}`, c.s)
-			var got map[string]Decimal128
-			err := json.Unmarshal([]byte(input), &got)
+		cases := make([]bigIntTestCase, 0, len(bigIntTestCases))
+		cases = append(cases, bigIntTestCases...)
+		cases = append(cases,
+			bigIntTestCase{s: "-0001231.453454000000565600000000E-21", h: 0xafe6000003faa269, l: 0x81cfeceaabdb1800},
+			bigIntTestCase{s: "12345E+21", h: 0x306a000000000000, l: 12345},
+			bigIntTestCase{s: "0.10000000000000000000000000000000000000000001", remark: "parse fail"},
+			bigIntTestCase{s: ".125e1", h: 0x303c000000000000, l: 125},
+			bigIntTestCase{s: ".125", h: 0x303a000000000000, l: 125})
 
-			switch c.remark {
-			case "overflow", "parse fail":
-				assert.NotNil(t, err, "expected Unmarshal error, got nil")
-			default:
-				assert.Nil(t, err, "Unmarshal error: %v", err)
-				gotDecimal := got["foo"]
-				assert.Equal(t, c.h, gotDecimal.h, "expected h: %v got: %v", c.h, gotDecimal.l)
-				assert.Equal(t, c.l, gotDecimal.l, "expected l: %v got: %v", c.l, gotDecimal.h)
-			}
+		for _, c := range cases {
+			t.Run(c.s, func(t *testing.T) {
+				input := fmt.Sprintf(`{"foo": %q}`, c.s)
+				var got map[string]Decimal128
+				err := json.Unmarshal([]byte(input), &got)
+
+				switch c.remark {
+				case "overflow", "parse fail":
+					assert.NotNil(t, err, "expected Unmarshal error, got nil")
+				default:
+					assert.Nil(t, err, "Unmarshal error: %v", err)
+					gotDecimal := got["foo"]
+					assert.Equal(t, c.h, gotDecimal.h, "expected h: %v got: %v", c.h, gotDecimal.l)
+					assert.Equal(t, c.l, gotDecimal.l, "expected l: %v got: %v", c.l, gotDecimal.h)
+				}
+			})
 		}
 	})
 }

mongo/client.go

@@ -694,15 +694,16 @@ func (c *Client) configure(opts *options.ClientOptions) error {
 		topology.WithClock(func(*session.ClusterClock) *session.ClusterClock { return c.clock }),
 		topology.WithConnectionOptions(func(...topology.ConnectionOption) []topology.ConnectionOption { return connOpts }),
 	)
-	c.topologyOptions = append(topologyOpts, topology.WithServerOptions(
+	topologyOpts = append(topologyOpts, topology.WithServerOptions(
 		func(...topology.ServerOption) []topology.ServerOption { return serverOpts },
 	))
+	c.topologyOptions = topologyOpts
 
 	// Deployment
 	if opts.Deployment != nil {
-		// topology options: WithSeedlist, WithURI, WithSRVServiceName and WithSRVMaxHosts
+		// topology options: WithSeedlist, WithURI, WithSRVServiceName, WithSRVMaxHosts, and WithServerOptions
 		// server options: WithClock and WithConnectionOptions + default maxPoolSize
-		if len(serverOpts) > 2+defaultOptions || len(topologyOpts) > 4 {
+		if len(serverOpts) > 2+defaultOptions || len(topologyOpts) > 5 {
 			return errors.New("cannot specify topology or server options with a deployment")
 		}
 		c.deployment = opts.Deployment

mongo/options/clientoptions.go

@@ -1013,7 +1013,9 @@ func addClientCertFromSeparateFiles(cfg *tls.Config, keyFile, certFile, keyPassw
 		return "", err
 	}
 
-	data := append(keyData, '\n')
+	data := make([]byte, 0, len(keyData)+len(certData)+1)
+	data = append(data, keyData...)
+	data = append(data, '\n')
 	data = append(data, certData...)
 	return addClientCertFromBytes(cfg, data, keyPassword)
 }

x/mongo/driver/connstring/connstring.go

@@ -294,7 +294,10 @@ func (p *parser) parse(original string) error {
 	}
 
 	// add connection arguments from URI and TXT records to connstring
-	connectionArgPairs := append(connectionArgsFromTXT, connectionArgsFromQueryString...)
+	connectionArgPairs := make([]string, 0, len(connectionArgsFromTXT)+len(connectionArgsFromQueryString))
+	connectionArgPairs = append(connectionArgPairs, connectionArgsFromTXT...)
+	connectionArgPairs = append(connectionArgPairs, connectionArgsFromQueryString...)
+
 	for _, pair := range connectionArgPairs {
 		err := p.addOption(pair)
 		if err != nil {