Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

respect variable shadowing and type aliasing #10

Merged
merged 2 commits into from Jun 25, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
12 changes: 6 additions & 6 deletions analyzer/analyzer.go
Expand Up @@ -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
firefart marked this conversation as resolved.
Show resolved Hide resolved
if findDeferWithVariableAssignment(funcBody, pass.TypesInfo, pass.TypesInfo.ObjectOf(n)) {
continue
}
}
Expand All @@ -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 {
Expand All @@ -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
}
Expand All @@ -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 {
Expand All @@ -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
}
Expand Down
69 changes: 63 additions & 6 deletions 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
Expand Down Expand Up @@ -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
Expand Down