]> Cypherpunks repositories - gostls13.git/commitdiff
go/printer: idempotent comment formatting
authorRobert Griesemer <gri@golang.org>
Wed, 10 Oct 2012 00:08:09 +0000 (17:08 -0700)
committerRobert Griesemer <gri@golang.org>
Wed, 10 Oct 2012 00:08:09 +0000 (17:08 -0700)
Also:

- Refactored testing framework to permit easier
idempotency testing.

- Applied gofmt -w src misc

This CL depends on CL 6639044 being applied first.

Formatting is not idempotent for all files: In those
files the comment position has changed (due to missing
precise location information) and/or the comment formatting
cannot/is not aware of independent code re-formatting.
In general it is very hard to make format idempotent when
running it in one pass only. Leaving that aside for now.

Fixes #1835.

R=r, rsc
CC=golang-dev
https://golang.org/cl/6624051

src/pkg/go/printer/printer.go
src/pkg/go/printer/printer_test.go
src/pkg/go/printer/testdata/comments2.golden [new file with mode: 0644]
src/pkg/go/printer/testdata/comments2.input [new file with mode: 0644]

index a027d32da89889e0ecacca8831904512f729f1ae..516c37161c13959b3fa433880e99202ce4640dc1 100644 (file)
@@ -449,11 +449,17 @@ func commonPrefix(a, b string) string {
        return a[0:i]
 }
 
+// stripCommonPrefix removes a common prefix from /*-style comment lines (unless no
+// comment line is indented, all but the first line have some form of space prefix).
+// The prefix is computed using heuristics such that is is likely that the comment
+// contents are nicely laid out after re-printing each line using the printer's
+// current indentation.
+//
 func stripCommonPrefix(lines []string) {
-       if len(lines) < 2 {
+       if len(lines) <= 1 {
                return // at most one line - nothing to do
        }
-       // len(lines) >= 2
+       // len(lines) > 1
 
        // The heuristic in this function tries to handle a few
        // common patterns of /*-style comments: Comments where
@@ -479,7 +485,7 @@ func stripCommonPrefix(lines []string) {
                for i, line := range lines[1 : len(lines)-1] {
                        switch {
                        case isBlank(line):
-                               lines[1+i] = "" // range starts at line 1
+                               lines[1+i] = "" // range starts with lines[1]
                        case first:
                                prefix = commonPrefix(line, line)
                                first = false
@@ -570,9 +576,9 @@ func stripCommonPrefix(lines []string) {
        }
 
        // Remove the common prefix from all but the first and empty lines.
-       for i, line := range lines[1:] {
-               if len(line) != 0 {
-                       lines[1+i] = line[len(prefix):] // range starts at line 1
+       for i, line := range lines {
+               if i > 0 && line != "" {
+                       lines[i] = line[len(prefix):]
                }
        }
 }
@@ -612,6 +618,19 @@ func (p *printer) writeComment(comment *ast.Comment) {
        // for /*-style comments, print line by line and let the
        // write function take care of the proper indentation
        lines := split(text)
+
+       // The comment started in the first column but is going
+       // to be indented. For an idempotent result, add indentation
+       // to all lines such that they look like they were indented
+       // before - this will make sure the common prefix computation
+       // is the same independent of how many times formatting is
+       // applied (was issue 1835).
+       if pos.IsValid() && pos.Column == 1 && p.indent > 0 {
+               for i, line := range lines[1:] {
+                       lines[1+i] = "   " + line
+               }
+       }
+
        stripCommonPrefix(lines)
 
        // write comment lines, separated by formfeed,
@@ -1140,7 +1159,7 @@ func (p *trimmer) Write(data []byte) (n int, err error) {
 // ----------------------------------------------------------------------------
 // Public interface
 
-// A Mode value is a set of flags (or 0). They coontrol printing. 
+// A Mode value is a set of flags (or 0). They control printing. 
 type Mode uint
 
 const (
index ab9e9b2ec8cefc02f0f74bbb1d43dbba86fa8216..36d1bf74d3ffe748f42b3ebaebd3e390aa456a93 100644 (file)
@@ -6,7 +6,9 @@ package printer
 
 import (
        "bytes"
+       "errors"
        "flag"
+       "fmt"
        "go/ast"
        "go/parser"
        "go/token"
@@ -25,33 +27,28 @@ var update = flag.Bool("update", false, "update golden files")
 
 var fset = token.NewFileSet()
 
-func lineString(text []byte, i int) string {
-       i0 := i
-       for i < len(text) && text[i] != '\n' {
-               i++
-       }
-       return string(text[i0:i])
-}
-
 type checkMode uint
 
 const (
        export checkMode = 1 << iota
        rawFormat
+       idempotent
 )
 
-func runcheck(t *testing.T, source, golden string, mode checkMode) {
-       // parse source
-       prog, err := parser.ParseFile(fset, source, nil, parser.ParseComments)
+// format parses src, prints the corresponding AST, verifies the resulting
+// src is syntactically correct, and returns the resulting src or an error
+// if any.
+func format(src []byte, mode checkMode) ([]byte, error) {
+       // parse src
+       f, err := parser.ParseFile(fset, "", src, parser.ParseComments)
        if err != nil {
-               t.Error(err)
-               return
+               return nil, fmt.Errorf("parse: %s\n%s", err, src)
        }
 
        // filter exports if necessary
        if mode&export != 0 {
-               ast.FileExports(prog) // ignore result
-               prog.Comments = nil   // don't print comments that are not in AST
+               ast.FileExports(f) // ignore result
+               f.Comments = nil   // don't print comments that are not in AST
        }
 
        // determine printer configuration
@@ -60,17 +57,72 @@ func runcheck(t *testing.T, source, golden string, mode checkMode) {
                cfg.Mode |= RawFormat
        }
 
-       // format source
+       // print AST
        var buf bytes.Buffer
-       if err := cfg.Fprint(&buf, fset, prog); err != nil {
-               t.Error(err)
+       if err := cfg.Fprint(&buf, fset, f); err != nil {
+               return nil, fmt.Errorf("print: %s", err)
        }
-       res := buf.Bytes()
 
-       // formatted source must be valid
+       // make sure formated output is syntactically correct
+       res := buf.Bytes()
        if _, err := parser.ParseFile(fset, "", res, 0); err != nil {
+               return nil, fmt.Errorf("re-parse: %s\n%s", err, buf.Bytes())
+       }
+
+       return res, nil
+}
+
+// lineAt returns the line in text starting at offset offs.
+func lineAt(text []byte, offs int) []byte {
+       i := offs
+       for i < len(text) && text[i] != '\n' {
+               i++
+       }
+       return text[offs:i]
+}
+
+// diff compares a and b.
+func diff(aname, bname string, a, b []byte) error {
+       var buf bytes.Buffer // holding long error message
+
+       // compare lengths
+       if len(a) != len(b) {
+               fmt.Fprintf(&buf, "\nlength changed: len(%s) = %d, len(%s) = %d", aname, len(a), bname, len(b))
+       }
+
+       // compare contents
+       line := 1
+       offs := 1
+       for i := 0; i < len(a) && i < len(b); i++ {
+               ch := a[i]
+               if ch != b[i] {
+                       fmt.Fprintf(&buf, "\n%s:%d:%d: %s", aname, line, i-offs+1, lineAt(a, offs))
+                       fmt.Fprintf(&buf, "\n%s:%d:%d: %s", bname, line, i-offs+1, lineAt(b, offs))
+                       fmt.Fprintf(&buf, "\n\n")
+                       break
+               }
+               if ch == '\n' {
+                       line++
+                       offs = i + 1
+               }
+       }
+
+       if buf.Len() > 0 {
+               return errors.New(buf.String())
+       }
+       return nil
+}
+
+func runcheck(t *testing.T, source, golden string, mode checkMode) {
+       src, err := ioutil.ReadFile(source)
+       if err != nil {
+               t.Error(err)
+               return
+       }
+
+       res, err := format(src, mode)
+       if err != nil {
                t.Error(err)
-               t.Logf("\n%s", res)
                return
        }
 
@@ -89,23 +141,19 @@ func runcheck(t *testing.T, source, golden string, mode checkMode) {
                return
        }
 
-       // compare lengths
-       if len(res) != len(gld) {
-               t.Errorf("len = %d, expected %d (= len(%s))", len(res), len(gld), golden)
+       // formatted source and golden must be the same
+       if err := diff(source, golden, res, gld); err != nil {
+               t.Error(err)
+               return
        }
 
-       // compare contents
-       for i, line, offs := 0, 1, 0; i < len(res) && i < len(gld); i++ {
-               ch := res[i]
-               if ch != gld[i] {
-                       t.Errorf("%s:%d:%d: %s", source, line, i-offs+1, lineString(res, offs))
-                       t.Errorf("%s:%d:%d: %s", golden, line, i-offs+1, lineString(gld, offs))
-                       t.Error()
-                       return
-               }
-               if ch == '\n' {
-                       line++
-                       offs = i + 1
+       if mode&idempotent != 0 {
+               // formatting golden must be idempotent
+               // (This is very difficult to achieve in general and for now
+               // it is only checked for files explicitly marked as such.)
+               res, err = format(gld, mode)
+               if err := diff(golden, fmt.Sprintf("format(%s)", golden), gld, res); err != nil {
+                       t.Errorf("golden is not idempotent: %s", err)
                }
        }
 }
@@ -142,15 +190,16 @@ type entry struct {
 
 // Use go test -update to create/update the respective golden files.
 var data = []entry{
-       {"empty.input", "empty.golden", 0},
+       {"empty.input", "empty.golden", idempotent},
        {"comments.input", "comments.golden", 0},
        {"comments.input", "comments.x", export},
-       {"linebreaks.input", "linebreaks.golden", 0},
-       {"expressions.input", "expressions.golden", 0},
-       {"expressions.input", "expressions.raw", rawFormat},
+       {"comments2.input", "comments2.golden", idempotent},
+       {"linebreaks.input", "linebreaks.golden", idempotent},
+       {"expressions.input", "expressions.golden", idempotent},
+       {"expressions.input", "expressions.raw", rawFormat | idempotent},
        {"declarations.input", "declarations.golden", 0},
        {"statements.input", "statements.golden", 0},
-       {"slow.input", "slow.golden", 0},
+       {"slow.input", "slow.golden", idempotent},
 }
 
 func TestFiles(t *testing.T) {
@@ -248,7 +297,7 @@ func testComment(t *testing.T, f *ast.File, srclen int, comment *ast.Comment) {
        }
 }
 
-// Verify that the printer produces always produces a correct program
+// Verify that the printer produces a correct program
 // even if the position information of comments introducing newlines
 // is incorrect.
 func TestBadComments(t *testing.T) {
@@ -421,21 +470,8 @@ func TestX(t *testing.T) {
 package p
 func _() {}
 `
-       // parse original
-       f, err := parser.ParseFile(fset, "src", src, parser.ParseComments)
+       _, err := format([]byte(src), 0)
        if err != nil {
-               t.Fatal(err)
-       }
-
-       // pretty-print original
-       var buf bytes.Buffer
-       if err = (&Config{Mode: UseSpaces, Tabwidth: 8}).Fprint(&buf, fset, f); err != nil {
-               t.Fatal(err)
-       }
-
-       // parse pretty printed original
-       if _, err := parser.ParseFile(fset, "", buf.Bytes(), 0); err != nil {
-               t.Fatalf("%s\n%s", err, buf.Bytes())
+               t.Error(err)
        }
-
 }
diff --git a/src/pkg/go/printer/testdata/comments2.golden b/src/pkg/go/printer/testdata/comments2.golden
new file mode 100644 (file)
index 0000000..d3b50bf
--- /dev/null
@@ -0,0 +1,79 @@
+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This is a package for testing comment placement by go/printer.
+//
+package main
+
+// Test cases for idempotent comment formatting (was issue 1835).
+/*
+c1a
+*/
+/*
+   c1b
+*/
+/* foo
+c1c
+*/
+/* foo
+   c1d
+*/
+/*
+c1e
+foo */
+/*
+   c1f
+   foo */
+
+func f() {
+       /*
+          c2a
+       */
+       /*
+          c2b
+       */
+       /* foo
+          c2c
+       */
+       /* foo
+          c2d
+       */
+       /*
+          c2e
+          foo */
+       /*
+          c2f
+          foo */
+}
+
+func g() {
+       /*
+          c3a
+       */
+       /*
+          c3b
+       */
+       /* foo
+          c3c
+       */
+       /* foo
+          c3d
+       */
+       /*
+          c3e
+          foo */
+       /*
+          c3f
+          foo */
+}
+
+// Test case taken literally from issue 1835.
+func main() {
+       /*
+          prints test 5 times
+       */
+       for i := 0; i < 5; i++ {
+               println("test")
+       }
+}
diff --git a/src/pkg/go/printer/testdata/comments2.input b/src/pkg/go/printer/testdata/comments2.input
new file mode 100644 (file)
index 0000000..6f8c85c
--- /dev/null
@@ -0,0 +1,79 @@
+// Copyright 2012 The Go Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style
+// license that can be found in the LICENSE file.
+
+// This is a package for testing comment placement by go/printer.
+//
+package main
+
+// Test cases for idempotent comment formatting (was issue 1835).
+/*
+c1a
+*/
+/*
+   c1b
+*/
+/* foo
+c1c
+*/
+/* foo
+   c1d
+*/
+/*
+c1e
+foo */
+/*
+   c1f
+   foo */
+
+func f() {
+/*
+c2a
+*/
+/*
+   c2b
+*/
+/* foo
+c2c
+*/
+/* foo
+   c2d
+*/
+/*
+c2e
+foo */
+/*
+   c2f
+   foo */
+}
+
+func g() {
+/*
+c3a
+*/
+/*
+   c3b
+*/
+/* foo
+c3c
+*/
+/* foo
+   c3d
+*/
+/*
+c3e
+foo */
+/*
+   c3f
+   foo */
+}
+
+// Test case taken literally from issue 1835.
+func main() {
+/*
+prints test 5 times
+*/
+   for i := 0; i < 5; i++ {
+      println("test")
+   }
+}
\ No newline at end of file