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: spec: interface literals #25860

Open
smasher164 opened this issue Jun 13, 2018 · 74 comments
Open

proposal: spec: interface literals #25860

smasher164 opened this issue Jun 13, 2018 · 74 comments
Labels
LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal
Milestone

Comments

@smasher164
Copy link
Member

Filing this for completeness sake, since it was mentioned in #21670 that this proposal had been discussed privately prior to Go 1. Just like literals allow the construction of slice, struct, and map values, I propose "interface literals" which specifically construct values that satisfy an interface. The syntax would mirror that of struct literals, where field names would correspond to method names. The original idea is proposed by @Sajmani in #21670 (comment).

Conceptually, this proposal would transform the following initializer

f := func(p int) { /* implementation */ }
x := interface{
	Name(p int)
}{f}

to the following type declaration and struct initializer

type _impl_Name_int struct {
	f func(p int)
}
func(v _impl_Name_int) Name(p int) { v.f(p) }
// … in some other scope
f := func(p int) { /* implementation */ }
var x interface{
	Name(p int)
} = _impl_Name_int{f}

As an extension to what’s mentioned in #21670, I propose that fields be addressable by both method names and field names, like in this example:

type ReadWriteSeekCloser interface {
	ReadWriteSeeker
	Closer
}

f := os.Open("file")
calls := 0
return ReadWriteSeekCloser{
	ReadWriteSeeker: f,
	Close: func() error {
		if calls < 1 {
			return f.Close()
		}
		return nil
	},
}

