refactor: Permit matching on full paths of allowed errors

Author: polyfloyd

errorlint/allowed.go

@@ -34,10 +34,10 @@ var allowedErrors = []struct {
 	{err: "io.EOF", fun: "(*bytes.Reader).ReadRune"},
 	{err: "io.EOF", fun: "(*bytes.Reader).ReadString"},
 	// pkg/database/sql
-	{err: "sql.ErrNoRows", fun: "(*database/sql.Row).Scan"},
+	{err: "database/sql.ErrNoRows", fun: "(*database/sql.Row).Scan"},
 	// pkg/debug/elf
-	{err: "io.EOF", fun: "elf.Open"},
-	{err: "io.EOF", fun: "elf.NewFile"},
+	{err: "io.EOF", fun: "debug/elf.Open"},
+	{err: "io.EOF", fun: "debug/elf.NewFile"},
 	// pkg/io
 	{err: "io.EOF", fun: "(io.Reader).Read"},
 	{err: "io.EOF", fun: "(io.ReaderAt).ReadAt"},
@@ -50,14 +50,14 @@ var allowedErrors = []struct {
 	{err: "io.EOF", fun: "io.ReadFull"},
 	{err: "io.ErrUnexpectedEOF", fun: "io.ReadFull"},
 	// pkg/net/http
-	{err: "http.ErrServerClosed", fun: "(*net/http.Server).ListenAndServe"},
-	{err: "http.ErrServerClosed", fun: "(*net/http.Server).ListenAndServeTLS"},
-	{err: "http.ErrServerClosed", fun: "(*net/http.Server).Serve"},
-	{err: "http.ErrServerClosed", fun: "(*net/http.Server).ServeTLS"},
-	{err: "http.ErrServerClosed", fun: "http.ListenAndServe"},
-	{err: "http.ErrServerClosed", fun: "http.ListenAndServeTLS"},
-	{err: "http.ErrServerClosed", fun: "http.Serve"},
-	{err: "http.ErrServerClosed", fun: "http.ServeTLS"},
+	{err: "net/http.ErrServerClosed", fun: "(*net/http.Server).ListenAndServe"},
+	{err: "net/http.ErrServerClosed", fun: "(*net/http.Server).ListenAndServeTLS"},
+	{err: "net/http.ErrServerClosed", fun: "(*net/http.Server).Serve"},
+	{err: "net/http.ErrServerClosed", fun: "(*net/http.Server).ServeTLS"},
+	{err: "net/http.ErrServerClosed", fun: "net/http.ListenAndServe"},
+	{err: "net/http.ErrServerClosed", fun: "net/http.ListenAndServeTLS"},
+	{err: "net/http.ErrServerClosed", fun: "net/http.Serve"},
+	{err: "net/http.ErrServerClosed", fun: "net/http.ServeTLS"},
 	// pkg/os
 	{err: "io.EOF", fun: "(*os.File).Read"},
 	{err: "io.EOF", fun: "(*os.File).ReadAt"},
@@ -80,7 +80,7 @@ func isAllowedErrAndFunc(err, fun string) bool {
 	return false
 }
 
-func isAllowedErrorComparison(info *TypesInfoExt, binExpr *ast.BinaryExpr) bool {
+func isAllowedErrorComparison(pass *TypesInfoExt, binExpr *ast.BinaryExpr) bool {
 	var errName string // `<package>.<name>`, e.g. `io.EOF`
 	var callExprs []*ast.CallExpr
 
@@ -91,11 +91,11 @@ func isAllowedErrorComparison(info *TypesInfoExt, binExpr *ast.BinaryExpr) bool
 		case *ast.SelectorExpr:
 			// A selector which we assume refers to a staticaly declared error
 			// in a package.
-			errName = selectorToString(t)
+			errName = selectorToString(pass, t)
 		case *ast.Ident:
 			// Identifier, most likely to be the `err` variable or whatever
 			// produces it.
-			callExprs = assigningCallExprs(info, t)
+			callExprs = assigningCallExprs(pass, t)
 		case *ast.CallExpr:
 			callExprs = append(callExprs, t)
 		}
@@ -115,11 +115,11 @@ func isAllowedErrorComparison(info *TypesInfoExt, binExpr *ast.BinaryExpr) bool
 			// allowed.
 			return false
 		}
-		if sel, ok := info.Selections[functionSelector]; ok {
+		if sel, ok := pass.TypesInfo.Selections[functionSelector]; ok {
 			functionNames[i] = fmt.Sprintf("(%s).%s", sel.Recv(), sel.Obj().Name())
 		} else {
 			// If there is no selection, assume it is a package.
-			functionNames[i] = selectorToString(callExpr.Fun.(*ast.SelectorExpr))
+			functionNames[i] = selectorToString(pass, callExpr.Fun.(*ast.SelectorExpr))
 		}
 	}
 
@@ -134,17 +134,17 @@ func isAllowedErrorComparison(info *TypesInfoExt, binExpr *ast.BinaryExpr) bool
 
 // assigningCallExprs finds all *ast.CallExpr nodes that are part of an
 // *ast.AssignStmt that assign to the subject identifier.
-func assigningCallExprs(info *TypesInfoExt, subject *ast.Ident) []*ast.CallExpr {
+func assigningCallExprs(pass *TypesInfoExt, subject *ast.Ident) []*ast.CallExpr {
 	if subject.Obj == nil {
 		return nil
 	}
 
 	// Find other identifiers that reference this same object. Make sure to
 	// exclude the subject identifier as it will cause an infinite recursion
 	// and is being used in a read operation anyway.
-	sobj := info.ObjectOf(subject)
+	sobj := pass.TypesInfo.ObjectOf(subject)
 	identifiers := []*ast.Ident{}
-	for _, ident := range info.IdentifiersForObject[sobj] {
+	for _, ident := range pass.IdentifiersForObject[sobj] {
 		if subject.Pos() != ident.Pos() {
 			identifiers = append(identifiers, ident)
 		}
@@ -153,7 +153,7 @@ func assigningCallExprs(info *TypesInfoExt, subject *ast.Ident) []*ast.CallExpr
 	// Find out whether the identifiers are part of an assignment statement.
 	var callExprs []*ast.CallExpr
 	for _, ident := range identifiers {
-		parent := info.NodeParent[ident]
+		parent := pass.NodeParent[ident]
 		switch declT := parent.(type) {
 		case *ast.AssignStmt:
 			// The identifier is LHS of an assignment.
@@ -181,7 +181,7 @@ func assigningCallExprs(info *TypesInfoExt, subject *ast.Ident) []*ast.CallExpr
 					continue
 				}
 				// The subject was the result of assigning from another identifier.
-				callExprs = append(callExprs, assigningCallExprs(info, assignT)...)
+				callExprs = append(callExprs, assigningCallExprs(pass, assignT)...)
 			default:
 				// TODO: inconclusive?
 			}
@@ -190,9 +190,7 @@ func assigningCallExprs(info *TypesInfoExt, subject *ast.Ident) []*ast.CallExpr
 	return callExprs
 }
 
-func selectorToString(selExpr *ast.SelectorExpr) string {
-	if ident, ok := selExpr.X.(*ast.Ident); ok {
-		return ident.Name + "." + selExpr.Sel.Name
-	}
-	return ""
+func selectorToString(pass *TypesInfoExt, selExpr *ast.SelectorExpr) string {
+	o := pass.TypesInfo.Uses[selExpr.Sel]
+	return fmt.Sprintf("%s.%s", o.Pkg().Path(), o.Name())
 }

errorlint/analysis.go

@@ -35,9 +35,9 @@ func init() {
 
 func run(pass *analysis.Pass) (interface{}, error) {
 	lints := []analysis.Diagnostic{}
-	extInfo := newTypesInfoExt(pass.TypesInfo)
+	extInfo := newTypesInfoExt(pass)
 	if checkComparison {
-		l := LintErrorComparisons(pass.Fset, extInfo)
+		l := LintErrorComparisons(extInfo)
 		lints = append(lints, l...)
 	}
 	if checkAsserts {
@@ -57,7 +57,7 @@ func run(pass *analysis.Pass) (interface{}, error) {
 }
 
 type TypesInfoExt struct {
-	types.Info
+	*analysis.Pass
 
 	// Maps AST nodes back to the node they are contained within.
 	NodeParent map[ast.Node]ast.Node
@@ -66,9 +66,9 @@ type TypesInfoExt struct {
 	IdentifiersForObject map[types.Object][]*ast.Ident
 }
 
-func newTypesInfoExt(info *types.Info) *TypesInfoExt {
+func newTypesInfoExt(pass *analysis.Pass) *TypesInfoExt {
 	nodeParent := map[ast.Node]ast.Node{}
-	for node := range info.Scopes {
+	for node := range pass.TypesInfo.Scopes {
 		file, ok := node.(*ast.File)
 		if !ok {
 			continue
@@ -86,15 +86,15 @@ func newTypesInfoExt(info *types.Info) *TypesInfoExt {
 	}
 
 	identifiersForObject := map[types.Object][]*ast.Ident{}
-	for node, obj := range info.Defs {
+	for node, obj := range pass.TypesInfo.Defs {
 		identifiersForObject[obj] = append(identifiersForObject[obj], node)
 	}
-	for node, obj := range info.Uses {
+	for node, obj := range pass.TypesInfo.Uses {
 		identifiersForObject[obj] = append(identifiersForObject[obj], node)
 	}
 
 	return &TypesInfoExt{
-		Info:                 *info,
+		Pass:                 pass,
 		NodeParent:           nodeParent,
 		IdentifiersForObject: identifiersForObject,
 	}

errorlint/lint.go

@@ -158,10 +158,10 @@ func isFmtErrorfCallExpr(info types.Info, expr ast.Expr) (*ast.CallExpr, bool) {
 	return nil, false
 }
 
-func LintErrorComparisons(fset *token.FileSet, info *TypesInfoExt) []analysis.Diagnostic {
+func LintErrorComparisons(info *TypesInfoExt) []analysis.Diagnostic {
 	lints := []analysis.Diagnostic{}
 
-	for expr := range info.Types {
+	for expr := range info.TypesInfo.Types {
 		// Find == and != operations.
 		binExpr, ok := expr.(*ast.BinaryExpr)
 		if !ok {
@@ -175,7 +175,7 @@ func LintErrorComparisons(fset *token.FileSet, info *TypesInfoExt) []analysis.Di
 			continue
 		}
 		// Find comparisons of which one side is a of type error.
-		if !isErrorComparison(info.Info, binExpr) {
+		if !isErrorComparison(info.TypesInfo, binExpr) {
 			continue
 		}
 		// Some errors that are returned from some functions are exempt.
@@ -193,7 +193,7 @@ func LintErrorComparisons(fset *token.FileSet, info *TypesInfoExt) []analysis.Di
 		})
 	}
 
-	for scope := range info.Scopes {
+	for scope := range info.TypesInfo.Scopes {
 		// Find value switch blocks.
 		switchStmt, ok := scope.(*ast.SwitchStmt)
 		if !ok {
@@ -203,7 +203,7 @@ func LintErrorComparisons(fset *token.FileSet, info *TypesInfoExt) []analysis.Di
 		if switchStmt.Tag == nil {
 			continue
 		}
-		tagType := info.Types[switchStmt.Tag]
+		tagType := info.TypesInfo.Types[switchStmt.Tag]
 		if tagType.Type.String() != "error" {
 			continue
 		}
@@ -233,7 +233,7 @@ func isNilComparison(binExpr *ast.BinaryExpr) bool {
 	return false
 }
 
-func isErrorComparison(info types.Info, binExpr *ast.BinaryExpr) bool {
+func isErrorComparison(info *types.Info, binExpr *ast.BinaryExpr) bool {
 	tx := info.Types[binExpr.X]
 	ty := info.Types[binExpr.Y]
 	return tx.Type.String() == "error" || ty.Type.String() == "error"
@@ -252,11 +252,11 @@ func isNodeInErrorIsFunc(info *TypesInfoExt, node ast.Node) bool {
 		return false
 	}
 	// There should be 1 argument of type error.
-	if ii := funcDecl.Type.Params.List; len(ii) != 1 || info.Types[ii[0].Type].Type.String() != "error" {
+	if ii := funcDecl.Type.Params.List; len(ii) != 1 || info.TypesInfo.Types[ii[0].Type].Type.String() != "error" {
 		return false
 	}
 	// The return type should be bool.
-	if ii := funcDecl.Type.Results.List; len(ii) != 1 || info.Types[ii[0].Type].Type.String() != "bool" {
+	if ii := funcDecl.Type.Results.List; len(ii) != 1 || info.TypesInfo.Types[ii[0].Type].Type.String() != "bool" {
 		return false
 	}