diff --git a/README.md b/README.md index 9b13423..1979597 100644 --- a/README.md +++ b/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: diff --git a/errchkjson.go b/errchkjson.go index 0c9c1e5..746709c 100644 --- a/errchkjson.go +++ b/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) } diff --git a/testdata/src/standard/a.go b/testdata/src/standard/a.go index 088557d..1867221 100644 --- a/testdata/src/standard/a.go +++ b/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