The default value for a method is nil. Calling a nil method will cause a panic. As a corollary, the interface can be made smaller to be any subset of the original declaration. The value can no longer be converted back to satisfy the original interface. See the following modified example (from @neild in #21670 (comment)):

type io interface {
  Read(p []byte) (n int, err error)
  ReadAt(p []byte, off int64) (n int, err error)
  WriteTo(w io.Writer) (n int64, err error)
}
// 3 method -> 2^3 = 8 subsets
func fn() io.Reader {
	return io{
		Read: strings.NewReader(“”),
	}
}

The nil values for ReadAt and WriteTo make it so the “downcasted” io.Reader can no longer be recast to an io. This provides a clean way to promote known methods, with the side effect that the struct transformation described above won't be a valid implementation of this proposal, since casting does not work this way when calling a nil function pointer through a struct.

Although this proposal brings parity between struct and interface initialization and provides easy promotion of known methods, I don’t think this feature would dramatically improve the way Go programs are written.

We may see more usage of closures like in this sorting example (now obviated because of sort.Slice):

arr := []int{1,2,3,4,5}
sort.Sort(sort.Interface{
	Len: func() int { return len(arr) },
	Swap: func(i, j int) {
		temp := arr[i]
		arr[i] = arr[j]
		arr[j] = temp
	},
	Less: func(i, j int) bool { return arr[i] < arr[j] },
})

Promotion of known methods also avoids a lot of boilerplate, although I’m not sure that it is a common enough use case to warrant a language feature.
For instance, if I wanted to wrap an io.Reader, but also let through implementations of io.ReaderAt, io.WriterTo, and io.Seeker, I would need seven different wrapper types, each of which embeds these types:

type wrapper1 struct {
	io.Reader
	io.ReaderAt
}
type wrapper2 struct {
	io.Reader
	io.WriterTo
}
type wrapper3 struct {
	io.Reader
	io.Seeker
}
type wrapper4 struct {
	io.Reader
	io.ReaderAt
	io.WriterTo
}
type wrapper5 struct {
	io.Reader
	io.ReaderAt
	io.Seeker
}
type wrapper6 struct {
	io.Reader
	io.WriterTo
	io.Seeker
}
type wrapper7 struct {
	io.Reader
	io.ReaderAt
	io.WriterTo
	io.Seeker
}

Here is the relevant change to the grammar (under the composite literal section of the spec):

LiteralType = StructType | InterfaceType | ArrayType | "[" "..." "]" ElementType | 
              SliceType | MapType | TypeName .
@gopherbot gopherbot added this to the Proposal milestone Jun 13, 2018
@oiooj oiooj added the v2 An incompatible library change label Jun 13, 2018
@gbbr gbbr changed the title Proposal: Go 2 -- Interface Literals Proposal: Go 2: Interface Literals Jun 13, 2018
@gbbr gbbr changed the title Proposal: Go 2: Interface Literals proposal: Go 2: Interface Literals Jun 13, 2018
@ianlancetaylor ianlancetaylor added the LanguageChange Suggested changes to the Go language label Jun 13, 2018
@themeeman
Copy link

How would this work in terms of reflection? What would be the result of calling reflect.ValueOf on one of these interface literals? This would create the case that isn't currently possible where an interface isn't actually "wrapping" some underlying value. There is also the question of how they would work with a switch t.(type) statement.

@smasher164
Copy link
Member Author

I assume you mean to ask that if
i is an interface literal as described above
iv := reflect.ValueOf(i)
u := iv.Interface()
uv := reflect.ValueOf(u)
What would be the kind of uv? Also what operations on uv are valid?

You are right in that there isn't an underlying value being wrapped. That being said, since type switches can already switch on interfaces, doing so on an interface literal would simply not satisfy cases that check for concrete types.

b := []byte("some randomly accessible string")
pos := 0
rs := io.ReadSeeker{
	Read: func(p []byte) (n int, err error) {
		/* implementation */
	}
	Seek: func(offset int64, whence int) (int64, error) {
		/* implementation */
	}
}
r := io.Reader(rs)
switch r.(type) {
case *bytes.Buffer:
	// does not satisfy
case io.ReadWriteCloser:
	// does not satisfy
case io.ReadSeeker:
	// does satisfy
}

As to the representation of a literal's reflected value, if the same reflect package is used for Go 2, the underlying value can be a MethodSet. This does not have to correspond to its runtime representation, but this is a simple abstraction for the reflect package.

A MethodSet is just an interface that references all methods in the underlying value. Operations on a MethodSet are nearly identical to operations on an Interface. From the above example, if uv.Kind() is a MethodSet, then uv.Interface() is no longer a valid operation.

ut := uv.Type() will return a type with all of the underlying methods. Similar to an interface type, ut.Method and ut.MethodByName will return Methods whose signatures do not have a receiver and whose Func fields are nil.

@ianlancetaylor ianlancetaylor added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Sep 25, 2018
@ianlancetaylor ianlancetaylor changed the title proposal: Go 2: Interface Literals proposal: Go 2: interface literals Sep 25, 2018
@smasher164
Copy link
Member Author

I think the primary problem with this proposal is that it allows nil methods, which panic when invoked.

  1. This violates a fundamental property of interfaces, where a type that implements an interface definitively implements a contract that its users can expect. Now a user of an interface cannot assume that a method is invocable, and the meaning of the interface lost.
  2. The runtime would have to guard against an invocation of a method. I could see this slowing down invocations to all interface methods.
  3. What is the difference, if any, between an interface that is nil, and an interface literal whose methods are all nil?

On the other hand, nil interfaces are extremely common, even though one could argue that they are also a violation of the contract described above, since users expect to be able to invoke its methods.

Additionally, I think allowing nil methods to prevent a downcasted interface from type-asserting into the original interface sounds nice since it allows the promotion of known methods. However, this behavior only exists because nil methods are allowed, and allows the runtime to convert an "unsafe" (non-invocable) interface to a "safe" (invocable) interface. This behavior implies that non-invocable interfaces shouldn't exist in the first place, and is too subtle and surprising.

The only alternative I can think of is to make a nil method provide some default no-op implementation of a method. Although this is safer than the previously mentioned, it seems just as subtle and surprising, but less powerful.

Ultimately, it appears impossible to require that no field in an interface literal is nil, since a value that is assigned to it could be nil at run-time. The only way to guarantee no field is nil would be to restrict each field to be a function literal or top-level function. However, this pretty much loses all of the power of interface literals, since it is only a marginal improvement (LoC-wise) over a type which is defined to implement an interface.

@carnott-snap
Copy link

Can this be added to #33892 for review?

@smasher164
Copy link
Member Author

/cc @golang/proposal-review

@ianlancetaylor
Copy link
Contributor

This was reviewed and marked as NeedsInvestigation. At some point we'll re-review these issues.

@beoran
Copy link

beoran commented Aug 1, 2021

I came here from #47487 which might be seen as competing with this issue.

Seeing that the interface literals proposed here have a serious problem with nil safety, as @smasher164 admits here: #25860 (comment) and considering that nil problems are a significant problem when developing software (look up "Null References: The Billion Dollar Mistake"), I strongly believe that any changes to the Go language should make it more nil safe and not less so.

Therefore I respectfully oppose this proposal, unless a way can be found to avoid the nil problem. #47487 on the other hand does not seem to have this nil problem, so I support that as an alternative.

@Merovius
Copy link
Contributor

Merovius commented Aug 1, 2021

@smasher164

This violates a fundamental property of interfaces, where a type that implements an interface definitively implements a contract that its users can expect. Now a user of an interface cannot assume that a method is invocable, and the meaning of the interface lost.

This is not necessarily true. For example:

package main

type T struct{}

func (*T) Foo() {}

func (T) Bar() {}

type FooBarer interface {
	Foo()
	Bar()
}

func main() {
	var x FooBarer = (*T)(nil)
	// Does not panic
	x.Foo()
	// Panics - a nil *T is dereferenced when calling a method with value receiver.
	x.Bar()
}

I included two methods here, to demonstrate that this isn't simply "calling a method on a nil-pointer might panic, if that method dereferences it". The panic here is caused on a language-level, very similar (if not equivalent) to what would happen if an interface-literal contains a nil method.

The runtime would have to guard against an invocation of a method. I could see this slowing down invocations to all interface methods.

This is also not a worry. The runtime does not do nil-checks in general - it just does a call of the function pointer which causes a hardware trap when it's nil, which is then translated into a SIGSEGV which is caught by the runtime. That is, panics on nil derefercning/calling are free at runtime, in the happy path.

What is the difference, if any, between an interface that is nil, and an interface literal whose methods are all nil?

I would argue it's similar, if not the same, as a pointer being stored in an interface, where all methods panic when called with a nil-pointer - it's a non-nil interface, which panics if called. This is, as shown above, already possible and well-defined without any real isues.

@smasher164
Copy link
Member Author

@Merovius Good point about the dereferencing when the method has a value receiver.

@zephyrtronium
Copy link
Contributor

Under this proposal, given an interface literal with a nil method, is the result of the method expression selecting that method nil or non-nil? E.g., is the output of the following program true or false?

package main

func main() {
	println(interface{ M() }{}.M == nil)
}

If the output of this program is true, it becomes possible to program around omitted methods. Contrarily, if it's possible, then people will do it, and I worry the end result would be complaints that interfaces cause boilerplate.

@smasher164
Copy link
Member Author

If nil methods are allowed, the output of the above program would be true. I would not want people to have to program around omitted methods. A nil method should only exist to aid narrowing conversions to a different interface type. Panicking when invoking a nil method should be similar to the invoked method panicking in its body.

@DeedleFake
Copy link

@beoran

#47487 can actually be used to replicate the functionality of this proposal in some circumstances, though it's a lot clunkier:

v := []int{3, 2, 5}

type Lesser interface{ Less(i1, i2 int) bool }
type Swapper interface{ Swap(i1, i2 int) }
type Lenner interface{ Len() int }
sort.Sort(struct {
	Lesser
	Swapper
	Lenner
}{
	Lesser:  Lesser(func(i1, i2 int) bool { return v[i1] < v[i2] }),
	Swapper: Swapper(func(i1, i2 int) { v[i1], v[i2] = v[i2], v[i1] }),
	Lenner:  Lenner(func() int { return len(v) }),
})

This pattern also works currently, but it requires top-level types to do the single-method-interface-implemented-by-a-function wrapper manually because there's currently no way to declare methods on a type inside another function.

@DeedleFake
Copy link

DeedleFake commented Aug 2, 2021

Is there any particular reason that this proposal can't be modified to require exhaustive implementation? I see a lot of arguments for and against it being accepted with nil methods, but what if methods could just simply never be nil? With a struct, adding more fields is a non-breaking change provided that any struct literals of that type use field names as the new ones will just be zero values, but adding a method to an interface is already a breaking change under almost all circumstances.

What's the benefit of allowing methods to be nil in this proposal at all instead of just requiring that all of them be implemented in any interface literal? The original proposal mentions something about downcasting them to smaller interfaces and being unable to revert that due to the lack of the method implementation, but why not just implement the smaller interface in the first place, then?

@smasher164
Copy link
Member Author

@DeedleFake

Is there any particular reason that this proposal can't be modified to require exhaustive implementation?

Even if we wanted to disallow nil methods, it is unclear how we would enforce that statically. Take this program for example, where readerFunc's being nil is determined by some runtime condition.

var readerFunc func([]byte) (int, error)
if runtimeCondition {
    readerFunc = func(p []byte) (int, error) { /* implementation */ }
}
reader := io.Reader{
    Read: readerFunc,
}

We could conservatively restrict methods in interface literals to either be top-level functions or function literals, but these rules are arbitrary and hard to teach. Additionally, a lot of the power of interface literals comes from the ability to change the implementation of some method by changing the variable binding for some closure. If you're going to define all of the functions up-front anyways, why not just make them methods on some basic type definition?

why not just implement the smaller interface in the first place, then?

The reason is to allow the promotion of known methods. Consider the example that @neild provides here: #21670 (comment). Basically, you know that some library wants to type-assert on your value for specific functionality (like fs.FS for example). You want to wrap that interface with your own functionality, but still allow some methods implemented by its underlying type to surface up.

Normally, you would have to type-assert for all combinations of methods you care about, and return a concrete type that implements that combination of methods. So if you wanted to wrap an fs.FS to do some additional work on its Open operation, but you still wanted StatFS and ReadDirFS to surface through, you would end up creating four concrete types.

The question is if this is even a problem worth solving for interface literals. However, even if it wasn't, nil methods would still exist, just for the ability to dynamically set an interface's method.

@Merovius
Copy link
Contributor

Merovius commented Aug 2, 2021

@DeedleFake

I see a lot of arguments for and against it being accepted with nil methods, but what if methods could just simply never be nil?

The same reasons I mention here.

@DeedleFake
Copy link

DeedleFake commented Aug 2, 2021

Ah, duh. I was mostly confused by the defaulting of unspecified methods to nil and somehow completely overlooked manually setting them to nil. I was thinking of Java's subclass literals which use a regular method definition syntax, rather than a field setting syntax. Something like

sort.Sort(sort.Interface{
  Less(i1, i2 int) bool { ... }
  Swap(i1, i2 int) { ... }
  Len() int { ... }
})

Maybe inverting the problem and making it anonymous type literals instead of interface literals would make more sense. I remember the reflect.MakeInterface() proposal running into a similar problem as they slowly realized that they weren't actually creating an interface, but were rather creating a new anonymous type with the right methods to satisfy an interface. Maybe it would help to approach this from the point of view creating an anonymous, data-less type with inline, closured methods, and then it can just satisfy whatever interfaces, the same as any other concrete type.

Edit: To clarify, I'm saying ditch the interface name completely and just define an inline method set, then let the type checker do the work:

// sort.Sort() already defines its argument as being sort.Interface,
// so the regular type checker can easily sort this out.
sort.Sort(type {
  Less(i1, i2 int) bool { ... }
  Swap(i1, i2 int) { ... }
  Len() int { ... }
})

@zigo101
Copy link

zigo101 commented Mar 20, 2024

@adonovan honestly, I don't very get your last reply.

Maybe I over-worried about it. Is it possible that the implementation struct type is a distinct zero-size struct (with an incomparable zero-size field) and the functions list is maintained outside of the struct values?

@ChrisHines
Copy link
Contributor

@griesemer I like your construction, but why it is important for "each interface literal has its own user-inaccessible type"? What does that add over having each interface type used in a literal have its own type?

I ask because when hand writing mocks/fakes that use this pattern today I usually only make one type per interface I am mocking/faking. Obviously that saves me effort that the compiler doesn't care about as much, but does making each literal generate a new type have a specific advantage?

@Merovius
Copy link
Contributor

@griesemer I would weakly argue in favor of requiring every method to be provided - if nothing else, then on the grounds that it is the more conservative choice and we can always relax it later, if we want to, but not the other way around.

Also, personally I would be in favor to have interface-literals just always be non-comparable.

@Merovius
Copy link
Contributor

@go101 There is no need to have func-valued fields.

func F() (*string, error) {
    x := new(string)
    return x, error{func() string { return *x }}
}

(for example) could be de-sugared into

type _tmp struct {
    p *string
}

func (v *_tmp) Error() string {
    return *v.p
}

func F() (*string, error) {
    x := new(string)
    return x, &_tmp{x}
}

But also, I don't think this is a super important argument. If it can be implemented efficiently, great. If it can't be implemented efficiently, then there isn't a way for the user to do so either (it's not like the user can write code the compiler couldn't generate), so that wouldn't be counted against this proposal. It's a red herring.

@griesemer
Copy link
Contributor

@ChrisHines This may not be a sufficiently convincing answer, but in the model translation, if there's exactly one struct type per interface type, comparison of interface literal values behaves differently. For instance, given an interface I:

  1. I{} == I{} is false with the proposed model where each literal has its own dynamic type (playground)
  2. I{} == I{} panics if the literals share the same dynamic type (playground) because now the literal values have the same type and can be compared, but the struct fields are incomparable.

Yet if I is actually any, then any{} == any{} is true (playground).

It seems a bit odd that comparing interface literals is ok if they are different interface types and the result is always false, but panics if they are the same interface type, unless the type is any in which case the result is always true.

If each interface literal has its own dynamic type, comparing interface literals is always ok and the result is always false. This seems easier to understand. It may also be simpler to implement.

@zigo101
Copy link

zigo101 commented Mar 20, 2024

@griesemer

Is it a good idea to make I{} == I{} always panic by combining @adonovan and @ChrisHines's idea?

@Merovius
Copy link
Contributor

The comparison only panics if both have the same type. It is allowed to compare uncomparable values of different types.

Requiring them to always panic comes with one theoretical complexity, which is that two interface literals from different packages would have to have the same type. I kind of get why that might be harder.

@griesemer
Copy link
Contributor

@Merovius I just note that with other literals we are not required to provide all elements. So the proposal matches existing "behavior". I don't have a stronger argument than that, and this may not be strong enough. And yes, we wouldn't be able to tighten the rules after the fact, while with your suggestion we would.

With respect to comparisons, if we panic when comparing interface literals, it seems that this leads to odd behavior in some cases. For instance, I{} == I{} where I is not the empty interface would panic. But any{} == any{} wouldn't panic with the proposed implementation (and probably would require some additional machinery to make it panic). Also, if we compare interface literals against other interface values (not created from literals) we would not panic (playground and playground).

It seems simpler to always have a new inaccessible dynamic type for each interface literal. Then they can always be compared and the result is always false.

@zephyrtronium
Copy link
Contributor

Without the different type per literal rule, there is also a question of identity: is fmt.Stringer{f} == interface { String() string }{f}? What about interface { String() string }{c.F} from two different packages?

I think a similar question applies given x := any{} => x == x, though. When we say "for each literal I{...}" there is a new type, is that lexical or per evaluation? I.e., given func g() any { return any{} }, is g() == g() allowed to be true?

@griesemer
Copy link
Contributor

@zephyrtronium Indeed. Consider multiple interface literals created in a loop (playground). So even with different types per literal, this probably has to be lexical. Which leaves some odd behavior after all, either way.

@zigo101
Copy link

zigo101 commented Mar 20, 2024

@griesemer

... and probably would require some additional machinery to make it panic

Yes, @adonovan provided a machinery: #25860 (comment)

Also, if we compare interface literals against other interface values (not created from literals) we would not panic

But what the problem is it? It looks normal to me.


BTW, I have no strong opinions on the implementation details. I mean, any design is okay.


@zephyrtronium

is fmt.Stringer{f} == interface { String() string }{f}?

I prefer true. But I have not a strong opinion on this. I mean false is also okay, and panicking is okay, too. ;)

What about interface { String() string }{c.F} from two different packages?

In my opinion, all of the dummy struct types should be declared in the universal space.

@zephyrtronium
Copy link
Contributor

I'm also a bit unclear on what "with a type name D that is inaccessible to the user" means. I assume "inaccessible" here means "unable to be referred to," along the lines of an unexported name in an imported package. But what is the result of reflect.TypeOf(any{}).String()?

@Merovius
Copy link
Contributor

Merovius commented Mar 20, 2024

Also, does the "different type per interface-literal" imply that every interface-literal has to allocate a new rtype, so that reflect.Type(x) != reflect.Type(y)? And if so, would that be potentially be a memory leak? I remember that there was a well-known issue that types created with reflect could not be garbage collected.

@neild
Copy link
Contributor

neild commented Mar 20, 2024

I fairly strongly think interface literals should be required to provide implementations of all methods.

It is true that with other literals we are not required to provide all elements. However, we do require implementations of an interface to implement all methods of the interface. We can argue consistency from either direction here.

Leaving consistency aside, a compile time error explaining that a method isn't implemented is substantially safer and more comprehensible than a run-time panic. A panic can occur at a point far distant from where the interface value was created. A compile error will immediately point at the source of the problem.

I also don't see much practical value to permitting implicit partial implementation of an interface. There may be some cases in tests where partial implementation is useful, but this doesn't seem common enough or useful enough to justify the reduction in comprehensibility and safety. If you need to partially implement an interface, you can do so explicitly: io.ReadCloser{Read: readFn, Close: nil}.

