fix golangci-lint detected issues

MODULE.bazel

@@ -32,7 +32,6 @@ use_repo(
     "com_github_goccy_go_yaml",
     "com_github_gogo_protobuf",
     "com_github_golang_mock",
-    "com_github_google_go_cmp",
     "com_github_stretchr_testify",
     "com_github_uber_go_tally",
     "org_golang_google_protobuf",

controller/getchangedtargetgraph.go

@@ -21,7 +21,7 @@ import (
 // GetChangedTargetGraph is the streaming RPC that will return the subgraph
 // induced by the changed targets between two revisions. It is currently a
 // stub: it records success/failure metrics and returns no data.
-func (c *controller) GetChangedTargetGraph(request *pb.GetChangedTargetGraphRequest, stream pb.TangoServiceGetChangedTargetGraphYARPCServer) (retErr error) {
+func (c *controller) GetChangedTargetGraph(_ *pb.GetChangedTargetGraphRequest, _ pb.TangoServiceGetChangedTargetGraphYARPCServer) (retErr error) {
 	scope := c.scope.SubScope("get_changed_target_graph")
 	defer func() {
 		if retErr != nil {

controller/getchangedtargets.go

@@ -48,6 +48,8 @@ type job struct {
 // GetChangedTargets returns the changed targets between two revisions. If the
 // client disconnects, the stream's context is cancelled and the function
 // returns with context.Canceled.
+//
+//nolint:gocyclo // orchestration method with inherently high branching
 func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, stream pb.TangoServiceGetChangedTargetsYARPCServer) (retErr error) {
 	scope := c.scope.SubScope("get_changed_targets")
 	scope.Counter("calls").Inc(1)
@@ -102,8 +104,8 @@ func (c *controller) GetChangedTargets(request *pb.GetChangedTargetsRequest, str
 
 	changedTargetsResponses, err := c.compareTargetGraphs(ctx, scope, logger, firstGraph, secondGraph, maxDist)
 	// Allow GC of raw graph data while the caching goroutine runs.
-	firstGraph = nil
-	secondGraph = nil
+	firstGraph = nil  //nolint:ineffassign // intentional: allow GC of large slice
+	secondGraph = nil //nolint:ineffassign // intentional: allow GC of large slice
 	if err != nil {
 		if ctx.Err() != nil {
 			err = ctx.Err()
@@ -397,21 +399,21 @@ func (c *controller) compareTargetGraphs(ctx context.Context, scope tally.Scope,
 		return nil, err
 	}
 	// Release raw chunk slices — individual target protos are now held by the ID maps.
-	firstGraph = nil
-	secondGraph = nil
+	firstGraph = nil  //nolint:ineffassign // intentional: allow GC of large slice
+	secondGraph = nil //nolint:ineffassign // intentional: allow GC of large slice
 	before, err := toDiffGraph(ctx, firstTargetsByID, firstMetadata)
 	if err != nil {
 		return nil, err
 	}
 	// Metadata and ID map are fully consumed by the name-resolved graph; drop them.
-	firstTargetsByID = nil
-	firstMetadata = nil
+	firstTargetsByID = nil //nolint:ineffassign // intentional: allow GC of duplicate map
+	firstMetadata = nil    //nolint:ineffassign // intentional: allow GC of duplicate map
 	after, err := toDiffGraph(ctx, secondTargetsByID, secondMetadata)
 	if err != nil {
 		return nil, err
 	}
-	secondTargetsByID = nil
-	secondMetadata = nil
+	secondTargetsByID = nil //nolint:ineffassign // intentional: allow GC of duplicate map
+	secondMetadata = nil    //nolint:ineffassign // intentional: allow GC of duplicate map
 	indexDuration := time.Since(indexStart)
 	compareScope.Timer("index_duration").Record(indexDuration)
 
@@ -426,8 +428,8 @@ func (c *controller) compareTargetGraphs(ctx context.Context, scope tally.Scope,
 		return nil, err
 	}
 	// Release the input graphs; only result is needed from here on.
-	before = nil
-	after = nil
+	before = nil //nolint:ineffassign // intentional: allow GC of large graph
+	after = nil  //nolint:ineffassign // intentional: allow GC of large graph
 	compareScope.Timer("compute_duration").Record(time.Since(computeStart))
 
 	if ctx.Err() != nil {

controller/getchangedtargets_test.go

@@ -340,7 +340,7 @@ func TestGetChangedTargets_streamChunks(t *testing.T) {
 	stream.EXPECT().Context().Return(t.Context())
 
 	var sentResponses []*pb.GetChangedTargetsResponse
-	stream.EXPECT().Send(gomock.Any()).DoAndReturn(func(resp *pb.GetChangedTargetsResponse, opts ...interface{}) error {
+	stream.EXPECT().Send(gomock.Any()).DoAndReturn(func(resp *pb.GetChangedTargetsResponse, _ ...interface{}) error {
 		sentResponses = append(sentResponses, resp)
 		return nil
 	}).Times(2)

controller/gettargetgraph_test.go

@@ -378,5 +378,5 @@ func newMockReadCloser(data []byte) io.ReadCloser {
 
 type errReadCloser struct{ err error }
 
-func (e *errReadCloser) Read(p []byte) (int, error) { return 0, e.err }
+func (e *errReadCloser) Read(_ []byte) (int, error) { return 0, e.err }
 func (e *errReadCloser) Close() error               { return nil }

core/bazel/bazel_test.go

@@ -36,7 +36,7 @@ func TestNewBazelClient(t *testing.T) {
 				WorkspacePath: "/tmp/test",
 				EnvVarsMap:    map[string]string{"FOO": "bar"},
 				Logger:        zap.NewNop().Sugar(),
-				ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander {
+				ExecCommandContext: func(_ context.Context, _ string, _ ...string) commander {
 					return nil
 				},
 			},

core/bazel/query_test.go

@@ -61,10 +61,11 @@ func TestExecuteQuery_Success(t *testing.T) {
 		WorkspacePath: "/tmp/test",
 		EnvVarsMap:    map[string]string{},
 		Logger:        zap.NewNop().Sugar(),
-		ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander {
+		ExecCommandContext: func(_ context.Context, _ string, _ ...string) commander {
 			return mockCmd
 		},
 	})
+	require.NoError(t, err)
 
 	resp, err := client.ExecuteQuery(context.Background(), &QueryRequest{Query: "//..."})
 	require.NoError(t, err)
@@ -105,7 +106,7 @@ func TestExecuteQuery_WithStartupOptions(t *testing.T) {
 		WorkspacePath: "/tmp/test",
 		EnvVarsMap:    map[string]string{},
 		Logger:        zap.NewNop().Sugar(),
-		ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander {
+		ExecCommandContext: func(_ context.Context, _ string, arg ...string) commander {
 			capturedArgs = arg
 			return mockCmd
 		},
@@ -156,7 +157,7 @@ func TestExecuteQueryInternal_ContextTimeout(t *testing.T) {
 		EnvVarsMap:    map[string]string{},
 		QueryTimeout:  10 * time.Millisecond, // Short timeout for test
 
-		ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander {
+		ExecCommandContext: func(ctx context.Context, _ string, _ ...string) commander {
 			// Simulate process behavior: when context is cancelled, close pipes
 			go func() {
 				<-ctx.Done()
@@ -169,7 +170,7 @@ func TestExecuteQueryInternal_ContextTimeout(t *testing.T) {
 	})
 	require.NoError(t, err)
 	result, err := client.executeQueryInternal(context.Background(), "//...", nil)
-	require.Nil(t, result)
+	require.Empty(t, result.GetTarget())
 	require.Error(t, err)
 	// Should get timeout or deadline exceeded error
 	assert.Contains(t, err.Error(), "deadline exceeded")
@@ -234,7 +235,7 @@ func TestExecuteQueryInternal_Failures(t *testing.T) {
 				WorkspacePath: "/tmp/test",
 				EnvVarsMap:    map[string]string{},
 				Logger:        zap.NewNop().Sugar(),
-				ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander {
+				ExecCommandContext: func(_ context.Context, _ string, _ ...string) commander {
 					return mockCmd
 				},
 			})
@@ -263,10 +264,11 @@ func TestExecuteQuery_ErrorCase(t *testing.T) {
 		WorkspacePath: "/tmp/test",
 		EnvVarsMap:    map[string]string{},
 		Logger:        zap.NewNop().Sugar(),
-		ExecCommandContext: func(ctx context.Context, name string, arg ...string) commander {
+		ExecCommandContext: func(_ context.Context, _ string, _ ...string) commander {
 			return mockCmd
 		},
 	})
+	require.NoError(t, err)
 
 	resp, err := client.ExecuteQuery(context.Background(), &QueryRequest{Query: "//..."})
 	require.Error(t, err)

core/git/git_test.go

@@ -227,8 +227,8 @@ func TestDefaultGit_FileHashes(t *testing.T) {
 				`100644 blob d236	file1
 100644 blob 9bcc	file2`),
 			wantHashes: map[string][]byte{
-				"file1": []byte{0xd2, 0x36},
-				"file2": []byte{0x9b, 0xcc},
+				"file1": {0xd2, 0x36},
+				"file2": {0x9b, 0xcc},
 			},
 		},
 		{

core/repomanager/repo_manager.go

@@ -68,7 +68,7 @@ type repoManager struct {
 // The origin directory holds the initial clone; workers are cheap local copies.
 type workerPool struct {
 	originDir string
-	originMu  sync.Mutex // one lock per repo for orginal clone
+	originMu  sync.Mutex // one lock per repo for original clone
 	cloned    bool
 
 	avail chan *workerSlot // available slots; pool capacity

core/storage/memstorage.go

@@ -36,7 +36,7 @@ func NewMemoryStorage() Storage {
 }
 
 // Get downloads a blob from the storage. Returns an error wrapping ErrNotFound when the blob is not found.
-func (m *memoryStorage) Get(ctx context.Context, req DownloadRequest) (DownloadResponse, error) {
+func (m *memoryStorage) Get(_ context.Context, req DownloadRequest) (DownloadResponse, error) {
 	m.mu.RLock()
 	defer m.mu.RUnlock()
 	b, ok := m.data[req.Key]
@@ -60,14 +60,14 @@ func (m *memoryStorage) Put(ctx context.Context, req UploadRequest) error {
 	return nil
 }
 
-func (m *memoryStorage) Exists(ctx context.Context, key string) (bool, error) {
+func (m *memoryStorage) Exists(_ context.Context, key string) (bool, error) {
 	m.mu.RLock()
 	defer m.mu.RUnlock()
 	_, ok := m.data[key]
 	return ok, nil
 }
 
-func (m *memoryStorage) List(ctx context.Context, prefix string) ([]string, error) {
+func (m *memoryStorage) List(_ context.Context, prefix string) ([]string, error) {
 	m.mu.RLock()
 	defer m.mu.RUnlock()
 	var keys []string

core/targethasher/BUILD.bazel

@@ -31,8 +31,6 @@ go_test(
         "@com_github_bazelbuild_buildtools//build_proto",
         "@com_github_deckarep_golang_set_v2//:golang-set",
         "@com_github_golang_mock//gomock",
-        "@com_github_google_go_cmp//cmp",
-        "@com_github_google_go_cmp//cmp/cmpopts",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
     ],

core/targethasher/graph_test.go

@@ -23,8 +23,6 @@ import (
 	buildpb "github.com/bazelbuild/buildtools/build_proto"
 	set "github.com/deckarep/golang-set/v2"
 	"github.com/golang/mock/gomock"
-	"github.com/google/go-cmp/cmp"
-	"github.com/google/go-cmp/cmp/cmpopts"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
@@ -77,7 +75,7 @@ func TestContextCancellation(t *testing.T) {
 
 	// verify fromProto honors context cancellation
 	qr := &buildpb.QueryResult{
-		Target: []*buildpb.Target{&buildpb.Target{}},
+		Target: []*buildpb.Target{{}},
 	}
 	result, err := fromProto(ctx, qr, nil, "", set.NewSet[string](), set.NewSet[string](), nil, false)
 	assert.Equal(t, EmptyResult(), result)
@@ -219,15 +217,15 @@ func Test_RemoveAttrs(t *testing.T) {
 		Rule: &buildpb.Rule{
 			Name: StringPtr("//pkg:go_default_library"),
 			Attribute: []*buildpb.Attribute{
-				&buildpb.Attribute{
+				{
 					Name:        StringPtr("url"),
 					StringValue: StringPtr("some_url"),
 				},
-				&buildpb.Attribute{
+				{
 					Name:            StringPtr("urls"),
 					StringListValue: []string{"url1", "url2"},
 				},
-				&buildpb.Attribute{
+				{
 					Name:        StringPtr("to_keep"),
 					StringValue: StringPtr("target1"),
 				},
@@ -245,15 +243,15 @@ func Test_RemoveAttrs(t *testing.T) {
 		Rule: &buildpb.Rule{
 			Name: StringPtr("//external:some_rule"),
 			Attribute: []*buildpb.Attribute{
-				&buildpb.Attribute{
+				{
 					Name:        StringPtr("url"),
 					StringValue: StringPtr("some_url"),
 				},
-				&buildpb.Attribute{
+				{
 					Name:            StringPtr("urls"),
 					StringListValue: []string{"url1", "url2"},
 				},
-				&buildpb.Attribute{
+				{
 					Name:        StringPtr("to_keep"),
 					StringValue: StringPtr("external"),
 				},
@@ -276,25 +274,6 @@ func Test_RemoveAttrs(t *testing.T) {
 	assert.Equal(t, []byte{0x7c, 0xde, 0x91, 0xc2, 0x94, 0x1a, 0x22, 0xf3, 0xb2, 0x18, 0x7c, 0x21, 0xbf, 0x32, 0x17, 0xc0, 0xa3, 0xf0, 0xc, 0x77}, external.HashWithoutDeps)
 }
 
-func validateResultIsStable(t *testing.T, baseResult, result Result) {
-	t.Helper()
-	require.ElementsMatch(t, baseResult.TargetNames, result.TargetNames)
-	for _, targetName := range baseResult.TargetNames {
-		base, ok := baseResult.Targets[targetName]
-		require.True(t, ok)
-		res, ok := result.Targets[targetName]
-		require.True(t, ok)
-		assert.Equal(t, base.Hash, res.Hash)
-	}
-}
-
-func assertEqualTargetHash(t *testing.T, expected, actual Target) {
-	opt := cmpopts.IgnoreUnexported(Target{})
-	// too many nested attributes to compare
-	ignore := cmpopts.IgnoreFields(Target{}, "Attributes", "SourceFile", "Rule")
-	assert.True(t, cmp.Equal(expected, actual, opt, ignore), cmp.Diff(expected, actual, opt))
-}
-
 func Test_fromProto(t *testing.T) {
 	ctrl := gomock.NewController(t)
 

core/targethasher/sourcehasher.go

@@ -72,7 +72,7 @@ func NewSourceHasher(p Params) SourceHasher {
 }
 
 // HashSourceFile does a no-op hash for the noOpHasher.
-func (hh *noOpHasher) HashSourceFile(sourceFile *buildpb.SourceFile) ([]byte, error) {
+func (hh *noOpHasher) HashSourceFile(_ *buildpb.SourceFile) ([]byte, error) {
 	return nil, nil
 }
 
@@ -123,7 +123,7 @@ func hashFile(path string) (hash.Hash, error) {
 	hash := newHash()
 	// Using same SHA1 hashing algorithm as git to ensure file hashes
 	// are always the same: https://alblue.bandlem.com/2011/08/git-tip-of-week-objects.html
-	hash.Write([]byte(fmt.Sprintf("blob %d\000", fi.Size())))
+	fmt.Fprintf(hash, "blob %d\000", fi.Size())
 	if _, err := io.Copy(hash, f); err != nil {
 		return nil, err
 	}
@@ -167,6 +167,8 @@ func externalTargetForRule(t string) string {
 }
 
 // HashRuleCommon hashes the common elements of a buildpb.Rule.
+//
+//nolint:errcheck // hash.Hash.Write never returns an error
 func HashRuleCommon(r *buildpb.Rule, h hash.Hash) {
 	// Name                        *string
 	io.WriteString(h, r.GetName())
@@ -210,6 +212,7 @@ func HashRuleCommon(r *buildpb.Rule, h hash.Hash) {
 	io.WriteString(h, r.GetSkylarkEnvironmentHashCode())
 }
 
+//nolint:errcheck // hash.Hash.Write never returns an error
 func hashAttributes(h hash.Hash, a *buildpb.Attribute) {
 	// generator_location is present if the rule is generated from a macro, but
 	// should not be hashed as the generating macro's location in a build file

example/client/client.go

@@ -50,12 +50,8 @@ func main() {
 
 	grpcTransport := yarpcgrpc.NewTransport()
 	out := grpcTransport.NewSingleOutbound(*addr)
-	zl, err := zap.NewDevelopment()
-	if err != nil {
-		fmt.Fprintf(os.Stderr, "init logger: %v\n", err)
-		os.Exit(1)
-	}
-	defer zl.Sync()
+	zl, _ := zap.NewDevelopment()
+	defer zl.Sync() //nolint:errcheck // best-effort flush
 	logger := zl.Sugar()
 	dispatcher := yarpc.NewDispatcher(yarpc.Config{
 		Name: "tango-client",
@@ -67,7 +63,7 @@ func main() {
 		logger.Errorf("start dispatcher: %w", err)
 		os.Exit(1)
 	}
-	defer dispatcher.Stop()
+	defer dispatcher.Stop() //nolint:errcheck // best-effort cleanup
 
 	client := pb.NewTangoYARPCClient(dispatcher.ClientConfig("tango"))
 
@@ -159,7 +155,7 @@ func callGetTargetGraph(ctx context.Context, client pb.TangoYARPCClient, logger
 	if err != nil {
 		return fmt.Errorf("GetTargetGraph: %w", err)
 	}
-	defer stream.CloseSend()
+	defer stream.CloseSend() //nolint:errcheck // best-effort cleanup
 
 	for {
 		msg, err := stream.Recv()
@@ -204,7 +200,7 @@ func callGetChangedTargets(ctx context.Context, client pb.TangoYARPCClient, logg
 	if err != nil {
 		return fmt.Errorf("GetChangedTargets: %w", err)
 	}
-	defer stream.CloseSend()
+	defer stream.CloseSend() //nolint:errcheck // best-effort cleanup
 
 	for {
 		msg, err := stream.Recv()

example/cmd/query-bench/main.go

@@ -66,7 +66,7 @@ func run() error {
 	if err != nil {
 		return fmt.Errorf("creating logger: %w", err)
 	}
-	defer logger.Sync()
+	defer logger.Sync() //nolint:errcheck // best-effort flush
 
 	parentCtx := context.Background()