Golang cmd/doc: Blank lines inserted into doc output in place of "directive" lines

xref https://github.com/golang/go/issues/56592

What version of Go are you using (go version)?

$ gotip version
go version devel go1.21-f00c54146c Mon Apr 10 22:52:22 2023 +0000 linux/amd64

Does this issue reproduce with the latest release?

This tests a change made on tip.

What operating system and processor architecture are you using (go env)?

NA

What did you do?

Given this file:

// Package p is a package.
//
//magic:true
package p

// T is a type
//
//magic:true
type T struct {
    // Field is a field
    //magic:true
    Field int
}

go 1.20 go doc shows:

$ go doc --all .
package p // import "hockin.org/p"

Package p is a package.

TYPES

type T struct {
    // Field is a field
    //magic:true
    Field int
}
    T is a type

Note the directive line is present. gotip doc says:

$ gotip doc --all .
package p // import "hockin.org/p"

Package p is a package.

TYPES

type T struct {
    // Field is a field

    Field int
}
    T is a type

The directive is gone (yay) but there's now a blank line. @ianlancetaylor explains it in https://github.com/golang/go/issues/56592#issuecomment-1502475206 and acknowledges that it is imperfect (but better than before).

Comment From: thockin

Following up, this still exists in go 1.24. Additionally, pkgsite does not even bother to elide directives in comments on struct fields.

Comment From: thockin

Digging a bit, to understand the scope:

go/ast.isDirective() is called from go/ast.CommentGroup.Text() from main.trimUnexportedFields() which specifically references https://github.com/golang/go/issues/56592.

cmd/doc/pkg.go :: func Package.emit() calls format.Node(), which calls config.Fprint() (go/printer.Config.Fprint()). That eventually calls go/printer.printer.printNode() -> printer.decl() -> printer.genDecl() -> printer.spec() -> printer.expr() -> printer.expr1() -> printer.fieldList(). At that point we can see that the directive has already been stripped out (as expected, given the above).

Tracking more, we eventually hit printer.print() which calls printer.flush() to line up for the next node, which includes printing comments associated with that node. Or that's what I would assume. In fact .intersperseComments() says "all comments that appear before the next token". The comment is not "linked to" the token any more, despite that several callframes back (fieldList()) had an ast.Field with a Name and a Doc. We tore the information apart, stuffed it into p in different places and then lost track of the fact that they go together. I think it can't tell between:

type T struct {
  // floating comment

  F int
}

and:

type T struct {
  // attached comment
  //some:directive
  F int
}

Oh, but the floating comment isn't represented in go doc, so we don't need to consider that case. I think the comment HAS TO BE attached to the current node. So in this code (printer.print())

  1000: next := p.pos // estimated/accurate position of next item
  1001: wroteNewline, droppedFF := p.flush(next, p.lastTok)
  1002: 
  1003: // intersperse extra newlines if present in the source and
  1004: // if they don't cause extra semicolons (don't do this in
  1005: // flush as it will cause extra newlines at the end of a file)
  1006: if !p.impliedSemi {
  1007:      n := nlimit(next.Line - p.pos.Line)
  1008:     // don't exceed maxNewlines if we already wrote one
  1009:     if wroteNewline && n == maxNewlines {
  1010:         n = maxNewlines - 1
  1011:     }
  1012:     if n > 0 {
  1013:         ch := byte('\n')
  1014:         if droppedFF {
  1015:             ch = '\f' // use formfeed since we dropped one before
  1016:         }
  1017:         p.writeByte(ch, n)
  1018:         impliedSemi = false
  1019:     }

Could we assume that if wroteNewline == true and next.Line != p.pos.Line that the pos is lying, and increment the current position (p.pos)? It seems to work for this case but fails tests. Maybe only for some node types this is true?