go/types: add errorcalls_test, apply it, and fix errorf call sites

The errorcalls_test makes sure that we use error instead of errorf
where possible. Copied from types2 and adjusted for go/types.

Change-Id: Ib0572308c87e4415bf89aec8d64e662abe94754b
Reviewed-on: https://go-review.googlesource.com/c/go/+/441958
Reviewed-by: Robert Findley <rfindley@google.com>
Run-TryBot: Robert Griesemer <gri@google.com>
Auto-Submit: Robert Griesemer <gri@google.com>
Reviewed-by: Robert Griesemer <gri@google.com>

diff --git a/src/go/types/builtins.go b/src/go/types/builtins.go
index 6fde9300d3b1b6c69fd599221d26d4871e6282d6..a923ef557ff52d5777a29c2dafdbcb32e7bf2000 100644 (file)
--- a/src/go/types/builtins.go
+++ b/src/go/types/builtins.go
@@ -522,7 +522,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
                        }
                }
                if len(sizes) == 2 && sizes[0] > sizes[1] {
-                       check.errorf(call.Args[1], SwappedMakeArgs, invalidArg+"length and capacity swapped")
+                       check.error(call.Args[1], SwappedMakeArgs, invalidArg+"length and capacity swapped")
                        // safe to continue
                }
                x.mode = value
@@ -605,7 +605,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
        case _Add:
                // unsafe.Add(ptr unsafe.Pointer, len IntegerType) unsafe.Pointer
                if !check.allowVersion(check.pkg, 1, 17) {
-                       check.errorf(call.Fun, UnsupportedFeature, "unsafe.Add requires go1.17 or later")
+                       check.error(call.Fun, UnsupportedFeature, "unsafe.Add requires go1.17 or later")
                        return
                }
 
@@ -731,7 +731,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
        case _Slice:
                // unsafe.Slice(ptr *T, len IntegerType) []T
                if !check.allowVersion(check.pkg, 1, 17) {
-                       check.errorf(call.Fun, UnsupportedFeature, "unsafe.Slice requires go1.17 or later")
+                       check.error(call.Fun, UnsupportedFeature, "unsafe.Slice requires go1.17 or later")
                        return
                }
 
@@ -756,7 +756,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
        case _SliceData:
                // unsafe.SliceData(slice []T) *T
                if !check.allowVersion(check.pkg, 1, 20) {
-                       check.errorf(call.Fun, UnsupportedFeature, "unsafe.SliceData requires go1.20 or later")
+                       check.error(call.Fun, UnsupportedFeature, "unsafe.SliceData requires go1.20 or later")
                        return
                }
 
@@ -775,7 +775,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
        case _String:
                // unsafe.String(ptr *byte, len IntegerType) string
                if !check.allowVersion(check.pkg, 1, 20) {
-                       check.errorf(call.Fun, UnsupportedFeature, "unsafe.String requires go1.20 or later")
+                       check.error(call.Fun, UnsupportedFeature, "unsafe.String requires go1.20 or later")
                        return
                }
 
@@ -799,7 +799,7 @@ func (check *Checker) builtin(x *operand, call *ast.CallExpr, id builtinId) (_ b
        case _StringData:
                // unsafe.StringData(str string) *byte
                if !check.allowVersion(check.pkg, 1, 20) {
-                       check.errorf(call.Fun, UnsupportedFeature, "unsafe.StringData requires go1.20 or later")
+                       check.error(call.Fun, UnsupportedFeature, "unsafe.StringData requires go1.20 or later")
                        return
                }
 

diff --git a/src/go/types/check.go b/src/go/types/check.go
index 73dbcca6cf26bdec0dc96238900d1be7512007b1..50d8afe4e373a75d5cf4a6f7a7c355c1cba70f20 100644 (file)
--- a/src/go/types/check.go
+++ b/src/go/types/check.go
@@ -272,7 +272,7 @@ func (check *Checker) initFiles(files []*ast.File) {
                        if name != "_" {
                                pkg.name = name
                        } else {
-                               check.errorf(file.Name, BlankPkgName, "invalid package name _")
+                               check.error(file.Name, BlankPkgName, "invalid package name _")
                        }
                        fallthrough
 

diff --git a/src/go/types/decl.go b/src/go/types/decl.go
index b9ac49e20942fb69d5712478b6dd239380bb8f80..467cb7ef706be21d35f6fdb52a1ca59e55d96f2b 100644 (file)
--- a/src/go/types/decl.go
+++ b/src/go/types/decl.go
@@ -581,7 +581,7 @@ func (check *Checker) typeDecl(obj *TypeName, tdecl *ast.TypeSpec, def *Named) {
        // alias declaration
        if alias {
                if !check.allowVersion(check.pkg, 1, 9) {
-                       check.errorf(atPos(tdecl.Assign), UnsupportedFeature, "type aliases requires go1.9 or later")
+                       check.error(atPos(tdecl.Assign), UnsupportedFeature, "type aliases requires go1.9 or later")
                }
 
                check.brokenAlias(obj)

diff --git a/src/go/types/errorcalls_test.go b/src/go/types/errorcalls_test.go
new file mode 100644 (file)
index 0000000..e4164d4
--- /dev/null
+++ b/src/go/types/errorcalls_test.go
@@ -0,0 +1,53 @@
+// Copyright 2021 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 ast.
+
+package types_test
+
+import (
+       "go/ast"
+       "go/token"
+       "testing"
+)
+
+const errorfMinArgCount = 4
+
+// TestErrorCalls makes sure that check.errorf calls have at least
+// errorfMinArgCount arguments (otherwise we should use check.error).
+func TestErrorCalls(t *testing.T) {
+       fset := token.NewFileSet()
+       files, err := pkgFiles(fset, ".", 0)
+       if err != nil {
+               t.Fatal(err)
+       }
+
+       for _, file := range files {
+               ast.Inspect(file, func(n ast.Node) bool {
+                       call, _ := n.(*ast.CallExpr)
+                       if call == nil {
+                               return true
+                       }
+                       selx, _ := call.Fun.(*ast.SelectorExpr)
+                       if selx == nil {
+                               return true
+                       }
+                       if !(isName(selx.X, "check") && isName(selx.Sel, "errorf")) {
+                               return true
+                       }
+                       // check.errorf calls should have at least errorfMinArgCount arguments:
+                       // position, code, format string, and arguments to format
+                       if n := len(call.Args); n < errorfMinArgCount {
+                               t.Errorf("%s: got %d arguments, want at least %d", fset.Position(call.Pos()), n, errorfMinArgCount)
+                               return false
+                       }
+                       return true
+               })
+       }
+}
+
+func isName(n ast.Node, name string) bool {
+       if n, ok := n.(*ast.Ident); ok {
+               return n.Name == name
+       }
+       return false
+}

diff --git a/src/go/types/expr.go b/src/go/types/expr.go
index e7d9658a6e76ad5f9696a70eeb21a8f19174bdf1..f7bf5d2b16b3c3fb6c1de9a55086d816947e5f5d 100644 (file)
--- a/src/go/types/expr.go
+++ b/src/go/types/expr.go
@@ -95,7 +95,7 @@ func (check *Checker) overflow(x *operand, opPos token.Pos) {
                // TODO(gri) We should report exactly what went wrong. At the
                //           moment we don't have the (go/constant) API for that.
                //           See also TODO in go/constant/value.go.
-               check.errorf(atPos(opPos), InvalidConstVal, "constant result is not representable")
+               check.error(atPos(opPos), InvalidConstVal, "constant result is not representable")
                return
        }
 
@@ -1145,7 +1145,7 @@ func (check *Checker) binary(x *operand, e ast.Expr, lhs, rhs ast.Expr, op token
        if op == token.QUO || op == token.REM {
                // check for zero divisor
                if (x.mode == constant_ || allInteger(x.typ)) && y.mode == constant_ && constant.Sign(y.val) == 0 {
-                       check.errorf(&y, DivByZero, invalidOp+"division by zero")
+                       check.error(&y, DivByZero, invalidOp+"division by zero")
                        x.mode = invalid
                        return
                }
@@ -1155,7 +1155,7 @@ func (check *Checker) binary(x *operand, e ast.Expr, lhs, rhs ast.Expr, op token
                        re, im := constant.Real(y.val), constant.Imag(y.val)
                        re2, im2 := constant.BinaryOp(re, token.MUL, re), constant.BinaryOp(im, token.MUL, im)
                        if constant.Sign(re2) == 0 && constant.Sign(im2) == 0 {
-                               check.errorf(&y, DivByZero, invalidOp+"division by zero")
+                               check.error(&y, DivByZero, invalidOp+"division by zero")
                                x.mode = invalid
                                return
                        }
@@ -1639,7 +1639,7 @@ func (check *Checker) exprInternal(x *operand, e ast.Expr, hint Type) exprKind {
 
        case *ast.KeyValueExpr:
                // key:value expressions are handled in composite literals
-               check.errorf(e, InvalidSyntaxTree, invalidAST+"no key:value expected")
+               check.error(e, InvalidSyntaxTree, invalidAST+"no key:value expected")
                goto Error
 
        case *ast.ArrayType, *ast.StructType, *ast.FuncType,

diff --git a/src/go/types/index.go b/src/go/types/index.go
index 2f87dcba316554eca7081aca2df1dedcbef14b01..e1ce74ff9fd39c4561aa9565670d05e5325be766 100644 (file)
--- a/src/go/types/index.go
+++ b/src/go/types/index.go
@@ -229,7 +229,7 @@ func (check *Checker) sliceExpr(x *operand, e *ast.SliceExpr) {
                                if at == nil {
                                        at = e // e.Index[2] should be present but be careful
                                }
-                               check.errorf(at, InvalidSliceExpr, invalidOp+"3-index slice of string")
+                               check.error(at, InvalidSliceExpr, invalidOp+"3-index slice of string")
                                x.mode = invalid
                                return
                        }
@@ -276,7 +276,7 @@ func (check *Checker) sliceExpr(x *operand, e *ast.SliceExpr) {
 
        // spec: "Only the first index may be omitted; it defaults to 0."
        if e.Slice3 && (e.High == nil || e.Max == nil) {
-               check.errorf(inNode(e, e.Rbrack), InvalidSyntaxTree, invalidAST+"2nd and 3rd index required in 3-index slice")
+               check.error(inNode(e, e.Rbrack), InvalidSyntaxTree, invalidAST+"2nd and 3rd index required in 3-index slice")
                x.mode = invalid
                return
        }
@@ -336,7 +336,7 @@ func (check *Checker) singleIndex(expr *typeparams.IndexExpr) ast.Expr {
        }
        if len(expr.Indices) > 1 {
                // TODO(rFindley) should this get a distinct error code?
-               check.errorf(expr.Indices[1], InvalidIndex, invalidOp+"more than one index")
+               check.error(expr.Indices[1], InvalidIndex, invalidOp+"more than one index")
        }
        return expr.Indices[0]
 }

diff --git a/src/go/types/interface.go b/src/go/types/interface.go
index 28c8325c71a3c6818a39f55482edf3fb08bb7dd7..2fb8e40119b569da0faf012da6ad6b4ec83713dd 100644 (file)
--- a/src/go/types/interface.go
+++ b/src/go/types/interface.go
@@ -174,7 +174,7 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
                // We have a method with name f.Names[0].
                name := f.Names[0]
                if name.Name == "_" {
-                       check.errorf(name, BlankIfaceMethod, "methods must have a unique non-blank name")
+                       check.error(name, BlankIfaceMethod, "methods must have a unique non-blank name")
                        continue // ignore
                }
 
@@ -195,7 +195,7 @@ func (check *Checker) interfaceType(ityp *Interface, iface *ast.InterfaceType, d
                        if ftyp, _ := f.Type.(*ast.FuncType); ftyp != nil && ftyp.TypeParams != nil {
                                at = ftyp.TypeParams
                        }
-                       check.errorf(at, InvalidMethodTypeParams, "methods cannot have type parameters")
+                       check.error(at, InvalidMethodTypeParams, "methods cannot have type parameters")
                }
 
                // use named receiver type if available (for better error messages)

diff --git a/src/go/types/mono.go b/src/go/types/mono.go
index f95d200b9315d78d6396a5bbe7ae01bb40ba2b84..cf3f5a8bdc746fdb8c8bcf77212ce0ad1f46af54 100644 (file)
--- a/src/go/types/mono.go
+++ b/src/go/types/mono.go
@@ -139,7 +139,7 @@ func (check *Checker) reportInstanceLoop(v int) {
        // TODO(mdempsky): Pivot stack so we report the cycle from the top?
 
        obj0 := check.mono.vertices[v].obj
-       check.errorf(obj0, InvalidInstanceCycle, "instantiation cycle:")
+       check.error(obj0, InvalidInstanceCycle, "instantiation cycle:")
 
        qf := RelativeTo(check.pkg)
        for _, v := range stack {

diff --git a/src/go/types/resolver.go b/src/go/types/resolver.go
index 7c7a68b01c4a605daff26b48def43853562a3626..c8ccaf4e6e08761777645fb6cbf399ab955d82a2 100644 (file)
--- a/src/go/types/resolver.go
+++ b/src/go/types/resolver.go
@@ -63,7 +63,7 @@ func (check *Checker) arityMatch(s, init *ast.ValueSpec) {
        case init == nil && r == 0:
                // var decl w/o init expr
                if s.Type == nil {
-                       check.errorf(s, code, "missing type or init expr")
+                       check.error(s, code, "missing type or init expr")
                }
        case l < r:
                if l < len(s.Values) {
@@ -107,14 +107,14 @@ func (check *Checker) declarePkgObj(ident *ast.Ident, obj Object, d *declInfo) {
        // spec: "A package-scope or file-scope identifier with name init
        // may only be declared to be a function with this (func()) signature."
        if ident.Name == "init" {
-               check.errorf(ident, InvalidInitDecl, "cannot declare init - must be func")
+               check.error(ident, InvalidInitDecl, "cannot declare init - must be func")
                return
        }
 
        // spec: "The main package must have package name main and declare
        // a function main that takes no arguments and returns no value."
        if ident.Name == "main" && check.pkg.name == "main" {
-               check.errorf(ident, InvalidMainDecl, "cannot declare main - must be func")
+               check.error(ident, InvalidMainDecl, "cannot declare main - must be func")
                return
        }
 
@@ -275,13 +275,13 @@ func (check *Checker) collectObjects() {
                                        name = d.spec.Name.Name
                                        if path == "C" {
                                                // match 1.17 cmd/compile (not prescribed by spec)
-                                               check.errorf(d.spec.Name, ImportCRenamed, `cannot rename import "C"`)
+                                               check.error(d.spec.Name, ImportCRenamed, `cannot rename import "C"`)
                                                return
                                        }
                                }
 
                                if name == "init" {
-                                       check.errorf(d.spec, InvalidInitDecl, "cannot import package as init - init must be a func")
+                                       check.error(d.spec, InvalidInitDecl, "cannot import package as init - init must be a func")
                                        return
                                }
 
@@ -530,7 +530,7 @@ L: // unpack receiver type
                                case *ast.BadExpr:
                                        // ignore - error already reported by parser
                                case nil:
-                                       check.errorf(ix.Orig, InvalidSyntaxTree, invalidAST+"parameterized receiver contains nil parameters")
+                                       check.error(ix.Orig, InvalidSyntaxTree, invalidAST+"parameterized receiver contains nil parameters")
                                default:
                                        check.errorf(arg, BadDecl, "receiver type parameter %s must be an identifier", arg)
                                }

diff --git a/src/go/types/signature.go b/src/go/types/signature.go
index cf184ed0d74605173e7d7d5e5ffdc1359eee4e53..d9c32b2287169b8d6323eff7de4c3feba3c700a0 100644 (file)
--- a/src/go/types/signature.go
+++ b/src/go/types/signature.go
@@ -173,7 +173,7 @@ func (check *Checker) funcType(sig *Signature, recvPar *ast.FieldList, ftyp *ast
                // (A separate check is needed when type-checking interface method signatures because
                // they don't have a receiver specification.)
                if recvPar != nil {
-                       check.errorf(ftyp.TypeParams, InvalidMethodTypeParams, "methods cannot have type parameters")
+                       check.error(ftyp.TypeParams, InvalidMethodTypeParams, "methods cannot have type parameters")
                }
        }
 
@@ -286,7 +286,7 @@ func (check *Checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
                        // named parameter
                        for _, name := range field.Names {
                                if name.Name == "" {
-                                       check.errorf(name, InvalidSyntaxTree, invalidAST+"anonymous parameter")
+                                       check.error(name, InvalidSyntaxTree, invalidAST+"anonymous parameter")
                                        // ok to continue
                                }
                                par := NewParam(name.Pos(), check.pkg, name.Name, typ)
@@ -304,7 +304,7 @@ func (check *Checker) collectParams(scope *Scope, list *ast.FieldList, variadicO
        }
 
        if named && anonymous {
-               check.errorf(list, InvalidSyntaxTree, invalidAST+"list contains both named and anonymous parameters")
+               check.error(list, InvalidSyntaxTree, invalidAST+"list contains both named and anonymous parameters")
                // ok to continue
        }
 

diff --git a/src/go/types/stmt.go b/src/go/types/stmt.go
index ccc9ffbd6872d45b74a56440c772cbba69340ed6..025844affad98ee0f2b3e16b1a181b7027f216c7 100644 (file)
--- a/src/go/types/stmt.go
+++ b/src/go/types/stmt.go
@@ -139,7 +139,7 @@ func (check *Checker) multipleDefaults(list []ast.Stmt) {
                                d = s
                        }
                default:
-                       check.errorf(s, InvalidSyntaxTree, invalidAST+"case/communication clause expected")
+                       check.error(s, InvalidSyntaxTree, invalidAST+"case/communication clause expected")
                }
                if d != nil {
                        if first != nil {
@@ -469,7 +469,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                switch s.Tok {
                case token.ASSIGN, token.DEFINE:
                        if len(s.Lhs) == 0 {
-                               check.errorf(s, InvalidSyntaxTree, invalidAST+"missing lhs in assignment")
+                               check.error(s, InvalidSyntaxTree, invalidAST+"missing lhs in assignment")
                                return
                        }
                        if s.Tok == token.DEFINE {
@@ -583,7 +583,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                case *ast.IfStmt, *ast.BlockStmt:
                        check.stmt(inner, s.Else)
                default:
-                       check.errorf(s.Else, InvalidSyntaxTree, invalidAST+"invalid else branch in if statement")
+                       check.error(s.Else, InvalidSyntaxTree, invalidAST+"invalid else branch in if statement")
                }
 
        case *ast.SwitchStmt:
@@ -617,7 +617,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                for i, c := range s.Body.List {
                        clause, _ := c.(*ast.CaseClause)
                        if clause == nil {
-                               check.errorf(c, InvalidSyntaxTree, invalidAST+"incorrect expression switch case")
+                               check.error(c, InvalidSyntaxTree, invalidAST+"incorrect expression switch case")
                                continue
                        }
                        check.caseValues(&x, clause.List, seen)
@@ -654,13 +654,13 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                        rhs = guard.X
                case *ast.AssignStmt:
                        if len(guard.Lhs) != 1 || guard.Tok != token.DEFINE || len(guard.Rhs) != 1 {
-                               check.errorf(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
+                               check.error(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
                                return
                        }
 
                        lhs, _ = guard.Lhs[0].(*ast.Ident)
                        if lhs == nil {
-                               check.errorf(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
+                               check.error(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
                                return
                        }
 
@@ -675,14 +675,14 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                        rhs = guard.Rhs[0]
 
                default:
-                       check.errorf(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
+                       check.error(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
                        return
                }
 
                // rhs must be of the form: expr.(type) and expr must be an ordinary interface
                expr, _ := rhs.(*ast.TypeAssertExpr)
                if expr == nil || expr.Type != nil {
-                       check.errorf(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
+                       check.error(s, InvalidSyntaxTree, invalidAST+"incorrect form of type switch guard")
                        return
                }
                var x operand
@@ -709,7 +709,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                for _, s := range s.Body.List {
                        clause, _ := s.(*ast.CaseClause)
                        if clause == nil {
-                               check.errorf(s, InvalidSyntaxTree, invalidAST+"incorrect type switch case")
+                               check.error(s, InvalidSyntaxTree, invalidAST+"incorrect type switch case")
                                continue
                        }
                        // Check each type in this type switch case.
@@ -936,7 +936,7 @@ func (check *Checker) stmt(ctxt stmtContext, s ast.Stmt) {
                check.stmt(inner, s.Body)
 
        default:
-               check.errorf(s, InvalidSyntaxTree, invalidAST+"invalid statement")
+               check.error(s, InvalidSyntaxTree, invalidAST+"invalid statement")
        }
 }
 

diff --git a/src/go/types/typexpr.go b/src/go/types/typexpr.go
index 3d7c765560fd0283506828555630009d2762d36e..ea8f58c42cc82d9a06501e3333ad4b05eaf7ebcd 100644 (file)
--- a/src/go/types/typexpr.go
+++ b/src/go/types/typexpr.go
@@ -85,7 +85,7 @@ func (check *Checker) ident(x *operand, e *ast.Ident, def *Named, wantType bool)
                }
                if obj == universeIota {
                        if check.iota == nil {
-                               check.errorf(e, InvalidIota, "cannot use iota outside constant declaration")
+                               check.error(e, InvalidIota, "cannot use iota outside constant declaration")
                                return
                        }
                        x.val = check.iota

diff --git a/src/go/types/version.go b/src/go/types/version.go
index 5c453ea8f1f71234a6827b7de43bde8abef26a7d..3958ec922cb75f6742f839910c266d4e6a82db91 100644 (file)
--- a/src/go/types/version.go
+++ b/src/go/types/version.go
@@ -23,7 +23,7 @@ func (check *Checker) langCompat(lit *ast.BasicLit) {
        }
        // len(s) > 2
        if strings.Contains(s, "_") {
-               check.errorf(lit, UnsupportedFeature, "underscores in numeric literals requires go1.13 or later")
+               check.error(lit, UnsupportedFeature, "underscores in numeric literals requires go1.13 or later")
                return
        }
        if s[0] != '0' {
@@ -31,15 +31,15 @@ func (check *Checker) langCompat(lit *ast.BasicLit) {
        }
        radix := s[1]
        if radix == 'b' || radix == 'B' {
-               check.errorf(lit, UnsupportedFeature, "binary literals requires go1.13 or later")
+               check.error(lit, UnsupportedFeature, "binary literals requires go1.13 or later")
                return
        }
        if radix == 'o' || radix == 'O' {
-               check.errorf(lit, UnsupportedFeature, "0o/0O-style octal literals requires go1.13 or later")
+               check.error(lit, UnsupportedFeature, "0o/0O-style octal literals requires go1.13 or later")
                return
        }
        if lit.Kind != token.INT && (radix == 'x' || radix == 'X') {
-               check.errorf(lit, UnsupportedFeature, "hexadecimal floating-point literals requires go1.13 or later")
+               check.error(lit, UnsupportedFeature, "hexadecimal floating-point literals requires go1.13 or later")
        }
 }