]> Cypherpunks repositories - gostls13.git/commitdiff
go/types: use commentMap to collect error comments
authorRobert Griesemer <gri@golang.org>
Thu, 8 Dec 2022 04:40:37 +0000 (20:40 -0800)
committerGopher Robot <gobot@golang.org>
Tue, 17 Jan 2023 19:53:51 +0000 (19:53 +0000)
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>

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

index 645b5b15721630e8cde2db2505e5e90426573e5a..0f97fe968054bf7a8c9e5090f8e6b5877b1f250d 100644 (file)
@@ -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)
                                }
                        }
index 215a836333eb00631dd8f25b0b60a4730e657336..81736f6623485c7d0a946858c97b796c6b82f7da 100644 (file)
@@ -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)
+                               }
                        }
                }
        }