go/types: use commentMap to collect error comments
Adjust the testFiles function to use the new commentMap
function. This makes it possible for testFiles to match
the types2.TestFiles logic more closely.
For #51006.
Change-Id: I6c5ecbeb86d095404ec04ba4452fb90d404b8280
Reviewed-on: https://go-review.googlesource.com/c/go/+/456137
Reviewed-by: Robert Findley <rfindley@google.com>
TryBot-Result: Gopher Robot <gobot@golang.org>
Reviewed-by: Robert Griesemer <gri@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go
index 645b5b1..0f97fe9 100644
--- a/src/cmd/compile/internal/types2/check_test.go
+++ b/src/cmd/compile/internal/types2/check_test.go
@@ -57,27 +57,23 @@
return files, errlist
}
-func unpackError(err error) syntax.Error {
+func unpackError(err error) (syntax.Pos, string) {
switch err := err.(type) {
case syntax.Error:
- return err
+ return err.Pos, err.Msg
case Error:
- return syntax.Error{Pos: err.Pos, Msg: err.Msg}
+ return err.Pos, err.Msg
default:
- return syntax.Error{Msg: err.Error()}
+ return nopos, err.Error()
}
}
// delta returns the absolute difference between x and y.
func delta(x, y uint) uint {
- switch {
- case x < y:
+ if x < y {
return y - x
- case x > y:
- return x - y
- default:
- return 0
}
+ return x - y
}
// Note: parseFlags is identical to the version in go/types which is
@@ -171,8 +167,8 @@
// sort errlist in source order
sort.Slice(errlist, func(i, j int) bool {
- pi := unpackError(errlist[i]).Pos
- pj := unpackError(errlist[j]).Pos
+ pi, _ := unpackError(errlist[i])
+ pj, _ := unpackError(errlist[j])
return pi.Cmp(pj) < 0
})
@@ -192,21 +188,20 @@
// match against found errors
for _, err := range errlist {
- got := unpackError(err)
+ gotPos, gotMsg := unpackError(err)
// find list of errors for the respective error line
- filename := got.Pos.Base().Filename()
+ filename := gotPos.Base().Filename()
filemap := errmap[filename]
- line := got.Pos.Line()
- var list []syntax.Error
+ line := gotPos.Line()
+ var errList []syntax.Error
if filemap != nil {
- list = filemap[line]
+ errList = filemap[line]
}
- // list may be nil
- // one of errors in list should match the current error
- index := -1 // list index of matching message, if any
- for i, want := range list {
+ // one of errors in errList should match the current error
+ index := -1 // errList index of matching message, if any
+ for i, want := range errList {
pattern := strings.TrimSpace(want.Msg[len(" ERROR "):])
if n := len(pattern); n >= 2 && pattern[0] == '"' && pattern[n-1] == '"' {
pattern = pattern[1 : n-1]
@@ -216,29 +211,29 @@
t.Errorf("%s:%d:%d: %v", filename, line, want.Pos.Col(), err)
continue
}
- if rx.MatchString(got.Msg) {
+ if rx.MatchString(gotMsg) {
index = i
break
}
}
if index < 0 {
- t.Errorf("%s: no error expected: %q", got.Pos, got.Msg)
+ t.Errorf("%s: no error expected: %q", gotPos, gotMsg)
continue
}
// column position must be within expected colDelta
- want := list[index]
- if delta(got.Pos.Col(), want.Pos.Col()) > colDelta {
- t.Errorf("%s: got col = %d; want %d", got.Pos, got.Pos.Col(), want.Pos.Col())
+ want := errList[index]
+ if delta(gotPos.Col(), want.Pos.Col()) > colDelta {
+ t.Errorf("%s: got col = %d; want %d", gotPos, gotPos.Col(), want.Pos.Col())
}
- // eliminate from list
- if n := len(list) - 1; n > 0 {
+ // eliminate from errList
+ if n := len(errList) - 1; n > 0 {
// not the last entry - slide entries down (don't reorder)
- copy(list[index:], list[index+1:])
- filemap[line] = list[:n]
+ copy(errList[index:], errList[index+1:])
+ filemap[line] = errList[:n]
} else {
- // last entry - remove list from filemap
+ // last entry - remove errList from filemap
delete(filemap, line)
}
@@ -252,8 +247,8 @@
if len(errmap) > 0 {
t.Errorf("--- %s: unreported errors:", pkgName)
for filename, filemap := range errmap {
- for line, list := range filemap {
- for _, err := range list {
+ for line, errList := range filemap {
+ for _, err := range errList {
t.Errorf("%s:%d:%d: %s", filename, line, err.Pos.Col(), err.Msg)
}
}
diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go
index 215a836..81736f6 100644
--- a/src/go/types/check_test.go
+++ b/src/go/types/check_test.go
@@ -36,6 +36,7 @@
"path/filepath"
"reflect"
"regexp"
+ "sort"
"strings"
"testing"
@@ -49,21 +50,6 @@
var fset = token.NewFileSet()
-// Positioned errors are of the form filename:line:column: message .
-var posMsgRx = regexp.MustCompile(`^(.*:\d+:\d+): *(?s)(.*)`)
-
-// splitError splits an error's error message into a position string
-// and the actual error message. If there's no position information,
-// pos is the empty string, and msg is the entire error message.
-func splitError(err error) (pos, msg string) {
- msg = err.Error()
- if m := posMsgRx.FindStringSubmatch(msg); len(m) == 3 {
- pos = m[1]
- msg = m[2]
- }
- return
-}
-
func parseFiles(t *testing.T, filenames []string, srcs [][]byte, mode parser.Mode) ([]*ast.File, []error) {
var files []*ast.File
var errlist []error
@@ -86,87 +72,22 @@
return files, errlist
}
-// ERROR comments must start with text `ERROR "rx"` or `ERROR rx` where
-// rx is a regular expression that matches the expected error message.
-// Space around "rx" or rx is ignored.
-var errRx = regexp.MustCompile(`^ *ERROR *"?([^"]*)"?`)
-
-// errMap collects the regular expressions of ERROR comments found
-// in files and returns them as a map of error positions to error messages.
-//
-// srcs must be a slice of the same length as files, containing the original
-// source for the parsed AST.
-func errMap(t *testing.T, files []*ast.File, srcs [][]byte) map[string][]string {
- // map of position strings to lists of error message patterns
- errmap := make(map[string][]string)
-
- for i, file := range files {
- tok := fset.File(file.Package)
- src := srcs[i]
- var s scanner.Scanner
- s.Init(tok, src, nil, scanner.ScanComments)
- var prev token.Pos // position of last non-comment, non-semicolon token
-
- scanFile:
- for {
- pos, tok, lit := s.Scan()
- switch tok {
- case token.EOF:
- break scanFile
- case token.COMMENT:
- if lit[1] == '*' {
- lit = lit[:len(lit)-2] // strip trailing */
- }
- if s := errRx.FindStringSubmatch(lit[2:]); len(s) == 2 {
- p := fset.Position(prev).String()
- errmap[p] = append(errmap[p], strings.TrimSpace(s[1]))
- }
- case token.SEMICOLON:
- // ignore automatically inserted semicolon
- if lit == "\n" {
- continue scanFile
- }
- fallthrough
- default:
- prev = pos
- }
- }
+func unpackError(fset *token.FileSet, err error) (token.Position, string) {
+ switch err := err.(type) {
+ case *scanner.Error:
+ return err.Pos, err.Msg
+ case Error:
+ return fset.Position(err.Pos), err.Msg
}
-
- return errmap
+ panic("unreachable")
}
-func eliminate(t *testing.T, errmap map[string][]string, errlist []error) {
- for _, err := range errlist {
- pos, gotMsg := splitError(err)
- list := errmap[pos]
- index := -1 // list index of matching message, if any
- // we expect one of the messages in list to match the error at pos
- for i, wantRx := range list {
- rx, err := regexp.Compile(wantRx)
- if err != nil {
- t.Errorf("%s: %v", pos, err)
- continue
- }
- if rx.MatchString(gotMsg) {
- index = i
- break
- }
- }
- if index >= 0 {
- // eliminate from list
- if n := len(list) - 1; n > 0 {
- // not the last entry - swap in last element and shorten list by 1
- list[index] = list[n]
- errmap[pos] = list[:n]
- } else {
- // last entry - remove list from map
- delete(errmap, pos)
- }
- } else {
- t.Errorf("%s: no error expected: %q", pos, gotMsg)
- }
+// delta returns the absolute difference between x and y.
+func delta(x, y int) int {
+ if x < y {
+ return y - x
}
+ return x - y
}
// parseFlags parses flags from the first line of the given source
@@ -262,28 +183,94 @@
return
}
- for _, err := range errlist {
- err, ok := err.(Error)
- if !ok {
- continue
- }
- code := readCode(err)
- if code == 0 {
- t.Errorf("missing error code: %v", err)
+ // sort errlist in source order
+ sort.Slice(errlist, func(i, j int) bool {
+ // TODO(gri) This is not correct as scanner.Errors
+ // don't have a correctly set Offset. But we only
+ // care about sorting when multiple equal errors
+ // appear on the same line, which happens with some
+ // type checker errors.
+ // For now this works. Will remove need for sorting
+ // in a subsequent CL.
+ pi, _ := unpackError(fset, errlist[i])
+ pj, _ := unpackError(fset, errlist[j])
+ return pi.Offset < pj.Offset
+ })
+
+ // collect expected errors
+ errmap := make(map[string]map[int][]comment)
+ for i, filename := range filenames {
+ if m := commentMap(srcs[i], regexp.MustCompile("^ ERROR ")); len(m) > 0 {
+ errmap[filename] = m
}
}
- // match and eliminate errors;
- // we are expecting the following errors
- errmap := errMap(t, files, srcs)
- eliminate(t, errmap, errlist)
+ // match against found errors
+ for _, err := range errlist {
+ gotPos, gotMsg := unpackError(fset, err)
+
+ // find list of errors for the respective error line
+ filename := gotPos.Filename
+ filemap := errmap[filename]
+ line := gotPos.Line
+ var errList []comment
+ if filemap != nil {
+ errList = filemap[line]
+ }
+
+ // one of errors in errList should match the current error
+ index := -1 // errList index of matching message, if any
+ for i, want := range errList {
+ pattern := strings.TrimSpace(want.text[len(" ERROR "):])
+ if n := len(pattern); n >= 2 && pattern[0] == '"' && pattern[n-1] == '"' {
+ pattern = pattern[1 : n-1]
+ }
+ rx, err := regexp.Compile(pattern)
+ if err != nil {
+ t.Errorf("%s:%d:%d: %v", filename, line, want.col, err)
+ continue
+ }
+ if rx.MatchString(gotMsg) {
+ index = i
+ break
+ }
+ }
+ if index < 0 {
+ t.Errorf("%s: no error expected: %q", gotPos, gotMsg)
+ continue
+ }
+
+ // column position must be within expected colDelta
+ const colDelta = 0
+ want := errList[index]
+ if delta(gotPos.Column, want.col) > colDelta {
+ t.Errorf("%s: got col = %d; want %d", gotPos, gotPos.Column, want.col)
+ }
+
+ // eliminate from errList
+ if n := len(errList) - 1; n > 0 {
+ // not the last entry - slide entries down (don't reorder)
+ copy(errList[index:], errList[index+1:])
+ filemap[line] = errList[:n]
+ } else {
+ // last entry - remove errList from filemap
+ delete(filemap, line)
+ }
+
+ // if filemap is empty, eliminate from errmap
+ if len(filemap) == 0 {
+ delete(errmap, filename)
+ }
+ }
// there should be no expected errors left
if len(errmap) > 0 {
- t.Errorf("--- %s: %d source positions with expected (but not reported) errors:", pkgName, len(errmap))
- for pos, list := range errmap {
- for _, rx := range list {
- t.Errorf("%s: %q", pos, rx)
+ t.Errorf("--- %s: unreported errors:", pkgName)
+ for filename, filemap := range errmap {
+ for line, errList := range filemap {
+ for _, err := range errList {
+ t.Errorf("%s:%d:%d: %s", filename, line, err.col, err.text)
+ }
}
}
}
