go.tools/go/types: more flexible error checking

- permit ERROR markers to be in full or line comments
- don't require ""s in /* ERROR "foo" */
- enable more std tests
- some minor cleanups

R=adonovan
CC=golang-dev
https://golang.org/cl/14169044

go/types/check_test.go

@@ -110,10 +110,11 @@ func parseFiles(t *testing.T, filenames []string) ([]*ast.File, []error) {
 	return files, errlist
 }
 
-// ERROR comments must be of the form /* ERROR "rx" */ and rx is
+// ERROR comments must start with text `ERROR "rx"` or `ERROR rx` where
-// a regular expression that matches the expected error message.
+// rx is a regular expression that matches the expected error message.
+// Space around "rx" or rx is ignored.
 //
-var errRx = regexp.MustCompile(`^/\* *ERROR *"([^"]*)" *\*/$`)
+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.
@@ -140,8 +141,11 @@ func errMap(t *testing.T, testname string, files []*ast.File) map[string][]strin
 			case token.EOF:
 				break scanFile
 			case token.COMMENT:
-				if s := errRx.FindStringSubmatch(lit); len(s) == 2 {
+				if lit[1] == '*' {
-					errmap[prev] = append(errmap[prev], s[1])
+					lit = lit[:len(lit)-2] // strip trailing */
+				}
+				if s := errRx.FindStringSubmatch(lit[2:]); len(s) == 2 {
+					errmap[prev] = append(errmap[prev], strings.TrimSpace(s[1]))
 				}
 			case token.SEMICOLON:
 				// ignore automatically inserted semicolon

go/types/stdlib_test.go

@@ -126,7 +126,6 @@ func TestStdtest(t *testing.T) {
 
 func TestStdfixed(t *testing.T) {
 	testTestDir(t, filepath.Join(runtime.GOROOT(), "test", "fixedbugs"),
-		"bug136.go", "bug179.go", "bug344.go", // TODO(gri) implement missing label checks
 		"bug251.go",                           // TODO(gri) incorrect cycle checks for interface types
 		"bug165.go",                           // TODO(gri) isComparable not working for incomplete struct type
 		"bug200.go",                           // TODO(gri) complete duplicate checking in expr switches

go/types/testdata/errors.src

@@ -36,3 +36,11 @@ func f(x int, m map[string]int) {
 	const s = "foo"
 	m /* ERROR "m\[s\] \(value, ok of type int\) is not used" */ [s]
 }
+
+// Valid ERROR comments can have a variety of forms.
+func _() {
+	0 /* ERROR "0 .* is not used" */
+	0 /* ERROR 0 .* is not used */
+	0 // ERROR "0 .* is not used"
+	0 // ERROR 0 .* is not used
+}

go/types/testdata/expr0.src

@@ -71,7 +71,7 @@ var (
 	f2 = +1
 	f3 = +f0
 	f4 float64 = +1
-	f5 float64 = +f4 /* ERROR not defined */
+	f5 float64 = +f4
 	f6 = -1
 	f7 = -f0
 	f8 float64 = -1
@@ -92,7 +92,7 @@ var (
 	c2 = +1
 	c3 = +c0
 	c4 complex128 = +1
-	c5 complex128 = +c4 /* ERROR not defined */
+	c5 complex128 = +c4
 	c6 = -1
 	c7 = -c0
 	c8 complex128 = -1

go/types/typexpr.go

@@ -415,7 +415,6 @@ func (check *checker) collectMethods(recv Type, list *ast.FieldList, cycleOk boo
 	var mset objset
 
 	for _, f := range list.List {
-		// TODO(gri) Consider calling funcType here.
 		typ := check.typ(f.Type, nil, cycleOk)
 		// the parser ensures that f.Tag is nil and we don't
 		// care if a constructed AST contains a non-nil tag