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

go/doc,x/pkgsite: struct fields inconsistently linked in doc comments #61394

Open
bcmills opened this issue Jul 17, 2023 · 8 comments
Open

go/doc,x/pkgsite: struct fields inconsistently linked in doc comments #61394

bcmills opened this issue Jul 17, 2023 · 8 comments
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. pkgsite Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@bcmills
Copy link
Contributor

bcmills commented Jul 17, 2023

https://go.dev/doc/comment#doclinks currently says:

Doc links are links of the form “[Name1]” or “[Name1.Name2]” to refer to exported identifiers in the current package

but it is somewhat ambiguous about whether the [Name1.Name2] form can refer to struct fields or only methods.


In practice, people do seem to intuitively expect references to struct fields to work; for example, this comment on types.Package.GoVersion (written by @rsc) refers to the go/ast.File.GoVersion struct field, and pkg.go.dev appears to render that reference correctly:
image


However, similar references within the same package are not rendered as links. For example, the reference to [CheckedFiles.SizeError] from zip.CheckedFiles.Err is currently rendered as plain text:
image

go/doc.Parser uses different functions to resolve intra- vs. inter-package references: it appears that the LookupPackage hook being used by pkgsite does resolve struct fields, whereas the LookupSym hook does not.

@bcmills bcmills added NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. Tools This label describes issues relating to any tools in the x/tools repository. pkgsite labels Jul 17, 2023
@gopherbot gopherbot added this to the Unreleased milestone Jul 17, 2023
@bcmills
Copy link
Contributor Author

bcmills commented Jul 17, 2023

I think the bug is in go/doc. Although the Package.syms field lacks a clarifying comment, in practice it appears not to include struct fields:
https://cs.opensource.google/go/go/+/refs/heads/master:src/go/doc/doc.go;l=159-171;drc=fe5af1532ab9c749d880c05e0ffe0e17bf874d7f

@bcmills bcmills changed the title x/pkgsite,go/doc: struct fields inconsistently linked in doc comments go/doc,x/pkgsite: struct fields inconsistently linked in doc comments Jul 17, 2023
@bcmills bcmills modified the milestones: Unreleased, Go1.22 Jul 17, 2023
@bcmills bcmills self-assigned this Jul 17, 2023
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/510315 mentions this issue: go/doc: recognize intra-package struct field references

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/528402 mentions this issue: go/doc: add a golden test that reproduces #62640

@bcmills
Copy link
Contributor Author

bcmills commented Feb 1, 2024

This is blocked awaiting code review, so moving to Backlog.

@bcmills bcmills modified the milestones: Go1.22, Go1.23, Backlog Feb 1, 2024
@bcmills bcmills removed their assignment Mar 15, 2024
@artyom
Copy link
Member

artyom commented May 10, 2024

I was about to file an issue for net/http.Transport documentation rendering and then I found this issue.

¶ go doc http.Transport | grep -A3 'Transport caches'
    By default, Transport caches connections for future re-use. This may
    leave many open connections when accessing many hosts. This behavior
    can be managed using Transport.CloseIdleConnections method and the
    [Transport.MaxIdleConnsPerHost] and [Transport.DisableKeepAlives] fields.

Hope https://go.dev/cl/510315 gets some attention soon.

@dmitshur dmitshur added NeedsFix The path to resolution is known, but the work has not been done. and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 19, 2024
@stevenh
Copy link
Contributor

stevenh commented Jul 22, 2024

I would love to see this hit 1.23 is there any chance of that @bcmills and is there anything I can do to help make that happen?

@bcmills
Copy link
Contributor Author

bcmills commented Jul 22, 2024

I'm no longer on the Go team so there's zero chance of my CL landing as-is — but you're welcome to clone it and shepherd it through the review process! 🙃

@cespare
Copy link
Contributor

cespare commented Aug 21, 2024

This is quite a visible issue with rendered package docs. There are about 30 instances where it occurs in the net/http documentation alone.

screen_20240821104748

@griesemer and @golang/pkgsite -- can you suggest a good person to own this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
FixPending Issues that have a fix which has not yet been reviewed or submitted. NeedsFix The path to resolution is known, but the work has not been done. pkgsite Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

6 participants