Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

proposal: encoding/json: return the postion for validation failures #67423

Closed
dveeden opened this issue May 16, 2024 · 6 comments
Closed

proposal: encoding/json: return the postion for validation failures #67423

dveeden opened this issue May 16, 2024 · 6 comments
Labels
Milestone

Comments

@dveeden
Copy link

dveeden commented May 16, 2024

Proposal Details

We use json.Valid() in https://github.com/pingcap/tidb to implement the JSON_VALID() SQL function and would like to return the position on which the validation failed to the user.

https://github.com/pingcap/tidb/blob/0875abde25467897e4e81ae6c18c5e8792ca28a9/pkg/expression/builtin_json.go#L1019

Example usage:

package main

import (
	"encoding/json"
	"fmt"
	"strings"
)

func main() {
	inputs := []string{
		`{}`,
		`{"foo": "bar"}`,
		`{"foo: "bar"}`,
		`{"foo": "bar", "bar": foo}`,
		`{"foo": "bar", "bar", foo}`,
	}
	for _, input := range inputs {
		v, pos := json.ValidPos([]byte(input))
		fmt.Printf("'%s': valid: %#v, pos: %d\n", input, v, pos)
		if !v {
			fmt.Printf(strings.Repeat(" ", int(pos)) + "^\n")
		}
	}
}

Output:

go run main.go 
'{}': valid: true, pos: 2
'{"foo": "bar"}': valid: true, pos: 14
'{"foo: "bar"}': valid: false, pos: 9
         ^
'{"foo": "bar", "bar": foo}': valid: false, pos: 24
                        ^
'{"foo": "bar", "bar", foo}': valid: false, pos: 21
                     ^

This would make it easier to see why validation failed, especially for larger JSON documents.

@gopherbot
Copy link

Change https://go.dev/cl/585995 mentions this issue: enconding/json: add ValidPos() function

@ianlancetaylor
Copy link
Contributor

CC @dsnet @mvdan

@ianlancetaylor ianlancetaylor changed the title proposal: encoding/json: Return the postion for validation failures. proposal: encoding/json: return the postion for validation failures May 20, 2024
@dsnet
Copy link
Member

dsnet commented May 20, 2024

The v2 json draft proposal includes a SyntacticError.ByteOffset field, which I suspect makes this feature redundant. I think we should defer on changing this until v2 lands.

@dveeden
Copy link
Author

dveeden commented May 22, 2024

The v2 json draft proposal includes a SyntacticError.ByteOffset field, which I suspect makes this feature redundant. I think we should defer on changing this until v2 lands.

I've tried to make my code work with the v2 draft, but this doesn't seem to work:

package main

import (
	"fmt"
	"strings"

	"github.com/go-json-experiment/json/jsontext"
)

func main() {
	inputs := []string{
		`{}`,
		`{"foo": "bar"}`,
		`{"foo: "bar"}`,
		`{"foo": "bar", "bar": foo}`,
		`{"foo": "bar", "bar", foo}`,
	}
	for _, input := range inputs {
		val := jsontext.Value([]byte(input))
		v := val.IsValid()
		pos := 0
		fmt.Printf("'%s': valid: %#v, pos: %d\n", input, v, pos)
		if !v {
			fmt.Printf(strings.Repeat(" ", int(pos)) + "^\n")
		}
	}
}
$ go run main.go 
'{}': valid: true, pos: 0
'{"foo": "bar"}': valid: true, pos: 0
'{"foo: "bar"}': valid: false, pos: 0
^
'{"foo": "bar", "bar": foo}': valid: false, pos: 0
^
'{"foo": "bar", "bar", foo}': valid: false, pos: 0
^

As you can see pos is always set to 0. As this doesn't return a SyntacticError, there is no ByteOffset.

I've noticed ExampleWithUnmarshalers_recordOffsets is dealing with offsets, but it doesn't look to easily be applied to my example.

The v2 json draft looks interesting, but I'm not fully convinced this should make this one redundant. Let's first see if my example can be modified to work with v2, then this can be used as feedback on v2. After that we can see if it makes sense to apply this to v1 or not.

@dsnet
Copy link
Member

dsnet commented May 22, 2024

Try this instead: https://go.dev/play/p/ze0FqocVe-8

@dveeden
Copy link
Author

dveeden commented May 22, 2024

Try this instead: https://go.dev/play/p/ze0FqocVe-8

Thanks. That's great. This covers my usecase to get the offsets and even adds a useful error message.

I think a ValidPos() in v1 would be useful but I don't think complicating v2 with this is worth it. So let's close this for now and hope that v2 makes it in.

@dveeden dveeden closed this as completed May 22, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants