From a3b3dbf14a80a49981563a6a5ab115c43949547a Mon Sep 17 00:00:00 2001 From: Lucas Bremgartner Date: Sun, 13 Feb 2022 21:21:04 +0100 Subject: [PATCH] Handle case when error is passed as arg Fixes #15 --- errchkjson.go | 64 ++++++++++++++++++++++++++++++-------- testdata/src/nosafe/a.go | 3 ++ testdata/src/standard/a.go | 3 ++ 3 files changed, 57 insertions(+), 13 deletions(-) diff --git a/errchkjson.go b/errchkjson.go index 5a5b6d1..0c9c1e5 100644 --- a/errchkjson.go +++ b/errchkjson.go @@ -59,11 +59,11 @@ func (e *errchkjson) run(pass *analysis.Pass) (interface{}, error) { switch fn.FullName() { case "encoding/json.Marshal", "encoding/json.MarshalIndent": - e.handleJSONMarshal(pass, ce, fn.FullName(), true) + e.handleJSONMarshal(pass, ce, fn.FullName(), blankIdentifier) case "(*encoding/json.Encoder).Encode": - e.handleJSONMarshal(pass, ce, fn.FullName(), true) + e.handleJSONMarshal(pass, ce, fn.FullName(), blankIdentifier) default: - return true + e.inspectArgs(pass, ce.Args) } return false } @@ -85,9 +85,9 @@ func (e *errchkjson) run(pass *analysis.Pass) (interface{}, error) { switch fn.FullName() { case "encoding/json.Marshal", "encoding/json.MarshalIndent": - e.handleJSONMarshal(pass, ce, fn.FullName(), blankIdentifier(as.Lhs[1])) + e.handleJSONMarshal(pass, ce, fn.FullName(), evaluateMarshalErrorTarget(as.Lhs[1])) case "(*encoding/json.Encoder).Encode": - e.handleJSONMarshal(pass, ce, fn.FullName(), blankIdentifier(as.Lhs[0])) + e.handleJSONMarshal(pass, ce, fn.FullName(), evaluateMarshalErrorTarget(as.Lhs[0])) default: return true } @@ -98,20 +98,28 @@ func (e *errchkjson) run(pass *analysis.Pass) (interface{}, error) { return nil, nil } -func blankIdentifier(n ast.Expr) bool { +func evaluateMarshalErrorTarget(n ast.Expr) marshalErrorTarget { if errIdent, ok := n.(*ast.Ident); ok { if errIdent.Name == "_" { - return true + return blankIdentifier } } - return false + return variableAssignment } -func (e *errchkjson) handleJSONMarshal(pass *analysis.Pass, ce *ast.CallExpr, fnName string, blankIdentifier bool) { +type marshalErrorTarget int + +const ( + blankIdentifier = iota // the returned error from the JSON marshal function is assigned to the blank identifier "_". + variableAssignment // the returned error from the JSON marshal function is assigned to a variable. + functionArgument // the returned error from the JSON marshal function is passed to an other function as argument. +) + +func (e *errchkjson) handleJSONMarshal(pass *analysis.Pass, ce *ast.CallExpr, fnName string, errorTarget marshalErrorTarget) { t := pass.TypesInfo.TypeOf(ce.Args[0]) if t == nil { // Not sure, if this is at all possible - if blankIdentifier { + if errorTarget == blankIdentifier { pass.Reportf(ce.Pos(), "Type of argument to `%s` could not be evaluated and error return value is not checked", fnName) } return @@ -131,15 +139,15 @@ func (e *errchkjson) handleJSONMarshal(pass *analysis.Pass, ce *ast.CallExpr, fn pass.Reportf(ce.Pos(), "Error argument passed to `%s` does not contain any exported field", fnName) } // Only care about unsafe types if they are assigned to the blank identifier. - if blankIdentifier { + if errorTarget == blankIdentifier { pass.Reportf(ce.Pos(), "Error return value of `%s` is not checked: %v", fnName, err) } } - if err == nil && !blankIdentifier && !e.omitSafe { + if err == nil && errorTarget == variableAssignment && !e.omitSafe { pass.Reportf(ce.Pos(), "Error return value of `%s` is checked but passed argument is safe", fnName) } // Report an error, if err for json.Marshal is not checked and safe types are omitted - if err == nil && blankIdentifier && e.omitSafe { + if err == nil && errorTarget == blankIdentifier && e.omitSafe { pass.Reportf(ce.Pos(), "Error return value of `%s` is not checked", fnName) } } @@ -269,6 +277,36 @@ func jsonSafeMapKey(t types.Type) error { } } +func (e *errchkjson) inspectArgs(pass *analysis.Pass, args []ast.Expr) { + for _, a := range args { + ast.Inspect(a, func(n ast.Node) bool { + if n == nil { + return true + } + + ce, ok := n.(*ast.CallExpr) + if !ok { + return false + } + + fn, _ := typeutil.Callee(pass.TypesInfo, ce).(*types.Func) + if fn == nil { + return true + } + + switch fn.FullName() { + case "encoding/json.Marshal", "encoding/json.MarshalIndent": + e.handleJSONMarshal(pass, ce, fn.FullName(), functionArgument) + case "(*encoding/json.Encoder).Encode": + e.handleJSONMarshal(pass, ce, fn.FullName(), functionArgument) + default: + e.inspectArgs(pass, ce.Args) + } + return false + }) + } +} + // Construct *types.Interface for interface encoding.TextMarshaler // type TextMarshaler interface { // MarshalText() (text []byte, err error) diff --git a/testdata/src/nosafe/a.go b/testdata/src/nosafe/a.go index de77a7b..9464f05 100644 --- a/testdata/src/nosafe/a.go +++ b/testdata/src/nosafe/a.go @@ -33,6 +33,7 @@ func JSONMarshalSafeTypesWithNoSafe() { json.Marshal(nil) // want "Error return value of `encoding/json.Marshal` is not checked" _, err = json.Marshal(nil) // nil is safe, but omit-safe is set _ = err + fmt.Print(json.Marshal(nil)) // nil is safe, error is passed as argument _, _ = json.MarshalIndent(nil, "", " ") // want "Error return value of `encoding/json.MarshalIndent` is not checked" json.MarshalIndent(nil, "", " ") // want "Error return value of `encoding/json.MarshalIndent` is not checked" @@ -409,6 +410,7 @@ func JSONMarshalUnsafeTypes() { _, _ = json.Marshal(f32) // want "Error return value of `encoding/json.Marshal` is not checked: unsafe type `float32` found" _, err = json.Marshal(f32) // err is checked _ = err + fmt.Print(json.Marshal(f32)) // err is passed and therefore considered as checked var f64 float64 _, _ = json.Marshal(f64) // want "Error return value of `encoding/json.Marshal` is not checked: unsafe type `float64` found" @@ -543,6 +545,7 @@ func JSONMarshalInvalidTypes() { _, _ = json.Marshal(c64) // want "`encoding/json.Marshal` for unsupported type `complex64` found" _, err = json.Marshal(c64) // want "`encoding/json.Marshal` for unsupported type `complex64` found" _ = err + fmt.Print(json.Marshal(c64)) // want "`encoding/json.Marshal` for unsupported type `complex64` found" var c128 complex128 _, _ = json.Marshal(c128) // want "`encoding/json.Marshal` for unsupported type `complex128` found" diff --git a/testdata/src/standard/a.go b/testdata/src/standard/a.go index 3780597..088557d 100644 --- a/testdata/src/standard/a.go +++ b/testdata/src/standard/a.go @@ -33,6 +33,7 @@ func JSONMarshalSafeTypes() { json.Marshal(nil) // nil is safe _, err = json.Marshal(nil) // want "Error return value of `encoding/json.Marshal` is checked but passed argument is safe" _ = err + fmt.Print(json.Marshal(nil)) // nil is safe, error is passed as argument _, _ = json.MarshalIndent(nil, "", " ") // nil is safe json.MarshalIndent(nil, "", " ") // nil is safe @@ -409,6 +410,7 @@ func JSONMarshalUnsafeTypes() { _, _ = json.Marshal(f32) // want "Error return value of `encoding/json.Marshal` is not checked: unsafe type `float32` found" _, err = json.Marshal(f32) // err is checked _ = err + fmt.Print(json.Marshal(f32)) // err is passed and therefore considered as checked var f64 float64 _, _ = json.Marshal(f64) // want "Error return value of `encoding/json.Marshal` is not checked: unsafe type `float64` found" @@ -543,6 +545,7 @@ func JSONMarshalInvalidTypes() { _, _ = json.Marshal(c64) // want "`encoding/json.Marshal` for unsupported type `complex64` found" _, err = json.Marshal(c64) // want "`encoding/json.Marshal` for unsupported type `complex64` found" _ = err + fmt.Print(json.Marshal(c64)) // want "`encoding/json.Marshal` for unsupported type `complex64` found" var c128 complex128 _, _ = json.Marshal(c128) // want "`encoding/json.Marshal` for unsupported type `complex128` found"