From 390eee7ab18a091662fd7580b08b4aa3515b5951 Mon Sep 17 00:00:00 2001 From: Robert Griesemer Date: Wed, 7 Dec 2022 20:40:37 -0800 Subject: [PATCH] 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 TryBot-Result: Gopher Robot Reviewed-by: Robert Griesemer Run-TryBot: Robert Griesemer Auto-Submit: Robert Griesemer --- src/cmd/compile/internal/types2/check_test.go | 61 +++--- src/go/types/check_test.go | 199 ++++++++---------- 2 files changed, 121 insertions(+), 139 deletions(-) diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go index 645b5b1572..0f97fe9680 100644 --- a/src/cmd/compile/internal/types2/check_test.go +++ b/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) } } diff --git a/src/go/types/check_test.go b/src/go/types/check_test.go index 215a836333..81736f6623 100644 --- a/src/go/types/check_test.go +++ b/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) + } } } } -- 2.50.0