From ae3ad166ceb727285c8a31e00cce421ed97585f9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Mart=C3=AD?= Date: Wed, 28 Aug 2024 17:00:54 +0100 Subject: [PATCH] internal/astinternal: revise API with more options MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Add two options which will soon be useful. First, an OmitEmpty boolean option, as invalid or empty lines like the ones below are usually not helpful: Optional: token.Pos("-") Constraint: token.Token("ILLEGAL") Attrs: []*ast.Attribute{} Second, add a Filter func option which gives flexibility in terms of what Go values we are interested in. For example, the TOML tests will soon use this to only print token.Pos values. For #3379. Signed-off-by: Daniel Martí Change-Id: Iaa1af9f987d68b3cafeaaece2ef697e2fa2b7678 Reviewed-on: https://review.gerrithub.io/c/cue-lang/cue/+/1200204 Unity-Result: CUE porcuepine Reviewed-by: Matthew Sackman TryBot-Result: CUEcueckoo --- internal/astinternal/debug.go | 144 +++++++++++------- internal/astinternal/debug_test.go | 28 +++- .../testdata/debugprint/comprehensions.txtar | 66 ++++++++ .../testdata/debugprint/fields.txtar | 114 ++++++++++++++ .../testdata/debugprint/file.txtar | 88 +++++++++++ .../testdata/debugprint/unify.txtar | 44 ++++++ internal/cmd/cue-ast-print/main.go | 14 +- 7 files changed, 442 insertions(+), 56 deletions(-) diff --git a/internal/astinternal/debug.go b/internal/astinternal/debug.go index 0cbd5131676..06d0e30fce7 100644 --- a/internal/astinternal/debug.go +++ b/internal/astinternal/debug.go @@ -27,29 +27,38 @@ import ( "cuelang.org/go/internal" ) -// DebugPrint writes a multi-line Go-like representation of a syntax tree node, +// AppendDebug writes a multi-line Go-like representation of a syntax tree node, // including node position information and any relevant Go types. -// -// Note that since this is an internal debugging API, [io.Writer] errors are ignored, -// as it is assumed that the caller is using a [bytes.Buffer] or directly -// writing to standard output. -func DebugPrint(w io.Writer, node ast.Node) { - d := &debugPrinter{w: w} - d.value(reflect.ValueOf(node), nil) - d.newline() +func AppendDebug(dst []byte, node ast.Node, config DebugConfig) []byte { + d := &debugPrinter{cfg: config} + dst = d.value(dst, reflect.ValueOf(node), nil) + dst = d.newline(dst) + return dst +} + +// DebugConfig configures the behavior of [AppendDebug]. +type DebugConfig struct { + // Filter is called before each value in a syntax tree. + // Values for which the function returns false are omitted. + Filter func(reflect.Value) bool + + // OmitEmpty causes empty strings, empty structs, empty lists, + // nil pointers, invalid positions, and missing tokens to be omitted. + OmitEmpty bool } type debugPrinter struct { w io.Writer + cfg DebugConfig level int } -func (d *debugPrinter) printf(format string, args ...any) { - fmt.Fprintf(d.w, format, args...) +func (d *debugPrinter) printf(dst []byte, format string, args ...any) []byte { + return fmt.Appendf(dst, format, args...) } -func (d *debugPrinter) newline() { - fmt.Fprintf(d.w, "\n%s", strings.Repeat("\t", d.level)) +func (d *debugPrinter) newline(dst []byte) []byte { + return fmt.Appendf(dst, "\n%s", strings.Repeat("\t", d.level)) } var ( @@ -57,15 +66,20 @@ var ( typeTokenToken = reflect.TypeFor[token.Token]() ) -func (d *debugPrinter) value(v reflect.Value, impliedType reflect.Type) { +func (d *debugPrinter) value(dst []byte, v reflect.Value, impliedType reflect.Type) []byte { + if d.cfg.Filter != nil && !d.cfg.Filter(v) { + return dst + } // Skip over interface types. if v.Kind() == reflect.Interface { v = v.Elem() } // Indirecting a nil interface gives a zero value. if !v.IsValid() { - d.printf("nil") - return + if !d.cfg.OmitEmpty { + dst = d.printf(dst, "nil") + } + return dst } // We print the original pointer type if there was one. @@ -74,54 +88,72 @@ func (d *debugPrinter) value(v reflect.Value, impliedType reflect.Type) { v = reflect.Indirect(v) // Indirecting a nil pointer gives a zero value. if !v.IsValid() { - d.printf("nil") - return + if !d.cfg.OmitEmpty { + dst = d.printf(dst, "nil") + } + return dst + } + + if d.cfg.OmitEmpty && v.IsZero() { + return dst } t := v.Type() switch t { // Simple types which can stringify themselves. case typeTokenPos, typeTokenToken: - d.printf("%s(%q)", t, v) - return + dst = d.printf(dst, "%s(%q)", t, v) + return dst } + undoValue := len(dst) switch t.Kind() { default: // We assume all other kinds are basic in practice, like string or bool. if t.PkgPath() != "" { // Mention defined and non-predeclared types, for clarity. - d.printf("%s(%#v)", t, v) + dst = d.printf(dst, "%s(%#v)", t, v) } else { - d.printf("%#v", v) + dst = d.printf(dst, "%#v", v) } case reflect.Slice: if origType != impliedType { - d.printf("%s", origType) - } - d.printf("{") - if v.Len() > 0 { - d.level++ - for i := 0; i < v.Len(); i++ { - d.newline() - ev := v.Index(i) - // Note: a slice literal implies the type of its elements - // so we can avoid mentioning the type - // of each element if it matches. - d.value(ev, t.Elem()) + dst = d.printf(dst, "%s", origType) + } + dst = d.printf(dst, "{") + d.level++ + anyElems := false + for i := 0; i < v.Len(); i++ { + ev := v.Index(i) + undoElem := len(dst) + dst = d.newline(dst) + // Note: a slice literal implies the type of its elements + // so we can avoid mentioning the type + // of each element if it matches. + if dst2 := d.value(dst, ev, t.Elem()); len(dst2) == len(dst) { + dst = dst[:undoElem] + } else { + dst = dst2 + anyElems = true } - d.level-- - d.newline() } - d.printf("}") + d.level-- + if !anyElems && d.cfg.OmitEmpty { + dst = dst[:undoValue] + } else { + if anyElems { + dst = d.newline(dst) + } + dst = d.printf(dst, "}") + } case reflect.Struct: if origType != impliedType { - d.printf("%s", origType) + dst = d.printf(dst, "%s", origType) } - d.printf("{") - printed := false + dst = d.printf(dst, "{") + anyElems := false d.level++ for i := 0; i < v.NumField(); i++ { f := t.Field(i) @@ -133,28 +165,38 @@ func (d *debugPrinter) value(v reflect.Value, impliedType reflect.Type) { case "Scope", "Node", "Unresolved": continue } - printed = true - d.newline() - d.printf("%s: ", f.Name) - d.value(v.Field(i), nil) + undoElem := len(dst) + dst = d.newline(dst) + dst = d.printf(dst, "%s: ", f.Name) + if dst2 := d.value(dst, v.Field(i), nil); len(dst2) == len(dst) { + dst = dst[:undoElem] + } else { + dst = dst2 + anyElems = true + } } val := v.Addr().Interface() if val, ok := val.(ast.Node); ok { // Comments attached to a node aren't a regular field, but are still useful. // The majority of nodes won't have comments, so skip them when empty. if comments := ast.Comments(val); len(comments) > 0 { - printed = true - d.newline() - d.printf("Comments: ") - d.value(reflect.ValueOf(comments), nil) + anyElems = true + dst = d.newline(dst) + dst = d.printf(dst, "Comments: ") + dst = d.value(dst, reflect.ValueOf(comments), nil) } } d.level-- - if printed { - d.newline() + if !anyElems && d.cfg.OmitEmpty { + dst = dst[:undoValue] + } else { + if anyElems { + dst = d.newline(dst) + } + dst = d.printf(dst, "}") } - d.printf("}") } + return dst } func DebugStr(x interface{}) (out string) { diff --git a/internal/astinternal/debug_test.go b/internal/astinternal/debug_test.go index 60c5fddc9d4..66dd713c645 100644 --- a/internal/astinternal/debug_test.go +++ b/internal/astinternal/debug_test.go @@ -15,9 +15,12 @@ package astinternal_test import ( + "path" + "reflect" "strings" "testing" + "cuelang.org/go/cue/ast" "cuelang.org/go/cue/parser" "cuelang.org/go/internal/astinternal" "cuelang.org/go/internal/cuetxtar" @@ -39,8 +42,29 @@ func TestDebugPrint(t *testing.T) { f, err := parser.ParseFile(file.Name, file.Data, parser.ParseComments) qt.Assert(t, qt.IsNil(err)) - w := t.Writer(file.Name) - astinternal.DebugPrint(w, f) + // The full syntax tree, as printed by default. + full := astinternal.AppendDebug(nil, f, astinternal.DebugConfig{}) + t.Writer(file.Name).Write(full) + + // A syntax tree which omits any empty values, + // and is only interested in showing string fields. + // We allow ast.Nodes and slices to not stop too early. + typNode := reflect.TypeFor[ast.Node]() + strings := astinternal.AppendDebug(nil, f, astinternal.DebugConfig{ + OmitEmpty: true, + Filter: func(v reflect.Value) bool { + if v.Type().Implements(typNode) { + return true + } + switch v.Kind() { + case reflect.Slice, reflect.String: + return true + default: + return false + } + }, + }) + t.Writer(path.Join(file.Name, "omitempty-strings")).Write(strings) } }) } diff --git a/internal/astinternal/testdata/debugprint/comprehensions.txtar b/internal/astinternal/testdata/debugprint/comprehensions.txtar index 1ee54421a24..7c8c23629df 100644 --- a/internal/astinternal/testdata/debugprint/comprehensions.txtar +++ b/internal/astinternal/testdata/debugprint/comprehensions.txtar @@ -107,3 +107,69 @@ for k, v in input if v > 2 { } Imports: []*ast.ImportSpec{} } +-- out/debugprint/comprehensions.cue/omitempty-strings -- +*ast.File{ + Filename: "comprehensions.cue" + Decls: []ast.Decl{ + *ast.Comprehension{ + Clauses: []ast.Clause{ + *ast.IfClause{ + Condition: *ast.Ident{ + Name: "condition" + } + } + } + Value: *ast.StructLit{ + Elts: []ast.Decl{ + *ast.Field{ + Label: *ast.Ident{ + Name: "a" + } + Value: *ast.BasicLit{ + Value: "true" + } + } + } + } + } + *ast.Comprehension{ + Clauses: []ast.Clause{ + *ast.ForClause{ + Key: *ast.Ident{ + Name: "k" + } + Value: *ast.Ident{ + Name: "v" + } + Source: *ast.Ident{ + Name: "input" + } + } + *ast.IfClause{ + Condition: *ast.BinaryExpr{ + X: *ast.Ident{ + Name: "v" + } + Y: *ast.BasicLit{ + Value: "2" + } + } + } + } + Value: *ast.StructLit{ + Elts: []ast.Decl{ + *ast.Field{ + Label: *ast.ParenExpr{ + X: *ast.Ident{ + Name: "k" + } + } + Value: *ast.Ident{ + Name: "v" + } + } + } + } + } + } +} diff --git a/internal/astinternal/testdata/debugprint/fields.txtar b/internal/astinternal/testdata/debugprint/fields.txtar index 32fa775e52c..e6b60b130ec 100644 --- a/internal/astinternal/testdata/debugprint/fields.txtar +++ b/internal/astinternal/testdata/debugprint/fields.txtar @@ -206,3 +206,117 @@ embed: { } Imports: []*ast.ImportSpec{} } +-- out/debugprint/fields.cue/omitempty-strings -- +*ast.File{ + Filename: "fields.cue" + Decls: []ast.Decl{ + *ast.Field{ + Label: *ast.Ident{ + Name: "a" + } + Value: *ast.BasicLit{ + Value: "1" + } + Attrs: []*ast.Attribute{ + { + Text: "@xml(,attr)" + } + } + } + *ast.Field{ + Label: *ast.Ident{ + Name: "b" + } + Value: *ast.BasicLit{ + Value: "2" + } + Attrs: []*ast.Attribute{ + { + Text: "@foo(a,b=4)" + } + { + Text: "@go(Foo)" + } + } + } + *ast.Field{ + Label: *ast.Ident{ + Name: "c" + } + Value: *ast.StructLit{ + Elts: []ast.Decl{ + *ast.Field{ + Label: *ast.Ident{ + Name: "d" + } + Value: *ast.Ident{ + Name: "string" + } + } + } + } + } + *ast.Field{ + Label: *ast.Alias{ + Ident: *ast.Ident{ + Name: "X" + } + Expr: *ast.Ident{ + Name: "e" + } + } + Value: *ast.StructLit{ + Elts: []ast.Decl{ + *ast.Field{ + Label: *ast.ListLit{ + Elts: []ast.Expr{ + *ast.Alias{ + Ident: *ast.Ident{ + Name: "Y" + } + Expr: *ast.Ident{ + Name: "string" + } + } + } + } + Value: *ast.Ident{ + Name: "int" + } + } + } + } + } + *ast.Field{ + Label: *ast.Ident{ + Name: "#Schema" + } + Value: *ast.StructLit{ + Elts: []ast.Decl{ + *ast.Field{ + Label: *ast.Ident{ + Name: "name" + } + Value: *ast.Ident{ + Name: "string" + } + } + } + } + } + *ast.Field{ + Label: *ast.Ident{ + Name: "embed" + } + Value: *ast.StructLit{ + Elts: []ast.Decl{ + *ast.EmbedDecl{ + Expr: *ast.Ident{ + Name: "#Schema" + } + } + } + } + } + } +} diff --git a/internal/astinternal/testdata/debugprint/file.txtar b/internal/astinternal/testdata/debugprint/file.txtar index 7ffee5e2cd1..080578f4439 100644 --- a/internal/astinternal/testdata/debugprint/file.txtar +++ b/internal/astinternal/testdata/debugprint/file.txtar @@ -145,3 +145,91 @@ import ( } } } +-- out/debugprint/empty.cue/omitempty-strings -- +*ast.File{ + Filename: "empty.cue" +} +-- out/debugprint/package_only.cue/omitempty-strings -- +*ast.File{ + Filename: "package_only.cue" + Decls: []ast.Decl{ + *ast.Package{ + Name: *ast.Ident{ + Name: "p" + } + } + } +} +-- out/debugprint/comments_only.cue/omitempty-strings -- +*ast.File{ + Filename: "comments_only.cue" + Comments: []*ast.CommentGroup{ + { + List: []*ast.Comment{ + { + Text: "// some" + } + { + Text: "// comment lines" + } + } + } + } +} +-- out/debugprint/imports.cue/omitempty-strings -- +*ast.File{ + Filename: "imports.cue" + Decls: []ast.Decl{ + *ast.Package{ + Name: *ast.Ident{ + Name: "p" + } + } + *ast.ImportDecl{ + Specs: []*ast.ImportSpec{ + { + Path: *ast.BasicLit{ + Value: "\"foo\"" + } + } + } + } + *ast.ImportDecl{ + Specs: []*ast.ImportSpec{ + { + Path: *ast.BasicLit{ + Value: "\"bar\"" + } + } + { + Name: *ast.Ident{ + Name: "name" + } + Path: *ast.BasicLit{ + Value: "\"baz\"" + } + } + } + } + } + Imports: []*ast.ImportSpec{ + { + Path: *ast.BasicLit{ + Value: "\"foo\"" + } + } + { + Path: *ast.BasicLit{ + Value: "\"bar\"" + } + } + { + Name: *ast.Ident{ + Name: "name" + } + Path: *ast.BasicLit{ + Value: "\"baz\"" + } + } + } +} diff --git a/internal/astinternal/testdata/debugprint/unify.txtar b/internal/astinternal/testdata/debugprint/unify.txtar index e5ee24c1031..b8bb50ac35b 100644 --- a/internal/astinternal/testdata/debugprint/unify.txtar +++ b/internal/astinternal/testdata/debugprint/unify.txtar @@ -74,3 +74,47 @@ disjunctoin: b | c | *d } Imports: []*ast.ImportSpec{} } +-- out/debugprint/unify.txtar/omitempty-strings -- +*ast.File{ + Filename: "unify.txtar" + Decls: []ast.Decl{ + *ast.Field{ + Label: *ast.Ident{ + Name: "unification" + } + Value: *ast.BinaryExpr{ + X: *ast.BinaryExpr{ + X: *ast.Ident{ + Name: "b" + } + Y: *ast.Ident{ + Name: "c" + } + } + Y: *ast.Ident{ + Name: "d" + } + } + } + *ast.Field{ + Label: *ast.Ident{ + Name: "disjunctoin" + } + Value: *ast.BinaryExpr{ + X: *ast.BinaryExpr{ + X: *ast.Ident{ + Name: "b" + } + Y: *ast.Ident{ + Name: "c" + } + } + Y: *ast.UnaryExpr{ + X: *ast.Ident{ + Name: "d" + } + } + } + } + } +} diff --git a/internal/cmd/cue-ast-print/main.go b/internal/cmd/cue-ast-print/main.go index 376f5633835..462845e641e 100644 --- a/internal/cmd/cue-ast-print/main.go +++ b/internal/cmd/cue-ast-print/main.go @@ -30,10 +30,17 @@ import ( func main() { flag.Usage = func() { - fmt.Fprintf(os.Stderr, "usage: cue-ast-print [file.cue]\n") - os.Exit(2) + fmt.Fprintf(flag.CommandLine.Output(), "usage: cue-ast-print [flags] [file.cue]\n") + flag.PrintDefaults() } + + var cfg astinternal.DebugConfig + flag.BoolVar(&cfg.OmitEmpty, "omitempty", false, "omit empty and invalid values") + // Note that DebugConfig also has a Filter func, but that doesn't lend itself well + // to a command line flag. Perhaps we could provide some commonly used filters, + // such as "positions only" or "skip positions". flag.Parse() + var filename string var src any switch flag.NArg() { @@ -53,5 +60,6 @@ func main() { if err != nil { log.Fatal(err) } - astinternal.DebugPrint(os.Stdout, file) + out := astinternal.AppendDebug(nil, file, cfg) + os.Stdout.Write(out) }