@doggedOwl
Copy link

doggedOwl commented Mar 20, 2024

@neild Partial implementations in tests is very common in my experience and literals would be especially helpful in that case where the ratio of types/method to implement is very close to 1. if it's always 1 yes we can use the #47487 but very often in tests I need to implement 2/3 methods of some extensive interfaces.
While it's true that large interfaces are not the best way to write go, sometimes it's neccessary.

@adonovan
Copy link
Member

adonovan commented Mar 20, 2024

We should not require that a given interface type has a single (unnameable) struct type for its closure, as this precludes optimizations that eliminate the double indirections both in control (method calls) and data (accesses to free variables). I like @griesemer's desugaring as an example of a legal (if suboptimial) implementation.

In other words, the identity of reflect.TypeOf(I{...}) should be implementation-defined.

@griesemer
Copy link
Contributor

@Merovius By "different type per interface-literal" I meant a different type per lexical interface literal - this is bounded by the size of the source and statically know.

@Merovius
Copy link
Contributor

@griesemer I think I'm confused, then by

If each interface literal has its own dynamic type, comparing interface literals is always ok and the result is always false. This seems easier to understand. It may also be simpler to implement.

ISTM that if we are talking "one type per lexical interface literal", with the model translation, this code would panic:

type J interface{ M() }
func F() J {
    return J{func() {}}
}
func main() {
    x, y := F(), F()
    x == y // panics: interface value have same, non-comparable type
}

@Merovius
Copy link
Contributor

I think I'd be most inclined to support just specifying "interface literals are always comparable and compare as false", i.e. have them behave like float NaNs. That doesn't cleanly map to the model-translation, but ISTM it gives the easiest to predict behavior without having to fix ourselves to implementation details.

@griesemer
Copy link
Contributor

The source-to-source transformation I proposed earlier can actually be adjusted easily to address some of the shortcomings that have been found so far: instead of using a dummy struct, we use a pointer to a dummy struct (playground).

Using a pointer in the interface variable (which an implementation would do anyway via boxing) addresses the comparison problems: two interface literals are equal only if they are in fact the same literal. Comparing an interface literal with any other interface will return false.

This leaves still open the following questions:

  1. Should an interface literal be required to provide functions for all methods?
  2. What is the dynamic type name of an interface literal as seen by reflection?

For 1) we just have to make a decision. The conservative approach is to require that all methods are present. This would be different from precedent for other composite literals.

For 2) I think the type name should be some artificial (compiler-created) name that makes it clear that these are special types. It may include the address of the actual type. The actual type may be marked such that it cannot be used via reflection (maybe?).

@griesemer
Copy link
Contributor

@Merovius I think my previous comment addresses most of your concerns.

@adonovan
Copy link
Member

adonovan commented Mar 22, 2024

https://go.dev/cl/573795 defines an analyzer to find types that are candidates for interface literals.

Here are 25 random results from the module mirror corpus.

https://go-mod-viewer.appspot.com/github.com/dop251/goja@v0.0.0-20240220182346-e401ed450204/func_test.go#L214: goja.testAsyncContextTracker is a one-off goja.AsyncContextTracker
https://go-mod-viewer.appspot.com/github.com/rancher/types@v0.0.0-20220328215343-4370ff10ecd5/apis/management.cattle.io/v3/zz_generated_cluster_monitor_graph_controller.go#L144: v3.clusterMonitorGraphLister is a one-off v3.ClusterMonitorGraphLister
https://go-mod-viewer.appspot.com/github.com/99designs/gqlgen@v0.17.45/codegen/testserver/singlefile/stub.go#L156: singlefile.stubBackedByInterface is a one-off singlefile.BackedByInterfaceResolver
https://go-mod-viewer.appspot.com/github.com/grpc-ecosystem/grpc-gateway/v2@v2.19.1/internal/httprule/parse.go#L204: httprule.variable is a one-off httprule.segment
https://go-mod-viewer.appspot.com/google.golang.org/grpc@v1.62.1/test/end2end_test.go#L752: test.nopDecompressor is a one-off grpc.Decompressor
https://go-mod-viewer.appspot.com/github.com/onsi/gomega@v1.32.0/gleak/ignoring_in_backtrace.go#L14: gleak.ignoringInBacktraceMatcher is a one-off types.GomegaMatcher
https://go-mod-viewer.appspot.com/github.com/dop251/goja@v0.0.0-20240220182346-e401ed450204/vm.go#L4346: goja._op_instanceof is a one-off goja.instruction
https://go-mod-viewer.appspot.com/google.golang.org/grpc@v1.62.1/test/end2end_test.go#L6277: test.bpbConfig is a one-off serviceconfig.LoadBalancingConfig
https://go-mod-viewer.appspot.com/github.com/dop251/goja@v0.0.0-20240220182346-e401ed450204/compiler_expr.go#L896: goja.throwConst is a one-off goja.instruction
https://go-mod-viewer.appspot.com/github.com/golang/dep@v0.5.4/gps/version.go#L678: gps.pvupgradeVersionSorter is a one-off sort.Interface
https://go-mod-viewer.appspot.com/github.com/dop251/goja@v0.0.0-20240220182346-e401ed450204/vm.go#L4688: goja._enumGet is a one-off goja.instruction
https://go-mod-viewer.appspot.com/google.golang.org/grpc@v1.62.1/benchmark/primitives/primitives_test.go#L450: primitives_test.alwaysNop is a one-off primitives_test.ifNop
https://go-mod-viewer.appspot.com/github.com/jackc/pgx/v5@v5.5.5/pgtype/ltree.go#L30: pgtype.encodeLtreeCodecBinaryByteSlice is a one-off pgtype.EncodePlan
https://go-mod-viewer.appspot.com/github.com/dop251/goja@v0.0.0-20240220182346-e401ed450204/compiler_stmt.go#L189: goja.enterFinally is a one-off goja.instruction
https://go-mod-viewer.appspot.com/github.com/go-kit/log@v0.2.1/nop_logger.go#L6: log.nopLogger is a one-off log.Logger
https://go-mod-viewer.appspot.com/github.com/pkg/sftp@v1.13.6/client_integration_test.go#L1352: sftp.lastChunkErrSequentialWriter is a one-off io.Writer
https://go-mod-viewer.appspot.com/github.com/jackc/pgx/v5@v5.5.5/pgtype/pgtype.go#L565: pgtype.underlyingTypeScanPlan is a one-off pgtype.WrappedScanPlanNextSetter
https://go-mod-viewer.appspot.com/github.com/hashicorp/go-plugin@v1.6.0/internal/plugin/grpc_stdio_grpc.pb.go#L46: plugin.gRPCStdioClient is a one-off plugin.GRPCStdioClient
https://go-mod-viewer.appspot.com/golang.org/x/oauth2@v0.18.0/google/externalaccount/executablecredsource.go#L167: externalaccount.runtimeEnvironment is a one-off externalaccount.environment
https://go-mod-viewer.appspot.com/github.com/rancher/types@v0.0.0-20220328215343-4370ff10ecd5/apis/management.cattle.io/v3/zz_generated_k8s_client.go#L739: v3.projectAlertClient is a one-off v3.ProjectAlertInterface
https://go-mod-viewer.appspot.com/github.com/grpc-ecosystem/grpc-gateway/v2@v2.19.1/examples/internal/proto/examplepb/response_body_service_grpc.pb.go#L36: examplepb.responseBodyServiceClient is a one-off examplepb.ResponseBodyServiceClient
https://go-mod-viewer.appspot.com/github.com/hyperledger/fabric-protos-go@v0.3.3/orderer/clusterserver.pb.go#L584: orderer.clusterNodeServiceStepServer is a one-off orderer.ClusterNodeService_StepServer
https://go-mod-viewer.appspot.com/golang.org/x/text@v0.14.0/encoding/unicode/utf32/utf32.go#L136: utf32.utf32Decoder is a one-off transform.Transformer
https://go-mod-viewer.appspot.com/github.com/rancher/types@v0.0.0-20220328215343-4370ff10ecd5/apis/management.cattle.io/v3/zz_generated_user_lifecycle_adapter.go#L57: v3.userLifecycleAdapter is a one-off lifecycle.ObjectLifecycle
https://go-mod-viewer.appspot.com/github.com/gogo/protobuf@v1.3.2/jsonpb/jsonpb_test.go#L983: jsonpb.funcResolver is a one-off jsonpb.AnyResolver

