For context, this is what users currently see on the package I'm working on: https://godoc.org/github.com/mvdan/sh/syntax#Command

The types are there, and you can select and do a search within the page, but that's just inconvenient for no reason.

In #18342, starting here, automatic linking is discussed and accepted.

That will be very useful in general but I don't think it's optimal for this case.

Especially in AST code, there's going to be many types involved and the links could be automatically generated without any user intervention automatically improving docs that do not list the types manually avoiding manual lists going out of sync.

The questions are how to link the types, where, and when.

In any case, it should only be limited to the types in a given package or godoc would have to check everything against everything and the lists generated could be quite long, even if it were practical to compute them in a reasonable amount of time.

Empty interfaces should always be skipped because there's no information gained and much noise produced.

Unexported interfaces would only make sense when ?m=all (or if #7823 is implemented and the unexported interface matches the definition of "accessible" given in that thread).

In the go/ast example essentially every type in that package implements ast.Node so listing the implementing types on ast.Node could be quite noisy, though it would be very useful when constructing type switches as you could see all the possible cases at a glance. But it also raises the question of whether it should list compatible interfaces such as ast.Spec. Maybe it could be in a collapsed-by-default fieldset, as examples are, if it exceeds more than a handful of entries.

Listing the local, non-empty interfaces a non-interface type implements would be brief and useful. A line below the doc string for, say, *ast.TypeSpec that says "implements: ast.Node, ast.Spec" (properly linked) is short and easy to follow.

This could be further limited to only interface types with unexported methods as those are the major pain point, but it seems generally useful.

I wasn't aware that automatic linking had already been proposed and accepted, thanks. I didn't find it because that issue doesn't mention any of this in the title.

That means that the first part of my proposal has already been proposed and acepted, so I'll retitle the issue to be more clear.

What you suggest - doing it for any interface within a package - seems OK, but I'm worried about it having unwanted effects or being too verbose. Like you say, some interface types may be very broad and you might end up with very long lists. Or some interfaces might not be of this "sum type" kind, and the list would not be appropriate.

Limiting it to interfaces with unexported methods would perhaps improve the situation, but I wonder if that would make the logic too convoluted. And perhaps there are other ways to represent sum types in current Go that I'm not aware of.

yeah, I had a vague recollection that automatic linking had come up at some point, but it took me forever to find it since it didn't have its own issue.

To be clear, since re-reading it seems somewhat ambiguous, I don't mean grouping anything in the table of contents, like listing implementing types that implement an interface under the interface the way constructors are listed under a type—that would get quite messy very quickly. I mean adding a note under https://golang.org/pkg/go/ast/#TypeSpec saying which interfaces TypeSpec implements that are declared in the same package and a collapsed-by-default list under https://golang.org/pkg/go/ast/#Node so it doesn't take up too much room but's there if you need it.

I think every interface should be considered because it's a natural expansion—same logic minus one proposition—and I can't think of a reason it would be harmful or unhelpful to make note of that information. Maybe there's some extant package out there that makes it obvious why doing so would be a bad idea, but I can't think of any.

(re sum types: *T is the sum 1 + T and const/iota can be 1 + 1 + ⋯ + 1 sum types, technically)

@jimmyfrasche perhaps you're right and doing it for all interfaces would be a good idea. This could be similar to how types list their constructors. You could make the same argument that a constructor could be a false positive, but it's still OK to list it there.

Let's see if there are any counter-arguments to doing it for all interfaces.

Here's a fancy mock up of the presentation I imagine:

@jimmyfrasche that looks good - we should have that implemented and ready in a CL by the time the 1.10 tree opens in August. For now I'd wait a few more weeks to see if there are any objections.

I also just realised I didn't write this as a proposal. Perhaps I should re-open as a formal proposal so that the weekly proposal review meeting can catch it.

For now, I've made it a proposal. Hopefully the proposal review meetings will catch it at some point soon.

This seems OK to surface, and not just for pseudo-sum types but any interface. Please do send a CL for Go 1.10.

Thank you, Russ. @jimmyfrasche would you like to collaborate on this?

@mvdan I'd be happy to

@mvdan I played with this a little tonight.

I couldn't figure out a way to get the source importer to interface with godoc's vfs. I rigged a fake importer that just always returned an incomplete and empty types.Package. That works surprisingly well, but it means it only works for interfaces that don't refer to types in other packages. So go/ast's out 👎

I looked at the source for the source importer. It's not that big, about 200 lines (and another 150 of tests), but it would still be a bit of a hassle to bolt it onto godoc and if looking at package A can load packages B through Z there might have to be caching and cache invalidation baked into it. No matter what the two copies would be somewhat divergent. That might be fine, but it has a lot of TODOs and bugs already.
. . .

golang.org/x/tools/go/gcexportdata looks like it could work but that requires things be built
Another option would be to rig an entirely source based mini-typechecker to figure it out. The big issues with that would be dealing with all the edge cases of different import names in different files.

At that point it would probably be easier to take the mostly filled in type info from the first approach and do some extra tests for interfaces that don't have full type info. That might be pretty simple, but I haven't had a chance to try it out yet and I've only been using the types package for 5 hours now so I am winging all of this

I am not familiar with godoc's vfs, nor with how it currently loads the packages from disk. I was not aware it had this layer of abstraction.

I do have some experience with go/types and package loading, but almost every single time I've ended up just using go/loader. I would imagine that's not an option here. It always loads from source too, working out the type info from scratch, so it would also make godoc noticeably slower, which I understand would be considered a regression.

I believe that the state of Go importers is a bit confusing at the moment. Some only load from cached build artifacts, and some only load from source. I do not know if a hybrid would be possible, and if so if one exists at the moment. But I think that is what we would want here.

Perhaps we should get some input from @rsc and @griesemer before we try to go down any of these routes.

yeah, go/loader looks like it's the only one with an option for a source importer that I could find but it always hits the real file system and there's not a mechanism to change that currently. But even if it did there'd be the issue with it having to recheck all deps all the time.

After sleeping on it though, I think the best way to do this would be to write a custom version of the code to see whether a concrete type satisfies an interface using the type info I've pulled out so far. It shouldn't be too complicated since it only needs to answer a very specific question in a very specific case.

There would be some false negatives in some edge cases (type aliases defined in external packages and interfaces that embed interfaces from other packages (unless the concrete type is a struct which also embeds that same interface)), but it looks like even with only type checking the package in isolation there's enough information to say "well I don't know what a token.Pos is but you both agree that you return whatever that is and the rest of it lines up . . .".

While that would have false negatives, like I said, I can't think of that having any false positives, which would be the unforgivable thing.

Worth a shot, at any rate.

Hopefully I'll have some time to play with that today, but will definitely have time to do so tomorrow. I'll try to throw what I have in a DO NOT REVIEW CL later, regardless of whether I make any progress, so you can take a look.

Change https://golang.org/cl/67192 mentions this issue: x/tools/cmd/godoc: List types that satisfy an interface within package

go/types sets all types that don't resolve to the same invalid type sentinel. We can get back to the original ast definition from the token.Pos and find the nth param/ret and see the type. As long as it's not from a dot import we can look up what package it came from in that ast.File and treat all (package, type) pairs as distinct and use that information to compute most of the interface satisfaction relation for the package, but at that point we might as well write our own purpose-built typechecker.

It seems like it would be less effort to have a custom source importer that works with godoc's vfs but it's not clear to me that that's desirable. That would also involve factoring out how godoc generates go/build contexts into its own package since that logic would be needed multiple places.

@mvdan I've updated the CL to use go/loader. It probably does too much work but it has the benefit that it works, or at least appears to work so far.

In typecheck.go there's a commented out config param. That's needed to integrate into the godoc vfs. Not exactly sure what needs to go into it yet, though.

This would be fantastic for a pattern similar to functional options but using a option interface and data types.

See https://news.ycombinator.com/item?id=16503839

Right now, the option interface and the types that implement it are not at linked in the godoc which makes the package docs hard to navigate.

Wait, isn't this already implemented?

Just needs #11251 to become enabled by default.

This is a documentation generation tool, so there's no reason to intrinsically limit it to just "within the package." It's fine if that is the default scope, but it should accept the scope at the command line so that APIs from different packages and modules that are intended to interact may be properly documented.