go/types: maintain "owner" information for interface methods and struct fields

[adonovan]

Hi Robert,

in our indexer for Go, when we load (from export data) a package that defines types such
as the following:

    type T struct { X int }
    type U T
    type S U

there is nothing in the go/types API that allows the client to determine that although
the *types.Var for X is a member of S, T and U, it was originally declared beneath T. 
(Underlying() returns the same struct type for S, T and U.)

An analogous situation holds for interface methods:

   type Closer interface { func Close() error }
   type WriterCloser interface { Writer; Closer }

i.e. there is no record of the fact that the Close() method was originally declared
beneath Closer, not WriterCloser.

The indexer needs this information because it must invent a globally unique stable name
for T.X that relates its definition in this package to its uses in other packages. 
Right now, it picks---consistently but arbitrarily---the lexicographically least name
from the set {S.X, T.X, U.X}, i.e. S.X.  So long as all invocations of go/types use
export data that includes S, then all will use the same least name S.X.  However, if the
package was loaded from partial export data that does not include the least name, the
result is a dangling link.

What the indexer needs is that go/types expose some information about the field X that
allows us to come up with a stable name for it each time we encounter it across
different invocations of the type checker.  Associating X with its "true"
owner (T.X) is the most logical approach and will doubtless have benefits for other
applications, but any stable name could work (e.g. the least name, or some fingerprint).

So: how should we do this?

[griesemer]

Comment 1:

Labels changed: added release-none, repo-tools.

Status changed to Thinking.

[griesemer]

@adonovan Is this still of interest? If so, please comment on this issue and mark for 1.11, "early in cycle" and I'll try to address it. Thanks.

[stapelberg]

I just ran into this limitation, too, when working on a tool to surface “// Deprecated: ” comments. (See Google-internal cl/478490109 for more context.)

My code works by iterating through info.Uses, constructing a stable name, and then matching said name against an index of known-deprecated symbols (which is produced by using ast.Inspect).

Currently, I can’t surface deprecation warnings for struct fields (only for the struct type itself, or methods on it) because of this limitation.

@griesemer Would you still be interested in looking into this? Timing-wise, the 1.21 cycle should start soon, so perhaps this is an opportune moment. Thank you!

[griesemer]

@stapelberg Marking for 1.21 so it's on the radar as we may look into some changes in this space. But exposing the information would be an API change which would require a (if small) proposal.

[stapelberg]

Thank you! Let me know if there’s anything I can help with, happy to try out a pending change or similar :)

[griesemer]

@adonovan @findleyr for feedback in whether this is something we need to prioritize ("early in cycle"). Moving to 1.22 for now.

[adonovan]

I have learned to live with this design, so don't keep this open on my account.

[stapelberg]

We’re still interested in this!

(Though, now that I read Alan’s comment, maybe I should try again to find a workaround for our use-case. I’ll try that as time permits.)

[adonovan]

FWIW, now that the export and import of type information preserves precise positions, you could use the position of the field to resolve the ambiguity: the named struct type whose position most closely precedes the field position is its "owner".

[griesemer]

Just to be clear, we could easily maintain an owner link but at least for go/types this would require an API change if it needs to be accessible externally. That would require a proposal. For now put on Backlog. Feel free to "reactivate" (and send out a proposal) if this continues to be important. Also, feel free to close if no need.

[stapelberg]

We managed to work around this issue as well: we now look at the AST, and when we find a KeyValueExpr, we go back up the stack to the containing CompositeLit to extract its type.

I’ll close this issue, but please report back in case anyone has a use-case where this limitation cannot be worked around.

[1268]

I ran into this as well, and ended up using a go/ast based solution, specifically because of this limitation. I think it would be a useful addition to go/types, or at a minimum it would be helpful if go/types documented it as a non-feature.