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

Question about Decoder.StackPointer #31

Open
elv-gilles opened this issue Apr 16, 2024 · 2 comments
Open

Question about Decoder.StackPointer #31

elv-gilles opened this issue Apr 16, 2024 · 2 comments

Comments

@elv-gilles
Copy link

The following code:

package config_arshal_any_or_hook

import (
	"bytes"
	"fmt"
	"reflect"
	"testing"

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

type Conf struct {
	Spaces
}

type Spaces struct {
	QSpaces []SubConf `json:"spaces"`
}

type SubConf struct {
	Id   string `json:"id"`
	Name string `json:"name"`
}

func (s *SubConf) initDef() {
	s.Name = "x"
}

func jsPath(s string) string {
	ret := s
	for len(ret) < 15 {
		ret += " "
	}
	return ret
}

func TestStackPointer(t *testing.T) {
	c := &Conf{}
	dec := jsontext.NewDecoder(bytes.NewReader([]byte(`{"spaces": [ {"id":"aa"}, {"id":"bb"} ] }`)))
	err := json.UnmarshalDecode(dec, c,
		json.WithUnmarshalers(
			json.UnmarshalFuncV2(func(dec *jsontext.Decoder, val any, opts json.Options) error {
				p := jsPath(dec.StackPointer())
				fmt.Println(p, "\t=>", reflect.TypeOf(val))
				switch x := val.(type) {
				case *SubConf:
					x.initDef()
				}
				return json.SkipFunc
			}),
		),
	)
	if err != nil {
		t.Fatalf("unexpected error: %v", err)
	}
}

produces the following output:

                	=> *config_arshal_any_or_hook.Conf
/spaces         	=> *[]config_arshal_any_or_hook.SubConf
/spaces         	=> *config_arshal_any_or_hook.SubConf
/spaces/0/id    	=> *string
/spaces/0       	=> *config_arshal_any_or_hook.SubConf
/spaces/1/id    	=> *string

Is that correct ?

In particular, I would have expected to see:

  • /spaces/0 before /spaces/0/id and
  • /spaces/1 before /spaces/1/id
@dsnet
Copy link
Collaborator

dsnet commented Nov 21, 2024

As currently documented, this is working as intended:

StackPointer returns a JSON Pointer RFC 6901 to the most recently read value. Object names are only present if AllowDuplicateNames is false, otherwise object members are represented using their index within the object.

At the point when we're processing *SubConf the first time, the last token processed was the [, so /spaces is the correct pointer to the "most recently read value", which is the nested JSON array. The second time processing *SubConf, the last token processed the }, so /spaces/0 is the correct pointer to the "most recently read value", which is the first element of the nested JSON array.

I can see why this goes against what you expect, since your intent was probably to get the pointer to the next token. We could change the semantics of StackPointer to point at the next token, but that will require a potential reading of the stream and therefore also the possibility to fail (and thus needing an error).

I think the current semantics of StackPointer is probably the right one to keep (since it also exactly matches jsontext.Encoder.StackPointer, but we could consider adding a fun (*Decoder) PeekStackPointer() (Pointer, error) method.

@elv-gilles
Copy link
Author

we could consider adding a fun (*Decoder) PeekStackPointer() (Pointer, error) method

This would be fun :-) and nice, thank you.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants