Repository: go
Issue: spec: disallow anonymous interface cycles
Labels: Documentation,Proposal,Proposal-Accepted

TL;DR: We should disallow declarations like `type I interface { m() interface { I } }`. They cause a lot of trouble for tools, and no one uses them in practice.

# Background

The Go spec allows interface types to [embed](https://go.dev/ref/spec#Embedded_interfaces) other declared interfaces. For example, package io declares [ReadCloser](https://pkg.go.dev/io#ReadCloser) using the type literal `interface { Reader; Closer }`. This type literal is [identical](https://go.dev/ref/spec#Type_identity) to `interface { Read([]byte) (int, error); Close() error }`, but also to `interface { Reader; Close() error }` and `interface { Read([]byte) (int, error); Closer }`.

Type identity is a core concept of the Go language, and it's useful for tools (e.g., type checkers, compilers, static analysis passes) to have a canonical representation that types can be referred to with (e.g., to ensure runtime type descriptors representing identical types in separate compilation units are deduplicated by the linker). For interface types, the natural canonical representation is the fully expanded form without any embedded interfaces.

However, this causes problems for some self-referential interface types. The simplest example is:

```go
type I interface { m() interface { I } }
```

Tools would like to expand the `interface { I }` to a canonical, embedding-free representation. However, expanding it produces `interface { m() interface { I } }`, which again contains `interface { I }` and requires expansion. Handled naively, this process never terminates.

The (accepted) type aliases proposal [stated](https://go.googlesource.com/proposal/+/master/design/18130-type-alias.md#type-cycles): "In contrast, aliases must be possible to “expand out”, and there is no way to expand out an alias like `type T = *T`."

The issue there was the same: given `type T = U`, we want to replace (expand) all occurrences of `T` with `U` to find a canonical type description. But if `U` is `*T`, then this process never terminates either.

**Proposal:** I propose the same principle should apply to interface embedding: it should be possible to finitely expand all embedded interfaces, and we should disallow declarations like `type I interface { m() interface { I } }`.

# Unused

Anonymous, cyclic interfaces appear unused in practice. I ran an analysis of every unique module path indexed by index.golang.org, and I found only 2 occurrences of cyclic interfaces:

* [https://github.com/gozelus/zelus_rest/blob/master/core/db/db.go#L20](https://github.com/gozelus/zelus_rest/blob/master/core/db/db.go#L20)
* [https://github.com/vetcher/go-astra/blob/master/test/assets/interfaces/source.go#L52](https://github.com/vetcher/go-astra/blob/master/test/assets/interfaces/source.go#L52)

However, neither of these packages appear to be imported anywhere, even within their own modules.

The first use case could be addressed by introducing a second named interface type like `type MySQLTx interface { MySQLDb; Rollback(); Commit() }`. (This would also allow [https://github.com/gozelus/zelus_rest/blob/master/core/db/db.go#L48](https://github.com/gozelus/zelus_rest/blob/master/core/db/db.go#L48) to be changed to `func (d *dbImp) Begin() MySQLTx { ... }`, instead of requiring the interface type literal to be repeated.)

The second appears like it's meant to be a testdata file instead.

# Broken

In the past, anonymous, cyclic interfaces have been a recurring issue that we've struggled to support: #10222, #16369, #25262, #29312.

Notably, packages whose package export data contained an anonymous interface cycle couldn't be imported prior to Go 1.7, because the old textual export data format couldn't handle them. (See [https://github.com/golang/go/issues/16369#issuecomment-232746915](https://github.com/golang/go/issues/16369#issuecomment-232746915).)

x/tools/go/ssa.Hasher has been known to mishandle them since 2018 (#26863), yet no end users have clamored for it to be fixed. This feature underpins many x/tools features like callgraph construction, points-to analysis, SSA building, gopls completion suggestions, and staticcheck. Moreover, [CL 439117](https://go-review.googlesource.com/c/tools/+/439117) was started to fix this issue, but has stalled as yet more failure cases are identified. The options at the moment are either: (1) continue to ignore the issue, (2) simplify interface hashing (at the risk of introducing collisions in realistic Go code), or (3) implement a considerably more complex algorithm.

Finally, while waiting for my module analysis code to execute, I manually discovered many more implementation issues with anonymous, cyclic interfaces: #56045, #56046, #56055, #56056, #56057, #56059, #56061, #56062, #56063, #56065.

# Why not tie to go.mod `go` version?

In #3939, it's been proposed to remove `int`->`string` conversions, because the semantics are surprising to new users. One recurring suggestion has been to tie this to the `go` line in go.mod files: old modules would continue to compile successfully, whereas new modules would get an error instead to protect users from misuse.

Could we do the same for anonymous interface cycles?

I argue no: the issue with anonymous interface cycles isn't that users accidentally use them (like `int`->`string` conversions), but that tools authors have to deal with them at all. As long as anonymous interface cycles are allowed anywhere, all tools authors have to deal with them (e.g., [CL 439117](https://go.dev/cl/439117) above). Users are better served by letting tools authors focus on features that users actually care about and use.