I'll update the note with the complete statistics when the job finishes.

Update: the job identified a whopping 100,277 one-off types in 6456 modules (from a total corpus of around 20K modules) That's an average of 16 in those modules, which seems almost implausibly high, though the ones I looked at by hand seemed plausible.

I was tempted to adjust the analyzer to reject candidates which have more methods than the interface type (such as a one-off io.Reader that also has a String method), but that would falsely reject one-off types with helper methods that might be more neatly expressed as local closures; also, one could easily locally define a broader interface such as ReaderStringer and use a literal of that type. Perhaps the analyzer should put a bound on the total number of lines of the one-off type's methods?

A file containing the first 10,000 is attached.

This file https://go-mod-viewer.appspot.com/github.com/greenpau/go-authcrunch@v1.0.50/pkg/acl/rule.go is the largest (apparently) non-generated example, with over 480 one-off types. Interface literals would save about 7000 lines of code in that file alone. There are dozens of files with over with 100 one-offs.

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/573795 mentions this issue: go/analysis/passes/oneoff: find "one-off" interface types

@adonovan
Copy link
Member

adonovan commented Mar 22, 2024

One possibility to permit partial implementations without compromising on the "fail open" principle would be to use an ellipsis to affirm that the interface demands more methods than are provided. A call to any of them would panic.

rw = io.ReadWriter{
   Read: readFromStdin,
   ...
}

Of course, it's a change to the syntax of composite literals, which would make everything much more costly.

@adonovan
Copy link
Member

I re-ran the analyzer across the corpus, this time modified to report only one-off types that satisfy an interface of no more than 3 methods, and whose methods together take no more than 20 lines of code.

Here are 100 findings selected at random from the first 30,000 to come out of the pipeline, which represents about 30% of the main module mirror corpus:

