From 62d28d1e5d5a32bd0f28a6a7929a85cd9e903bf0 Mon Sep 17 00:00:00 2001 From: Marat Reymers Date: Fri, 24 Jun 2022 22:38:01 +0300 Subject: [PATCH 1/2] respect variable shadowing and type aliasing --- analyzer/analyzer.go | 12 ++-- .../allow_error_in_defer.go | 69 +++++++++++++++++-- 2 files changed, 69 insertions(+), 12 deletions(-) diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go index 594a0fc..430c571 100644 --- a/analyzer/analyzer.go +++ b/analyzer/analyzer.go @@ -71,8 +71,8 @@ func run(pass *analysis.Pass) (interface{}, error) { } if allowErrorInDefer { - if ident, ok := p.Type.(*ast.Ident); ok { - if ident.Name == "error" && findDeferWithErrorAssignment(funcBody, n.Name) { + if pass.TypesInfo.TypeOf(p.Type).String() == "error" { // with package prefix, so only built-it error fits + if findDeferWithVariableAssignment(funcBody, pass.TypesInfo, pass.TypesInfo.ObjectOf(n)) { continue } } @@ -86,7 +86,7 @@ func run(pass *analysis.Pass) (interface{}, error) { return nil, nil } -func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool { +func findDeferWithVariableAssignment(body *ast.BlockStmt, info *types.Info, variable types.Object) bool { found := false ast.Inspect(body, func(node ast.Node) bool { @@ -96,7 +96,7 @@ func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool { if d, ok := node.(*ast.DeferStmt); ok { if fn, ok2 := d.Call.Fun.(*ast.FuncLit); ok2 { - if findErrorAssignment(fn.Body, name) { + if findVariableAssignment(fn.Body, info, variable) { found = true return false } @@ -109,7 +109,7 @@ func findDeferWithErrorAssignment(body *ast.BlockStmt, name string) bool { return found } -func findErrorAssignment(body *ast.BlockStmt, name string) bool { +func findVariableAssignment(body *ast.BlockStmt, info *types.Info, variable types.Object) bool { found := false ast.Inspect(body, func(node ast.Node) bool { @@ -120,7 +120,7 @@ func findErrorAssignment(body *ast.BlockStmt, name string) bool { if a, ok := node.(*ast.AssignStmt); ok { for _, lh := range a.Lhs { if i, ok2 := lh.(*ast.Ident); ok2 { - if i.Name == name { + if info.ObjectOf(i) == variable { found = true return false } diff --git a/testdata/src/allow-error-in-defer/allow_error_in_defer.go b/testdata/src/allow-error-in-defer/allow_error_in_defer.go index 31fa5ff..f8477be 100644 --- a/testdata/src/allow-error-in-defer/allow_error_in_defer.go +++ b/testdata/src/allow-error-in-defer/allow_error_in_defer.go @@ -1,5 +1,7 @@ package main +import "errors" + func simple() (err error) { defer func() { err = nil @@ -43,32 +45,87 @@ func errorIsNoAssigned() (err error) { // want `named return "err" with type "er return } -func shadowVariable() (err error) { +func shadowVariable() (err error) { // want `named return "err" with type "error" found` defer func() { - err := 123 // linter doesn't understand that this is different variable (even if different type) (yet?) + err := errors.New("xxx") _ = err }() return } -func shadowVariable2() (err error) { +func shadowVariableButAssign() (err error) { + defer func() { + { + err := errors.New("xxx") + _ = err + } + err = nil + }() + return +} + +func shadowVariable2() (err error) { // want `named return "err" with type "error" found` defer func() { - a, err := doSomething() // linter doesn't understand that this is different variable (yet?) + a, err := doSomething() _ = a _ = err }() return } -type myError = error // linter doesn't understand that this is the same type (yet?) +type errorAlias = error + +func errorAliasIsTheSame() (err errorAlias) { + defer func() { + err = nil + }() + return +} + +type myError error // linter doesn't check underlying type (yet?) + +func customTypeWithErrorUnderline() (err myError) { // want `named return "err" with type "myError" found` + defer func() { + err = nil + }() + return +} + +type myError2 interface{ error } // linter doesn't check interfaces -func customType() (err myError) { // want `named return "err" with type "myError" found` +func customTypeWithTheSameInterface() (err myError2) { // want `named return "err" with type "myError2" found` defer func() { err = nil }() return } +var _ error = myError3{} + +type myError3 struct{} // linter doesn't check interfaces + +func (m myError3) Error() string { return "" } + +func customTypeImplementingErrorInterface() (err myError3) { // want `named return "err" with type "myError3" found` + defer func() { + err = struct{}{} + }() + return +} + +func shadowErrorType() { + type error interface { // linter understands that this is not built-in error, even if it has the same name + Error() string + } + do := func() (err error) { // want `named return "err" with type "error" found` + defer func() { + err = nil + }() + return + } + do() +} + func notTheLast() (err error, _ int) { defer func() { err = nil From 8435743f4359b41a364b3afc9ec6bd43a80a83dc Mon Sep 17 00:00:00 2001 From: Marat Reymers Date: Sat, 25 Jun 2022 12:43:16 +0300 Subject: [PATCH 2/2] find error type in `types.Universe` --- analyzer/analyzer.go | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/analyzer/analyzer.go b/analyzer/analyzer.go index 430c571..83fe839 100644 --- a/analyzer/analyzer.go +++ b/analyzer/analyzer.go @@ -28,6 +28,7 @@ func flags() flag.FlagSet { func run(pass *analysis.Pass) (interface{}, error) { allowErrorInDefer := pass.Analyzer.Flags.Lookup(FlagAllowErrorInDefer).Value.String() == "true" + errorType := types.Universe.Lookup("error").Type() inspector := pass.ResultOf[inspect.Analyzer].(*inspector.Inspector) @@ -70,12 +71,10 @@ func run(pass *analysis.Pass) (interface{}, error) { continue } - if allowErrorInDefer { - if pass.TypesInfo.TypeOf(p.Type).String() == "error" { // with package prefix, so only built-it error fits - if findDeferWithVariableAssignment(funcBody, pass.TypesInfo, pass.TypesInfo.ObjectOf(n)) { - continue - } - } + if allowErrorInDefer && + types.Identical(pass.TypesInfo.TypeOf(p.Type), errorType) && + findDeferWithVariableAssignment(funcBody, pass.TypesInfo, pass.TypesInfo.ObjectOf(n)) { + continue } pass.Reportf(node.Pos(), "named return %q with type %q found", n.Name, types.ExprString(p.Type))