lint: add new deferunlockcheck pass

This commit updates the previous iteration of the linter
to only trigger the report if we exceed a certain number of
manual unlocks. This is due to the fact that while some
cases of manual unlocks are ok (and are fine passing via the nolint)
there are others which should be fixed. Discerning which of those that
need to be fixed and then fixing them will require considerable effort
and will likely not be completed in the short term. Having the linter
report over a number of manual locks will enforce future behaviour
to either defer unlocks or apply the nolint.

Release note: None

pkg/BUILD.bazel

@@ -589,7 +589,6 @@ ALL_TESTS = [
     "//pkg/testutils/fingerprintutils:fingerprintutils_test",
     "//pkg/testutils/floatcmp:floatcmp_test",
     "//pkg/testutils/keysutils:keysutils_test",
-    "//pkg/testutils/lint/passes/deferunlockcheck:deferunlockcheck_test",
     "//pkg/testutils/lint/passes/errcmp:errcmp_test",
     "//pkg/testutils/lint/passes/errwrap:errwrap_test",
     "//pkg/testutils/lint/passes/fmtsafe:fmtsafe_test",
@@ -2151,7 +2150,6 @@ GO_TARGETS = [
     "//pkg/testutils/keysutils:keysutils_test",
     "//pkg/testutils/kvclientutils:kvclientutils",
     "//pkg/testutils/lint/passes/deferunlockcheck:deferunlockcheck",
-    "//pkg/testutils/lint/passes/deferunlockcheck:deferunlockcheck_test",
     "//pkg/testutils/lint/passes/descriptormarshal:descriptormarshal",
     "//pkg/testutils/lint/passes/errcheck:errcheck",
     "//pkg/testutils/lint/passes/errcmp:errcmp",

pkg/testutils/lint/passes/deferunlockcheck/BUILD.bazel

@@ -1,4 +1,4 @@
-load("@io_bazel_rules_go//go:def.bzl", "go_library", "go_test")
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
 
 go_library(
     name = "deferunlockcheck",
@@ -12,19 +12,3 @@ go_library(
         "@org_golang_x_tools//go/ast/inspector",
     ],
 )
-
-go_test(
-    name = "deferunlockcheck_test",
-    srcs = ["deferunlockcheck_test.go"],
-    args = ["-test.timeout=295s"],
-    data = glob(["testdata/**"]) + [
-        "@go_sdk//:files",
-    ],
-    deps = [
-        ":deferunlockcheck",
-        "//pkg/build/bazel",
-        "//pkg/testutils/datapathutils",
-        "//pkg/testutils/skip",
-        "@org_golang_x_tools//go/analysis/analysistest",
-    ],
-)

pkg/testutils/lint/passes/deferunlockcheck/deferunlockcheck.go

@@ -24,6 +24,9 @@ const Doc = "checks that usages of mutex Unlock() are deferred."
 
 const noLintName = "deferunlock"
 
+// Please try to avoid updating this value, use the nolint comment instead.
+const unlocksLimit = 1037
+
 // Analyzer is an analysis pass that checks for mutex unlocks which
 // aren't deferred.
 var Analyzer = &analysis.Analyzer{
@@ -40,6 +43,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 	filter := []ast.Node{
 		(*ast.ExprStmt)(nil),
 	}
+	var unlocksFound int
 	astInspector.Preorder(filter, func(n ast.Node) {
 		stmt := n.(*ast.ExprStmt)
 		expr, ok := stmt.X.(*ast.CallExpr)
@@ -50,8 +54,16 @@ func run(pass *analysis.Pass) (interface{}, error) {
 		if !ok {
 			return
 		}
+		// Increase the count if we found a violation.
 		if sel.Sel != nil && (sel.Sel.Name == "Unlock" || sel.Sel.Name == "RUnlock") && !passesutil.HasNolintComment(pass, n, noLintName) {
-			pass.Reportf(sel.Pos(), "Mutex %s not deferred", sel.Sel.Name)
+			unlocksFound++
+		}
+		// If we exceed the limit then the reports trigger.
+		if unlocksFound > unlocksLimit {
+			pass.Report(analysis.Diagnostic{
+				Pos:     n.Pos(),
+				Message: "Too many Unlocks() not being deferred, check code changes (//nolint:deferunlock to pass)",
+			})
 		}
 	})