fix: targets spuriously returned in cache case

Remove the "NewTarget" case, which can never be reached because:
1. beforeMetadata.TransitiveConfiguredTargets is a superset of
   beforeMetadata.MatchingTargets, and
2. at this point in the function, beforeMetadata.MatchingTargets is
   known to contain that same label and configuration.

Doing this also addresses an issue where spurious targets were returned
on a result cache hit: the codepath was looking for configured target
structs, but when retrieving hashes from the cache, these are not
populated in the first place, only the resulting hashes are.

Author: sitaktif

pkg/BUILD.bazel

@@ -38,6 +38,7 @@ go_test(
         "hash_cache_test.go",
         "normalizer_test.go",
         "target_determinator_test.go",
+        "walker_test.go",
     ],
     data = ["//testdata/HelloWorld:all_srcs"],
     embed = [":pkg"],

pkg/walker.go

@@ -88,35 +88,29 @@ func DiffSingleLabel(beforeMetadata, afterMetadata *QueryResults, includeDiffere
 			callback(label, differences, configuredTarget)
 			return nil
 		}
-		_, ok := beforeMetadata.TransitiveConfiguredTargets[label][configuration]
-		if !ok {
-			collectDifference(Difference{Category: "NewTarget"})
-			callback(label, differences, configuredTarget)
-			return nil
-		} else {
-			labelAndConfiguration := LabelAndConfiguration{
-				Label:         label,
-				Configuration: configuration,
-			}
-			hashBefore, err := beforeMetadata.TargetHashCache.Hash(labelAndConfiguration)
-			if err != nil {
-				return err
-			}
-			hashAfter, err := afterMetadata.TargetHashCache.Hash(labelAndConfiguration)
+		labelAndConfiguration := LabelAndConfiguration{
+			Label:         label,
+			Configuration: configuration,
+		}
+
+		hashBefore, err := beforeMetadata.TargetHashCache.Hash(labelAndConfiguration)
+		if err != nil {
+			return err
+		}
+		hashAfter, err := afterMetadata.TargetHashCache.Hash(labelAndConfiguration)
+		if err != nil {
+			return err
+		}
+		if bytes.Equal(hashBefore, hashAfter) {
+			continue
+		}
+		if includeDifferences {
+			differences, err = WalkDiffs(beforeMetadata.TargetHashCache, afterMetadata.TargetHashCache, labelAndConfiguration)
 			if err != nil {
 				return err
 			}
-			if bytes.Equal(hashBefore, hashAfter) {
-				continue
-			}
-			if includeDifferences {
-				differences, err = WalkDiffs(beforeMetadata.TargetHashCache, afterMetadata.TargetHashCache, labelAndConfiguration)
-				if err != nil {
-					return err
-				}
-			}
-			callback(label, differences, configuredTarget)
 		}
+		callback(label, differences, configuredTarget)
 	}
 	return nil
 }

pkg/walker_test.go

@@ -0,0 +1,57 @@
+package pkg
+
+import (
+	"testing"
+
+	ss "github.com/bazel-contrib/target-determinator/common/sorted_set"
+	"github.com/bazel-contrib/target-determinator/third_party/protobuf/bazel/analysis"
+	gazelle_label "github.com/bazelbuild/bazel-gazelle/label"
+)
+
+func makeMatchingTargets(lbl gazelle_label.Label, config Configuration) *MatchingTargets {
+	return &MatchingTargets{
+		labels: ss.NewSortedSetFn([]gazelle_label.Label{lbl}, CompareLabels),
+		labelsToConfigurations: map[gazelle_label.Label]*ss.SortedSet[Configuration]{
+			lbl: ss.NewSortedSetFn([]Configuration{config}, ConfigurationLess),
+		},
+	}
+}
+
+// TestDiffSingleLabel_NoDifferenceWhenBothFromCache verifies that when both before and after
+// metadata are loaded from cache (TransitiveConfiguredTargets == nil, pre-computed hashes
+// present), DiffSingleLabel does not report any differences for an unchanged target.
+func TestDiffSingleLabel_NoDifferenceWhenBothFromCache(t *testing.T) {
+	const bazelRelease = "release 7.0.0"
+	lbl := mustParseLabel("//foo:bar")
+	config := NormalizeConfiguration("deadcafe")
+
+	mt := makeMatchingTargets(lbl, config)
+	fakeHash := []byte{0xde, 0xad, 0xca, 0xfe}
+	hashKey := lbl.String() + "\x00" + config.String()
+
+	makeFromCache := func() *QueryResults {
+		thc := NewTargetHashCache(nil, &Normalizer{}, bazelRelease)
+		if err := thc.RestoreHashes(map[string][]byte{hashKey: fakeHash}); err != nil {
+			t.Fatalf("RestoreHashes: %v", err)
+		}
+		return &QueryResults{
+			MatchingTargets:             mt,
+			TransitiveConfiguredTargets: nil, // as set by LoadFromCache
+			TargetHashCache:             thc,
+			BazelRelease:                bazelRelease,
+		}
+	}
+
+	beforeMetadata := makeFromCache()
+	afterMetadata := makeFromCache()
+
+	err := DiffSingleLabel(
+		beforeMetadata, afterMetadata, false, lbl,
+		func(_ gazelle_label.Label, diffs []Difference, _ *analysis.ConfiguredTarget) {
+			t.Errorf("callback called for unchanged target (diffs when includeDifferences=false are always nil: %v)", diffs)
+		},
+	)
+	if err != nil {
+		t.Fatalf("DiffSingleLabel returned unexpected error: %v", err)
+	}
+}