From aec354cd0b9af19f8af6c09ffa915e8ebbb85ebf Mon Sep 17 00:00:00 2001 From: Stefan Haller Date: Thu, 9 Nov 2023 10:15:25 +0100 Subject: [PATCH] Shorten variable types (#3535) * Add ShortenType function Taken from https://github.com/aarzilli/gdlv/blob/master/internal/prettyprint/short.go with kind permission by @aarzilli. * Shorten type names in variable values The variables view in VS Code is a lot easier to read if long type names are shortened in much the same way as we shorten them for functions in the call stack view. We only shorten them in the value strings; the Type field of dap.Variable is kept as is. Since this only appears in a tooltip, it isn't a problem to have the full type visible there. --- service/api/prettyprint.go | 55 +++++++++++------ service/api/shorten_type.go | 102 +++++++++++++++++++++++++++++++ service/api/shorten_type_test.go | 36 +++++++++++ service/dap/server.go | 8 +-- service/dap/server_test.go | 2 +- 5 files changed, 179 insertions(+), 24 deletions(-) create mode 100644 service/api/shorten_type.go create mode 100644 service/api/shorten_type_test.go diff --git a/service/api/prettyprint.go b/service/api/prettyprint.go index a5159e3d..123dab34 100644 --- a/service/api/prettyprint.go +++ b/service/api/prettyprint.go @@ -24,11 +24,13 @@ const ( prettyTop prettyFlags = 1 << iota prettyNewlines prettyIncludeType + prettyShortenType ) func (flags prettyFlags) top() bool { return flags&prettyTop != 0 } func (flags prettyFlags) includeType() bool { return flags&prettyIncludeType != 0 } func (flags prettyFlags) newlines() bool { return flags&prettyNewlines != 0 } +func (flags prettyFlags) shortenType() bool { return flags&prettyShortenType != 0 } func (flags prettyFlags) set(flag prettyFlags, v bool) prettyFlags { if v { @@ -45,6 +47,13 @@ func (v *Variable) SinglelineString() string { return buf.String() } +// SinglelineStringWithShortTypes returns a representation of v on a single line, with types shortened. +func (v *Variable) SinglelineStringWithShortTypes() string { + var buf bytes.Buffer + v.writeTo(&buf, prettyTop|prettyIncludeType|prettyShortenType, "", "") + return buf.String() +} + // SinglelineStringFormatted returns a representation of v on a single line, using the format specified by fmtstr. func (v *Variable) SinglelineStringFormatted(fmtstr string) string { var buf bytes.Buffer @@ -59,6 +68,14 @@ func (v *Variable) MultilineString(indent, fmtstr string) string { return buf.String() } +func (v *Variable) typeStr(flags prettyFlags) string { + if flags.shortenType() { + return ShortenType(v.Type) + } + + return v.Type +} + func (v *Variable) writeTo(buf io.Writer, flags prettyFlags, indent, fmtstr string) { if v.Unreadable != "" { fmt.Fprintf(buf, "(unreadable %s)", v.Unreadable) @@ -67,7 +84,7 @@ func (v *Variable) writeTo(buf io.Writer, flags prettyFlags, indent, fmtstr stri if !flags.top() && v.Addr == 0 && v.Value == "" { if flags.includeType() && v.Type != "void" { - fmt.Fprintf(buf, "%s nil", v.Type) + fmt.Fprintf(buf, "%s nil", v.typeStr(flags)) } else { fmt.Fprint(buf, "nil") } @@ -83,10 +100,10 @@ func (v *Variable) writeTo(buf io.Writer, flags prettyFlags, indent, fmtstr stri if v.Type == "" || len(v.Children) == 0 { fmt.Fprint(buf, "nil") } else if v.Children[0].OnlyAddr && v.Children[0].Addr != 0 { - v.writePointerTo(buf) + v.writePointerTo(buf, flags) } else { if flags.top() && flags.newlines() && v.Children[0].Addr != 0 { - v.writePointerTo(buf) + v.writePointerTo(buf, flags) fmt.Fprint(buf, "\n") } fmt.Fprint(buf, "*") @@ -103,14 +120,14 @@ func (v *Variable) writeTo(buf io.Writer, flags prettyFlags, indent, fmtstr stri v.writeStructTo(buf, flags, indent, fmtstr) } else { if len(v.Children) == 0 { - fmt.Fprintf(buf, "%s nil", v.Type) + fmt.Fprintf(buf, "%s nil", v.typeStr(flags)) } else { - fmt.Fprintf(buf, "%s %s/%s", v.Type, v.Children[0].Value, v.Children[1].Value) + fmt.Fprintf(buf, "%s %s/%s", v.typeStr(flags), v.Children[0].Value, v.Children[1].Value) } } case reflect.Struct: if v.Value != "" { - fmt.Fprintf(buf, "%s(%s)", v.Type, v.Value) + fmt.Fprintf(buf, "%s(%s)", v.typeStr(flags), v.Value) flags = flags.set(prettyIncludeType, false) } v.writeStructTo(buf, flags, indent, fmtstr) @@ -123,13 +140,13 @@ func (v *Variable) writeTo(buf io.Writer, flags prettyFlags, indent, fmtstr stri } if flags.includeType() { if v.Children[0].Kind == reflect.Invalid { - fmt.Fprintf(buf, "%s ", v.Type) + fmt.Fprintf(buf, "%s ", v.typeStr(flags)) if v.Children[0].Addr == 0 { fmt.Fprint(buf, "nil") return } } else { - fmt.Fprintf(buf, "%s(%s) ", v.Type, v.Children[0].Type) + fmt.Fprintf(buf, "%s(%s) ", v.typeStr(flags), v.Children[0].Type) } } data := v.Children[0] @@ -145,9 +162,9 @@ func (v *Variable) writeTo(buf io.Writer, flags prettyFlags, indent, fmtstr stri } } else if data.OnlyAddr { if strings.Contains(v.Type, "/") { - fmt.Fprintf(buf, "*(*%q)(%#x)", v.Type, v.Addr) + fmt.Fprintf(buf, "*(*%q)(%#x)", v.typeStr(flags), v.Addr) } else { - fmt.Fprintf(buf, "*(*%s)(%#x)", v.Type, v.Addr) + fmt.Fprintf(buf, "*(*%s)(%#x)", v.typeStr(flags), v.Addr) } } else { v.Children[0].writeTo(buf, flags.set(prettyTop, false).set(prettyIncludeType, !flags.includeType()), indent, fmtstr) @@ -165,11 +182,11 @@ func (v *Variable) writeTo(buf io.Writer, flags prettyFlags, indent, fmtstr stri } } -func (v *Variable) writePointerTo(buf io.Writer) { +func (v *Variable) writePointerTo(buf io.Writer, flags prettyFlags) { if strings.Contains(v.Type, "/") { - fmt.Fprintf(buf, "(%q)(%#x)", v.Type, v.Children[0].Addr) + fmt.Fprintf(buf, "(%q)(%#x)", v.typeStr(flags), v.Children[0].Addr) } else { - fmt.Fprintf(buf, "(%s)(%#x)", v.Type, v.Children[0].Addr) + fmt.Fprintf(buf, "(%s)(%#x)", v.typeStr(flags), v.Children[0].Addr) } } @@ -248,7 +265,7 @@ func ExtractIntValue(s string) string { func (v *Variable) writeSliceTo(buf io.Writer, flags prettyFlags, indent, fmtstr string) { if flags.includeType() { - fmt.Fprintf(buf, "%s len: %d, cap: %d, ", v.Type, v.Len, v.Cap) + fmt.Fprintf(buf, "%s len: %d, cap: %d, ", v.typeStr(flags), v.Len, v.Cap) } if v.Base == 0 && len(v.Children) == 0 { fmt.Fprintf(buf, "nil") @@ -259,7 +276,7 @@ func (v *Variable) writeSliceTo(buf io.Writer, flags prettyFlags, indent, fmtstr func (v *Variable) writeArrayTo(buf io.Writer, flags prettyFlags, indent, fmtstr string) { if flags.includeType() { - fmt.Fprintf(buf, "%s ", v.Type) + fmt.Fprintf(buf, "%s ", v.typeStr(flags)) } v.writeSliceOrArrayTo(buf, flags, indent, fmtstr) } @@ -267,15 +284,15 @@ func (v *Variable) writeArrayTo(buf io.Writer, flags prettyFlags, indent, fmtstr func (v *Variable) writeStructTo(buf io.Writer, flags prettyFlags, indent, fmtstr string) { if int(v.Len) != len(v.Children) && len(v.Children) == 0 { if strings.Contains(v.Type, "/") { - fmt.Fprintf(buf, "(*%q)(%#x)", v.Type, v.Addr) + fmt.Fprintf(buf, "(*%q)(%#x)", v.typeStr(flags), v.Addr) } else { - fmt.Fprintf(buf, "(*%s)(%#x)", v.Type, v.Addr) + fmt.Fprintf(buf, "(*%s)(%#x)", v.typeStr(flags), v.Addr) } return } if flags.includeType() { - fmt.Fprintf(buf, "%s ", v.Type) + fmt.Fprintf(buf, "%s ", v.typeStr(flags)) } nl := v.shouldNewlineStruct(flags.newlines()) @@ -310,7 +327,7 @@ func (v *Variable) writeStructTo(buf io.Writer, flags prettyFlags, indent, fmtst func (v *Variable) writeMapTo(buf io.Writer, flags prettyFlags, indent, fmtstr string) { if flags.includeType() { - fmt.Fprintf(buf, "%s ", v.Type) + fmt.Fprintf(buf, "%s ", v.typeStr(flags)) } if v.Base == 0 && len(v.Children) == 0 { fmt.Fprintf(buf, "nil") diff --git a/service/api/shorten_type.go b/service/api/shorten_type.go new file mode 100644 index 00000000..0e3ca8e3 --- /dev/null +++ b/service/api/shorten_type.go @@ -0,0 +1,102 @@ +package api + +import ( + "strings" + "unicode" +) + +func ShortenType(typ string) string { + out, ok := shortenTypeEx(typ) + if !ok { + return typ + } + return out +} + +func shortenTypeEx(typ string) (string, bool) { + switch { + case strings.HasPrefix(typ, "["): + for i := range typ { + if typ[i] == ']' { + sub, ok := shortenTypeEx(typ[i+1:]) + return typ[:i+1] + sub, ok + } + } + return "", false + case strings.HasPrefix(typ, "*"): + sub, ok := shortenTypeEx(typ[1:]) + return "*" + sub, ok + case strings.HasPrefix(typ, "map["): + depth := 1 + for i := 4; i < len(typ); i++ { + switch typ[i] { + case '[': + depth++ + case ']': + depth-- + if depth == 0 { + key, keyok := shortenTypeEx(typ[4:i]) + val, valok := shortenTypeEx(typ[i+1:]) + return "map[" + key + "]" + val, keyok && valok + } + } + } + return "", false + case typ == "interface {}" || typ == "interface{}": + return typ, true + case typ == "struct {}" || typ == "struct{}": + return typ, true + default: + if containsAnonymousType(typ) { + return "", false + } + + if lbrk := strings.Index(typ, "["); lbrk >= 0 { + if typ[len(typ)-1] != ']' { + return "", false + } + typ0, ok := shortenTypeEx(typ[:lbrk]) + if !ok { + return "", false + } + args := strings.Split(typ[lbrk+1:len(typ)-1], ",") + for i := range args { + var ok bool + args[i], ok = shortenTypeEx(strings.TrimSpace(args[i])) + if !ok { + return "", false + } + } + return typ0 + "[" + strings.Join(args, ", ") + "]", true + } + + slashnum := 0 + slash := -1 + for i, ch := range typ { + if !unicode.IsLetter(ch) && !unicode.IsDigit(ch) && ch != '_' && ch != '.' && ch != '/' && ch != '@' && ch != '%' && ch != '-' { + return "", false + } + if ch == '/' { + slash = i + slashnum++ + } + } + if slashnum <= 1 || slash < 0 { + return typ, true + } + return typ[slash+1:], true + } +} + +func containsAnonymousType(typ string) bool { + for _, thing := range []string{"interface {", "interface{", "struct {", "struct{", "func (", "func("} { + idx := strings.Index(typ, thing) + if idx >= 0 && idx+len(thing) < len(typ) { + ch := typ[idx+len(thing)] + if ch != '}' && ch != ')' { + return true + } + } + } + return false +} diff --git a/service/api/shorten_type_test.go b/service/api/shorten_type_test.go new file mode 100644 index 00000000..8a2595a8 --- /dev/null +++ b/service/api/shorten_type_test.go @@ -0,0 +1,36 @@ +package api + +import "testing" + +func TestShortenType(t *testing.T) { + tests := []struct { + input string + expected string + }{ + {"long/package/path/pkg.A", "pkg.A"}, + {"[]long/package/path/pkg.A", "[]pkg.A"}, + {"map[long/package/path/pkg.A]long/package/path/pkg.B", "map[pkg.A]pkg.B"}, + {"map[long/package/path/pkg.A]interface {}", "map[pkg.A]interface {}"}, + {"map[long/package/path/pkg.A]interface{}", "map[pkg.A]interface{}"}, + {"map[long/package/path/pkg.A]struct {}", "map[pkg.A]struct {}"}, + {"map[long/package/path/pkg.A]struct{}", "map[pkg.A]struct{}"}, + {"map[long/package/path/pkg.A]map[long/package/path/pkg.B]long/package/path/pkg.C", "map[pkg.A]map[pkg.B]pkg.C"}, + {"map[long/package/path/pkg.A][]long/package/path/pkg.B", "map[pkg.A][]pkg.B"}, + {"map[uint64]*github.com/aarzilli/dwarf5/dwarf.typeUnit", "map[uint64]*dwarf.typeUnit"}, + {"uint8", "uint8"}, + {"encoding/binary", "encoding/binary"}, + {"*github.com/go-delve/delve/pkg/proc.Target", "*proc.Target"}, + {"long/package/path/pkg.Parametric[long/package/path/pkg.A, map[long/package/path/pkg.B]long/package/path/pkg.A]", "pkg.Parametric[pkg.A, map[pkg.B]pkg.A]"}, + {"[]long/package/path/pkg.Parametric[long/package/path/pkg.A]", "[]pkg.Parametric[pkg.A]"}, + {"[24]long/package/path/pkg.A", "[24]pkg.A"}, + {"chan func()", "chan func()"}, + } + for _, tt := range tests { + t.Run(tt.input, func(t *testing.T) { + result := ShortenType(tt.input) + if result != tt.expected { + t.Errorf("shortenTypeEx() got = %v, want %v", result, tt.expected) + } + }) + } +} diff --git a/service/dap/server.go b/service/dap/server.go index 21e47022..1880fd2e 100644 --- a/service/dap/server.go +++ b/service/dap/server.go @@ -2589,7 +2589,7 @@ func (s *Session) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr } return s.variableHandles.create(&fullyQualifiedVariable{v, qualifiedNameOrExpr, false /*not a scope*/, 0}) } - value = api.ConvertVar(v).SinglelineString() + value = api.ConvertVar(v).SinglelineStringWithShortTypes() if v.Unreadable != nil { return value, 0 } @@ -2603,7 +2603,7 @@ func (s *Session) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr // TODO(polina): Get *proc.Variable object from debugger instead. Export a function to set v.loaded to false // and call v.loadValue gain with a different load config. It's more efficient, and it's guaranteed to keep // working with generics. - value = api.ConvertVar(v).SinglelineString() + value = api.ConvertVar(v).SinglelineStringWithShortTypes() typeName := api.PrettyTypeName(v.DwarfType) loadExpr := fmt.Sprintf("*(*%q)(%#x)", typeName, v.Addr) s.config.log.Debugf("loading %s (type %s) with %s", qualifiedNameOrExpr, typeName, loadExpr) @@ -2617,7 +2617,7 @@ func (s *Session) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr } else { v.Children = vLoaded.Children v.Value = vLoaded.Value - value = api.ConvertVar(v).SinglelineString() + value = api.ConvertVar(v).SinglelineStringWithShortTypes() } return value } @@ -2649,7 +2649,7 @@ func (s *Session) convertVariableWithOpts(v *proc.Variable, qualifiedNameOrExpr } else { cLoaded.Name = v.Children[0].Name // otherwise, this will be the pointer expression v.Children = []proc.Variable{*cLoaded} - value = api.ConvertVar(v).SinglelineString() + value = api.ConvertVar(v).SinglelineStringWithShortTypes() } } else { value = reloadVariable(v, qualifiedNameOrExpr) diff --git a/service/dap/server_test.go b/service/dap/server_test.go index 611ddde2..c0cfae0d 100644 --- a/service/dap/server_test.go +++ b/service/dap/server_test.go @@ -2502,7 +2502,7 @@ func TestGlobalScopeAndVariables(t *testing.T) { client.VariablesRequest(globalsScope) globals := client.ExpectVariablesResponse(t) checkChildren(t, globals, "Globals", 1) - ref := checkVarExact(t, globals, 0, "SomeVar", "github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeVar", "github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType {X: 0}", "github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType", hasChildren) + ref := checkVarExact(t, globals, 0, "SomeVar", "github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeVar", "pkg.SomeType {X: 0}", "github.com/go-delve/delve/_fixtures/internal/dir0/pkg.SomeType", hasChildren) if ref > 0 { client.VariablesRequest(ref)