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

Handle case when error is passed as arg #16

Merged
merged 1 commit into from
Mar 1, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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