net/url: add a "SetQuery" helper function

[HaraldNordgren]

Hi, I want to create a new helper function called "SetQueryParameters" in net/url. Is this something the community wants too?

Already from my first experience with Go, I was confused about the round-about way that we set query parameters:

v := u.Query()
v.Set("a", 1)
v.Set("b", 2)
u.RawQuery = v.Encode()

So instead we could have a helper function

u.SetQueryParameters(url.Values{
	"a": {"1"},
	"b": {"2"},
})

to hopefully make life a tiny bit easier for me and other developers.

[HaraldNordgren]

Possible implementation: #24528

[mvdan]

It seems to me like your second piece of code could be:

u.RawQuery = url.Values{
	"a": {"1"},
	"b": {"2"},
}.Encode()

What are you trying to solve here? If it's less code, I think both ways to do it currently are fine. If it's about the code being confusing, then perhaps better documentation or an example could help. Or perhaps even a proposal to improve a future version of net/url, such as replacing RawQuery with something else.

[HaraldNordgren]

@mvdan I think the problem with using RawQuery and Encode() is the amount of voodoo involved.

It feels strange to me that the normal way to construct a URL with query parameters is to assign anything called Raw…. I would expect the internals to handle the "raw" stuff.

So I was trying to take something that confused and bewildered me about Go when I started writing it professionally a year ago (and still looks odd to me), and then simplify it for the next guy.

[ianlancetaylor]

CC @bradfitz @tombergan

[bradfitz]

There are no existing Setter functions on net/url.(*URL), so I'm somewhat reluctant to start.

But if we added one, I'd name it SetQuery, to match the naming of https://golang.org/pkg/net/url/#URL.Query

And the implementation could be simpler. I replied on https://go-review.googlesource.com/c/go/+/102476

/cc @bcmills

[bcmills]

I don't have a strong opinion on whether we should add a SetQuery method, but there is a strange asymmetry between RawQuery and RawPath. If one assigns to RawPath without Path, it appears that the EscapedPath method will ignore it. That seems to support @HaraldNordgren's unease at setting the RawQuery field, even though it is safe to do.

That suggests a slightly broader change to bring Query and Path closer to parallel: two setters instead of one.

func (u *URL) SetEscapedPath(rawPath string) error {
	path, err := PathUnescape(path)
	if err != nil {
		return err
	}
	u.RawPath = rawPath
	u.Path = path
	return nil
}

func (u *URL) SetQuery(Values) {
	[…]
}

[HaraldNordgren]

@bcmills Should there also be a path parameter to the SetEscapedPath function? I'm not exactly sure I follow the idea, but it seems like the underlying problem remains if you have to pass anything "raw" to the function to make it work.

[bcmills]

@HaraldNordgren
There is no need for a path parameter, because there is only one valid Path for a given RawPath. (The relationship is 1∶N, not M∶N.)

Since the Path field overrides RawPath, if the user has an already-unescaped path they can simply set the Path field — no need for a method call.

[rsc]

Please, let's not start designing new API for url.URL. The URL struct is not set up for incremental modification. It's meant for reading URLs, not to be a kitchen sink for writing them. There's a reason that APIs that consume URLs typically take type string.

[bradfitz]

Yeah, I'd prefer we keep URL as it is, without any setters.

We can rethink the URL type for Go 2, perhaps.