proposal: x/tools/go/analysis: new `godoc` analyzer

[Antonboom]

Hello!

I need to know if the Go team is interested in having Go Doc checker in x/tools or if we implement it in a third party module. Go Doc comments is common place for issues (especially after recent HTML support) and I thought maybe Gopls interested in it.

Example of checks:

https://go.dev/doc/comment#packages

Before:

// path implements utility routines for manipulating slash-separated paths.

After:

// Package path implements utility routines for manipulating slash-separated paths.

Or typo:

// Package path implements utility routines for manipulating slash-separated paths.
package patch

https://go.dev/doc/comment#links

Before:

// Package json implements encoding and decoding of JSON as defined in
// https://tools.ietf.org/html/rfc7159. The mapping between JSON and Go values is described
// in the documentation for the Marshal and Unmarshal functions.
//
// For an introduction to this package, see the article
// https://golang.org/doc/articles/json_and_go.html
package json

After:

// Package json implements encoding and decoding of JSON as defined in
// [RFC 7159]. The mapping between JSON and Go values is described
// in the documentation for the Marshal and Unmarshal functions.
//
// For an introduction to this package, see the article
// “[JSON and Go].”
//
// [RFC 7159]: https://tools.ietf.org/html/rfc7159
// [JSON and Go]: https://golang.org/doc/articles/json_and_go.html
package json

(+ check URL formatting)

And so on and so forth.

---

Related:

• https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#comment-spacings
• https://github.com/mgechev/revive/blob/master/RULES_DESCRIPTIONS.md#package-comments
• https://github.com/tetafro/godot
• https://github.com/client9/misspell
• https://github.com/Abirdcfly/dupword
• https://github.com/stretchr/testify/commit/43b2ef12b3f933c888167b0029430c083837ad65
• https://staticcheck.dev/docs/checks/#ST1000

---

P.S. Based on https://github.com/golangci/golangci-lint/issues/4147 (cc) @500poundbear

[timothy-king]

Checks that enforce comments to be correct "Doc Comments" seem plausible as a cmd/vet (or gopls) check IMO. If we have a strong reason to suspect that a comment is a doc comment and it is a 'buggy' comment, that will cause an incorrect rendering of the documentation for the package. I think this is on the right side of the vet correctness criteria. (In this case it is the execution of the documentation programs on the package.)

cmd/vet should not be too opinionated about non-"Doc Comments". So I think a lot of these will hinge on having enough context to decide if something is intended to be a "Doc Comment". I don't think there is sufficient context from just this:

// path implements ...

I do think there is sufficient context once this is the comment above a package declaration and the first word matches the package name:

// path implements ...
package path // warning

I also think there is sufficient context to warn on that the word after "Package" does not match the package name:

// Package path implements utility routines for manipulating slash-separated paths.
package patch // warning

From https://go.dev/doc/comment#links :

Plain text that is recognized as a URL is automatically linked in HTML renderings.

IIUC this correctly the links given are are style issues not correctness. That is not to say they are not valuable checks. Just not a fit for vet (IMO).

Or typo:

I am not really sure what other typos you have in mind. Regardless, it will need to be typos according to some dictionary (system dictionary, then personal dictionaries, etc.) I am not really sure this a good match for cmd/vet, which tends towards zero config. This may be okay for gopls. @findleyr ?

[seankhliao]

I think I already see the first one in gopls via staticcheck's ST1000. @dominikh placed this as a style check, maybe not strong enough to fail a vet check for?

[Antonboom]

If we have a strong reason to suspect that a comment is a doc comment and it is a 'buggy' comment, that will cause an incorrect rendering of the documentation for the package

Exactly! And this is true for links and other HTML.

I am not really sure what other typos you have in mind.

E.g. the common error – do not change documentation after function name changing

// WasClosed reports whether the connection was closed.
func (c *conn) Closed() bool

Of course there is some analysis of godoc checker's scope needed. I thought it is the next step after decision "yes, the nice idea, we accept it".

maybe not strong enough to fail a vet check for

I'm not sure that go vet should rely on the opinions of third-party linters. Moreover, @timothy-king explained above why this is not about style. Added ST1000 to "Related" section, thanks!

[adonovan]

This proposal overlaps with https://github.com/golang/go/issues/57963, which is specifically about broken doc links.