Fix/stop using subfield labels for unexpanded fields in textlog

reference.go

@@ -134,13 +134,19 @@ func (r *Reference) ToTextLabel() string {
 	return r.textLabel
 }
 
-// TextReferenceResolver is an interface that can be implemented by a struct to specify custom text labels for its fields
-// that are used in references.
+// TextReferenceResolver is an interface that can be implemented by a struct to
+// specify custom text labels for its fields that are used in references.
 type TextReferenceResolver interface {
-	// RelativeTextPointer returns a label for the given field of the object that implements this interface.
-	// If the field is not found, nil is returned.
+	// RelativeTextPointer returns a label for the given field of the object that
+	// implements this interface.
 	// The given field must be a pointer to a field of the object.
-	RelativeTextPointer(pointee any) (string, bool)
+	// Fields are marked virtual if the resolver provides a distinct label for
+	// this field but a corresponding TextlogMarshaler method does not list this
+	// field/label as a separate key.
+	// RelativeTextPointer returns the label as a string, a bool indicating
+	// whether the label is virtual, and a bool indicating whether the field was
+	// found.
+	RelativeTextPointer(pointee any) (string, bool, bool)
 }
 
 func findTextLabel(base reflect.Value, pointedField reflect.Value) (string, bool) {
@@ -165,10 +171,12 @@ func findTextLabel(base reflect.Value, pointedField reflect.Value) (string, bool
 		}
 		var label string
 		var labelFound bool
+		var labelVirtual bool
+		resolver, isResolver := fieldPointer.Interface().(TextReferenceResolver)
 		if fieldPointer.Equal(pointedField) {
 			label, labelFound = "", true
-		} else if resolver, isResolver := fieldPointer.Interface().(TextReferenceResolver); isResolver {
-			label, labelFound = resolver.RelativeTextPointer(pointedField.Interface())
+		} else if isResolver {
+			label, labelVirtual, labelFound = resolver.RelativeTextPointer(pointedField.Interface())
 		} else {
 			label, labelFound = findTextLabel(field, pointedField)
 		}
@@ -180,12 +188,21 @@ func findTextLabel(base reflect.Value, pointedField reflect.Value) (string, bool
 		fieldlabel := strings.ToUpper(tagModifiers[0])
 		tagModifiers = tagModifiers[1:]
 		var fullLabel string
-		if slices.Contains(tagModifiers, "expand") {
-			fullLabel = ConcatTextLabels(fieldlabel, label)
+		if typefield.Anonymous || slices.Contains(tagModifiers, TextlogModifierExpand) || isResolver {
+			if labelVirtual {
+				// Virtual fields are actually not present in the text log (hence the
+				// name), so it does not make sense to combine the existent field label
+				// with the non-existent subfield label which would confuse the reader.
+				fullLabel = label
+			} else {
+				fullLabel = ConcatTextLabels(fieldlabel, label)
+			}
 		} else if label == "" {
 			fullLabel = fieldlabel
 		} else {
-			fullLabel = label
+			// Unexpanded fields should not use labels of any subfield because it
+			// cannot be resolved unambiguously.
+			return "", false
 		}
 		return fullLabel, true
 	}

reference_test.go

@@ -1,6 +1,7 @@
 package jsonlog
 
 import (
+	"strings"
 	"testing"
 
 	"github.com/NextronSystems/jsonlog/jsonpointer"
@@ -27,7 +28,7 @@ type testObject struct {
 
 	Subfield5 string `json:"subfield5" textlog:"subfield5"`
 
-	Valuer TestEventValuer `json:"valuer" textlog:"valuer"`
+	Resolver TestResolver `json:"resolver" textlog:"resolver"`
 
 	SubObject *SubObject `json:"subobject" textlog:"subobject,expand"`
 }
@@ -50,23 +51,32 @@ func (u UnexpandedSubstruct) String() string {
 	return u.SubField4
 }
 
-type TestEventValuer struct {
+type TestResolver struct {
 	Subfield6 string `json:"subfield6"`
 	Subfield7 string `json:"subfield7"`
 	Ignore    string
 }
 
-func (t *TestEventValuer) RelativeTextPointer(pointee any) (string, bool) {
+func (t TestResolver) String() string {
+	return strings.Join([]string{t.Subfield6, t.Subfield7}, ", ")
+}
+
+var _ TextReferenceResolver = (*TestResolver)(nil)
+
+func (t *TestResolver) RelativeTextPointer(pointee any) (string, bool, bool) {
+	virtual := true // We do not implement TextlogMarshaler
 	if pointee == &t.Subfield6 {
-		return "subfield6", true
+		return "subfield6", virtual, true
 	}
 	if pointee == &t.Subfield7 {
-		return "subfield7", true
+		return "subfield7", virtual, true
 	}
-	return "", false
+	return "", false, false
 }
 
-func (t *TestEventValuer) RelativeJsonPointer(pointee any) jsonpointer.Pointer {
+var _ JsonReferenceResolver = (*TestResolver)(nil)
+
+func (t *TestResolver) RelativeJsonPointer(pointee any) jsonpointer.Pointer {
 	if pointee == &t.Subfield6 {
 		return jsonpointer.New("subfield6")
 	}
@@ -88,8 +98,8 @@ func TestReference_ToJsonPointer(t *testing.T) {
 	test.Nested.Substruct.SubField3 = "subfield3"
 	test.Unexpanded.SubField4 = "subfield4"
 	test.Subfield5 = "subfield5"
-	test.Valuer.Subfield6 = "subfield6"
-	test.Valuer.Subfield7 = "subfield7"
+	test.Resolver.Subfield6 = "subfield6"
+	test.Resolver.Subfield7 = "subfield7"
 	test.SubObject = &SubObject{Subfield8: "subfield8"}
 	test.Recursive = NewReference(&test, &test.Substruct)
 
@@ -106,9 +116,9 @@ func TestReference_ToJsonPointer(t *testing.T) {
 		{&test.Unexpanded, "/unexpanded"},
 		{&test.Unexpanded.SubField4, "/unexpanded/subfield4"},
 		{&test.Subfield5, "/subfield5"},
-		{&test.Valuer, "/valuer"},
-		{&test.Valuer.Subfield6, "/valuer/subfield6"},
-		{&test.Valuer.Subfield7, "/valuer/subfield7"},
+		{&test.Resolver, "/resolver"},
+		{&test.Resolver.Subfield6, "/resolver/subfield6"},
+		{&test.Resolver.Subfield7, "/resolver/subfield7"},
 		{&test.SubObject, "/subobject"},
 		{&test.SubObject.Subfield8, "/subobject/subfield8"},
 		// test.Recursive not required here: if there is a flaw in the pointer
@@ -129,9 +139,10 @@ func TestReference_ToTextPointer(t *testing.T) {
 	test.Nested.Substruct.SubField3 = "subfield3"
 	test.Unexpanded.SubField4 = "subfield4"
 	test.Subfield5 = "subfield5"
-	test.Valuer.Subfield6 = "subfield6"
-	test.Valuer.Subfield7 = "subfield7"
+	test.Resolver.Subfield6 = "subfield6"
+	test.Resolver.Subfield7 = "subfield7"
 	test.SubObject = &SubObject{Subfield8: "subfield8"}
+	test.Recursive = NewReference(&test, &test.Substruct)
 
 	var tests = []struct {
 		PointedField any
@@ -144,11 +155,11 @@ func TestReference_ToTextPointer(t *testing.T) {
 		{&test.Nested.Substruct, "NESTED"},
 		{&test.Nested.Substruct.SubField3, "NESTED_SUBFIELD3"},
 		{&test.Unexpanded, "UNEXPANDED"},
-		{&test.Unexpanded.SubField4, "SUBFIELD4"},
+		{&test.Unexpanded.SubField4, ""},
 		{&test.Subfield5, "SUBFIELD5"},
-		{&test.Valuer, "VALUER"},
-		{&test.Valuer.Subfield6, "subfield6"},
-		{&test.Valuer.Subfield7, "subfield7"},
+		{&test.Resolver, "RESOLVER"},
+		{&test.Resolver.Subfield6, "subfield6"},
+		{&test.Resolver.Subfield7, "subfield7"},
 		{&test.SubObject, "SUBOBJECT"},
 		{&test.SubObject.Subfield8, "SUBOBJECT_SUBFIELD8"},
 	}

textlog.go

@@ -66,12 +66,10 @@ func (t TextlogFormatter) Format(object any) TextlogEntry {
 }
 
 const (
-	// modifierExpand, when applied to a struct field, causes the field to be expanded into its subfields
-	modifierExpand = "expand"
-	// modifierOmitempty causes a field to be omitted if it is the zero value or implements IsZero() and it returns true
-	modifierOmitempty = "omitempty"
-	// modifierExplicit causes a field to be marshalled even if the tag name is empty
-	modifierExplicit = "explicit"
+	// TextlogModifierExpand, when applied to a struct field, causes the field to be expanded into its subfields
+	TextlogModifierExpand = "expand"
+	// TextlogModifierOmitempty causes a field to be omitted if it is the zero value or implements IsZero() and it returns true
+	TextlogModifierOmitempty = "omitempty"
 )
 
 type TextlogMarshaler interface {
@@ -117,16 +115,16 @@ func (t TextlogFormatter) toEntry(object reflect.Value) TextlogEntry {
 			if logfield == "-" {
 				continue
 			}
-			if !typeField.Anonymous && textlogTag == "" && !slices.Contains(tagModifiers, modifierExplicit) {
+			if !typeField.Anonymous && textlogTag == "" {
 				continue
 			}
-			if slices.Contains(tagModifiers, modifierOmitempty) && isZero(field) {
+			if slices.Contains(tagModifiers, TextlogModifierOmitempty) && isZero(field) {
 				continue
 			}
 			if t.Omit != nil && t.Omit(tagModifiers, field.Interface()) {
 				continue
 			}
-			if typeField.Anonymous || slices.Contains(tagModifiers, modifierExpand) {
+			if typeField.Anonymous || slices.Contains(tagModifiers, TextlogModifierExpand) {
 				// Use the tag as a prefix for the subfields
 				subentry := t.toEntry(field)
 				for _, subentryValue := range subentry {

textlog_test.go

@@ -8,7 +8,9 @@ import (
 	"github.com/stretchr/testify/assert"
 )
 
-type TestObject struct {
+// TextlogTestObject is a test object to test textlog-specific details, mainly
+// that is the TextlogFormatter's behavior.
+type TextlogTestObject struct {
 	ObjectHeader
 	Element1   string `json:"element1" textlog:"element1"`
 	_          string `json:"-" textlog:"ignored"`
@@ -28,9 +30,9 @@ type SimpleSubstruct struct {
 }
 
 func TestToDetails(t *testing.T) {
-	var test = TestObject{
+	var test = TextlogTestObject{
 		ObjectHeader: ObjectHeader{
-			Type: "testobject",
+			Type: "textlogtestobject",
 		},
 		Element1:   "element1",
 		Element2:   "element2",
@@ -63,3 +65,33 @@ func TestToDetails(t *testing.T) {
 		{"TIME", "0001-01-01T00:00:00Z"},
 	}, details)
 }
+
+func TestTextlogFormatting(t *testing.T) {
+	var test testObject
+	test.Substruct.SubField1 = "subfield1"
+	test.SubField2 = "subfield2"
+	test.Nested.Substruct.SubField3 = "subfield3"
+	test.Unexpanded.SubField4 = "subfield4"
+	test.Subfield5 = "subfield5"
+	test.Resolver.Subfield6 = "subfield6"
+	test.Resolver.Subfield7 = "subfield7"
+	test.SubObject = &SubObject{Subfield8: "subfield8"}
+	test.Recursive = NewReference(&test, &test.Substruct)
+
+	formatter := TextlogFormatter{
+		FormatValue: func(data any, modifiers []string) string {
+			return fmt.Sprint(data)
+		},
+	}
+	details := formatter.Format(test)
+	t.Log(details)
+	assert.Equal(t, TextlogEntry{
+		{"SUBSTRUCT_SUBFIELD1", "subfield1"},
+		{"SUBFIELD2", "subfield2"},
+		{"NESTED_SUBFIELD3", "subfield3"},
+		{"UNEXPANDED", "subfield4"},
+		{"SUBFIELD5", "subfield5"},
+		{"RESOLVER", "subfield6, subfield7"},
+		{"SUBOBJECT_SUBFIELD8", "subfield8"},
+	}, details)
+}

thorlog/v3/kvlist.go

@@ -6,6 +6,7 @@ import (
 	"errors"
 	"strings"
 
+	"github.com/NextronSystems/jsonlog"
 	"github.com/NextronSystems/jsonlog/jsonpointer"
 )
 
@@ -97,17 +98,22 @@ func (d KeyValueList) RelativeJsonPointer(pointee any) jsonpointer.Pointer {
 	return nil
 }
 
-func (d KeyValueList) RelativeTextPointer(pointee any) (string, bool) {
+var _ jsonlog.TextReferenceResolver = KeyValueList{}
+
+func (d KeyValueList) RelativeTextPointer(pointee any) (string, bool, bool) {
+	// Pure virtual: String() combines all key value pairs thus individual keys are not separate fields in the text log
+	virtual := true
+
 	stringPointer, isStringPointer := pointee.(*string)
 	if !isStringPointer {
-		return "", false
+		return "", false, false
 	}
 	for i := range d {
 		if &d[i].Value == stringPointer {
-			return d[i].Key, true
+			return d[i].Key, virtual, true
 		}
 	}
-	return "", false
+	return "", false, false
 }
 
 func (d KeyValueList) Find(key string) *string {

thorlog/v3/matchstrings.go

@@ -187,7 +187,9 @@ func (f MatchString) String() string {
 			matchString += fmt.Sprintf(" at %#x", *f.Offset)
 		}
 	}
-	if f.Field != nil {
+	// Field may not be set. But its (text) label also might be empty, e.g., for
+	// matches on unexpanded fields.
+	if f.Field != nil && f.Field.String() != "" {
 		matchString += " in " + f.Field.String()
 	}
 

thorlog/v3/process.go

@@ -74,20 +74,25 @@ type Section struct {
 	Permissions RwxPermissions `json:"permissions"`
 }
 
+var _ jsonlog.TextReferenceResolver = (*Sections)(nil)
+
 // RelativeTextPointer implements the jsonlog.TextReferenceResolver interface for Sections.
 // It resolves a reference to a Section's SparseData field to a human-readable string.
-func (s *Sections) RelativeTextPointer(pointee any) (string, bool) {
+func (s *Sections) RelativeTextPointer(pointee any) (string, bool, bool) {
+	// Pure virtual: Sections are omitted in the textlog.
+	virtual := true
+
 	for i := range *s {
 		section := &(*s)[i]
 		if pointee == &section.SparseData {
 			if section.Name != "" {
-				return section.Name, true
+				return section.Name, virtual, true
 			} else {
-				return fmt.Sprintf("0x%x", section.Address), true
+				return fmt.Sprintf("0x%x", section.Address), virtual, true
 			}
 		}
 	}
-	return "", false
+	return "", false, false
 }
 
 type ProcessConnections struct {