[januththedev]

Complex encoding emits 1+-0i for a negative-zero imaginary part

Description

(*jsonEncoder).appendComplex chooses the sign of the imaginary part with if i >= 0, against the invariant stated in its own comment — "If imaginary part is less than 0, minus (-) sign is added by default by AppendFloat."

IEEE-754 makes -0.0 >= 0 true, but strconv still renders negative zero as -0. So for a negative-zero imaginary part the code prepends '+' and AppendFloat emits -0, producing two sign characters.

Why it's wrong

The encoder's contract — already asserted by the pre-existing table case "complex128/negative_i" -> "1-2i" — is <real><sign><imag>i. The output is "1+-0i", a malformed complex literal. It affects AddComplex128/AddComplex64 on ObjectEncoder and AppendComplex128/AppendComplex64 on ArrayEncoder, i.e. every JSON- or console-encoded complex field and every Complex128s/Complex64s array.

It is reachable from ordinary code: any arithmetic that underflows to -0 yields complex(a, -0.0) — e.g. x * -1 where x == 0.

The fix

// If imaginary part is less than 0, minus (-) sign is added by default
// by AppendFloat. Note that negative zero compares >= 0, so it needs to
// be excluded explicitly: AppendFloat still renders it as "-0", and
// prepending '+' would produce a malformed "1+-0i".
if i >= 0 && !math.Signbit(i) {
    enc.buf.AppendByte('+')
}

math.Signbit distinguishes -0 from +0, which a comparison cannot.

Tests

Four new table entries: complex128/negative_zero_i and complex64/negative_zero_i, in both TestJSONEncoderObjectFields and TestJSONEncoderArrays, e.g.:

{"complex128/negative_zero_i", `"k":"1-0i"`, func(e Encoder) { e.AddComplex128("k", complex(1, math.Copysign(0, -1))) }},

• Before: all four fail with expected: "\"k\":\"1-0i\"" actual: "\"k\":\"1+-0i\"". After: pass.
• go test -count=1 ./... -v: 824 PASS, 14 FAIL — the identical set of 14 that fails on unmodified HEAD, all Windows-environment artifacts (URL parsing of file://localhostC:\..., and a TempDir cleanup blocked by an open handle). The sorted --- FAIL diff before vs after is identical apart from timing digits, so there are zero regressions and 4 net-new passing subtests.
• go vet ./zapcore/ clean. gofmt -l flags the two files, but the same two HEAD files are flagged identically (CRLF checkout vs LF) — pre-existing, not introduced here.

This HEAD is upstream master's tip, so the defect is unfixed there. Searching open issues for complex, negative zero, and AppendComplex returns zero results, and no open PR touches appendComplex (nearest are #1553 NaN/Inf→null and #1588 AppendNumber).

I also checked and did not report: LevelEncoder.UnmarshalText docs "colored" but matches "color" (a doc bug, not code); TrimmedPath dropping /v2 for major-version modules (open PR #1559); and zapgrpc's V() sending unknown levels to InfoLevel (issue #1544, PRs #1583/#1546). Several other candidates — Field.Equals panicking on uncomparable Stringer, encodeStringer emitting PANIC= for a nil Stringer, missing nil guards in EncodeCaller/EncodeTime, and the multiWriteSyncer short-write accounting — all have open PRs (#1569, #1555, #1566, #1584, #1590) and are out of scope.

Note this repository requires a CLA signature before merging; that is the contributor's action, not something the change can address.

[CLAassistant]

All committers have signed the CLA.

[januththedev]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

Januth Nimnal seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

Ok i have signed in