Merge pull request #17 from breml/issue_14

Omit safe for encoding/json.Encoder

Author: breml

README.md

@@ -114,6 +114,12 @@ Forbidden basic types:
 Any composed types (struct, map, slice, array) containing a forbidden basic type. Any map
 using a key with a forbidden type (`bool`, `float32`, `float64`, `struct`).
 
+## Accepted edge case
+
+For `encoding/json.MarshalIndent`, there is a (pathological) edge case, where this
+function could [return an error](https://cs.opensource.google/go/go/+/refs/tags/go1.18:src/encoding/json/scanner.go;drc=refs%2Ftags%2Fgo1.18;l=181) for an otherwise safe argument, if the argument has
+a nesting depth larger than [`10000`](https://cs.opensource.google/go/go/+/refs/tags/go1.18:src/encoding/json/scanner.go;drc=refs%2Ftags%2Fgo1.18;l=144) (as of Go 1.18).
+
 ## Bugs found during development
 
 During the development of `errcheckjson`, the following issues in package `encoding/json` of the Go standard library have been found and PR have been merged:

errchkjson.go

@@ -59,9 +59,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)
+					e.handleJSONMarshal(pass, ce, fn.FullName(), blankIdentifier, e.omitSafe)
 				case "(*encoding/json.Encoder).Encode":
-					e.handleJSONMarshal(pass, ce, fn.FullName(), blankIdentifier)
+					e.handleJSONMarshal(pass, ce, fn.FullName(), blankIdentifier, true)
 				default:
 					e.inspectArgs(pass, ce.Args)
 				}
@@ -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(), evaluateMarshalErrorTarget(as.Lhs[1]))
+				e.handleJSONMarshal(pass, ce, fn.FullName(), evaluateMarshalErrorTarget(as.Lhs[1]), e.omitSafe)
 			case "(*encoding/json.Encoder).Encode":
-				e.handleJSONMarshal(pass, ce, fn.FullName(), evaluateMarshalErrorTarget(as.Lhs[0]))
+				e.handleJSONMarshal(pass, ce, fn.FullName(), evaluateMarshalErrorTarget(as.Lhs[0]), true)
 			default:
 				return true
 			}
@@ -115,7 +115,7 @@ const (
 	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) {
+func (e *errchkjson) handleJSONMarshal(pass *analysis.Pass, ce *ast.CallExpr, fnName string, errorTarget marshalErrorTarget, omitSafe bool) {
 	t := pass.TypesInfo.TypeOf(ce.Args[0])
 	if t == nil {
 		// Not sure, if this is at all possible
@@ -143,11 +143,11 @@ func (e *errchkjson) handleJSONMarshal(pass *analysis.Pass, ce *ast.CallExpr, fn
 			pass.Reportf(ce.Pos(), "Error return value of `%s` is not checked: %v", fnName, err)
 		}
 	}
-	if err == nil && errorTarget == variableAssignment && !e.omitSafe {
+	if err == nil && errorTarget == variableAssignment && !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 && errorTarget == blankIdentifier && e.omitSafe {
+	if err == nil && errorTarget == blankIdentifier && omitSafe {
 		pass.Reportf(ce.Pos(), "Error return value of `%s` is not checked", fnName)
 	}
 }
@@ -296,9 +296,9 @@ func (e *errchkjson) inspectArgs(pass *analysis.Pass, args []ast.Expr) {
 
 			switch fn.FullName() {
 			case "encoding/json.Marshal", "encoding/json.MarshalIndent":
-				e.handleJSONMarshal(pass, ce, fn.FullName(), functionArgument)
+				e.handleJSONMarshal(pass, ce, fn.FullName(), functionArgument, e.omitSafe)
 			case "(*encoding/json.Encoder).Encode":
-				e.handleJSONMarshal(pass, ce, fn.FullName(), functionArgument)
+				e.handleJSONMarshal(pass, ce, fn.FullName(), functionArgument, true)
 			default:
 				e.inspectArgs(pass, ce.Args)
 			}

testdata/src/standard/a.go

@@ -41,9 +41,9 @@ func JSONMarshalSafeTypes() {
 	_ = err
 
 	enc := json.NewEncoder(ioutil.Discard)
-	_ = enc.Encode(nil)   // nil is safe
-	enc.Encode(nil)       // nil is safe
-	err = enc.Encode(nil) // want "Error return value of `\\([*]encoding/json.Encoder\\).Encode` is checked but passed argument is safe"
+	_ = enc.Encode(nil)   // want "Error return value of `\\([*]encoding/json.Encoder\\).Encode` is not checked"
+	enc.Encode(nil)       // want "Error return value of `\\([*]encoding/json.Encoder\\).Encode` is not checked"
+	err = enc.Encode(nil) // nil is safe, but encoding/json.Encoder may return non json related errors
 	_ = err
 
 	var b bool
@@ -611,7 +611,7 @@ func JSONMarshalInvalidTypes() {
 	_, err = json.Marshal(mapC128Str) // want "`encoding/json.Marshal` for unsupported type `complex128` as map key found"
 	_ = err
 
-	mapStructStr := map[structKey]string{structKey{1}: "str"}
+	mapStructStr := map[structKey]string{{1}: "str"}
 	_, _ = json.Marshal(mapStructStr)   // want "`encoding/json.Marshal` for unsupported type `standard.structKey` as map key found"
 	_, err = json.Marshal(mapStructStr) // want "`encoding/json.Marshal` for unsupported type `standard.structKey` as map key found"
 	_ = err