x/tools/go/analysis: pkgsite documentation for golang.org/x/tools/go/analysis/passes/* often lacks details

[hyangah]

I am not sure if this is a feature request for pkgsite, or it's just because the go analysis analyzer code was structured in a way not friendly with go documentation. Analyzers often place the helpful details in Doc constant, but this long doc is hidden from documentation

https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/nilness

$ go doc
package nilness // import "golang.org/x/tools/go/analysis/passes/nilness"

Package nilness inspects the control-flow graph of an SSA function and reports
errors such as nil pointer dereferences and degenerate nil pointer comparisons.

const Doc = ...
var Analyzer = &analysis.Analyzer{ ... }

$ go doc Doc

Would be nice if the documentation page of analyzers presents sufficient details.
The pattern I saw in similar situation was to generate the package doc (doc.go) from the types but not sure if that's the best way.

cc @adonovan @golang/tools-team

[adonovan]

I agree that doc.go makes sense. See https://go.dev/cl/474935 for a sketch of a starting point.

xtools$ go run ./go/analysis/passes/nilness/cmd/nilness/main.go 
nilness: check for redundant or impossible nil comparisons

Usage: nilness [-flag] [package]

The nilness checker inspects the control-flow graph of each function in
a package and reports nil pointer dereferences, degenerate nil
pointers, and panics with nil values. A degenerate comparison is of the form
x==nil or x!=nil where x is statically known to be nil or non-nil. These are
often a mistake, especially in control flow related to errors. Panics with nil
values are checked because they are not detectable by
...

$ go run golang.org/x/pkgsite/cmd/pkgsite@latest & 
$ open http://localhost:8080/golang.org/x/tools/go/analysis/passes/nilness

[gopherbot]

Change https://go.dev/cl/474935 mentions this issue: go/analysis: ExtractDoc unifies Analyzer.Doc and package doc comment

[seankhliao]

I wonder if that ExtractDoc function is something that could go in maybe the standard library go/doc?
It's something I've wanted a few times

[hyangah]

Thanks @adonovan

https://go.dev/cl/474935's approach is

• place the doc for an analyzer in the package document
• extract the relevant section from the package document and wire it to analysis.Analyzer.Doc.
• provide a helper function ExtractDoc to extract the section

(+) One copy and place where the document is
(-) To support multiple analyzers in one package, we may end up with some convention in the package documentation organization.
(-) Some features available in comment doc (cross-reference, etc) may leak to the analyzer doc which.

I was originally thinking a different approach

• have a tool to extract analysis.Analyzer.Doc contents
• populate doc.go using the tool which can be automated with go:generate
• may have a test that ensures consistency of doc.go.

(+) No magic keyword or convention in the package documentation.
(-) Two copies of the same contents.
(-) Consistency between two copies is not automatically enforced.

[adonovan]

I think the benefit of having a single copy (that is automatically readable in pkgsite) probably offsets the risk of markup (e.g. cross-refs) being accidentally included in the plain text version. Perhaps we can apply the go/doc/comment parser and format such things, so we get the best of both worlds: HTML in the browser and plain text in the terminal?

I agree we need to support multiple analyzers. Perhaps we should require a section called # Analyzer foo, and make ExtractDoc(doc, "foo") fail if if doesn't find a section of that name.

[findleyr]

Looks good at master: https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/nilness@master

Closing this as the work is done, and x/tools will automatically be released at some point.