encoding/json/jsontext: make trailing comma error more clear

[dsnet]

Go version

go1.27

Output of go env in your module/workspace:

n/a

What did you do?

Do this:

v := jsontext.Value(`["foo", "bar", "baz", ]`)
fmt.Println(v.Compact())

What did you see happen?

It correctly reports an error like:

jsontext: invalid character ']' at start of value within "/3" after offset 22

What did you expect to see?

However, it would be more helpful to report:

jsontext: invalid trailing ',' before ']' after offset 20

An accidental trailing comma is a common mistake in hand-modified JSON.
While the current syntactic error is correct (and actually the easiest to produce given the parsing context), it is not particularly easy for a human to diagnose the problem.

[ilindan-dev]

Hi @dsnet, is this issue still relevant and open for contributions? If no one else is currently working on it, I'd love to take a stab at it.
I was looking into the jsontext internals and I think there's a clean way to handle this without impacting the fast path. Instead of modifying the hot switch in reformatValue directly, we could intercept the m == 0 error inside the loops of reformatArray and reformatObject. If it immediately follows a comma and the next token is a closing bracket, we can return the clearer syntax error.
Does this architectural approach make sense to you for the main tree? If so, I'd be happy to put together a draft CL!

[dsnet]

Feel free to take a shot at it. I think we'll need to adjust the error generation logic it in three places (the code path for Decoder.ReadToken, Decoder.ReadValue, and Encoder.WriteValue). You mentioned reformatValue, which is only for the Encoder.WriteValue code path.

[ilindan-dev]

Awesome, thanks for the pointer! I'll make sure to cover all three code paths.

[odeke-em]

Thank you @dsnet and @ilindan-dev! Just an FYI that we encounter the same error when using the legacy encoding/json package, and it isn’t a deviation from legacy per https://go.dev/play/p/sxjhxjRxJy8 or inlined below

package main

import "encoding/json"

func main() {
	d := []byte(`["foo", "bar", "baz", ]`)
	var sl []string
	if err := json.Unmarshal(d, &sl); err != nil {
		panic(err)
	}
}

which produces

panic: invalid character ']' looking for beginning of value

goroutine 1 [running]:
main.main()
	/tmp/sandbox1267793750/prog.go:9 +0x88

which to me doesn’t spell urgency for Go1.27.

[ilindan-dev]

Thanks for the context, @dsnet! I agree that this isn't a critical regression, but I believe that having clearer error messages is a great step forward for the jsontext library to improve developer experience. I've almost updated CL to handle these cases consistently. Thanks for the example, @odeke-em.

[gopherbot]

Change https://go.dev/cl/795101 mentions this issue: encoding/json/jsontext: make trailing comma error more clear

[ilindan-dev]

Just a quick heads-up: I initially opened a PR in the go-json-experiment/json repo, missing the fact that it's now a read-only mirror since the package is being integrated upstream. I've ported my changes directly to the main golang/go tree. Thanks for your understanding.