(Im)mutability in the toolkit APIs

[mickael-menu]

I'm starting this discussion in response to a PR fixing a data race in the toolkit, as we need to agree on a shared vision for the Go toolkit APIs.

I'm a proponent of immutability when possible, as it prevents some of the most challenging bugs: data races and race conditions. In my experience, it also makes the code more understandable as the data cannot change while we use it.

Both Swift and Kotlin toolkits have immutable models. We haven't come across a use case where mutability was truly necessary, but let me know if your mileage varies.

I'll end with a quote from Russ Cox (main maintainer of Go):

The race detector is one of Go’s most loved features. But not having races would be even better. I would love it if there were some reasonable way to integrate reference immutability into Go, so that programmers can make clear, checked assertions about what can and cannot be written and thereby eliminate certain races at compile time. Go already has one immutable type, string; it would be nice to retroactively define that string is a named type (or type alias) for immutable []byte. I don’t think that will happen this year, but I’d like to understand the solution space better. Javari, Midori, Pony, and Rust have all staked out interesting points in the solution space, and there are plenty of research papers beyond those.

In the long-term, if we could statically eliminate the possibility of races, that would eliminate the need for most of the memory model. That may well be an impossible dream, but again I’d like to understand the solution space better.

How to keep the toolkit immutable?

Prefer copy-by-value

I'm not advocating for hiding all the fields of a struct and exposing only read-only functions. Fortunately, Go uses a copy-by-value strategy with structs by default.

So this fine, as a caller can change the property of a var link: Link locally, but this won't cause data races:

type Link struct {
	Href      string
	Templated bool
}

However, we should avoid returning or referencing pointers.

type Contributor struct {
	Position *float64 // Here be dragons!
}

The case for nullable values

The previous pattern is often used to represent a value that can be null.

Ideally we would use an Optional<float64> type, which might be feasible now that Go supports generics. But I don't think it's very idiomatic in the Go world?

So I think it's fine to compromise and use Position *float64 to represent an optional value, if we explicitly states that integrators should not mutate the value itself.

That means that with var contr: Contributor, this is valid:

contr.Position = &(float64){3}
// or
contr.Position = nil

But this should not be officially supported by the toolkit, as it mutates a shared reference:

*contr.Position = 3

Note that this is already the de facto behavior in the toolkit, except that we didn't explicitly said that we're not thread-safe.

Slices and maps

Unfortunately, slices and maps are copied by reference by default. So this is not safe, as we can modify the map concurrently:

type Link struct {
	Properties map[string]interface{}
}

Here are some alternatives, by order of preference:

1. Using immutable map / list types. There are a couple of third-party solutions, or we could roll out our own.
2. Hide the map field and expose custom accessors (e.g. Link.GetProperty("authenticate")).
3. Like with nullable values, do nothing and simply state that the toolkit is not thread-safe and that maps and lists should not be mutated.