Merge pull request #16 from breml/issue_15

Handle case when error is passed as arg

Author: breml

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)

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"

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"