https://go-mod-viewer.appspot.com/github.com/linuxboot/fiano@v1.2.0/pkg/log/logger.go#L29: log.logWrapper is a one-off log.Logger (3 methods)
https://go-mod-viewer.appspot.com/github.com/openconfig/goyang@v1.4.5/pkg/yang/types_builtin.go#L680: yang.int64Slice is a one-off sort.Interface (3 methods)
https://go-mod-viewer.appspot.com/git.prognetwork.ru/x0r/utls@v1.3.3/tls_test.go#L301: tls.readerFunc is a one-off io.Reader (1 methods)
https://go-mod-viewer.appspot.com/github.com/jhump/protoreflect@v1.16.0/desc/sourceinfo/wrappers.go#L35: sourceinfo.imports is a one-off protoreflect.FileImports (3 methods)
https://go-mod-viewer.appspot.com/trpc.group/trpc-go/trpc-go@v1.0.2/plugin/setup_test.go#L90: plugin_test.mockTimeoutPlugin is a one-off plugin.Factory (2 methods)
https://go-mod-viewer.appspot.com/github.com/contiv/libOpenflow@v0.0.0-20210609050114-d967b14cc688/stream_test.go#L86: libOpenflow.parserIntf is a one-off util.Parser (1 methods)
https://go-mod-viewer.appspot.com/github.com/aacfactory/fns@v1.2.85/barriers/barrier.go#L42: barriers.barrier is a one-off barriers.Barrier (1 methods)
https://go-mod-viewer.appspot.com/go.uber.org/yarpc@v1.72.1/api/middleware/inbound.go#L47: middleware.nopUnaryInbound is a one-off middleware.UnaryInbound (1 methods)
https://go-mod-viewer.appspot.com/github.com/ZihuaZhang/fabric-protos-go@v1.0.7/peer/chaincode_shim.pb.go#L1273: peer.chaincodeClient is a one-off peer.ChaincodeClient (1 methods)
https://go-mod-viewer.appspot.com/github.com/bodgit/sevenzip@v1.5.1/internal/bzip2/reader.go#L38: bzip2.readCloser is a one-off io.ReadCloser (2 methods)
https://go-mod-viewer.appspot.com/github.com/tetratelabs/wazero@v1.7.1/internal/sysfs/adapter_test.go#L178: sysfs.zeroInoOsFile is a one-off fs.File (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/emr/pulumiTypes.go#L1490: emr.clusterCoreInstanceGroupPtrType is a one-off emr.ClusterCoreInstanceGroupPtrInput (3 methods)
https://go-mod-viewer.appspot.com/launchpad.net/gocheck@v0.0.0-20140225173054-000000000087/checkers.go#L167: gocheck.equalsChecker is a one-off gocheck.Checker (2 methods)
https://go-mod-viewer.appspot.com/github.com/hellofresh/janus@v0.0.0-20230925145208-ce8de8183c67/cassandra/wrapper/init.go#L79: wrapper.sessionInitializer is a one-off wrapper.Initializer (1 methods)
https://go-mod-viewer.appspot.com/github.com/readium/readium-lcp-server@v0.0.0-20240101192032-6e95190e99f1/sign/sign.go#L120: sign.rsaSigner is a one-off sign.Signer (1 methods)
https://go-mod-viewer.appspot.com/github.com/jenkins-x/jx-api@v0.0.24/pkg/client/listers/jenkins.io/v1/sourcerepositorygroup.go#L41: v1.sourceRepositoryGroupNamespaceLister is a one-off v1.SourceRepositoryGroupNamespaceLister (2 methods)
https://go-mod-viewer.appspot.com/github.com/microsoft/moc@v0.17.1/rpc/nodeagent/security/moc_nodeagent_authentication.pb.go#L176: security.authenticationAgentClient is a one-off security.AuthenticationAgentClient (1 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/quicksight/pulumiTypes.go#L8435: quicksight.dataSetRefreshPropertiesPtrType is a one-off quicksight.DataSetRefreshPropertiesPtrInput (3 methods)
https://go-mod-viewer.appspot.com/volcano.sh/apis@v1.8.2/pkg/client/listers/flow/v1alpha1/jobflow.go#L58: v1alpha1.jobFlowNamespaceLister is a one-off v1alpha1.JobFlowNamespaceLister (2 methods)
https://go-mod-viewer.appspot.com/github.com/admpub/mail@v0.0.0-20170408110349-d63147b0317b/smtp.go#L429: mail.loginAuth is a one-off smtp.Auth (2 methods)
https://go-mod-viewer.appspot.com/github.com/Carcraftz/utls@v0.0.0-20220413235215-6b7c52fd78b6/handshake_messages.go#L24: tls.marshalingFunction is a one-off cryptobyte.MarshalingValue (1 methods)
https://go-mod-viewer.appspot.com/github.com/bazelbuild/remote-apis-sdks@v0.0.0-20240425170053-8a36686a6350/go/pkg/outerr/outerr.go#L76: outerr.outWriter is a one-off io.Writer (1 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v3@v3.30.2/go/kubernetes/core/v1/pulumiTypes.go#L50270: v1.replicationControllerSpecPtrType is a one-off v1.ReplicationControllerSpecPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v2@v2.9.1/go/kubernetes/policy/v1beta1/pulumiTypes.go#L2611: v1beta1.runtimeClassStrategyOptionsPtrType is a one-off v1beta1.RuntimeClassStrategyOptionsPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/s3/pulumiTypes.go#L4336: s3.bucketLifecycleRuleNoncurrentVersionExpirationPtrType is a one-off s3.BucketLifecycleRuleNoncurrentVersionExpirationPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v4@v4.11.0/go/kubernetes/core/v1/pulumiTypes.go#L31854: v1.nodeDaemonEndpointsPtrType is a one-off v1.NodeDaemonEndpointsPtrInput (3 methods)
https://go-mod-viewer.appspot.com/k8c.io/api/v3@v3.0.0-20230904060738-b0a93889c0b6/pkg/generated/listers/kubermatic/v1/clustertemplateinstance.go#L30: v1.clusterTemplateInstanceLister is a one-off v1.ClusterTemplateInstanceLister (2 methods)
https://go-mod-viewer.appspot.com/github.com/bluenviron/mediacommon@v1.9.3/pkg/formats/mpegts/reader.go#L62: mpegts.recordReader is a one-off io.Reader (1 methods)
https://go-mod-viewer.appspot.com/github.com/saucesteals/fhttp@v0.0.0-20240117033920-7657833eb7a7/h2_bundle.go#L5082: http.http2writeGoAway is a one-off http.http2writeFramer (2 methods)
https://go-mod-viewer.appspot.com/github.com/nsqio/nsq@v1.3.0/internal/http_api/http_server.go#L27: http_api.logWriter is a one-off io.Writer (1 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/pipes/pulumiTypes.go#L3491: pipes.pipeSourceParametersSqsQueueParametersPtrType is a one-off pipes.PipeSourceParametersSqsQueueParametersPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/comprehend/pulumiTypes.go#L1797: comprehend.entityRecognizerVpcConfigPtrType is a one-off comprehend.EntityRecognizerVpcConfigPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/filecoin-project/specs-actors/v4@v4.0.2/actors/migration/nv10/top.go#L326: nv10.cachedMigrator is a one-off nv10.actorMigration (2 methods)
https://go-mod-viewer.appspot.com/github.com/micro/go-micro/v2@v2.9.1/util/log/log.go#L32: log.elog is a one-off log.Log (3 methods)
https://go-mod-viewer.appspot.com/github.com/hyperledger/aries-framework-go@v0.3.2/pkg/didcomm/packer/authcrypt/pack_test.go#L642: authcrypt.noopAEAD is a one-off tink.AEAD (2 methods)
https://go-mod-viewer.appspot.com/github.com/fastly/go-fastly/v6@v6.8.0/fastly/newrelic.go#L67: fastly.newrelicByName is a one-off sort.Interface (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/ecs/pulumiTypes.go#L4739: ecs.taskDefinitionVolumeDockerVolumeConfigurationPtrType is a one-off ecs.TaskDefinitionVolumeDockerVolumeConfigurationPtrInput (3 methods)
https://go-mod-viewer.appspot.com/go.opentelemetry.io/contrib/instrumentation/github.com/Shopify/sarama/otelsarama@v0.43.0/option_test.go#L47: otelsarama.fakeTracerProvider is a one-off trace.TracerProvider (1 methods)
https://go-mod-viewer.appspot.com/github.com/bazelbuild/rules_webtesting@v0.2.0/go/wtl/proxy/driverhub/debugger/debugger_test.go#L174: debugger.fakeReadCloser is a one-off io.ReadCloser (2 methods)
https://go-mod-viewer.appspot.com/github.com/bingoohuang/gg@v0.0.0-20240325092523-45da7dee9335/pkg/netx/proxy.go#L22: netx.socks5ProxyClient is a one-off proxy.Dialer (1 methods)
https://go-mod-viewer.appspot.com/github.com/m3db/m3@v1.5.0/src/query/graphite/native/expression.go#L235: native.rootASTNode is a one-off native.ASTNode (3 methods)
https://go-mod-viewer.appspot.com/github.com/kubeflow/common@v0.4.7/test_job/client/informers/externalversions/test_job/interface.go#L38: test_job.group is a one-off test_job.Interface (1 methods)
https://go-mod-viewer.appspot.com/github.com/newrelic/go-agent@v3.26.0+incompatible/internal/sysinfo/bootid.go#L48: sysinfo.invalidBootID is a one-off error (1 methods)
https://go-mod-viewer.appspot.com/go.skia.org/infra@v0.0.0-20240430042052-0589073495d2/go/autoroll/autoroll.go#L339: autoroll.tryResultSlice is a one-off sort.Interface (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v4@v4.11.0/go/kubernetes/core/v1/pulumiTypes.go#L57341: v1.scaleIOVolumeSourcePatchPtrType is a one-off v1.ScaleIOVolumeSourcePatchPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v3@v3.30.2/go/kubernetes/flowcontrol/v1beta1/pulumiTypes.go#L3783: v1beta1.priorityLevelConfigurationReferencePatchPtrType is a one-off v1beta1.PriorityLevelConfigurationReferencePatchPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/s3/pulumiTypes.go#L6474: s3.bucketObjectLockConfigurationV2RulePtrType is a one-off s3.BucketObjectLockConfigurationV2RulePtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/securityhub/pulumiTypes.go#L213: securityhub.automationRuleActionFindingFieldsUpdatePtrType is a one-off securityhub.AutomationRuleActionFindingFieldsUpdatePtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/zmap/zlint@v1.1.0/lints/lint_ev_business_category_missing.go#L47: lints.evNoBiz is a one-off lints.LintInterface (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v3@v3.30.2/go/kubernetes/core/v1/pulumiTypes.go#L35759: v1.persistentVolumeClaimStatusPtrType is a one-off v1.PersistentVolumeClaimStatusPtrInput (3 methods)
https://go-mod-viewer.appspot.com/sigs.k8s.io/external-dns@v0.14.1/source/gateway_tcproute.go#L31: source.gatewayTCPRouteInformer is a one-off source.gatewayRouteInformer (2 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/wafv2/pulumiTypes.go#L20576: wafv2.ruleGroupRuleStatementRateBasedStatementScopeDownStatementSizeConstraintStatementFieldToMatchPtrType is a one-off wafv2.RuleGroupRuleStatementRateBasedStatementScopeDownStatementSizeConstraintStatementFieldToMatchPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/waldiirawan/apm-agent-go/v2@v2.2.2/internal/apmcloudutil/provider_test.go#L57: apmcloudutil.targetedRoundTripper is a one-off http.RoundTripper (1 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/s3/pulumiTypes.go#L4169: s3.bucketLifecycleRuleExpirationPtrType is a one-off s3.BucketLifecycleRuleExpirationPtrInput (3 methods)
https://go-mod-viewer.appspot.com/go.etcd.io/etcd@v3.3.27+incompatible/client/client_test.go#L519: client.staticHTTPAction is a one-off client.httpAction (1 methods)
https://go-mod-viewer.appspot.com/github.com/juju/juju@v0.0.0-20240430160146-1752b71fcf00/worker/uniter/uniter_test.go#L208: uniter_test.removeCharmDir is a one-off uniter_test.stepper (1 methods)
https://go-mod-viewer.appspot.com/github.com/gdamore/mangos@v1.4.0/transport/inproc/inproc.go#L298: inproc.inprocTran is a one-off mangos.Transport (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/apprunner/pulumiTypes.go#L2948: apprunner.serviceSourceConfigurationImageRepositoryImageConfigurationPtrType is a one-off apprunner.ServiceSourceConfigurationImageRepositoryImageConfigurationPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/sagemaker/pulumiTypes.go#L9035: sagemaker.domainDefaultUserSettingsJupyterServerAppSettingsDefaultResourceSpecPtrType is a one-off sagemaker.DomainDefaultUserSettingsJupyterServerAppSettingsDefaultResourceSpecPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/medialive/pulumiTypes.go#L7967: medialive.channelEncoderSettingsGlobalConfigurationPtrType is a one-off medialive.ChannelEncoderSettingsGlobalConfigurationPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/Cloud-Foundations/Dominator@v0.3.4/lib/concurrent/incrementer.go#L17: concurrent.incrementer is a one-off concurrent.putter (1 methods)
https://go-mod-viewer.appspot.com/github.com/uber-go/tally/v4@v4.1.16/thirdparty/github.com/apache/thrift/lib/go/thrift/transport_factory.go#L38: thrift.tTransportFactory is a one-off thrift.TTransportFactory (1 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/acmpca/pulumiTypes.go#L300: acmpca.certificateAuthorityCertificateAuthorityConfigurationSubjectPtrType is a one-off acmpca.CertificateAuthorityCertificateAuthorityConfigurationSubjectPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/wafv2/pulumiTypes.go#L7723: wafv2.ruleGroupRuleStatementRateBasedStatementCustomKeyCookiePtrType is a one-off wafv2.RuleGroupRuleStatementRateBasedStatementCustomKeyCookiePtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v4@v4.11.0/go/kubernetes/extensions/v1beta1/pulumiTypes.go#L7069: v1beta1.ingressBackendPatchPtrType is a one-off v1beta1.IngressBackendPatchPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/connect/pulumiTypes.go#L1308: connect.instanceStorageConfigStorageConfigS3ConfigPtrType is a one-off connect.InstanceStorageConfigStorageConfigS3ConfigPtrInput (3 methods)
https://go-mod-viewer.appspot.com/go.mercari.io/datastore@v1.8.2/option.go#L99: datastore.withGRPCDialOption is a one-off datastore.ClientOption (1 methods)
https://go-mod-viewer.appspot.com/github.com/goravel/framework@v1.13.9/contracts/cache/mocks/Lock.go#L91: mocks.mockConstructorTestingTNewLock is a one-off mock.TestingT (3 methods)
https://go-mod-viewer.appspot.com/github.com/line/line-bot-sdk-go/v7@v7.21.0/linebot/audience.go#L709: linebot.withListAudienceGroupCallSize is a one-off linebot.IListAudienceGroupOption (1 methods)
https://go-mod-viewer.appspot.com/github.com/cayleygraph/cayley@v0.7.7/graph/memstore/quadstore.go#L318: memstore.quadWriter is a one-off quad.WriteCloser (3 methods)
https://go-mod-viewer.appspot.com/github.com/vmware/govmomi@v0.37.1/object/authorization_manager_internal.go#L48: object.disableMethodsBody is a one-off soap.HasFault (1 methods)
https://go-mod-viewer.appspot.com/github.com/edwarnicke/govpp@v0.0.0-20230130211138-14ef5d20b1d0/binapi/urpf/urpf_rpc.ba.go#L22: urpf.serviceClient is a one-off urpf.RPCService (2 methods)
https://go-mod-viewer.appspot.com/github.com/Laisky/zap@v1.27.0/array.go#L39: zap.bools is a one-off zapcore.ArrayMarshaler (1 methods)
https://go-mod-viewer.appspot.com/github.com/googleapis/api-linter@v1.65.2/cmd/api-linter/rules.go#L48: main.listedRulesByName is a one-off sort.Interface (3 methods)
https://go-mod-viewer.appspot.com/github.com/starshine-sys/bcr@v0.21.0/bot/bot.go#L64: bot.botModule is a one-off bot.Module (2 methods)
https://go-mod-viewer.appspot.com/github.com/newrelic/go-agent@v3.26.0+incompatible/internal_test.go#L291: newrelic.sampleResponseWriter is a one-off http.ResponseWriter (3 methods)
https://go-mod-viewer.appspot.com/github.com/ipld/go-ipld-prime@v0.21.0/traversal/selector/builder/builder.go#L148: builder.exploreFieldsSpecBuilder is a one-off builder.ExploreFieldsSpecBuilder (1 methods)
https://go-mod-viewer.appspot.com/github.com/onflow/flow-go@v0.33.17/network/mocknetwork/ping_info_provider.go#L60: mocknetwork.mockConstructorTestingTNewPingInfoProvider is a one-off mock.TestingT (3 methods)
https://go-mod-viewer.appspot.com/gitlab.com/gitlab-org/labkit@v1.21.0/log/access_logger.go#L28: log.notifiableResponseWriter is a one-off http.ResponseWriter (3 methods)
https://go-mod-viewer.appspot.com/github.com/core-coin/go-core@v1.1.7/p2p/enr/entries.go#L55: enr.generic is a one-off enr.Entry (1 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/lex/pulumiTypes.go#L44239: lex.v2modelsIntentConfirmationSettingConfirmationConditionalDefaultBranchResponsePtrType is a one-off lex.V2modelsIntentConfirmationSettingConfirmationConditionalDefaultBranchResponsePtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/alwitt/goutils@v0.6.4/timer.go#L160: goutils.exponentialSequence is a one-off goutils.Sequencer (1 methods)
https://go-mod-viewer.appspot.com/github.com/peterbourgon/diskv@v2.0.1+incompatible/diskv.go#L326: diskv.closingReader is a one-off io.Reader (1 methods)
https://go-mod-viewer.appspot.com/github.com/cilium/ebpf@v0.15.0/examples/tcprtt/bpf_bpfel.go#L79: main.bpfMaps is a one-off io.Closer (1 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/wafv2/pulumiTypes.go#L6363: wafv2.ruleGroupRuleStatementGeoMatchStatementForwardedIpConfigPtrType is a one-off wafv2.RuleGroupRuleStatementGeoMatchStatementForwardedIpConfigPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/uber/kraken@v0.1.4/origin/blobclient/cluster_client.go#L63: blobclient.clientResolver is a one-off blobclient.ClientResolver (1 methods)
https://go-mod-viewer.appspot.com/github.com/annwntech/go-micro/v2@v2.9.5/api/router/router_test.go#L63: router_test.testServer is a one-off test.TestHandler (3 methods)
https://go-mod-viewer.appspot.com/github.com/rudderlabs/rudder-go-kit@v0.30.0/testhelper/docker/resource/postgres/postgres_test.go#L36: postgres_test.testCleaner is a one-off resource.Cleaner (3 methods)
https://go-mod-viewer.appspot.com/gitee.com/liuxuezhan/go-micro-v1.18.0@v1.0.0/server/proto/server.pb.micro.go#L94: go_micro_server.serverHandler is a one-off go_micro_server.server (2 methods)
https://go-mod-viewer.appspot.com/github.com/TrueCloudLab/frostfs-api-go/v2@v2.0.0-20230228134343-196241c4e79a/reputation/grpc/service_grpc.pb.go#L47: reputation.reputationServiceClient is a one-off reputation.ReputationServiceClient (2 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/wafv2/pulumiTypes.go#L43815: wafv2.ruleGroupRuleStatementXssMatchStatementFieldToMatchSingleQueryArgumentPtrType is a one-off wafv2.RuleGroupRuleStatementXssMatchStatementFieldToMatchSingleQueryArgumentPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v4@v4.11.0/go/kubernetes/extensions/v1beta1/pulumiTypes.go#L3639: v1beta1.deploymentSpecPtrType is a one-off v1beta1.DeploymentSpecPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/vugu/vugu@v0.3.5/lifecycle.go#L94: vugu.destroyCtx is a one-off vugu.DestroyCtx (1 methods)
https://go-mod-viewer.appspot.com/github.com/minio/console@v1.3.0/api/user_login_test.go#L58: api.consoleCredentialsMock is a one-off api.ConsoleCredentialsI (3 methods)
https://go-mod-viewer.appspot.com/github.com/pingcap/tidb/parser@v0.0.0-20231013125129-93a834a6bf8d/parser_test.go#L5022: parser_test.subqueryChecker is a one-off ast.Visitor (2 methods)
https://go-mod-viewer.appspot.com/github.com/micro/go-micro/examples@v0.0.0-20210105173217-bf4ab679e18b/secure/srv/proto/hello/hello.micro.go#L60: go_micro_srv_greeter.sayService is a one-off go_micro_srv_greeter.SayService (1 methods)
https://go-mod-viewer.appspot.com/github.com/kubernetes-csi/external-snapshotter/client/v4@v4.2.0/informers/externalversions/volumesnapshot/v1beta1/interface.go#L58: v1beta1.volumeSnapshotContentInformer is a one-off v1beta1.VolumeSnapshotContentInformer (2 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v3@v3.30.2/go/kubernetes/autoscaling/v1/pulumiTypes.go#L261: v1.crossVersionObjectReferencePatchPtrType is a one-off v1.CrossVersionObjectReferencePatchPtrInput (3 methods)
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-kubernetes/sdk/v3@v3.30.2/go/kubernetes/resource/v1alpha1/pulumiTypes.go#L2752: v1alpha1.resourceClaimStatusPtrType is a one-off v1alpha1.ResourceClaimStatusPtrInput (3 methods)
https://go-mod-viewer.appspot.com/k8c.io/api/v3@v3.0.0-20230904060738-b0a93889c0b6/pkg/generated/listers/ee.kubermatic/v1/resourcequota.go#L30: v1.resourceQuotaLister is a one-off v1.ResourceQuotaLister (2 methods)

