bazel: run returncheck lint in nogo

Unfortunately `returncheck` is not implemented as an `Analyzer` so it
can't be integrated with `nogo` directly. Instead I've copied/adapted
the existing code from https://github.com/cockroachdb/returncheck. The
vendored `returncheck` can be deleted from tree when we no longer need
to support `make lint`.

Closes #73391.

Release note: None

BUILD.bazel

@@ -193,6 +193,7 @@ nogo(
             "//pkg/testutils/lint/passes/leaktestcall",
             "//pkg/testutils/lint/passes/nilness",
             "//pkg/testutils/lint/passes/nocopy",
+            "//pkg/testutils/lint/passes/returncheck",
             "//pkg/testutils/lint/passes/returnerrcheck",
             "//pkg/testutils/lint/passes/timer",
             "//pkg/testutils/lint/passes/unconvert",

build/bazelutil/nogo_config.json

@@ -111,6 +111,15 @@
             "cockroach/pkg/.*$": "first-party code"
         }
     },
+    "returncheck": {
+        "exclude_files": {
+            "cockroach/pkg/kv/kvclient/kvcoord/txn_interceptor_span_refresher_test.go": "existing failure",
+            "cockroach/pkg/kv/txn_external_test.go": "existing failure"
+        },
+        "only_files": {
+            "cockroach/pkg/.*$": "first-party code"
+        }
+    },
     "returnerrcheck": {
         "only_files": {
             "cockroach/pkg/.*$": "first-party code"

pkg/kv/kvserver/client_raft_test.go

@@ -4253,8 +4253,11 @@ func TestInitRaftGroupOnRequest(t *testing.T) {
 		log.Errorf(ctx, "expected raft group to be uninitialized")
 	}
 	// Send an increment and verify that initializes the Raft group.
-	kv.SendWrapped(ctx,
+	_, pErr := kv.SendWrapped(ctx,
 		followerStore.TestSender(), incrementArgs(splitKey, 1))
+	if pErr != nil {
+		t.Fatal(pErr)
+	}
 
 	if !repl.IsRaftGroupInitialized() {
 		t.Fatal("expected raft group to be initialized")

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

@@ -0,0 +1,13 @@
+load("@io_bazel_rules_go//go:def.bzl", "go_library")
+
+go_library(
+    name = "returncheck",
+    srcs = ["returncheck.go"],
+    importpath = "github.com/cockroachdb/cockroach/pkg/testutils/lint/passes/returncheck",
+    visibility = ["//visibility:public"],
+    deps = [
+        "@org_golang_x_tools//go/analysis",
+        "@org_golang_x_tools//go/analysis/passes/inspect",
+        "@org_golang_x_tools//go/ast/inspector",
+    ],
+)

pkg/testutils/lint/passes/returncheck/returncheck.go

@@ -0,0 +1,105 @@
+// Copyright 2022 The Cockroach Authors.
+//
+// Use of this software is governed by the Business Source License
+// included in the file licenses/BSL.txt.
+//
+// As of the Change Date specified in that file, in accordance with
+// the Business Source License, use of this software will be governed
+// by the Apache License, Version 2.0, included in the file
+// licenses/APL.txt.
+
+// Package returncheck defines an Analyzer that detects unused or
+// discarded roachpb.Error objects.
+package returncheck
+
+import (
+	"go/ast"
+	"go/types"
+
+	"golang.org/x/tools/go/analysis"
+	"golang.org/x/tools/go/analysis/passes/inspect"
+	"golang.org/x/tools/go/ast/inspector"
+)
+
+// Analyzer is an analysis.Analyzer that checks for unused or discarded
+// roachpb.Error objects from function calls.
+var Analyzer = &analysis.Analyzer{
+	Name:     "returncheck",
+	Doc:      "`returncheck` : `roachpb.Error` :: `errcheck` : (stdlib)`error`",
+	Requires: []*analysis.Analyzer{inspect.Analyzer},
+	Run:      runAnalyzer,
+}
+
+func runAnalyzer(pass *analysis.Pass) (interface{}, error) {
+	inspect := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector)
+	inspect.Preorder([]ast.Node{
+		(*ast.AssignStmt)(nil),
+		(*ast.DeferStmt)(nil),
+		(*ast.ExprStmt)(nil),
+		(*ast.GoStmt)(nil),
+	}, func(n ast.Node) {
+		switch stmt := n.(type) {
+		case *ast.AssignStmt:
+			// Find "_" in the left-hand side of the assigments and check if the corressponding
+			// right-hand side expression is a call that returns the target type.
+			for i := 0; i < len(stmt.Lhs); i++ {
+				if id, ok := stmt.Lhs[i].(*ast.Ident); ok && id.Name == "_" {
+					var rhs ast.Expr
+					if len(stmt.Rhs) == 1 {
+						// ..., stmt.Lhs[i], ... := stmt.Rhs[0]
+						rhs = stmt.Rhs[0]
+					} else {
+						// ..., stmt.Lhs[i], ... := ..., stmt.Rhs[i], ...
+						rhs = stmt.Rhs[i]
+					}
+					if call, ok := rhs.(*ast.CallExpr); ok {
+						recordUnchecked(pass, call, i)
+					}
+				}
+			}
+		case *ast.ExprStmt:
+			if call, ok := stmt.X.(*ast.CallExpr); ok {
+				recordUnchecked(pass, call, -1)
+			}
+		case *ast.GoStmt:
+			recordUnchecked(pass, stmt.Call, -1)
+		case *ast.DeferStmt:
+			recordUnchecked(pass, stmt.Call, -1)
+		}
+	})
+	return nil, nil
+}
+
+// recordUnchecked records an error if a given calls has an unchecked
+// return. If pos is not a negative value and the call returns a
+// tuple, check if the return value at the specified position is of type
+// roachpb.Error.
+func recordUnchecked(pass *analysis.Pass, call *ast.CallExpr, pos int) {
+	isTarget := false
+	switch t := pass.TypesInfo.Types[call].Type.(type) {
+	case *types.Named:
+		isTarget = isTargetType(t)
+	case *types.Pointer:
+		isTarget = isTargetType(t.Elem())
+	case *types.Tuple:
+		for i := 0; i < t.Len(); i++ {
+			if pos >= 0 && i != pos {
+				continue
+			}
+			switch et := t.At(i).Type().(type) {
+			case *types.Named:
+				isTarget = isTargetType(et)
+			case *types.Pointer:
+				isTarget = isTargetType(et.Elem())
+			}
+		}
+	}
+
+	if isTarget {
+		pass.Reportf(call.Pos(), "unchecked roachpb.Error value")
+	}
+}
+
+func isTargetType(t types.Type) bool {
+	return t.String() == "github.com/cockroachdb/cockroach/pkg/roachpb.Error"
+}