Support bool/int/string_list attribute values in the target graph mapper

mapper/BUILD.bazel

@@ -20,6 +20,8 @@ go_test(
     embed = [":mapper"],
     deps = [
         "//core/targethasher",
+        "//entity",
+        "@com_github_bazelbuild_buildtools//build_proto",
         "@com_github_stretchr_testify//assert",
         "@com_github_stretchr_testify//require",
     ],

mapper/mapper.go

@@ -3,6 +3,9 @@ package mapper
 import (
 	"context"
 	"encoding/hex"
+	"encoding/json"
+	"slices"
+	"strconv"
 
 	buildpb "github.com/bazelbuild/buildtools/build_proto"
 	"github.com/uber/tango/core/targethasher"
@@ -69,8 +72,11 @@ func ResultToTargetGraph(ctx context.Context, result targethasher.Result) ([]ent
 		if len(t.Attributes) > 0 {
 			attrs := make(map[int32]int32, len(t.Attributes))
 			for _, attr := range t.Attributes {
-				if attr.GetType() == buildpb.Attribute_STRING && attr.Name != nil && attr.StringValue != nil {
-					attrs[attrNameMapper.ID(*attr.Name)] = attrStrValMapper.ID(*attr.StringValue)
+				if attr.Name == nil {
+					continue
+				}
+				if val, ok := attributeValue(attr); ok {
+					attrs[attrNameMapper.ID(*attr.Name)] = attrStrValMapper.ID(val)
 				}
 			}
 			if len(attrs) > 0 {
@@ -143,3 +149,50 @@ func ResultToGraphChunks(ctx context.Context, result targethasher.Result, maxByt
 
 	return chunks, nil
 }
+
+// attributeValue returns the canonical string form of a Bazel attribute's
+// value for STRING, BOOLEAN, INTEGER, and STRING_LIST typed attributes.
+// Other collection/dict/label types are not yet supported and are rejected
+// rather than stringified, since a naive string join is not safe for them
+// (ambiguous delimiters, unstable ordering).
+//
+// STRING_LIST values are sorted before encoding: Bazel query does not
+// guarantee list ordering is stable across runs when nothing semantically
+// changed, so treating order as significant would produce spurious "changed"
+// results. This matches core/targethasher/sourcehasher.go's encodeAttribute,
+// which sorts string_list_value for the same reason. The sorted list is
+// JSON-encoded (not delimiter-joined) so that elements containing arbitrary
+// characters can't collide with a different list's encoding.
+func attributeValue(attr *buildpb.Attribute) (string, bool) {
+	switch attr.GetType() {
+	case buildpb.Attribute_STRING:
+		if attr.StringValue == nil {
+			return "", false
+		}
+		return *attr.StringValue, true
+	case buildpb.Attribute_BOOLEAN:
+		if attr.BooleanValue == nil {
+			return "", false
+		}
+		return strconv.FormatBool(*attr.BooleanValue), true
+	case buildpb.Attribute_INTEGER:
+		if attr.IntValue == nil {
+			return "", false
+		}
+		return strconv.FormatInt(int64(*attr.IntValue), 10), true
+	case buildpb.Attribute_STRING_LIST:
+		vals := attr.GetStringListValue()
+		if len(vals) == 0 {
+			return "", false
+		}
+		sorted := slices.Clone(vals)
+		slices.Sort(sorted)
+		encoded, err := json.Marshal(sorted)
+		if err != nil {
+			return "", false
+		}
+		return string(encoded), true
+	default:
+		return "", false
+	}
+}

mapper/mapper_test.go

@@ -4,12 +4,20 @@ import (
 	"context"
 	"testing"
 
+	buildpb "github.com/bazelbuild/buildtools/build_proto"
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 
 	"github.com/uber/tango/core/targethasher"
+	"github.com/uber/tango/entity"
 )
 
+func attrPtr(s string) *string { return &s }
+func boolPtr(b bool) *bool     { return &b }
+func int32Ptr(i int32) *int32  { return &i }
+
+func discPtr(d buildpb.Attribute_Discriminator) *buildpb.Attribute_Discriminator { return &d }
+
 func TestResultToTargetGraph_EmptyResult(t *testing.T) {
 	t.Parallel()
 
@@ -114,3 +122,93 @@ func TestResultToGraphChunks(t *testing.T) {
 		assert.Equal(t, hashes, got)
 	})
 }
+
+func TestResultToTargetGraph_ScalarAttributes(t *testing.T) {
+	t.Parallel()
+
+	result := targethasher.Result{
+		TargetNames: []string{"//a:a"},
+		Targets: map[string]*targethasher.Target{
+			"//a:a": {
+				Attributes: []*buildpb.Attribute{
+					{Name: attrPtr("srcs"), Type: discPtr(buildpb.Attribute_STRING), StringValue: attrPtr("main.go")},
+					{Name: attrPtr("testonly"), Type: discPtr(buildpb.Attribute_BOOLEAN), BooleanValue: boolPtr(true)},
+					{Name: attrPtr("size"), Type: discPtr(buildpb.Attribute_INTEGER), IntValue: int32Ptr(42)},
+					{Name: attrPtr("tags"), Type: discPtr(buildpb.Attribute_STRING_LIST), StringListValue: []string{"b", "a"}},
+					// Unsupported type: dropped, not stringified (even though it
+					// happens to reuse the StringListValue field, its type is LABEL_LIST).
+					{Name: attrPtr("deps"), Type: discPtr(buildpb.Attribute_LABEL_LIST), StringListValue: []string{"//x:x"}},
+				},
+			},
+		},
+	}
+
+	targets, meta, err := ResultToTargetGraph(t.Context(), result)
+	require.NoError(t, err)
+	require.Len(t, targets, 1)
+
+	attrs := targets[0].Attributes
+	assert.Len(t, attrs, 4, "unsupported LABEL_LIST attribute should be dropped")
+
+	got := make(map[string]string, len(attrs))
+	for nameID, valID := range attrs {
+		got[meta.AttributeNameMapping[nameID]] = meta.AttributeStringValueMapping[valID]
+	}
+	assert.Equal(t, map[string]string{
+		"srcs":     "main.go",
+		"testonly": "true",
+		"size":     "42",
+		"tags":     `["a","b"]`,
+	}, got)
+}
+
+func TestResultToTargetGraph_StringListAttributeOrderInsensitive(t *testing.T) {
+	t.Parallel()
+
+	// Bazel query does not guarantee stable list ordering across runs when
+	// nothing semantically changed, so two snapshots differing only in
+	// order must intern to the same value (no spurious "changed" attribute).
+	before := targethasher.Result{
+		TargetNames: []string{"//a:a"},
+		Targets: map[string]*targethasher.Target{
+			"//a:a": {
+				Attributes: []*buildpb.Attribute{
+					{Name: attrPtr("tags"), Type: discPtr(buildpb.Attribute_STRING_LIST), StringListValue: []string{"a", "b", "c"}},
+				},
+			},
+		},
+	}
+	after := targethasher.Result{
+		TargetNames: []string{"//a:a"},
+		Targets: map[string]*targethasher.Target{
+			"//a:a": {
+				Attributes: []*buildpb.Attribute{
+					{Name: attrPtr("tags"), Type: discPtr(buildpb.Attribute_STRING_LIST), StringListValue: []string{"c", "a", "b"}},
+				},
+			},
+		},
+	}
+
+	beforeTargets, beforeMeta, err := ResultToTargetGraph(t.Context(), before)
+	require.NoError(t, err)
+	afterTargets, afterMeta, err := ResultToTargetGraph(t.Context(), after)
+	require.NoError(t, err)
+
+	beforeAttrs := attributeValuesByName(beforeTargets[0].Attributes, beforeMeta)
+	afterAttrs := attributeValuesByName(afterTargets[0].Attributes, afterMeta)
+	require.Contains(t, beforeAttrs, "tags")
+	require.Contains(t, afterAttrs, "tags")
+
+	beforeVal := beforeMeta.AttributeStringValueMapping[beforeAttrs["tags"]]
+	afterVal := afterMeta.AttributeStringValueMapping[afterAttrs["tags"]]
+	assert.Equal(t, beforeVal, afterVal)
+}
+
+// attributeValuesByName maps attribute name to its interned value ID.
+func attributeValuesByName(attrs map[int32]int32, meta *entity.Metadata) map[string]int32 {
+	byName := make(map[string]int32, len(attrs))
+	for nameID, valID := range attrs {
+		byName[meta.AttributeNameMapping[nameID]] = valID
+	}
+	return byName
+}