Skip to content

Commit

Permalink
Handle case when error is passed as arg
Browse files Browse the repository at this point in the history
Fixes #15
  • Loading branch information
breml committed Feb 14, 2022
1 parent 7e195c2 commit a3b3dbf
Show file tree
Hide file tree
Showing 3 changed files with 57 additions and 13 deletions.
64 changes: 51 additions & 13 deletions errchkjson.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand All @@ -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
}
Expand All @@ -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
Expand All @@ -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)
}
}
Expand Down Expand Up @@ -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)
Expand Down
3 changes: 3 additions & 0 deletions testdata/src/nosafe/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down
3 changes: 3 additions & 0 deletions testdata/src/standard/a.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"
Expand Down Expand Up @@ -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"
Expand Down

0 comments on commit a3b3dbf

Please sign in to comment.