@rsc
Copy link
Contributor

rsc commented May 8, 2024

I'm just not seeing compelling use cases here yet.

I clicked on a handful at random and all of them look better to me with an explicit type. You need very trivial functions for this to be a win. After that I think the benefit of having a real type name and therefore a real method name in debuggers and debug prints becomes far more important than saving a few lines.

https://go-mod-viewer.appspot.com/github.com/aacfactory/fns@v1.2.85/barriers/barrier.go#L42
https://go-mod-viewer.appspot.com/github.com/jhump/protoreflect@v1.16.0/desc/sourceinfo/wrappers.go#L35
https://go-mod-viewer.appspot.com/github.com/tetratelabs/wazero@v1.7.1/internal/sysfs/adapter_test.go#L178
https://go-mod-viewer.appspot.com/github.com/pulumi/pulumi-aws/sdk/v6@v6.32.0/go/aws/quicksight/pulumiTypes.go#L8435
https://go-mod-viewer.appspot.com/github.com/fastly/go-fastly/v6@v6.8.0/fastly/newrelic.go#L67 (this should use slices.SortStableFunc anyway)
https://go-mod-viewer.appspot.com/go.skia.org/infra@v0.0.0-20240430042052-0589073495d2/go/autoroll/autoroll.go#L339 (another use for slices.SortStableFunc)
https://go-mod-viewer.appspot.com/github.com/waldiirawan/apm-agent-go/v2@v2.2.2/internal/apmcloudutil/provider_test.go#L57

