cmd/compile/internal/types2: review of check_test.go

The changes between (equivalent, and reviewed) go/types/check_test.go
and check_test.go can be seen by comparing patchset 1 and 2. The actual
changes are removing the "// UNREVIEWED" marker, and minor adjustments
to get the code slightly closer to go/types/check_test.go.

The primary differences compared to go/types are:
- use of syntax rather than go/ast package
- re-implemented mechanism for error matching and elimination
  based on the syntax.ErrorMap mechanism (there's no exported
  access to the syntax scanner)
- error matching permits for column tolerances because types2
  column information doesn't match go/types column information

Change-Id: I8ae6bc93dfa2b517673b642064a1f09166755286
Reviewed-on: https://go-review.googlesource.com/c/go/+/305573
Trust: Robert Griesemer <gri@golang.org>
Run-TryBot: Robert Griesemer <gri@golang.org>
TryBot-Result: Go Bot <gobot@golang.org>
Reviewed-by: Robert Findley <rfindley@google.com>

diff --git a/src/cmd/compile/internal/types2/check_test.go b/src/cmd/compile/internal/types2/check_test.go
index fc6f46b4b8e31fe43c480d12402a45e6074a9612..ac21c3458e2a7296d215d8d0c9c2be1f55bc43f9 100644 (file)
--- a/src/cmd/compile/internal/types2/check_test.go
+++ b/src/cmd/compile/internal/types2/check_test.go
@@ -1,4 +1,3 @@
-// UNREVIEWED
 // Copyright 2011 The Go Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style
 // license that can be found in the LICENSE file.
@@ -29,9 +28,7 @@ package types2_test
 import (
        "cmd/compile/internal/syntax"
        "flag"
-       "fmt"
        "internal/testenv"
-       "io/ioutil"
        "os"
        "path/filepath"
        "regexp"
@@ -73,6 +70,7 @@ func unpackError(err error) syntax.Error {
        }
 }
 
+// delta returns the absolute difference between x and y.
 func delta(x, y uint) uint {
        switch {
        case x < y:
@@ -98,17 +96,17 @@ func asGoVersion(s string) string {
        return ""
 }
 
-func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint, trace bool) {
-       if len(sources) == 0 {
+func checkFiles(t *testing.T, filenames []string, goVersion string, colDelta uint, trace bool) {
+       if len(filenames) == 0 {
                t.Fatal("no source files")
        }
 
        var mode syntax.Mode
-       if strings.HasSuffix(sources[0], ".go2") {
+       if strings.HasSuffix(filenames[0], ".go2") {
                mode |= syntax.AllowGenerics
        }
        // parse files and collect parser errors
-       files, errlist := parseFiles(t, sources, mode)
+       files, errlist := parseFiles(t, filenames, mode)
 
        pkgName := "<no package>"
        if len(files) > 0 {
@@ -133,7 +131,7 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint,
        conf.AcceptMethodTypeParams = true
        conf.InferFromConstraints = true
        // special case for importC.src
-       if len(sources) == 1 && strings.HasSuffix(sources[0], "importC.src") {
+       if len(filenames) == 1 && strings.HasSuffix(filenames[0], "importC.src") {
                conf.FakeImportC = true
        }
        conf.Trace = trace
@@ -156,7 +154,7 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint,
 
        // collect expected errors
        errmap := make(map[string]map[uint][]syntax.Error)
-       for _, filename := range sources {
+       for _, filename := range filenames {
                f, err := os.Open(filename)
                if err != nil {
                        t.Error(err)
@@ -169,16 +167,16 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint,
        }
 
        // match against found errors
+       // TODO(gri) sort err list to avoid mismatched when having multiple errors
        for _, err := range errlist {
                got := unpackError(err)
 
                // find list of errors for the respective error line
                filename := got.Pos.Base().Filename()
                filemap := errmap[filename]
-               var line uint
+               line := got.Pos.Line()
                var list []syntax.Error
                if filemap != nil {
-                       line = got.Pos.Line()
                        list = filemap[line]
                }
                // list may be nil
@@ -210,6 +208,7 @@ func checkFiles(t *testing.T, sources []string, goVersion string, colDelta uint,
                // eliminate from list
                if n := len(list) - 1; n > 0 {
                        // not the last entry - swap in last element and shorten list by 1
+                       // TODO(gri) avoid changing the order of entries
                        list[index] = list[n]
                        filemap[line] = list[:n]
                } else {
@@ -247,48 +246,41 @@ func TestCheck(t *testing.T) {
        checkFiles(t, strings.Split(*testFiles, ","), *goVersion, 0, testing.Verbose())
 }
 
-func TestTestdata(t *testing.T)  { DefPredeclaredTestFuncs(); testDir(t, 75, "testdata") } // TODO(gri) narrow column tolerance
-func TestExamples(t *testing.T)  { testDir(t, 0, "examples") }
-func TestFixedbugs(t *testing.T) { testDir(t, 0, "fixedbugs") }
+// TODO(gri) go/types has an extra TestLongConstants test
+
+func TestTestdata(t *testing.T)  { DefPredeclaredTestFuncs(); testDir(t, "testdata", 75) } // TODO(gri) narrow column tolerance
+func TestExamples(t *testing.T)  { testDir(t, "examples", 0) }
+func TestFixedbugs(t *testing.T) { testDir(t, "fixedbugs", 0) }
 
-func testDir(t *testing.T, colDelta uint, dir string) {
+func testDir(t *testing.T, dir string, colDelta uint) {
        testenv.MustHaveGoBuild(t)
 
-       fis, err := ioutil.ReadDir(dir)
+       fis, err := os.ReadDir(dir)
        if err != nil {
                t.Error(err)
                return
        }
 
-       for count, fi := range fis {
+       for _, fi := range fis {
                path := filepath.Join(dir, fi.Name())
 
                // if fi is a directory, its files make up a single package
+               var filenames []string
                if fi.IsDir() {
-                       if testing.Verbose() {
-                               fmt.Printf("%3d %s\n", count, path)
-                       }
-                       fis, err := ioutil.ReadDir(path)
+                       fis, err := os.ReadDir(path)
                        if err != nil {
                                t.Error(err)
                                continue
                        }
-                       files := make([]string, len(fis))
-                       for i, fi := range fis {
-                               // if fi is a directory, checkFiles below will complain
-                               files[i] = filepath.Join(path, fi.Name())
-                               if testing.Verbose() {
-                                       fmt.Printf("\t%s\n", files[i])
-                               }
+                       for _, fi := range fis {
+                               filenames = append(filenames, filepath.Join(path, fi.Name()))
                        }
-                       checkFiles(t, files, "", colDelta, false)
-                       continue
+               } else {
+                       filenames = []string{path}
                }
 
-               // otherwise, fi is a stand-alone file
-               if testing.Verbose() {
-                       fmt.Printf("%3d %s\n", count, path)
-               }
-               checkFiles(t, []string{path}, "", colDelta, false)
+               t.Run(filepath.Base(path), func(t *testing.T) {
+                       checkFiles(t, filenames, *goVersion, colDelta, false)
+               })
        }
 }