filter comments outside of interface declaration (#50)

[korylprince]

This is a fix for Issue #50. It works by filtering comments outside of the interface declaration.

Arguably, this could be fixed in the ast package itself, as it's the ast package that assigns a comment at the end of the file to the last parsed node, but maybe that's expected behavior?

[josharian]

Thanks.

Yes, this ought to be fixed upstream. See golang/go#21755. Feel free to pursue that instead of you'd prefer.

If you want to fix in impl, rather than reconstructing the comment groups here, it seems like it'd be easier to adjust flattenCommentMap (remove the panic, do the position checks inline).

This PR could also use a test.

[korylprince]

It seems like there's some semi-recent activity on fixing this in go/ast. While that's definitely the desired approach, it's a bigger challenge than I'm able to take on at the moment. I'd definitely like to add a fix/workaround to impl, as I don't think that will take a huge amount of work.

In some more comprehensive testing, this PR isn't sufficient for all cases (it specifically fails with the // after Func2 comment below). Before working on this further, I'd like to establish what exactly you're targeting with comments.

Given the following file:

package pkg

// before Interface

// Interface doc
type Interface interface {
	// before Func

	// Func doc
	Func() // end Func

	// after Func

	// Func2 doc
	Func2() // end Func2

	// after Func2

} // end Interface

// after Interface

impl 't T' Interface currently outputs (only removing the panic in flattenCommentMap):

// before Func
// Func doc
// end Func
func (t T) Func() {
	panic("not implemented") // TODO: Implement
}

// after Func2
// after Interface
// after Func
// Func2 doc
// end Func2
func (t T) Func2() {
	panic("not implemented") // TODO: Implement
}

I would assert that the output should be:

// Func doc
// end Func
func (t T) Func() {
	panic("not implemented") // TODO: Implement
}

// Func2 doc
// end Func2
func (t T) Func2() {
	panic("not implemented") // TODO: Implement
}

For reference, go doc -all . outputs:

package pkg // import "test"


TYPES

type Interface interface {

	// Func doc
	Func() // end Func

	// Func2 doc
	Func2() // end Func2

} // end Interface
    Interface doc

In other words, only comments directly before (separated by exactly one newline) or after (separated by no newlines) a function should be output. Do you agree with that?

[josharian]

I'd be happy to mimic go doc. I'd also be happy to have just Func2 doc and not end Func2 if it simplifies things.

[josharian]

Or maybe we should just remove the panic. End-of-stuff comments are rare, and easy to delete if unwanted.

[korylprince]

I've taken some time to dig into this issue further, particularly into how go doc creates its output.

It seems there's no good reason to mess with an ast.CommentMap in the first place. Each *ast.Field (e.g. each method of the interface) has a .Doc field that contains the correctly filtered doc comment that go doc uses.

I've changed this PR to change ast.CommentMap usage with *ast.Field.Doc. I've also included a test for free-floating comments.

If desired, the trailing comment (e.g. the // end Func from above) can also be accessed in *ast.Field.Comment, though I didn't include that in this PR. I'm not sure I've ever seen trailing comments on interface methods...

[josharian]

Very nice. Thank you.