The only defensible ones are the calls to sort.Sort but even there having to write three methods gets to the point where it's probably not a win to inline it. I've always found it much clearer to move that code to a separate place in the file and have the call site just say

sort.Sort(byBuilder(x))

That's less to skim at a glance when reading it than

sort.Sort(sort.Interface{
    Len: func() int { return len(x) }, 
    Swap: func(i, j int) { x[i], x[j] = x[j], x[i] },
    Less: func(i, j int) bool { return x[i].Builder < x[j].Builder },
})

Writing the literal seems to optimize for writing the code and not for reading it, and that's the wrong optimization.

And as I noted these should use slices.SortStableFunc or sort.Slice anyway, and then there's no interface at all anymore.

Any use of sort.Sort should be dropped from the analysis, since we already have a better API for sorting. (Making a language change to improve the use of an API we've already added a better replacement for isn't worth it.)

@ianlancetaylor ianlancetaylor changed the title proposal: Go 2: interface literals proposal: spec: interface literals Aug 6, 2024
@ianlancetaylor ianlancetaylor added LanguageChangeReview Discussed by language change review committee and removed v2 An incompatible library change NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Aug 6, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
LanguageChange Suggested changes to the Go language LanguageChangeReview Discussed by language change review committee Proposal
Projects
None yet
Development

No branches or pull requests