diff --git a/CHANGELOG.md b/CHANGELOG.md index aaf74a7..bb315fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,13 @@ The format is based on [Keep a Changelog], and this project adheres to [keep a changelog]: https://keepachangelog.com/en/1.0.0/ [semantic versioning]: https://semver.org/spec/v2.0.0.html +## [0.4.5] - 2022-08-07 + +### Fixed + +- `StringerFilter` now takes precedence over all other filters +- `FilterPrinter.Fallback()` now only skips the filter it was called from + ## [0.4.4] - 2022-08-07 ### Added diff --git a/filter.go b/filter.go index 67a70d3..dc80c63 100644 --- a/filter.go +++ b/filter.go @@ -37,16 +37,17 @@ type FilterPrinter interface { type filterPrinter struct { *visitor - value Value + currentFilter uintptr + value Value } func (p filterPrinter) Fallback(w io.Writer, c Config) { p.leave(p.value) vis := &visitor{ - config: c, - ignoreFilters: true, - recursionSet: p.recursionSet, + config: c, + skipFilter: p.currentFilter, + recursionSet: p.recursionSet, } vis.Write(w, p.value) diff --git a/filterprotobuf_test.go b/filterprotobuf_test.go index 35dfa86..2baa3b4 100644 --- a/filterprotobuf_test.go +++ b/filterprotobuf_test.go @@ -20,6 +20,9 @@ func TestPrinter_ProtobufFilter(t *testing.T) { NestedA: "foo", NestedB: []byte(""), }, + Stringer: &fixtures.Stringer{ + Value: "", + }, } // Trigger population of internal state to make sure it does not @@ -29,14 +32,15 @@ func TestPrinter_ProtobufFilter(t *testing.T) { actual := dapper.Format(m) expected := strings.Join([]string{ `*github.com/dogmatiq/dapper/internal/fixtures.Message{`, - ` Str: "hello"`, - ` Enum: 1`, - ` Nested: {`, + ` Str: "hello"`, + ` Enum: 1`, + ` Nested: {`, ` NestedA: "foo"`, ` NestedB: {`, ` 00000000 3c 62 79 74 65 73 3e ||`, ` }`, ` }`, + ` Stringer: []`, `}`, }, "\n") @@ -62,6 +66,9 @@ func TestPrinter_ProtobufFilter(t *testing.T) { "it renders a protocol buffers message properly when nested within a regular struct", func(t *testing.T) { m := &fixtures.Message{ Str: "hello", + Stringer: &fixtures.Stringer{ + Value: "", + }, } outerStruct := struct { @@ -74,9 +81,10 @@ func TestPrinter_ProtobufFilter(t *testing.T) { `{`, ` foo: "hi"`, ` bar: *github.com/dogmatiq/dapper/internal/fixtures.Message{`, - ` Str: "hello"`, - ` Enum: 0`, - ` Nested: nil`, + ` Str: "hello"`, + ` Enum: 0`, + ` Nested: nil`, + ` Stringer: []`, ` }`, `}`, }, "\n") diff --git a/filterstringer.go b/filterstringer.go index 9801d83..82ded73 100644 --- a/filterstringer.go +++ b/filterstringer.go @@ -27,16 +27,17 @@ func StringerFilter( return nil } + s := v.Value.Interface().(Stringer).DapperString() + if s == "" { + return nil + } + if v.IsAmbiguousType() { must.WriteString(w, p.FormatTypeName(v)) must.WriteByte(w, ' ') } - must.Fprintf( - w, - "[%s]", - v.Value.Interface().(Stringer).DapperString(), - ) + must.Fprintf(w, "[%s]", s) return nil } diff --git a/internal/fixtures/protostub.go b/internal/fixtures/protostub.go new file mode 100644 index 0000000..2f3c1bd --- /dev/null +++ b/internal/fixtures/protostub.go @@ -0,0 +1,7 @@ +package fixtures + +// DapperString is used to ensure that dapper.Stringer takes precedence over +// other filters (such as the protobuf filter). +func (x *Stringer) DapperString() string { + return x.GetValue() +} diff --git a/internal/fixtures/protostub.pb.go b/internal/fixtures/protostub.pb.go index 01c7da8..4e4fed3 100644 --- a/internal/fixtures/protostub.pb.go +++ b/internal/fixtures/protostub.pb.go @@ -74,9 +74,10 @@ type Message struct { sizeCache protoimpl.SizeCache unknownFields protoimpl.UnknownFields - Str string `protobuf:"bytes,1,opt,name=str,proto3" json:"str,omitempty"` - Enum Enum `protobuf:"varint,2,opt,name=enum,proto3,enum=dogmatiq.dapper.fixtures.Enum" json:"enum,omitempty"` - Nested *Nested `protobuf:"bytes,3,opt,name=nested,proto3" json:"nested,omitempty"` + Str string `protobuf:"bytes,1,opt,name=str,proto3" json:"str,omitempty"` + Enum Enum `protobuf:"varint,2,opt,name=enum,proto3,enum=dogmatiq.dapper.fixtures.Enum" json:"enum,omitempty"` + Nested *Nested `protobuf:"bytes,3,opt,name=nested,proto3" json:"nested,omitempty"` + Stringer *Stringer `protobuf:"bytes,4,opt,name=stringer,proto3" json:"stringer,omitempty"` } func (x *Message) Reset() { @@ -132,6 +133,13 @@ func (x *Message) GetNested() *Nested { return nil } +func (x *Message) GetStringer() *Stringer { + if x != nil { + return x.Stringer + } + return nil +} + type Nested struct { state protoimpl.MessageState sizeCache protoimpl.SizeCache @@ -187,6 +195,53 @@ func (x *Nested) GetNestedB() []byte { return nil } +type Stringer struct { + state protoimpl.MessageState + sizeCache protoimpl.SizeCache + unknownFields protoimpl.UnknownFields + + Value string `protobuf:"bytes,1,opt,name=value,proto3" json:"value,omitempty"` +} + +func (x *Stringer) Reset() { + *x = Stringer{} + if protoimpl.UnsafeEnabled { + mi := &file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_msgTypes[2] + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + ms.StoreMessageInfo(mi) + } +} + +func (x *Stringer) String() string { + return protoimpl.X.MessageStringOf(x) +} + +func (*Stringer) ProtoMessage() {} + +func (x *Stringer) ProtoReflect() protoreflect.Message { + mi := &file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_msgTypes[2] + if protoimpl.UnsafeEnabled && x != nil { + ms := protoimpl.X.MessageStateOf(protoimpl.Pointer(x)) + if ms.LoadMessageInfo() == nil { + ms.StoreMessageInfo(mi) + } + return ms + } + return mi.MessageOf(x) +} + +// Deprecated: Use Stringer.ProtoReflect.Descriptor instead. +func (*Stringer) Descriptor() ([]byte, []int) { + return file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_rawDescGZIP(), []int{2} +} + +func (x *Stringer) GetValue() string { + if x != nil { + return x.Value + } + return "" +} + var File_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto protoreflect.FileDescriptor var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_rawDesc = []byte{ @@ -195,7 +250,7 @@ var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_rawDesc = 0x65, 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x2f, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x73, 0x74, 0x75, 0x62, 0x2e, 0x70, 0x72, 0x6f, 0x74, 0x6f, 0x12, 0x18, 0x64, 0x6f, 0x67, 0x6d, 0x61, 0x74, 0x69, 0x71, 0x2e, 0x64, 0x61, 0x70, 0x70, 0x65, 0x72, 0x2e, - 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x22, 0x89, 0x01, 0x0a, 0x07, 0x4d, 0x65, 0x73, + 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x22, 0xc9, 0x01, 0x0a, 0x07, 0x4d, 0x65, 0x73, 0x73, 0x61, 0x67, 0x65, 0x12, 0x10, 0x0a, 0x03, 0x73, 0x74, 0x72, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x03, 0x73, 0x74, 0x72, 0x12, 0x32, 0x0a, 0x04, 0x65, 0x6e, 0x75, 0x6d, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0e, 0x32, 0x1e, 0x2e, 0x64, 0x6f, 0x67, 0x6d, 0x61, 0x74, 0x69, 0x71, 0x2e, @@ -204,17 +259,23 @@ var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_rawDesc = 0x73, 0x74, 0x65, 0x64, 0x18, 0x03, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x20, 0x2e, 0x64, 0x6f, 0x67, 0x6d, 0x61, 0x74, 0x69, 0x71, 0x2e, 0x64, 0x61, 0x70, 0x70, 0x65, 0x72, 0x2e, 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x2e, 0x4e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x52, 0x06, 0x6e, 0x65, - 0x73, 0x74, 0x65, 0x64, 0x22, 0x3e, 0x0a, 0x06, 0x4e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x12, 0x19, + 0x73, 0x74, 0x65, 0x64, 0x12, 0x3e, 0x0a, 0x08, 0x73, 0x74, 0x72, 0x69, 0x6e, 0x67, 0x65, 0x72, + 0x18, 0x04, 0x20, 0x01, 0x28, 0x0b, 0x32, 0x22, 0x2e, 0x64, 0x6f, 0x67, 0x6d, 0x61, 0x74, 0x69, + 0x71, 0x2e, 0x64, 0x61, 0x70, 0x70, 0x65, 0x72, 0x2e, 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, + 0x73, 0x2e, 0x53, 0x74, 0x72, 0x69, 0x6e, 0x67, 0x65, 0x72, 0x52, 0x08, 0x73, 0x74, 0x72, 0x69, + 0x6e, 0x67, 0x65, 0x72, 0x22, 0x3e, 0x0a, 0x06, 0x4e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x12, 0x19, 0x0a, 0x08, 0x6e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x5f, 0x61, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, 0x07, 0x6e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x41, 0x12, 0x19, 0x0a, 0x08, 0x6e, 0x65, 0x73, 0x74, 0x65, 0x64, 0x5f, 0x62, 0x18, 0x02, 0x20, 0x01, 0x28, 0x0c, 0x52, 0x07, 0x6e, 0x65, 0x73, - 0x74, 0x65, 0x64, 0x42, 0x2a, 0x25, 0x0a, 0x04, 0x45, 0x6e, 0x75, 0x6d, 0x12, 0x0b, 0x0a, 0x07, - 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x07, 0x0a, 0x03, 0x46, 0x4f, 0x4f, - 0x10, 0x01, 0x12, 0x07, 0x0a, 0x03, 0x42, 0x41, 0x52, 0x10, 0x02, 0x42, 0x2e, 0x5a, 0x2c, 0x67, - 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x64, 0x6f, 0x67, 0x6d, 0x61, 0x74, - 0x69, 0x71, 0x2f, 0x64, 0x61, 0x70, 0x70, 0x65, 0x72, 0x2f, 0x69, 0x6e, 0x74, 0x65, 0x72, 0x6e, - 0x61, 0x6c, 0x2f, 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x62, 0x06, 0x70, 0x72, 0x6f, - 0x74, 0x6f, 0x33, + 0x74, 0x65, 0x64, 0x42, 0x22, 0x20, 0x0a, 0x08, 0x53, 0x74, 0x72, 0x69, 0x6e, 0x67, 0x65, 0x72, + 0x12, 0x14, 0x0a, 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x18, 0x01, 0x20, 0x01, 0x28, 0x09, 0x52, + 0x05, 0x76, 0x61, 0x6c, 0x75, 0x65, 0x2a, 0x25, 0x0a, 0x04, 0x45, 0x6e, 0x75, 0x6d, 0x12, 0x0b, + 0x0a, 0x07, 0x55, 0x4e, 0x4b, 0x4e, 0x4f, 0x57, 0x4e, 0x10, 0x00, 0x12, 0x07, 0x0a, 0x03, 0x46, + 0x4f, 0x4f, 0x10, 0x01, 0x12, 0x07, 0x0a, 0x03, 0x42, 0x41, 0x52, 0x10, 0x02, 0x42, 0x2e, 0x5a, + 0x2c, 0x67, 0x69, 0x74, 0x68, 0x75, 0x62, 0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x64, 0x6f, 0x67, 0x6d, + 0x61, 0x74, 0x69, 0x71, 0x2f, 0x64, 0x61, 0x70, 0x70, 0x65, 0x72, 0x2f, 0x69, 0x6e, 0x74, 0x65, + 0x72, 0x6e, 0x61, 0x6c, 0x2f, 0x66, 0x69, 0x78, 0x74, 0x75, 0x72, 0x65, 0x73, 0x62, 0x06, 0x70, + 0x72, 0x6f, 0x74, 0x6f, 0x33, } var ( @@ -230,20 +291,22 @@ func file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_rawDescGZ } var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_enumTypes = make([]protoimpl.EnumInfo, 1) -var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_msgTypes = make([]protoimpl.MessageInfo, 2) +var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_msgTypes = make([]protoimpl.MessageInfo, 3) var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_goTypes = []interface{}{ - (Enum)(0), // 0: dogmatiq.dapper.fixtures.Enum - (*Message)(nil), // 1: dogmatiq.dapper.fixtures.Message - (*Nested)(nil), // 2: dogmatiq.dapper.fixtures.Nested + (Enum)(0), // 0: dogmatiq.dapper.fixtures.Enum + (*Message)(nil), // 1: dogmatiq.dapper.fixtures.Message + (*Nested)(nil), // 2: dogmatiq.dapper.fixtures.Nested + (*Stringer)(nil), // 3: dogmatiq.dapper.fixtures.Stringer } var file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_depIdxs = []int32{ 0, // 0: dogmatiq.dapper.fixtures.Message.enum:type_name -> dogmatiq.dapper.fixtures.Enum 2, // 1: dogmatiq.dapper.fixtures.Message.nested:type_name -> dogmatiq.dapper.fixtures.Nested - 2, // [2:2] is the sub-list for method output_type - 2, // [2:2] is the sub-list for method input_type - 2, // [2:2] is the sub-list for extension type_name - 2, // [2:2] is the sub-list for extension extendee - 0, // [0:2] is the sub-list for field type_name + 3, // 2: dogmatiq.dapper.fixtures.Message.stringer:type_name -> dogmatiq.dapper.fixtures.Stringer + 3, // [3:3] is the sub-list for method output_type + 3, // [3:3] is the sub-list for method input_type + 3, // [3:3] is the sub-list for extension type_name + 3, // [3:3] is the sub-list for extension extendee + 0, // [0:3] is the sub-list for field type_name } func init() { file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_init() } @@ -276,6 +339,18 @@ func file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_init() { return nil } } + file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_msgTypes[2].Exporter = func(v interface{}, i int) interface{} { + switch v := v.(*Stringer); i { + case 0: + return &v.state + case 1: + return &v.sizeCache + case 2: + return &v.unknownFields + default: + return nil + } + } } type x struct{} out := protoimpl.TypeBuilder{ @@ -283,7 +358,7 @@ func file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_init() { GoPackagePath: reflect.TypeOf(x{}).PkgPath(), RawDescriptor: file_github_com_dogmatiq_dapper_internal_fixtures_protostub_proto_rawDesc, NumEnums: 1, - NumMessages: 2, + NumMessages: 3, NumExtensions: 0, NumServices: 0, }, diff --git a/internal/fixtures/protostub.proto b/internal/fixtures/protostub.proto index 0ad9191..9622b7a 100644 --- a/internal/fixtures/protostub.proto +++ b/internal/fixtures/protostub.proto @@ -7,6 +7,7 @@ message Message { string str = 1; Enum enum = 2; Nested nested = 3; + Stringer stringer = 4; } message Nested { @@ -14,6 +15,10 @@ message Nested { bytes nested_b = 2; } +message Stringer{ + string value = 1; +} + enum Enum { UNKNOWN = 0; FOO = 1; diff --git a/map.go b/map.go index e105536..807cf72 100644 --- a/map.go +++ b/map.go @@ -22,7 +22,7 @@ func (vis *visitor) visitMap(w io.Writer, v Value) { Map: v, KeyType: v.DynamicType.Key(), ValueType: v.DynamicType.Elem(), - Printer: &filterPrinter{vis, v}, + Printer: &filterPrinter{vis, 0, v}, Indent: vis.config.Indent, } diff --git a/printer.go b/printer.go index 743e737..fe6e339 100644 --- a/printer.go +++ b/printer.go @@ -128,10 +128,10 @@ func (p *Printer) Format(v interface{}) string { var DefaultPrinter = Printer{ Config: Config{ Filters: []Filter{ + StringerFilter, DurationFilter, ProtobufFilter, ReflectTypeFilter, - StringerFilter, SyncFilter, TimeFilter, }, diff --git a/visitor.go b/visitor.go index 4ec8e5d..bcef230 100644 --- a/visitor.go +++ b/visitor.go @@ -19,9 +19,15 @@ type visitor struct { // config is the printer's configuration. config Config - // ignoreFilters indicates whether the visitor should skip filters when - // rendering values. - ignoreFilters bool + // skipFilter causes the filter at the given address to be skipped when + // rendering the next value. + // + // HACK: We're using function pointers to compare functions. This behavior + // is technically undefined and may stop working in the future. + // + // The Filter system needs to be refactored to allow filters to have + // identity, perhaps by converting Filter to an interface. + skipFilter uintptr // recursionSet is the set of potentially recursive values that are // currently being visited. @@ -44,25 +50,32 @@ func (vis *visitor) Write(w io.Writer, v Value) { v.Value = unsafereflect.MakeMutable(v.Value) - if !vis.ignoreFilters { - w := count.NewWriter(w) + cw := count.NewWriter(w) - for _, f := range vis.config.Filters { - p := filterPrinter{ - visitor: vis, - value: v, - } + for _, f := range vis.config.Filters { + ptr := reflect.ValueOf(f).Pointer() - if err := f(w, v, vis.config, p); err != nil { - panic(must.PanicSentinel{Cause: err}) - } + if ptr == vis.skipFilter { + continue + } - if w.Count() > 0 { - return - } + p := filterPrinter{ + visitor: vis, + currentFilter: ptr, + value: v, + } + + if err := f(cw, v, vis.config, p); err != nil { + panic(must.PanicSentinel{Cause: err}) + } + + if cw.Count() > 0 { + return } } + vis.skipFilter = 0 + switch v.DynamicType.Kind() { case reflect.String: vis.visitString(w, v)