changing-the-public-api.md

Changing the Public API

When I started looking for ways to help on the "github.com/alecthomas/gozmq" package a few months ago, a recurring topic in discussions was that an earlier decision to provide a hidden struct type through a public interface type had become restrictive. We couldn't just drop the interface and make the struct public because that would be backwards-incompatible. Or could we?

No

This had to be the first considered option, and was the standing decision at the time. If we could continue providing what the package aims to provide without breaking backwards compatibility, then by all means, we should not break it.

Unfortunately this meant we couldn't provide all that we aimed to provide. Or could we?

Yes, if we change the import path too

The Way of Go is to fork the repo to a new import path for backwards-incompatible changes. This preserves the original import path for all dependent packages and applications whose maintainers do not immediately update their code. However, two import paths means two projects to maintain, and we are not many maintainers with not much time to devote. While we didn't seriously consider this option, one contributor left to start a new project with similar goals and a different design. We had to do something. Could we?

Yes, if we provide a code fixing tool too

One thing I've been impressed with in Go is the go fix command, which can smartly update old Go code when the language or standard libraries have changed. What if we could provide something similar for gozmq? The source for go fix, under src/cmd/fix in the Go source tree, is governed by a BSD-style license and, while I'm no lawyer, I know this means it can be freely copied, modified, and redistributed.

The code is already well-organized (no surprise there) so it isn't hard to pick out the parts I need:

• src/cmd/fix/fix.go
• src/cmd/fix/main.go
• src/cmd/fix/main_test.go
• LICENSE

For gozmq, I added these files to make a new command, gozmqfix. It took some real work to figure out the next part: I had to dig through some of the rewrites in go fix and dive into "go/ast" and "go/token". You can see the result in gozmqfix/zmqstruct.go.

The heart of this is transforming an AST node once it's been found. The following snippet creates a new type expression that is a pointer to the type identified by s.Sel.Name (a string) in the package identified by zmq (also a string).

&ast.StarExpr{
	X: &ast.SelectorExpr{
		X: ast.NewIdent(zmq),
		Sel: ast.NewIdent(s.Sel.Name),
	},
}

One of the subtleties is that if a package is imported with an alias, the alias must be used in the new type expression. Conveniently, the files copied from Go's src/cmd/fix provide a convenience function importSpec that finds this alias.

spec := importSpec(f, "github.com/alecthomas/gozmq")
if spec == nil {
	return false
}
zmq := spec.Name.Name

What really tripped me was getting all the cases. To do this right, I had to search the "go/ast" documentation for all AST node types that have Type fields. This turned out to be quite a few:

switch node := n.(type) {
	case *ast.ArrayType:
		t := zmqstructtype(zmq, node.Elt)
		if t != nil {
			node.Elt = t
			fixed = true
		}
	case *ast.CompositeLit:
		// This is irrelevant only because the original type is an
		// interface i.e. cannot be the type of a composite literal.
	case *ast.Ellipsis:
		t := zmqstructtype(zmq, node.Elt)
		if t != nil {
			node.Elt = t
			fixed = true
		}
	case *ast.Field:
		t := zmqstructtype(zmq, node.Type)
		if t != nil {
			node.Type = t
			fixed = true
		}
	case *ast.MapType:
		t := zmqstructtype(zmq, node.Key)
		if t != nil {
			node.Key = t
			fixed = true
		}
		t = zmqstructtype(zmq, node.Value)
		if t != nil {
			node.Value = t
			fixed = true
		}
	case *ast.Object:
		// Does something need to be done here with node.Type?
		// What does it take to trigger this case?
	case *ast.TypeAssertExpr:
		t := zmqstructtype(zmq, node.Type)
		if t != nil {
			node.Type = t
			fixed = true
		}
	case *ast.TypeSpec:
		t := zmqstructtype(zmq, node.Type)
		if t != nil {
			node.Type = t
			fixed = true
		}
	case *ast.ValueSpec:
		t := zmqstructtype(zmq, node.Type)
		if t != nil {
			node.Type = t
			fixed = true
		}
}

The test harness copied over in main_test.go was very handy and I was able to verify each case as it was added.

Since there was already a good collection of sample code built on gozmq in imatix/zguide/examples/Go, I was able to find the cases I'd missed the first time around and satisfy myself that the command finally worked as expected. As a result, we decided to drop interfaces and publish structs in "github.com/alecthomas/gozmq" and other pent-up improvements have been released.

A note about upgrading was included in the package's README, changes made by gozmqfix in imatix/zguide were merged, no issues have been filed about the change, and I am pretty darned pleased.

Doubts

Should we have forked to a new import path instead? Should we have tried harder to work within the constraints of backwards-compatibility? Might a proliferation of "fix"-enabled updates damage stability of the Go package ecosystem?

In retrospect, I believe that providing a "fix" command, while it has worked out well enough, was not the best thing we could have done. I strongly suspect that we become much better coders by taking responsibility for past design decisions, and in code that includes maintaining backwards-compatibility.

Comment here or in Go+.