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

Regression since dace191 (fix fromjson to keep integer precision (close #172)) #178

Closed
wader opened this issue Jun 26, 2022 · 4 comments
Closed

Comments

@wader
Copy link
Sponsor Contributor

wader commented Jun 26, 2022

$ git co dace191026f576cb7d8ffbea0c29e8c13097a7c1
$ go run cmd/gojq/main.go -n '"123abc" | fromjson'
123

$ git co dace191026f576cb7d8ffbea0c29e8c13097a7c1~1
$ go run cmd/gojq/main.go -n '"123abc" | fromjson'
gojq: invalid character 'a' after top-level value
exit status 5

Same for "{}asd" and "[]abc". I think we're hitting the support for decoding a stream of JSON values.

As I guess we still need to use UseNumber() maybe a possible solutions could be something like this:

func funcFromJSON(v interface{}) interface{} {
	switch v := v.(type) {
	case string:
		var w interface{}
		dec := json.NewDecoder(strings.NewReader(v))
		dec.UseNumber()
		err := dec.Decode(&w)
		if err != nil {
			return err
		}
		var n interface{}
		err = dec.Decode(&n)
		if err != io.EOF {
			// would be nice with better error, the decode err might be confusing to show?
                        // for "123abc" err is "invalid character 'a' looking for beginning of value"
			return fmt.Errorf("invalid character after top-level value")
		}
		return normalizeNumbers(w)
	default:
		return &funcTypeError{"fromjson", v}
	}
}

Work fine with "123 \n" etc also.

@itchyny
Copy link
Owner

itchyny commented Jun 26, 2022

What's the actual problem here? I don't think this is a regression.

@itchyny
Copy link
Owner

itchyny commented Jun 27, 2022

Hmm, I'll reconsider later. Maybe checking Token() is enough.

@wader
Copy link
Sponsor Contributor Author

wader commented Jun 27, 2022

What's the actual problem here? I don't think this is a regression.

That is used to throw error on unexpected trailing data, same as jq does:

$ jq -n '"{} 123" | fromjson'
jq: error (at <unknown>): Unexpected extra JSON values (while parsing '{} 123')
$ jq -n '"123 abc" | fromjson'
jq: error (at <unknown>): Invalid numeric literal at EOF at line 1, column 7 (while parsing '123 abc')

Hmm, I'll reconsider later. Maybe checking Token() is enough.

👍 yes that might be better. The double call to Decode and check for EOF feel a bit ugly.

@itchyny itchyny closed this as completed in 8483f3f Jul 3, 2022
@wader
Copy link
Sponsor Contributor Author

wader commented Jul 3, 2022

Thanks!

wader added a commit to wader/fq that referenced this issue Jul 3, 2022
Expose number parse validate functions, will be used later by text format decoders:
gojq.ValidNumber(s string)
gojq.NormalizeNumber(v json.Number)
gojq.NormalizeNumbers(v interface{})

From upstream:
Fixes: Regression since dace191 (fix fromjson to keep integer precision (close #172)) (itchyny/gojq#178)
otherwise mostly cosmetic and cleanups since last rebase
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