diff options
Diffstat (limited to 'src/pkg/go')
25 files changed, 758 insertions, 220 deletions
diff --git a/src/pkg/go/ast/commentmap.go b/src/pkg/go/ast/commentmap.go index 1fb4867dd..ac999d627 100644 --- a/src/pkg/go/ast/commentmap.go +++ b/src/pkg/go/ast/commentmap.go @@ -149,7 +149,7 @@ func NewCommentMap(fset *token.FileSet, node Node, comments []*CommentGroup) Com // set up comment reader r tmp := make([]*CommentGroup, len(comments)) - copy(tmp, comments) // don't change incomming comments + copy(tmp, comments) // don't change incoming comments sortComments(tmp) r := commentListReader{fset: fset, list: tmp} // !r.eol() because len(comments) > 0 r.next() diff --git a/src/pkg/go/ast/example_test.go b/src/pkg/go/ast/example_test.go index 632bfcfd0..d2e734f2c 100644 --- a/src/pkg/go/ast/example_test.go +++ b/src/pkg/go/ast/example_test.go @@ -5,8 +5,10 @@ package ast_test import ( + "bytes" "fmt" "go/ast" + "go/format" "go/parser" "go/token" ) @@ -134,3 +136,75 @@ func main() { // 57 . } // 58 } } + +// This example illustrates how to remove a variable declaration +// in a Go program while maintaining correct comment association +// using an ast.CommentMap. +func ExampleCommentMap() { + // src is the input for which we create the AST that we + // are going to manipulate. + src := ` +// This is the package comment. +package main + +// This comment is associated with the hello constant. +const hello = "Hello, World!" // line comment 1 + +// This comment is associated with the foo variable. +var foo = hello // line comment 2 + +// This comment is associated with the main function. +func main() { + fmt.Println(hello) // line comment 3 +} +` + + // Create the AST by parsing src. + fset := token.NewFileSet() // positions are relative to fset + f, err := parser.ParseFile(fset, "src.go", src, parser.ParseComments) + if err != nil { + panic(err) + } + + // Create an ast.CommentMap from the ast.File's comments. + // This helps keeping the association between comments + // and AST nodes. + cmap := ast.NewCommentMap(fset, f, f.Comments) + + // Remove the first variable declaration from the list of declarations. + f.Decls = removeFirstVarDecl(f.Decls) + + // Use the comment map to filter comments that don't belong anymore + // (the comments associated with the variable declaration), and create + // the new comments list. + f.Comments = cmap.Filter(f).Comments() + + // Print the modified AST. + var buf bytes.Buffer + if err := format.Node(&buf, fset, f); err != nil { + panic(err) + } + fmt.Printf("%s", buf.Bytes()) + + // output: + // // This is the package comment. + // package main + // + // // This comment is associated with the hello constant. + // const hello = "Hello, World!" // line comment 1 + // + // // This comment is associated with the main function. + // func main() { + // fmt.Println(hello) // line comment 3 + // } +} + +func removeFirstVarDecl(list []ast.Decl) []ast.Decl { + for i, decl := range list { + if gen, ok := decl.(*ast.GenDecl); ok && gen.Tok == token.VAR { + copy(list[i:], list[i+1:]) + return list[:len(list)-1] + } + } + panic("variable declaration not found") +} diff --git a/src/pkg/go/build/build.go b/src/pkg/go/build/build.go index 50d2fb4ae..412abea3a 100644 --- a/src/pkg/go/build/build.go +++ b/src/pkg/go/build/build.go @@ -292,10 +292,10 @@ func defaultContext() Context { // say "+build go1.x", and code that should only be built before Go 1.x // (perhaps it is the stub to use in that case) should say "+build !go1.x". // - // When we reach Go 1.3 the line will read - // c.ReleaseTags = []string{"go1.1", "go1.2", "go1.3"} + // When we reach Go 1.4 the line will read + // c.ReleaseTags = []string{"go1.1", "go1.2", "go1.3", "go1.4"} // and so on. - c.ReleaseTags = []string{"go1.1", "go1.2"} + c.ReleaseTags = []string{"go1.1", "go1.2", "go1.3"} switch os.Getenv("CGO_ENABLED") { case "1": @@ -303,8 +303,7 @@ func defaultContext() Context { case "0": c.CgoEnabled = false default: - // golang.org/issue/5141 - // cgo should be disabled for cross compilation builds + // cgo must be explicitly enabled for cross compilation builds if runtime.GOARCH == c.GOARCH && runtime.GOOS == c.GOOS { c.CgoEnabled = cgoEnabled[c.GOOS+"/"+c.GOARCH] break @@ -358,6 +357,7 @@ type Package struct { IgnoredGoFiles []string // .go source files ignored for this build CFiles []string // .c source files CXXFiles []string // .cc, .cpp and .cxx source files + MFiles []string // .m (Objective-C) source files HFiles []string // .h, .hh, .hpp and .hxx source files SFiles []string // .s source files SwigFiles []string // .swig files @@ -622,6 +622,9 @@ Found: case ".cc", ".cpp", ".cxx": p.CXXFiles = append(p.CXXFiles, name) continue + case ".m": + p.MFiles = append(p.MFiles, name) + continue case ".h", ".hh", ".hpp", ".hxx": p.HFiles = append(p.HFiles, name) continue @@ -789,7 +792,7 @@ func (ctxt *Context) matchFile(dir, name string, returnImports bool, allTags map } switch ext { - case ".go", ".c", ".cc", ".cxx", ".cpp", ".s", ".h", ".hh", ".hpp", ".hxx", ".S", ".swig", ".swigcxx": + case ".go", ".c", ".cc", ".cxx", ".cpp", ".m", ".s", ".h", ".hh", ".hpp", ".hxx", ".S", ".swig", ".swigcxx": // tentatively okay - read to make sure case ".syso": // binary, no reading @@ -1207,7 +1210,7 @@ func ArchChar(goarch string) (string, error) { switch goarch { case "386": return "8", nil - case "amd64": + case "amd64", "amd64p32": return "6", nil case "arm": return "5", nil diff --git a/src/pkg/go/build/deps_test.go b/src/pkg/go/build/deps_test.go index dd162c7db..7421e144f 100644 --- a/src/pkg/go/build/deps_test.go +++ b/src/pkg/go/build/deps_test.go @@ -8,6 +8,7 @@ package build import ( + "runtime" "sort" "testing" ) @@ -29,7 +30,7 @@ var pkgDeps = map[string][]string{ "errors": {}, "io": {"errors", "sync"}, "runtime": {"unsafe"}, - "sync": {"sync/atomic", "unsafe"}, + "sync": {"runtime", "sync/atomic", "unsafe"}, "sync/atomic": {"unsafe"}, "unsafe": {}, @@ -125,7 +126,7 @@ var pkgDeps = map[string][]string{ "os": {"L1", "os", "syscall", "time"}, "path/filepath": {"L2", "os", "syscall"}, "io/ioutil": {"L2", "os", "path/filepath", "time"}, - "os/exec": {"L2", "os", "syscall"}, + "os/exec": {"L2", "os", "path/filepath", "syscall"}, "os/signal": {"L2", "os", "syscall"}, // OS enables basic operating system functionality, @@ -301,7 +302,7 @@ var pkgDeps = map[string][]string{ // SSL/TLS. "crypto/tls": { "L4", "CRYPTO-MATH", "CGO", "OS", - "crypto/x509", "encoding/pem", "net", "syscall", + "container/list", "crypto/x509", "encoding/pem", "net", "syscall", }, "crypto/x509": { "L4", "CRYPTO-MATH", "OS", "CGO", @@ -359,7 +360,7 @@ func allowed(pkg string) map[string]bool { } var bools = []bool{false, true} -var geese = []string{"darwin", "dragonfly", "freebsd", "linux", "netbsd", "openbsd", "plan9", "windows"} +var geese = []string{"darwin", "dragonfly", "freebsd", "linux", "nacl", "netbsd", "openbsd", "plan9", "solaris", "windows"} var goarches = []string{"386", "amd64", "arm"} type osPkg struct { @@ -374,6 +375,11 @@ var allowedErrors = map[osPkg]bool{ } func TestDependencies(t *testing.T) { + if runtime.GOOS == "nacl" { + // NaCl tests run in a limited file system and we do not + // provide access to every source file. + t.Skip("skipping on NaCl") + } var all []string for k := range pkgDeps { @@ -387,6 +393,9 @@ func TestDependencies(t *testing.T) { if isMacro(pkg) { continue } + if pkg == "runtime/cgo" && !ctxt.CgoEnabled { + continue + } p, err := ctxt.Import(pkg, "", 0) if err != nil { if allowedErrors[osPkg{ctxt.GOOS, pkg}] { diff --git a/src/pkg/go/build/doc.go b/src/pkg/go/build/doc.go index b2f04ea45..f17f76ccc 100644 --- a/src/pkg/go/build/doc.go +++ b/src/pkg/go/build/doc.go @@ -57,11 +57,15 @@ // // Build Constraints // -// A build constraint is a line comment beginning with the directive +build +// A build constraint, also known as a build tag, is a line comment that begins +// +// // +build +// // that lists the conditions under which a file should be included in the package. // Constraints may appear in any kind of source file (not just Go), but // they must appear near the top of the file, preceded -// only by blank lines and other line comments. +// only by blank lines and other line comments. These rules mean that in Go +// files a build constraint must appear before the package clause. // // To distinguish build constraints from package documentation, a series of // build constraints must be followed by a blank line. @@ -95,6 +99,7 @@ // - "cgo", if ctxt.CgoEnabled is true // - "go1.1", from Go version 1.1 onward // - "go1.2", from Go version 1.2 onward +// - "go1.3", from Go version 1.3 onward // - any additional words listed in ctxt.BuildTags // // If a file's name, after stripping the extension and a possible _test suffix, diff --git a/src/pkg/go/build/syslist.go b/src/pkg/go/build/syslist.go index e1fbf6330..5c42b946b 100644 --- a/src/pkg/go/build/syslist.go +++ b/src/pkg/go/build/syslist.go @@ -4,5 +4,5 @@ package build -const goosList = "darwin dragonfly freebsd linux netbsd openbsd plan9 windows " -const goarchList = "386 amd64 arm " +const goosList = "darwin dragonfly freebsd linux nacl netbsd openbsd plan9 solaris windows " +const goarchList = "386 amd64 amd64p32 arm " diff --git a/src/pkg/go/doc/comment.go b/src/pkg/go/doc/comment.go index 5c8c43e0c..f414ca409 100644 --- a/src/pkg/go/doc/comment.go +++ b/src/pkg/go/doc/comment.go @@ -45,13 +45,13 @@ func commentEscape(w io.Writer, text string, nice bool) { const ( // Regexp for Go identifiers - identRx = `[a-zA-Z_][a-zA-Z_0-9]*` // TODO(gri) ASCII only for now - fix this + identRx = `[\pL_][\pL_0-9]*` // Regexp for URLs - protocol = `(https?|ftp|file|gopher|mailto|news|nntp|telnet|wais|prospero):` + protocol = `https?|ftp|file|gopher|mailto|news|nntp|telnet|wais|prospero` hostPart = `[a-zA-Z0-9_@\-]+` - filePart = `[a-zA-Z0-9_?%#~&/\-+=]+` - urlRx = protocol + `//` + // http:// + filePart = `[a-zA-Z0-9_?%#~&/\-+=()]+` // parentheses may not be matching; see pairedParensPrefixLen + urlRx = `(` + protocol + `)://` + // http:// hostPart + `([.:]` + hostPart + `)*/?` + // //www.google.com:8080/ filePart + `([:.,]` + filePart + `)*` ) @@ -73,6 +73,29 @@ var ( html_endh = []byte("</h3>\n") ) +// pairedParensPrefixLen returns the length of the longest prefix of s containing paired parentheses. +func pairedParensPrefixLen(s string) int { + parens := 0 + l := len(s) + for i, ch := range s { + switch ch { + case '(': + if parens == 0 { + l = i + } + parens++ + case ')': + parens-- + if parens == 0 { + l = len(s) + } else if parens < 0 { + return i + } + } + } + return l +} + // Emphasize and escape a line of text for HTML. URLs are converted into links; // if the URL also appears in the words map, the link is taken from the map (if // the corresponding map value is the empty string, the URL is not converted @@ -92,18 +115,26 @@ func emphasize(w io.Writer, line string, words map[string]string, nice bool) { // write text before match commentEscape(w, line[0:m[0]], nice) - // analyze match + // adjust match if necessary match := line[m[0]:m[1]] + if n := pairedParensPrefixLen(match); n < len(match) { + // match contains unpaired parentheses (rare); + // redo matching with shortened line for correct indices + m = matchRx.FindStringSubmatchIndex(line[:m[0]+n]) + match = match[:n] + } + + // analyze match url := "" italics := false if words != nil { - url, italics = words[string(match)] + url, italics = words[match] } if m[2] >= 0 { // match against first parenthesized sub-regexp; must be match against urlRx if !italics { // no alternative URL in words list, use match instead - url = string(match) + url = match } italics = false // don't italicize URLs } @@ -392,7 +423,9 @@ func ToText(w io.Writer, text string, indent, preIndent string, width int) { case opPre: w.Write(nl) for _, line := range b.lines { - if !isBlank(line) { + if isBlank(line) { + w.Write([]byte("\n")) + } else { w.Write([]byte(preIndent)) w.Write([]byte(line)) } diff --git a/src/pkg/go/doc/comment_test.go b/src/pkg/go/doc/comment_test.go index aa21b8d1b..ad65c2a27 100644 --- a/src/pkg/go/doc/comment_test.go +++ b/src/pkg/go/doc/comment_test.go @@ -42,8 +42,9 @@ func TestIsHeading(t *testing.T) { } var blocksTests = []struct { - in string - out []block + in string + out []block + text string }{ { in: `Para 1. @@ -59,6 +60,22 @@ Para 3. pre1 Para 4. + + pre + pre1 + + pre2 + +Para 5. + + + pre + + + pre1 + pre2 + +Para 6. pre pre2 `, @@ -69,8 +86,44 @@ Para 4. {opPara, []string{"Para 3.\n"}}, {opPre, []string{"pre\n", "pre1\n"}}, {opPara, []string{"Para 4.\n"}}, + {opPre, []string{"pre\n", "pre1\n", "\n", "pre2\n"}}, + {opPara, []string{"Para 5.\n"}}, + {opPre, []string{"pre\n", "\n", "\n", "pre1\n", "pre2\n"}}, + {opPara, []string{"Para 6.\n"}}, {opPre, []string{"pre\n", "pre2\n"}}, }, + text: `. Para 1. Para 1 line 2. + +. Para 2. + + +. Section + +. Para 3. + +$ pre +$ pre1 + +. Para 4. + +$ pre +$ pre1 + +$ pre2 + +. Para 5. + +$ pre + + +$ pre1 +$ pre2 + +. Para 6. + +$ pre +$ pre2 +`, }, } @@ -83,14 +136,28 @@ func TestBlocks(t *testing.T) { } } +func TestToText(t *testing.T) { + var buf bytes.Buffer + for i, tt := range blocksTests { + ToText(&buf, tt.in, ". ", "$\t", 40) + if have := buf.String(); have != tt.text { + t.Errorf("#%d: mismatch\nhave: %s\nwant: %s\nhave vs want:\n%q\n%q", i, have, tt.text, have, tt.text) + } + buf.Reset() + } +} + var emphasizeTests = []struct { - in string - out string + in, out string }{ {"http://www.google.com/", `<a href="http://www.google.com/">http://www.google.com/</a>`}, {"https://www.google.com/", `<a href="https://www.google.com/">https://www.google.com/</a>`}, {"http://www.google.com/path.", `<a href="http://www.google.com/path">http://www.google.com/path</a>.`}, + {"http://en.wikipedia.org/wiki/Camellia_(cipher)", `<a href="http://en.wikipedia.org/wiki/Camellia_(cipher)">http://en.wikipedia.org/wiki/Camellia_(cipher)</a>`}, {"(http://www.google.com/)", `(<a href="http://www.google.com/">http://www.google.com/</a>)`}, + {"http://gmail.com)", `<a href="http://gmail.com">http://gmail.com</a>)`}, + {"((http://gmail.com))", `((<a href="http://gmail.com">http://gmail.com</a>))`}, + {"http://gmail.com ((http://gmail.com)) ()", `<a href="http://gmail.com">http://gmail.com</a> ((<a href="http://gmail.com">http://gmail.com</a>)) ()`}, {"Foo bar http://example.com/ quux!", `Foo bar <a href="http://example.com/">http://example.com/</a> quux!`}, {"Hello http://example.com/%2f/ /world.", `Hello <a href="http://example.com/%2f/">http://example.com/%2f/</a> /world.`}, {"Lorem http: ipsum //host/path", "Lorem http: ipsum //host/path"}, @@ -107,3 +174,34 @@ func TestEmphasize(t *testing.T) { } } } + +var pairedParensPrefixLenTests = []struct { + in, out string +}{ + {"", ""}, + {"foo", "foo"}, + {"()", "()"}, + {"foo()", "foo()"}, + {"foo()()()", "foo()()()"}, + {"foo()((()()))", "foo()((()()))"}, + {"foo()((()()))bar", "foo()((()()))bar"}, + {"foo)", "foo"}, + {"foo))", "foo"}, + {"foo)))))", "foo"}, + {"(foo", ""}, + {"((foo", ""}, + {"(((((foo", ""}, + {"(foo)", "(foo)"}, + {"((((foo))))", "((((foo))))"}, + {"foo()())", "foo()()"}, + {"foo((()())", "foo"}, + {"foo((()())) (() foo ", "foo((()())) "}, +} + +func TestPairedParensPrefixLen(t *testing.T) { + for i, tt := range pairedParensPrefixLenTests { + if out := tt.in[:pairedParensPrefixLen(tt.in)]; out != tt.out { + t.Errorf("#%d: mismatch\nhave: %q\nwant: %q", i, out, tt.out) + } + } +} diff --git a/src/pkg/go/doc/example.go b/src/pkg/go/doc/example.go index 2358ed389..c414e548c 100644 --- a/src/pkg/go/doc/example.go +++ b/src/pkg/go/doc/example.go @@ -32,6 +32,17 @@ type Example struct { // Examples returns the examples found in the files, sorted by Name field. // The Order fields record the order in which the examples were encountered. +// +// Playable Examples must be in a package whose name ends in "_test". +// An Example is "playable" (the Play field is non-nil) in either of these +// circumstances: +// - The example function is self-contained: the function references only +// identifiers from other packages (or predeclared identifiers, such as +// "int") and the test file does not include a dot import. +// - The entire test file is the example: the file contains exactly one +// example function, zero test or benchmark functions, and at least one +// top-level function, type, variable, or constant declaration other +// than the example function. func Examples(files ...*ast.File) []*Example { var list []*Example for _, file := range files { @@ -244,7 +255,7 @@ func playExample(file *ast.File, body *ast.BlockStmt) *ast.File { } } - // Strip "Output:" commment and adjust body end position. + // Strip "Output:" comment and adjust body end position. body, comments = stripOutputComment(body, comments) // Synthesize import declaration. @@ -307,7 +318,7 @@ func playExampleFile(file *ast.File) *ast.File { return &f } -// stripOutputComment finds and removes an "Output:" commment from body +// stripOutputComment finds and removes an "Output:" comment from body // and comments, and adjusts the body block's end position. func stripOutputComment(body *ast.BlockStmt, comments []*ast.CommentGroup) (*ast.BlockStmt, []*ast.CommentGroup) { // Do nothing if no "Output:" comment found. diff --git a/src/pkg/go/parser/error_test.go b/src/pkg/go/parser/error_test.go index d4d4f909d..8506077ce 100644 --- a/src/pkg/go/parser/error_test.go +++ b/src/pkg/go/parser/error_test.go @@ -59,8 +59,11 @@ func getPos(filename string, offset int) token.Pos { // ERROR comments must be of the form /* ERROR "rx" */ and rx is // a regular expression that matches the expected error message. +// The special form /* ERROR HERE "rx" */ must be used for error +// messages that appear immediately after a token, rather than at +// a token's position. // -var errRx = regexp.MustCompile(`^/\* *ERROR *"([^"]*)" *\*/$`) +var errRx = regexp.MustCompile(`^/\* *ERROR *(HERE)? *"([^"]*)" *\*/$`) // expectedErrors collects the regular expressions of ERROR comments found // in files and returns them as a map of error positions to error messages. @@ -74,6 +77,7 @@ func expectedErrors(t *testing.T, filename string, src []byte) map[token.Pos]str // not match the position information collected by the parser s.Init(getFile(filename), src, nil, scanner.ScanComments) var prev token.Pos // position of last non-comment, non-semicolon token + var here token.Pos // position immediately after the token at position prev for { pos, tok, lit := s.Scan() @@ -82,11 +86,22 @@ func expectedErrors(t *testing.T, filename string, src []byte) map[token.Pos]str return errors case token.COMMENT: s := errRx.FindStringSubmatch(lit) - if len(s) == 2 { - errors[prev] = string(s[1]) + if len(s) == 3 { + pos := prev + if s[1] == "HERE" { + pos = here + } + errors[pos] = string(s[2]) } default: prev = pos + var l int // token length + if tok.IsLiteral() { + l = len(lit) + } else { + l = len(tok.String()) + } + here = prev + token.Pos(l) } } } diff --git a/src/pkg/go/parser/interface.go b/src/pkg/go/parser/interface.go index 0f83ca931..57da4ddcd 100644 --- a/src/pkg/go/parser/interface.go +++ b/src/pkg/go/parser/interface.go @@ -182,6 +182,13 @@ func ParseExpr(x string) (ast.Expr, error) { p.closeScope() assert(p.topScope == nil, "unbalanced scopes") + // If a semicolon was inserted, consume it; + // report an error if there's more tokens. + if p.tok == token.SEMICOLON { + p.next() + } + p.expect(token.EOF) + if p.errors.Len() > 0 { p.errors.Sort() return nil, p.errors.Err() diff --git a/src/pkg/go/parser/parser.go b/src/pkg/go/parser/parser.go index c4523318f..00dd532b2 100644 --- a/src/pkg/go/parser/parser.go +++ b/src/pkg/go/parser/parser.go @@ -492,6 +492,26 @@ func syncDecl(p *parser) { } } +// safePos returns a valid file position for a given position: If pos +// is valid to begin with, safePos returns pos. If pos is out-of-range, +// safePos returns the EOF position. +// +// This is hack to work around "artificial" end positions in the AST which +// are computed by adding 1 to (presumably valid) token positions. If the +// token positions are invalid due to parse errors, the resulting end position +// may be past the file's EOF position, which would lead to panics if used +// later on. +// +func (p *parser) safePos(pos token.Pos) (res token.Pos) { + defer func() { + if recover() != nil { + res = token.Pos(p.file.Base() + p.file.Size()) // EOF position + } + }() + _ = p.file.Offset(pos) // trigger a panic if position is out-of-range + return pos +} + // ---------------------------------------------------------------------------- // Identifiers @@ -679,7 +699,7 @@ func (p *parser) parseFieldDecl(scope *ast.Scope) *ast.Field { if n := len(list); n > 1 || !isTypeName(deref(typ)) { pos := typ.Pos() p.errorExpected(pos, "anonymous field") - typ = &ast.BadExpr{From: pos, To: list[n-1].End()} + typ = &ast.BadExpr{From: pos, To: p.safePos(list[n-1].End())} } } @@ -1168,16 +1188,19 @@ func (p *parser) parseIndexOrSlice(x ast.Expr) ast.Expr { defer un(trace(p, "IndexOrSlice")) } + const N = 3 // change the 3 to 2 to disable 3-index slices lbrack := p.expect(token.LBRACK) p.exprLev++ - var index [3]ast.Expr // change the 3 to 2 to disable slice expressions w/ cap + var index [N]ast.Expr + var colons [N - 1]token.Pos if p.tok != token.COLON { index[0] = p.parseRhs() } ncolons := 0 - for p.tok == token.COLON && ncolons < len(index)-1 { - p.next() + for p.tok == token.COLON && ncolons < len(colons) { + colons[ncolons] = p.pos ncolons++ + p.next() if p.tok != token.COLON && p.tok != token.RBRACK && p.tok != token.EOF { index[ncolons] = p.parseRhs() } @@ -1187,7 +1210,21 @@ func (p *parser) parseIndexOrSlice(x ast.Expr) ast.Expr { if ncolons > 0 { // slice expression - return &ast.SliceExpr{X: x, Lbrack: lbrack, Low: index[0], High: index[1], Max: index[2], Slice3: ncolons == 2, Rbrack: rbrack} + slice3 := false + if ncolons == 2 { + slice3 = true + // Check presence of 2nd and 3rd index here rather than during type-checking + // to prevent erroneous programs from passing through gofmt (was issue 7305). + if index[1] == nil { + p.error(colons[0], "2nd index required in 3-index slice") + index[1] = &ast.BadExpr{From: colons[0] + 1, To: colons[1]} + } + if index[2] == nil { + p.error(colons[1], "3rd index required in 3-index slice") + index[2] = &ast.BadExpr{From: colons[1] + 1, To: rbrack} + } + } + return &ast.SliceExpr{X: x, Lbrack: lbrack, Low: index[0], High: index[1], Max: index[2], Slice3: slice3, Rbrack: rbrack} } return &ast.IndexExpr{X: x, Lbrack: lbrack, Index: index[0], Rbrack: rbrack} @@ -1320,7 +1357,7 @@ func (p *parser) checkExpr(x ast.Expr) ast.Expr { default: // all other nodes are not proper expressions p.errorExpected(x.Pos(), "expression") - x = &ast.BadExpr{From: x.Pos(), To: x.End()} + x = &ast.BadExpr{From: x.Pos(), To: p.safePos(x.End())} } return x } @@ -1383,7 +1420,7 @@ func (p *parser) checkExprOrType(x ast.Expr) ast.Expr { case *ast.ArrayType: if len, isEllipsis := t.Len.(*ast.Ellipsis); isEllipsis { p.error(len.Pos(), "expected array length, found '...'") - x = &ast.BadExpr{From: x.Pos(), To: x.End()} + x = &ast.BadExpr{From: x.Pos(), To: p.safePos(x.End())} } } @@ -1669,14 +1706,14 @@ func (p *parser) parseSimpleStmt(mode int) (ast.Stmt, bool) { return &ast.ExprStmt{X: x[0]}, false } -func (p *parser) parseCallExpr() *ast.CallExpr { +func (p *parser) parseCallExpr(callType string) *ast.CallExpr { x := p.parseRhsOrType() // could be a conversion: (some type)(x) if call, isCall := x.(*ast.CallExpr); isCall { return call } if _, isBad := x.(*ast.BadExpr); !isBad { // only report error if it's a new one - p.errorExpected(x.Pos(), "function/method call") + p.error(p.safePos(x.End()), fmt.Sprintf("function must be invoked in %s statement", callType)) } return nil } @@ -1687,7 +1724,7 @@ func (p *parser) parseGoStmt() ast.Stmt { } pos := p.expect(token.GO) - call := p.parseCallExpr() + call := p.parseCallExpr("go") p.expectSemi() if call == nil { return &ast.BadStmt{From: pos, To: pos + 2} // len("go") @@ -1702,7 +1739,7 @@ func (p *parser) parseDeferStmt() ast.Stmt { } pos := p.expect(token.DEFER) - call := p.parseCallExpr() + call := p.parseCallExpr("defer") p.expectSemi() if call == nil { return &ast.BadStmt{From: pos, To: pos + 5} // len("defer") @@ -1745,15 +1782,15 @@ func (p *parser) parseBranchStmt(tok token.Token) *ast.BranchStmt { return &ast.BranchStmt{TokPos: pos, Tok: tok, Label: label} } -func (p *parser) makeExpr(s ast.Stmt) ast.Expr { +func (p *parser) makeExpr(s ast.Stmt, kind string) ast.Expr { if s == nil { return nil } if es, isExpr := s.(*ast.ExprStmt); isExpr { return p.checkExpr(es.X) } - p.error(s.Pos(), "expected condition, found simple statement") - return &ast.BadExpr{From: s.Pos(), To: s.End()} + p.error(s.Pos(), fmt.Sprintf("expected %s, found simple statement (missing parentheses around composite literal?)", kind)) + return &ast.BadExpr{From: s.Pos(), To: p.safePos(s.End())} } func (p *parser) parseIfStmt() *ast.IfStmt { @@ -1779,7 +1816,7 @@ func (p *parser) parseIfStmt() *ast.IfStmt { p.next() x = p.parseRhs() } else { - x = p.makeExpr(s) + x = p.makeExpr(s, "boolean expression") s = nil } } @@ -1910,7 +1947,7 @@ func (p *parser) parseSwitchStmt() ast.Stmt { return &ast.TypeSwitchStmt{Switch: pos, Init: s1, Assign: s2, Body: body} } - return &ast.SwitchStmt{Switch: pos, Init: s1, Tag: p.makeExpr(s2), Body: body} + return &ast.SwitchStmt{Switch: pos, Init: s1, Tag: p.makeExpr(s2, "switch expression"), Body: body} } func (p *parser) parseCommClause() *ast.CommClause { @@ -2035,7 +2072,7 @@ func (p *parser) parseForStmt() ast.Stmt { key = as.Lhs[0] default: p.errorExpected(as.Lhs[0].Pos(), "1 or 2 expressions") - return &ast.BadStmt{From: pos, To: body.End()} + return &ast.BadStmt{From: pos, To: p.safePos(body.End())} } // parseSimpleStmt returned a right-hand side that // is a single unary expression of the form "range x" @@ -2055,7 +2092,7 @@ func (p *parser) parseForStmt() ast.Stmt { return &ast.ForStmt{ For: pos, Init: s1, - Cond: p.makeExpr(s2), + Cond: p.makeExpr(s2, "boolean or range expression"), Post: s3, Body: body, } @@ -2282,7 +2319,7 @@ func (p *parser) parseReceiver(scope *ast.Scope) *ast.FieldList { p.errorExpected(base.Pos(), "(unqualified) identifier") } par.List = []*ast.Field{ - {Type: &ast.BadExpr{From: recv.Pos(), To: recv.End()}}, + {Type: &ast.BadExpr{From: recv.Pos(), To: p.safePos(recv.End())}}, } } diff --git a/src/pkg/go/parser/parser_test.go b/src/pkg/go/parser/parser_test.go index 0a34b7e50..2797ea518 100644 --- a/src/pkg/go/parser/parser_test.go +++ b/src/pkg/go/parser/parser_test.go @@ -78,7 +78,7 @@ func TestParseExpr(t *testing.T) { } // sanity check if _, ok := x.(*ast.BinaryExpr); !ok { - t.Errorf("ParseExpr(%s): got %T, expected *ast.BinaryExpr", src, x) + t.Errorf("ParseExpr(%s): got %T, want *ast.BinaryExpr", src, x) } // a valid type expression @@ -89,17 +89,24 @@ func TestParseExpr(t *testing.T) { } // sanity check if _, ok := x.(*ast.StructType); !ok { - t.Errorf("ParseExpr(%s): got %T, expected *ast.StructType", src, x) + t.Errorf("ParseExpr(%s): got %T, want *ast.StructType", src, x) } // an invalid expression src = "a + *" _, err = ParseExpr(src) if err == nil { - t.Fatalf("ParseExpr(%s): %v", src, err) + t.Fatalf("ParseExpr(%s): got no error", src) + } + + // a valid expression followed by extra tokens is invalid + src = "a[i] := x" + _, err = ParseExpr(src) + if err == nil { + t.Fatalf("ParseExpr(%s): got no error", src) } - // it must not crash + // ParseExpr must not crash for _, src := range valids { ParseExpr(src) } diff --git a/src/pkg/go/parser/short_test.go b/src/pkg/go/parser/short_test.go index 0ef0c560c..b79406099 100644 --- a/src/pkg/go/parser/short_test.go +++ b/src/pkg/go/parser/short_test.go @@ -48,14 +48,14 @@ var invalids = []string{ `package p; func f() { if { /* ERROR "expected operand" */ } };`, `package p; func f() { if ; { /* ERROR "expected operand" */ } };`, `package p; func f() { if f(); { /* ERROR "expected operand" */ } };`, - `package p; func f() { if _ /* ERROR "expected condition" */ = range x; true {} };`, - `package p; func f() { switch _ /* ERROR "expected condition" */ = range x; true {} };`, + `package p; func f() { if _ /* ERROR "expected boolean expression" */ = range x; true {} };`, + `package p; func f() { switch _ /* ERROR "expected switch expression" */ = range x; true {} };`, `package p; func f() { for _ = range x ; /* ERROR "expected '{'" */ ; {} };`, `package p; func f() { for ; ; _ = range /* ERROR "expected operand" */ x {} };`, - `package p; func f() { for ; _ /* ERROR "expected condition" */ = range x ; {} };`, - `package p; func f() { switch t /* ERROR "expected condition" */ = t.(type) {} };`, - `package p; func f() { switch t /* ERROR "expected condition" */ , t = t.(type) {} };`, - `package p; func f() { switch t /* ERROR "expected condition" */ = t.(type), t {} };`, + `package p; func f() { for ; _ /* ERROR "expected boolean or range expression" */ = range x ; {} };`, + `package p; func f() { switch t /* ERROR "expected switch expression" */ = t.(type) {} };`, + `package p; func f() { switch t /* ERROR "expected switch expression" */ , t = t.(type) {} };`, + `package p; func f() { switch t /* ERROR "expected switch expression" */ = t.(type), t {} };`, `package p; var a = [ /* ERROR "expected expression" */ 1]int;`, `package p; var a = [ /* ERROR "expected expression" */ ...]int;`, `package p; var a = struct /* ERROR "expected expression" */ {}`, @@ -76,8 +76,19 @@ var invalids = []string{ `package p; func f() { _ = x = /* ERROR "expected '=='" */ 0 {}};`, `package p; func f() { _ = 1 == func()int { var x bool; x = x = /* ERROR "expected '=='" */ true; return x }() };`, `package p; func f() { var s []int; _ = s[] /* ERROR "expected operand" */ };`, - `package p; func f() { var s []int; _ = s[::: /* ERROR "expected ']'" */ ] };`, + `package p; func f() { var s []int; _ = s[i:j: /* ERROR "3rd index required" */ ] };`, + `package p; func f() { var s []int; _ = s[i: /* ERROR "2nd index required" */ :k] };`, + `package p; func f() { var s []int; _ = s[i: /* ERROR "2nd index required" */ :] };`, + `package p; func f() { var s []int; _ = s[: /* ERROR "2nd index required" */ :] };`, + `package p; func f() { var s []int; _ = s[: /* ERROR "2nd index required" */ ::] };`, `package p; func f() { var s []int; _ = s[i:j:k: /* ERROR "expected ']'" */ l] };`, + `package p; func f() { for x /* ERROR "boolean or range expression" */ = []string {} }`, + `package p; func f() { for x /* ERROR "boolean or range expression" */ := []string {} }`, + `package p; func f() { for i /* ERROR "boolean or range expression" */ , x = []string {} }`, + `package p; func f() { for i /* ERROR "boolean or range expression" */ , x := []string {} }`, + `package p; func f() { go f /* ERROR HERE "function must be invoked" */ }`, + `package p; func f() { defer func() {} /* ERROR HERE "function must be invoked" */ }`, + `package p; func f() { go func() { func() { f(x func /* ERROR "expected '\)'" */ (){}) } } }`, } func TestInvalid(t *testing.T) { diff --git a/src/pkg/go/printer/nodes.go b/src/pkg/go/printer/nodes.go index 583c6c370..04b5f1a76 100644 --- a/src/pkg/go/printer/nodes.go +++ b/src/pkg/go/printer/nodes.go @@ -378,10 +378,6 @@ func (p *printer) setLineComment(text string) { p.setComment(&ast.CommentGroup{List: []*ast.Comment{{Slash: token.NoPos, Text: text}}}) } -func (p *printer) isMultiLine(n ast.Node) bool { - return p.lineFor(n.End())-p.lineFor(n.Pos()) > 0 -} - func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) { lbrace := fields.Opening list := fields.List @@ -428,13 +424,14 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) if len(list) == 1 { sep = blank } - newSection := false + var line int for i, f := range list { if i > 0 { - p.linebreak(p.lineFor(f.Pos()), 1, ignore, newSection) + p.linebreak(p.lineFor(f.Pos()), 1, ignore, p.linesFrom(line) > 0) } extraTabs := 0 p.setComment(f.Doc) + p.recordLine(&line) if len(f.Names) > 0 { // named fields p.identList(f.Names, false) @@ -460,7 +457,6 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) } p.setComment(f.Comment) } - newSection = p.isMultiLine(f) } if isIncomplete { if len(list) > 0 { @@ -472,12 +468,13 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) } else { // interface - newSection := false + var line int for i, f := range list { if i > 0 { - p.linebreak(p.lineFor(f.Pos()), 1, ignore, newSection) + p.linebreak(p.lineFor(f.Pos()), 1, ignore, p.linesFrom(line) > 0) } p.setComment(f.Doc) + p.recordLine(&line) if ftyp, isFtyp := f.Type.(*ast.FuncType); isFtyp { // method p.expr(f.Names[0]) @@ -487,7 +484,6 @@ func (p *printer) fieldList(fields *ast.FieldList, isStruct, isIncomplete bool) p.expr(f.Type) } p.setComment(f.Comment) - newSection = p.isMultiLine(f) } if isIncomplete { if len(list) > 0 { @@ -826,10 +822,16 @@ func (p *printer) expr1(expr ast.Expr, prec1, depth int) { } p.print(x.Lbrace, token.LBRACE) p.exprList(x.Lbrace, x.Elts, 1, commaTerm, x.Rbrace) - // do not insert extra line breaks because of comments before - // the closing '}' as it might break the code if there is no - // trailing ',' - p.print(noExtraLinebreak, x.Rbrace, token.RBRACE, noExtraLinebreak) + // do not insert extra line break following a /*-style comment + // before the closing '}' as it might break the code if there + // is no trailing ',' + mode := noExtraLinebreak + // do not insert extra blank following a /*-style comment + // before the closing '}' unless the literal is empty + if len(x.Elts) > 0 { + mode |= noExtraBlank + } + p.print(mode, x.Rbrace, token.RBRACE, mode) case *ast.Ellipsis: p.print(token.ELLIPSIS) @@ -901,20 +903,31 @@ func (p *printer) stmtList(list []ast.Stmt, nindent int, nextIsRBrace bool) { if nindent > 0 { p.print(indent) } - multiLine := false + var line int i := 0 for _, s := range list { // ignore empty statements (was issue 3466) if _, isEmpty := s.(*ast.EmptyStmt); !isEmpty { - // _indent == 0 only for lists of switch/select case clauses; + // nindent == 0 only for lists of switch/select case clauses; // in those cases each clause is a new section if len(p.output) > 0 { // only print line break if we are not at the beginning of the output // (i.e., we are not printing only a partial program) - p.linebreak(p.lineFor(s.Pos()), 1, ignore, i == 0 || nindent == 0 || multiLine) + p.linebreak(p.lineFor(s.Pos()), 1, ignore, i == 0 || nindent == 0 || p.linesFrom(line) > 0) } + p.recordLine(&line) p.stmt(s, nextIsRBrace && i == len(list)-1) - multiLine = p.isMultiLine(s) + // labeled statements put labels on a separate line, but here + // we only care about the start line of the actual statement + // without label - correct line for each label + for t := s; ; { + lt, _ := t.(*ast.LabeledStmt) + if lt == nil { + break + } + line++ + t = lt.Stmt + } i++ } } @@ -1375,22 +1388,22 @@ func (p *printer) genDecl(d *ast.GenDecl) { // two or more grouped const/var declarations: // determine if the type column must be kept keepType := keepTypeColumn(d.Specs) - newSection := false + var line int for i, s := range d.Specs { if i > 0 { - p.linebreak(p.lineFor(s.Pos()), 1, ignore, newSection) + p.linebreak(p.lineFor(s.Pos()), 1, ignore, p.linesFrom(line) > 0) } + p.recordLine(&line) p.valueSpec(s.(*ast.ValueSpec), keepType[i]) - newSection = p.isMultiLine(s) } } else { - newSection := false + var line int for i, s := range d.Specs { if i > 0 { - p.linebreak(p.lineFor(s.Pos()), 1, ignore, newSection) + p.linebreak(p.lineFor(s.Pos()), 1, ignore, p.linesFrom(line) > 0) } + p.recordLine(&line) p.spec(s, n, false) - newSection = p.isMultiLine(s) } } p.print(unindent, formfeed) @@ -1448,13 +1461,16 @@ func (p *printer) bodySize(b *ast.BlockStmt, maxSize int) int { // opening and closing brace are on different lines - don't make it a one-liner return maxSize + 1 } - if len(b.List) > 5 || p.commentBefore(p.posFor(pos2)) { - // too many statements or there is a comment inside - don't make it a one-liner + if len(b.List) > 5 { + // too many statements - don't make it a one-liner return maxSize + 1 } // otherwise, estimate body size - bodySize := 0 + bodySize := p.commentSizeBefore(p.posFor(pos2)) for i, s := range b.List { + if bodySize > maxSize { + break // no need to continue + } if i > 0 { bodySize += 2 // space for a semicolon and blank } @@ -1488,7 +1504,7 @@ func (p *printer) adjBlock(headerSize int, sep whiteSpace, b *ast.BlockStmt) { } p.print(blank) } - p.print(b.Rbrace, token.RBRACE) + p.print(noExtraLinebreak, b.Rbrace, token.RBRACE, noExtraLinebreak) return } diff --git a/src/pkg/go/printer/printer.go b/src/pkg/go/printer/printer.go index e06d2edfb..280c697a0 100644 --- a/src/pkg/go/printer/printer.go +++ b/src/pkg/go/printer/printer.go @@ -39,9 +39,17 @@ const ( type pmode int const ( - noExtraLinebreak pmode = 1 << iota + noExtraBlank pmode = 1 << iota // disables extra blank after /*-style comment + noExtraLinebreak // disables extra line break after /*-style comment ) +type commentInfo struct { + cindex int // current comment index + comment *ast.CommentGroup // = printer.comments[cindex]; or nil + commentOffset int // = printer.posFor(printer.comments[cindex].List[0].Pos()).Offset; or infinity + commentNewline bool // true if the comment group contains newlines +} + type printer struct { // Configuration (does not change after initialization) Config @@ -52,7 +60,8 @@ type printer struct { indent int // current indentation mode pmode // current printer mode impliedSemi bool // if set, a linebreak implies a semicolon - lastTok token.Token // the last token printed (token.ILLEGAL if it's whitespace) + lastTok token.Token // last token printed (token.ILLEGAL if it's whitespace) + prevOpen token.Token // previous non-brace "open" token (, [, or token.ILLEGAL wsbuf []whiteSpace // delayed white space // Positions @@ -61,19 +70,17 @@ type printer struct { // white space). If there's a difference and SourcePos is set in // ConfigMode, //line comments are used in the output to restore // original source positions for a reader. - pos token.Position // current position in AST (source) space - out token.Position // current position in output space - last token.Position // value of pos after calling writeString + pos token.Position // current position in AST (source) space + out token.Position // current position in output space + last token.Position // value of pos after calling writeString + linePtr *int // if set, record out.Line for the next token in *linePtr // The list of all source comments, in order of appearance. comments []*ast.CommentGroup // may be nil - cindex int // current comment index useNodeComments bool // if not set, ignore lead and line comments of nodes // Information about p.comments[p.cindex]; set up by nextComment. - comment *ast.CommentGroup // = p.comments[p.cindex]; or nil - commentOffset int // = p.posFor(p.comments[p.cindex].List[0].Pos()).Offset; or infinity - commentNewline bool // true if the comment group contains newlines + commentInfo // Cache of already computed node sizes. nodeSizes map[ast.Node]int @@ -93,6 +100,14 @@ func (p *printer) init(cfg *Config, fset *token.FileSet, nodeSizes map[ast.Node] p.cachedPos = -1 } +func (p *printer) internalError(msg ...interface{}) { + if debug { + fmt.Print(p.pos.String() + ": ") + fmt.Println(msg...) + panic("go/printer") + } +} + // commentsHaveNewline reports whether a list of comments belonging to // an *ast.CommentGroup contains newlines. Because the position information // may only be partially correct, we also have to read the comment text. @@ -129,12 +144,49 @@ func (p *printer) nextComment() { p.commentOffset = infinity } -func (p *printer) internalError(msg ...interface{}) { - if debug { - fmt.Print(p.pos.String() + ": ") - fmt.Println(msg...) - panic("go/printer") +// commentBefore returns true iff the current comment group occurs +// before the next position in the source code and printing it does +// not introduce implicit semicolons. +// +func (p *printer) commentBefore(next token.Position) bool { + return p.commentOffset < next.Offset && (!p.impliedSemi || !p.commentNewline) +} + +// commentSizeBefore returns the estimated size of the +// comments on the same line before the next position. +// +func (p *printer) commentSizeBefore(next token.Position) int { + // save/restore current p.commentInfo (p.nextComment() modifies it) + defer func(info commentInfo) { + p.commentInfo = info + }(p.commentInfo) + + size := 0 + for p.commentBefore(next) { + for _, c := range p.comment.List { + size += len(c.Text) + } + p.nextComment() } + return size +} + +// recordLine records the output line number for the next non-whitespace +// token in *linePtr. It is used to compute an accurate line number for a +// formatted construct, independent of pending (not yet emitted) whitespace +// or comments. +// +func (p *printer) recordLine(linePtr *int) { + p.linePtr = linePtr +} + +// linesFrom returns the number of output lines between the current +// output line and the line argument, ignoring any pending (not yet +// emitted) whitespace or comments. It is used to compute an accurate +// size (in number of lines) for a formatted construct. +// +func (p *printer) linesFrom(line int) int { + return p.out.Line - line } func (p *printer) posFor(pos token.Pos) token.Position { @@ -675,10 +727,14 @@ func (p *printer) intersperseComments(next token.Position, tok token.Token) (wro if last != nil { // if the last comment is a /*-style comment and the next item - // follows on the same line but is not a comma or a "closing" - // token, add an extra blank for separation - if last.Text[1] == '*' && p.lineFor(last.Pos()) == next.Line && tok != token.COMMA && - tok != token.RPAREN && tok != token.RBRACK && tok != token.RBRACE { + // follows on the same line but is not a comma, and not a "closing" + // token immediately following its corresponding "opening" token, + // add an extra blank for separation unless explicitly disabled + if p.mode&noExtraBlank == 0 && + last.Text[1] == '*' && p.lineFor(last.Pos()) == next.Line && + tok != token.COMMA && + (tok != token.RPAREN || p.prevOpen == token.LPAREN) && + (tok != token.RBRACK || p.prevOpen == token.LBRACK) { p.writeByte(' ', 1) } // ensure that there is a line break after a //-style comment, @@ -735,12 +791,8 @@ func (p *printer) writeWhitespace(n int) { } // shift remaining entries down - i := 0 - for ; n < len(p.wsbuf); n++ { - p.wsbuf[i] = p.wsbuf[n] - i++ - } - p.wsbuf = p.wsbuf[0:i] + l := copy(p.wsbuf, p.wsbuf[n:]) + p.wsbuf = p.wsbuf[:l] } // ---------------------------------------------------------------------------- @@ -790,6 +842,17 @@ func (p *printer) print(args ...interface{}) { var isLit bool var impliedSemi bool // value for p.impliedSemi after this arg + // record previous opening token, if any + switch p.lastTok { + case token.ILLEGAL: + // ignore (white space) + case token.LPAREN, token.LBRACK: + p.prevOpen = p.lastTok + default: + // other tokens followed any opening token + p.prevOpen = token.ILLEGAL + } + switch x := arg.(type) { case pmode: // toggle printer mode @@ -899,19 +962,17 @@ func (p *printer) print(args ...interface{}) { } } + // the next token starts now - record its line number if requested + if p.linePtr != nil { + *p.linePtr = p.out.Line + p.linePtr = nil + } + p.writeString(next, data, isLit) p.impliedSemi = impliedSemi } } -// commentBefore returns true iff the current comment group occurs -// before the next position in the source code and printing it does -// not introduce implicit semicolons. -// -func (p *printer) commentBefore(next token.Position) (result bool) { - return p.commentOffset < next.Offset && (!p.impliedSemi || !p.commentNewline) -} - // flush prints any pending comments and whitespace occurring textually // before the position of the next token tok. The flush result indicates // if a newline was written or if a formfeed was dropped from the whitespace diff --git a/src/pkg/go/printer/printer_test.go b/src/pkg/go/printer/printer_test.go index 8454ac12b..306928a69 100644 --- a/src/pkg/go/printer/printer_test.go +++ b/src/pkg/go/printer/printer_test.go @@ -63,7 +63,7 @@ func format(src []byte, mode checkMode) ([]byte, error) { return nil, fmt.Errorf("print: %s", err) } - // make sure formated output is syntactically correct + // make sure formatted 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()) @@ -179,7 +179,7 @@ func check(t *testing.T, source, golden string, mode checkMode) { // test running past time out t.Errorf("%s: running too slowly", source) case <-cc: - // test finished within alloted time margin + // test finished within allotted time margin } } @@ -212,7 +212,7 @@ func TestFiles(t *testing.T) { } } -// TestLineComments, using a simple test case, checks that consequtive line +// TestLineComments, using a simple test case, checks that consecutive line // comments are properly terminated with a newline even if the AST position // information is incorrect. // diff --git a/src/pkg/go/printer/testdata/comments.golden b/src/pkg/go/printer/testdata/comments.golden index 610a42a68..b1af7958a 100644 --- a/src/pkg/go/printer/testdata/comments.golden +++ b/src/pkg/go/printer/testdata/comments.golden @@ -494,16 +494,21 @@ func _() { func _( /* this */ x /* is */ /* an */ int) { } -func _( /* no params */) {} +func _( /* no params - extra blank before and after comment */ ) {} +func _(a, b int /* params - no extra blank after comment */) {} + +func _() { f( /* no args - extra blank before and after comment */ ) } +func _() { f(a, b /* args - no extra blank after comment */) } func _() { - f( /* no args */) + f( /* no args - extra blank before and after comment */ ) + f(a, b /* args - no extra blank after comment */) } func ( /* comment1 */ T /* comment2 */) _() {} -func _() { /* one-line functions with comments are formatted as multi-line functions */ -} +func _() { /* "short-ish one-line functions with comments are formatted as multi-line functions */ } +func _() { x := 0; /* comment */ y = x /* comment */ } func _() { _ = 0 diff --git a/src/pkg/go/printer/testdata/comments.input b/src/pkg/go/printer/testdata/comments.input index d121dd4be..983e2b2c9 100644 --- a/src/pkg/go/printer/testdata/comments.input +++ b/src/pkg/go/printer/testdata/comments.input @@ -500,15 +500,21 @@ func _() { func _(/* this */x/* is *//* an */ int) { } -func _(/* no params */) {} +func _(/* no params - extra blank before and after comment */) {} +func _(a, b int /* params - no extra blank after comment */) {} + +func _() { f(/* no args - extra blank before and after comment */) } +func _() { f(a, b /* args - no extra blank after comment */) } func _() { - f(/* no args */) + f(/* no args - extra blank before and after comment */) + f(a, b /* args - no extra blank after comment */) } func (/* comment1 */ T /* comment2 */) _() {} -func _() { /* one-line functions with comments are formatted as multi-line functions */ } +func _() { /* "short-ish one-line functions with comments are formatted as multi-line functions */ } +func _() { x := 0; /* comment */ y = x /* comment */ } func _() { _ = 0 diff --git a/src/pkg/go/printer/testdata/comments2.golden b/src/pkg/go/printer/testdata/comments2.golden index d3b50bf3e..7676a26c1 100644 --- a/src/pkg/go/printer/testdata/comments2.golden +++ b/src/pkg/go/printer/testdata/comments2.golden @@ -77,3 +77,29 @@ func main() { println("test") } } + +func issue5623() { +L: + _ = yyyyyyyyyyyyyyyy // comment - should be aligned + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */ + + _ = yyyyyyyyyyyyyyyy /* comment - should be aligned */ + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment + +LLLLLLL: + _ = yyyyyyyyyyyyyyyy // comment - should be aligned + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment + +LL: +LLLLL: + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */ + _ = yyyyyyyyyyyyyyyy /* comment - should be aligned */ + + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment + _ = yyyyyyyyyyyyyyyy // comment - should be aligned + + // test case from issue +label: + mask := uint64(1)<<c - 1 // Allocation mask + used := atomic.LoadUint64(&h.used) // Current allocations +} diff --git a/src/pkg/go/printer/testdata/comments2.input b/src/pkg/go/printer/testdata/comments2.input index 6f8c85c94..4a055c827 100644 --- a/src/pkg/go/printer/testdata/comments2.input +++ b/src/pkg/go/printer/testdata/comments2.input @@ -76,4 +76,30 @@ prints test 5 times for i := 0; i < 5; i++ { println("test") } -}
\ No newline at end of file +} + +func issue5623() { +L: + _ = yyyyyyyyyyyyyyyy // comment - should be aligned + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */ + + _ = yyyyyyyyyyyyyyyy /* comment - should be aligned */ + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment + +LLLLLLL: + _ = yyyyyyyyyyyyyyyy // comment - should be aligned + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment + +LL: +LLLLL: + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx /* comment */ + _ = yyyyyyyyyyyyyyyy /* comment - should be aligned */ + + _ = xxxxxxxxxxxxxxxxxxxxxxxxxxxx // comment + _ = yyyyyyyyyyyyyyyy // comment - should be aligned + +// test case from issue +label: + mask := uint64(1)<<c - 1 // Allocation mask + used := atomic.LoadUint64(&h.used) // Current allocations +} diff --git a/src/pkg/go/printer/testdata/declarations.golden b/src/pkg/go/printer/testdata/declarations.golden index 0331615e5..a27f21fc8 100644 --- a/src/pkg/go/printer/testdata/declarations.golden +++ b/src/pkg/go/printer/testdata/declarations.golden @@ -397,6 +397,21 @@ func _() { } } +// use the formatted output rather than the input to decide when to align +// (was issue 4505) +const ( + short = 2 * (1 + 2) + aMuchLongerName = 3 +) + +var ( + short = X{} + aMuchLongerName = X{} + + x1 = X{} // foo + x2 = X{} // foo +) + func _() { type ( xxxxxx int @@ -723,7 +738,8 @@ func _() int { } // making function declarations safe for new semicolon rules -func _() { /* multi-line func because of comment */ +func _() { /* single-line function because of "short-ish" comment */ } +func _() { /* multi-line function because of "long-ish" comment - much more comment text is following here */ /* and more */ } func _() { diff --git a/src/pkg/go/printer/testdata/declarations.input b/src/pkg/go/printer/testdata/declarations.input index dbdbdfe74..d9951d386 100644 --- a/src/pkg/go/printer/testdata/declarations.input +++ b/src/pkg/go/printer/testdata/declarations.input @@ -409,6 +409,24 @@ func _() { } } +// use the formatted output rather than the input to decide when to align +// (was issue 4505) +const ( + short = 2 * ( + 1 + 2) + aMuchLongerName = 3 +) + +var ( + short = X{ + } + aMuchLongerName = X{} + + x1 = X{} // foo + x2 = X{ + } // foo +) + func _() { type ( xxxxxx int @@ -737,7 +755,8 @@ func _() int { // making function declarations safe for new semicolon rules -func _() { /* multi-line func because of comment */ } +func _() { /* single-line function because of "short-ish" comment */ } +func _() { /* multi-line function because of "long-ish" comment - much more comment text is following here */ /* and more */ } func _() { /* multi-line func because block is on multiple lines */ } diff --git a/src/pkg/go/scanner/scanner.go b/src/pkg/go/scanner/scanner.go index 1e259d5ed..cec82ea10 100644 --- a/src/pkg/go/scanner/scanner.go +++ b/src/pkg/go/scanner/scanner.go @@ -148,11 +148,14 @@ func (s *Scanner) interpretLineComment(text []byte) { // get filename and line number, if any if i := bytes.LastIndex(text, []byte{':'}); i > 0 { if line, err := strconv.Atoi(string(text[i+1:])); err == nil && line > 0 { - // valid //line filename:line comment; - filename := filepath.Clean(string(text[len(prefix):i])) - if !filepath.IsAbs(filename) { - // make filename relative to current directory - filename = filepath.Join(s.dir, filename) + // valid //line filename:line comment + filename := string(bytes.TrimSpace(text[len(prefix):i])) + if filename != "" { + filename = filepath.Clean(filename) + if !filepath.IsAbs(filename) { + // make filename relative to current directory + filename = filepath.Join(s.dir, filename) + } } // update scanner position s.file.AddLineInfo(s.lineOffset+len(text)+1, filename, line) // +len(text)+1 since comment applies to next line @@ -358,73 +361,94 @@ exit: return tok, string(s.src[offs:s.offset]) } -func (s *Scanner) scanEscape(quote rune) { +// scanEscape parses an escape sequence where rune is the accepted +// escaped quote. In case of a syntax error, it stops at the offending +// character (without consuming it) and returns false. Otherwise +// it returns true. +func (s *Scanner) scanEscape(quote rune) bool { offs := s.offset - var i, base, max uint32 + var n int + var base, max uint32 switch s.ch { case 'a', 'b', 'f', 'n', 'r', 't', 'v', '\\', quote: s.next() - return + return true case '0', '1', '2', '3', '4', '5', '6', '7': - i, base, max = 3, 8, 255 + n, base, max = 3, 8, 255 case 'x': s.next() - i, base, max = 2, 16, 255 + n, base, max = 2, 16, 255 case 'u': s.next() - i, base, max = 4, 16, unicode.MaxRune + n, base, max = 4, 16, unicode.MaxRune case 'U': s.next() - i, base, max = 8, 16, unicode.MaxRune + n, base, max = 8, 16, unicode.MaxRune default: - s.next() // always make progress - s.error(offs, "unknown escape sequence") - return + msg := "unknown escape sequence" + if s.ch < 0 { + msg = "escape sequence not terminated" + } + s.error(offs, msg) + return false } var x uint32 - for ; i > 0 && s.ch != quote && s.ch >= 0; i-- { + for n > 0 { d := uint32(digitVal(s.ch)) if d >= base { - s.error(s.offset, "illegal character in escape sequence") - break + msg := fmt.Sprintf("illegal character %#U in escape sequence", s.ch) + if s.ch < 0 { + msg = "escape sequence not terminated" + } + s.error(s.offset, msg) + return false } x = x*base + d s.next() + n-- } - // in case of an error, consume remaining chars - for ; i > 0 && s.ch != quote && s.ch >= 0; i-- { - s.next() - } + if x > max || 0xD800 <= x && x < 0xE000 { s.error(offs, "escape sequence is invalid Unicode code point") + return false } + + return true } -func (s *Scanner) scanChar() string { +func (s *Scanner) scanRune() string { // '\'' opening already consumed offs := s.offset - 1 + valid := true n := 0 - for s.ch != '\'' { + for { ch := s.ch - n++ - s.next() if ch == '\n' || ch < 0 { - s.error(offs, "character literal not terminated") - n = 1 + // only report error if we don't have one already + if valid { + s.error(offs, "rune literal not terminated") + valid = false + } break } + s.next() + if ch == '\'' { + break + } + n++ if ch == '\\' { - s.scanEscape('\'') + if !s.scanEscape('\'') { + valid = false + } + // continue to read to closing quote } } - s.next() - - if n != 1 { - s.error(offs, "illegal character literal") + if valid && n != 1 { + s.error(offs, "illegal rune literal") } return string(s.src[offs:s.offset]) @@ -434,11 +458,14 @@ func (s *Scanner) scanString() string { // '"' opening already consumed offs := s.offset - 1 - for s.ch != '"' { + for { ch := s.ch - s.next() if ch == '\n' || ch < 0 { - s.error(offs, "string not terminated") + s.error(offs, "string literal not terminated") + break + } + s.next() + if ch == '"' { break } if ch == '\\' { @@ -446,8 +473,6 @@ func (s *Scanner) scanString() string { } } - s.next() - return string(s.src[offs:s.offset]) } @@ -468,20 +493,21 @@ func (s *Scanner) scanRawString() string { offs := s.offset - 1 hasCR := false - for s.ch != '`' { + for { ch := s.ch + if ch < 0 { + s.error(offs, "raw string literal not terminated") + break + } s.next() + if ch == '`' { + break + } if ch == '\r' { hasCR = true } - if ch < 0 { - s.error(offs, "string not terminated") - break - } } - s.next() - lit := s.src[offs:s.offset] if hasCR { lit = stripCR(lit) @@ -617,7 +643,7 @@ scanAgain: case '\'': insertSemi = true tok = token.CHAR - lit = s.scanChar() + lit = s.scanRune() case '`': insertSemi = true tok = token.STRING diff --git a/src/pkg/go/scanner/scanner_test.go b/src/pkg/go/scanner/scanner_test.go index 8c64c2b95..fc450d8a6 100644 --- a/src/pkg/go/scanner/scanner_test.go +++ b/src/pkg/go/scanner/scanner_test.go @@ -493,9 +493,9 @@ var segments = []segment{ {"\nline3 //line File1.go:100", filepath.Join("dir", "TestLineComments"), 3}, // bad line comment, ignored {"\nline4", filepath.Join("dir", "TestLineComments"), 4}, {"\n//line File1.go:100\n line100", filepath.Join("dir", "File1.go"), 100}, + {"\n//line \t :42\n line1", "", 42}, {"\n//line File2.go:200\n line200", filepath.Join("dir", "File2.go"), 200}, - {"\n//line :1\n line1", "dir", 1}, - {"\n//line foo:42\n line42", filepath.Join("dir", "foo"), 42}, + {"\n//line foo\t:42\n line42", filepath.Join("dir", "foo"), 42}, {"\n //line foo:42\n line44", filepath.Join("dir", "foo"), 44}, // bad line comment, ignored {"\n//line foo 42\n line46", filepath.Join("dir", "foo"), 46}, // bad line comment, ignored {"\n//line foo:42 extra text\n line48", filepath.Join("dir", "foo"), 48}, // bad line comment, ignored @@ -631,7 +631,7 @@ type errorCollector struct { pos token.Position // last error position encountered } -func checkError(t *testing.T, src string, tok token.Token, pos int, err string) { +func checkError(t *testing.T, src string, tok token.Token, pos int, lit, err string) { var s Scanner var h errorCollector eh := func(pos token.Position, msg string) { @@ -640,13 +640,12 @@ func checkError(t *testing.T, src string, tok token.Token, pos int, err string) h.pos = pos } s.Init(fset.AddFile("", fset.Base(), len(src)), []byte(src), eh, ScanComments|dontInsertSemis) - _, tok0, _ := s.Scan() - _, tok1, _ := s.Scan() + _, tok0, lit0 := s.Scan() if tok0 != tok { t.Errorf("%q: got %s, expected %s", src, tok0, tok) } - if tok1 != token.EOF { - t.Errorf("%q: got %s, expected EOF", src, tok1) + if tok0 != token.ILLEGAL && lit0 != lit { + t.Errorf("%q: got literal %q, expected %q", src, lit0, lit) } cnt := 0 if err != "" { @@ -667,43 +666,71 @@ var errors = []struct { src string tok token.Token pos int + lit string err string }{ - {"\a", token.ILLEGAL, 0, "illegal character U+0007"}, - {`#`, token.ILLEGAL, 0, "illegal character U+0023 '#'"}, - {`…`, token.ILLEGAL, 0, "illegal character U+2026 '…'"}, - {`' '`, token.CHAR, 0, ""}, - {`''`, token.CHAR, 0, "illegal character literal"}, - {`'\8'`, token.CHAR, 2, "unknown escape sequence"}, - {`'\08'`, token.CHAR, 3, "illegal character in escape sequence"}, - {`'\x0g'`, token.CHAR, 4, "illegal character in escape sequence"}, - {`'\Uffffffff'`, token.CHAR, 2, "escape sequence is invalid Unicode code point"}, - {`'`, token.CHAR, 0, "character literal not terminated"}, - {`""`, token.STRING, 0, ""}, - {`"`, token.STRING, 0, "string not terminated"}, - {"``", token.STRING, 0, ""}, - {"`", token.STRING, 0, "string not terminated"}, - {"/**/", token.COMMENT, 0, ""}, - {"/*", token.COMMENT, 0, "comment not terminated"}, - {"077", token.INT, 0, ""}, - {"078.", token.FLOAT, 0, ""}, - {"07801234567.", token.FLOAT, 0, ""}, - {"078e0", token.FLOAT, 0, ""}, - {"078", token.INT, 0, "illegal octal number"}, - {"07800000009", token.INT, 0, "illegal octal number"}, - {"0x", token.INT, 0, "illegal hexadecimal number"}, - {"0X", token.INT, 0, "illegal hexadecimal number"}, - {"\"abc\x00def\"", token.STRING, 4, "illegal character NUL"}, - {"\"abc\x80def\"", token.STRING, 4, "illegal UTF-8 encoding"}, - {"\ufeff\ufeff", token.ILLEGAL, 3, "illegal byte order mark"}, // only first BOM is ignored - {"//\ufeff", token.COMMENT, 2, "illegal byte order mark"}, // only first BOM is ignored - {"'\ufeff" + `'`, token.CHAR, 1, "illegal byte order mark"}, // only first BOM is ignored - {`"` + "abc\ufeffdef" + `"`, token.STRING, 4, "illegal byte order mark"}, // only first BOM is ignored + {"\a", token.ILLEGAL, 0, "", "illegal character U+0007"}, + {`#`, token.ILLEGAL, 0, "", "illegal character U+0023 '#'"}, + {`…`, token.ILLEGAL, 0, "", "illegal character U+2026 '…'"}, + {`' '`, token.CHAR, 0, `' '`, ""}, + {`''`, token.CHAR, 0, `''`, "illegal rune literal"}, + {`'12'`, token.CHAR, 0, `'12'`, "illegal rune literal"}, + {`'123'`, token.CHAR, 0, `'123'`, "illegal rune literal"}, + {`'\0'`, token.CHAR, 3, `'\0'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\07'`, token.CHAR, 4, `'\07'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\8'`, token.CHAR, 2, `'\8'`, "unknown escape sequence"}, + {`'\08'`, token.CHAR, 3, `'\08'`, "illegal character U+0038 '8' in escape sequence"}, + {`'\x'`, token.CHAR, 3, `'\x'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\x0'`, token.CHAR, 4, `'\x0'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\x0g'`, token.CHAR, 4, `'\x0g'`, "illegal character U+0067 'g' in escape sequence"}, + {`'\u'`, token.CHAR, 3, `'\u'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\u0'`, token.CHAR, 4, `'\u0'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\u00'`, token.CHAR, 5, `'\u00'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\u000'`, token.CHAR, 6, `'\u000'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\u000`, token.CHAR, 6, `'\u000`, "escape sequence not terminated"}, + {`'\u0000'`, token.CHAR, 0, `'\u0000'`, ""}, + {`'\U'`, token.CHAR, 3, `'\U'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U0'`, token.CHAR, 4, `'\U0'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U00'`, token.CHAR, 5, `'\U00'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U000'`, token.CHAR, 6, `'\U000'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U0000'`, token.CHAR, 7, `'\U0000'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U00000'`, token.CHAR, 8, `'\U00000'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U000000'`, token.CHAR, 9, `'\U000000'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U0000000'`, token.CHAR, 10, `'\U0000000'`, "illegal character U+0027 ''' in escape sequence"}, + {`'\U0000000`, token.CHAR, 10, `'\U0000000`, "escape sequence not terminated"}, + {`'\U00000000'`, token.CHAR, 0, `'\U00000000'`, ""}, + {`'\Uffffffff'`, token.CHAR, 2, `'\Uffffffff'`, "escape sequence is invalid Unicode code point"}, + {`'`, token.CHAR, 0, `'`, "rune literal not terminated"}, + {`'\`, token.CHAR, 2, `'\`, "escape sequence not terminated"}, + {"'\n", token.CHAR, 0, "'", "rune literal not terminated"}, + {"'\n ", token.CHAR, 0, "'", "rune literal not terminated"}, + {`""`, token.STRING, 0, `""`, ""}, + {`"abc`, token.STRING, 0, `"abc`, "string literal not terminated"}, + {"\"abc\n", token.STRING, 0, `"abc`, "string literal not terminated"}, + {"\"abc\n ", token.STRING, 0, `"abc`, "string literal not terminated"}, + {"``", token.STRING, 0, "``", ""}, + {"`", token.STRING, 0, "`", "raw string literal not terminated"}, + {"/**/", token.COMMENT, 0, "/**/", ""}, + {"/*", token.COMMENT, 0, "/*", "comment not terminated"}, + {"077", token.INT, 0, "077", ""}, + {"078.", token.FLOAT, 0, "078.", ""}, + {"07801234567.", token.FLOAT, 0, "07801234567.", ""}, + {"078e0", token.FLOAT, 0, "078e0", ""}, + {"078", token.INT, 0, "078", "illegal octal number"}, + {"07800000009", token.INT, 0, "07800000009", "illegal octal number"}, + {"0x", token.INT, 0, "0x", "illegal hexadecimal number"}, + {"0X", token.INT, 0, "0X", "illegal hexadecimal number"}, + {"\"abc\x00def\"", token.STRING, 4, "\"abc\x00def\"", "illegal character NUL"}, + {"\"abc\x80def\"", token.STRING, 4, "\"abc\x80def\"", "illegal UTF-8 encoding"}, + {"\ufeff\ufeff", token.ILLEGAL, 3, "\ufeff\ufeff", "illegal byte order mark"}, // only first BOM is ignored + {"//\ufeff", token.COMMENT, 2, "//\ufeff", "illegal byte order mark"}, // only first BOM is ignored + {"'\ufeff" + `'`, token.CHAR, 1, "'\ufeff" + `'`, "illegal byte order mark"}, // only first BOM is ignored + {`"` + "abc\ufeffdef" + `"`, token.STRING, 4, `"` + "abc\ufeffdef" + `"`, "illegal byte order mark"}, // only first BOM is ignored } func TestScanErrors(t *testing.T) { for _, e := range errors { - checkError(t, e.src, e.tok, e.pos, e.err) + checkError(t, e.src, e.tok, e.pos, e.lit, e.err) } } |