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: add a generic Decode function #59053

Open
joerdav opened this issue Mar 15, 2023 · 8 comments
Open

proposal: encoding/json: add a generic Decode function #59053

joerdav opened this issue Mar 15, 2023 · 8 comments
Labels
Milestone

Comments

@joerdav
Copy link

joerdav commented Mar 15, 2023

I believe with generics now in place a new more convenient Decode function could be added to the encoding/json package, amongst other encoding/* packages.

Proposed:

func handler(w http.ResponseWriter, r *http.Request) {
	request, err := json.NewDecoder(r.Body).DecodeInto[MyStruct]()
	if err != nil {
		...
	}
	...
}

Existing:

func handler(w http.ResponseWriter, r *http.Request) {
	var request MyStruct
	err := json.NewDecoder(r.Body).Decode(&request)
	if err != nil {
		...
	}
	...
}

The benefit of this API is not only is it more concise, but it removes the possibility of a json: Unmarshal(nil) error.

@joerdav joerdav changed the title proposal: encoding/json: proposal: encoding/json: add a generic Decode function Mar 15, 2023
@gopherbot gopherbot added this to the Proposal milestone Mar 15, 2023
@ianlancetaylor ianlancetaylor moved this to Incoming in Proposals Mar 15, 2023
@ianlancetaylor
Copy link
Member

CC @dsnet @mvdan

@a-h
Copy link
Contributor

a-h commented Mar 15, 2023

I think I remember reading somewhere that the idea of APIs that take a pointer is that you can reuse the MyStruct multiple times to avoid allocations. However, I can't think of a time I ever used the var request MyStruct more than once.

At the moment, you can't have generics on a method, and the design above has DecodeInto being a method on the json.Decoder type which has type parameters.

So I think you'd have to have the type on the json.Decoder type as per this example: https://go.dev/play/p/ts7x6mZP74j

This has the benefit of adding strong typing to the existing json.Decoder package's signature in a backwards compatible way.

package main

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

type Data struct {
	Name string `json:"name"`
	Age  int    `json:"age"`
}

func main() {
	r := strings.NewReader(`{ "name": "test", "age": 31 }`)

	result, err := NewDecoder[Data](r).Result()
	if err != nil {
		log.Fatalf("failed to decode: %v", err)
	}
	fmt.Printf("name: %v\n", result.Name)
}

func NewDecoder[T any](r io.Reader) *Decoder[T] {
	return &Decoder[T]{r: r}
}

type Decoder[T any] struct {
	r io.Reader
}

func (d *Decoder[T]) Decode(v *T) error {
	return json.NewDecoder(d.r).Decode(&v)
}

func (d *Decoder[T]) Result() (v T, err error) {
	err = d.Decode(&v)
	return
}

@joerdav
Copy link
Author

joerdav commented Mar 15, 2023

Yes that's a good point, you would need a new Decoder type, unless we got #49085 first.

@a-h
Copy link
Contributor

a-h commented Mar 15, 2023

Actually, my idea doesn't work for the existing API - https://go.dev/play/p/iRU0oLt4wPK

This is because there's no way to have a constraint that enforces that a generic parameter is a struct as per the conversation here https://groups.google.com/g/golang-nuts/c/UxVAj75L-rg/m/raCOyQRjAAAJ

@earthboundkid
Copy link
Contributor

https://go.dev/blog/when-generics

If some operation has to support even types that don’t have methods (so that interface types don’t help), and if the operation is different for each type (so that type parameters aren’t appropriate), use reflection.
An example of this is the encoding/json package. We don’t want to require that every type that we encode have a MarshalJSON method, so we can’t use interface types. But encoding an interface type is nothing like encoding a struct type, so we shouldn’t use type parameters. Instead, the package uses reflection. The code is not simple, but it works.

@dsnet
Copy link
Member

dsnet commented Mar 15, 2023

I'm not sure a generic variant provides sufficient benefit.

Consider these two patterns:

var v T
if err := dec.Decode(&v); err != nil {
    ...
}

versus:

v, err := dec.Decode[T]()
if err != nil {
    ...
}

Both are 4 lines.

@leaxoy
Copy link

leaxoy commented Mar 16, 2023

I'm not sure a generic variant provides sufficient benefit.

Consider these two patterns:

var v T
if err := dec.Decode(&v); err != nil {
    ...
}

versus:

v, err := dec.Decode[T]()
if err != nil {
    ...
}

Both are 4 lines.

I thinks this is error handling limitation, not generics.

@seankhliao
Copy link
Member

I think we should decline this for the same reason we should decline #57975
This is an unnecessary use of generics.

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

No branches or pull requests

8 participants