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 golang#51006.

Change-Id: I6c5ecbeb86d095404ec04ba4452fb90d404b8280
Reviewed-on: https://go-review.googlesource.com/c/go/+/456137
Reviewed-by: Robert Findley <[email protected]>
TryBot-Result: Gopher Robot <[email protected]>
Reviewed-by: Robert Griesemer <[email protected]>
Run-TryBot: Robert Griesemer <[email protected]>
Auto-Submit: Robert Griesemer <[email protected]>

Author: griesemer

src/cmd/compile/internal/types2/check_test.go

@@ -57,27 +57,23 @@ func parseFiles(t *testing.T, filenames []string, mode syntax.Mode) ([]*syntax.F
 	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 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 
 	// 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 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 
 	// 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 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 				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 @@ func testFiles(t *testing.T, filenames []string, colDelta uint, manual bool) {
 	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)
 				}
 			}

src/go/types/check_test.go

@@ -36,6 +36,7 @@ import (
 	"path/filepath"
 	"reflect"
 	"regexp"
+	"sort"
 	"strings"
 	"testing"
 
@@ -49,21 +50,6 @@ var (
 
 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 @@ func parseFiles(t *testing.T, filenames []string, srcs [][]byte, mode parser.Mod
 	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 @@ func testFiles(t *testing.T, sizes Sizes, filenames []string, srcs [][]byte, man
 		return
 	}
 
+	// 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 against found errors
 	for _, err := range errlist {
-		err, ok := err.(Error)
-		if !ok {
+		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
 		}
-		code := readCode(err)
-		if code == 0 {
-			t.Errorf("missing error code: %v", err)
+
+		// 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)
 		}
-	}
 
-	// match and eliminate errors;
-	// we are expecting the following errors
-	errmap := errMap(t, files, srcs)
-	eliminate(t, errmap, errlist)
+		// 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)
+				}
 			}
 		}
 	}