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/parser: comments to parameters are not processed #25814

Closed
wzshiming opened this issue Jun 11, 2018 · 5 comments
Closed

go/parser: comments to parameters are not processed #25814

wzshiming opened this issue Jun 11, 2018 · 5 comments

Comments

@wzshiming
Copy link

wzshiming commented Jun 11, 2018

Comments to parameters are not processed here

field := &ast.Field{Names: idents, Type: typ}

I think this comment should be processed

package main

import (
	"go/ast"
	"go/parser"
	"go/token"

	ffmt "gopkg.in/ffmt.v1"
)

const f = `
package a

func A(
	// Why isn't this a doc on x
	x int , // Why isn't this a comment on x
) {

}
`

func main() {
	fset := token.NewFileSet()
	f, err := parser.ParseFile(fset, "_", f, parser.ParseComments)
	if err != nil {
		ffmt.Mark(err)
		return
	}
	for _, v := range f.Decls {
		switch t := v.(type) {
		case *ast.FuncDecl:
			ffmt.P(t)
			//&go/ast.FuncDecl{
			// Doc:  <nil>
			// Recv: <nil>
			// Name: <A>
			// Type: &go/ast.FuncType{
			//  Func:   go/token.Pos(13)
			//  Params: &go/ast.FieldList{
			//   Opening: go/token.Pos(19)
			//   List:    slice[
			//    &go/ast.Field{
			//     Doc:     <nil>
			//     Names:   slice([]*ast.Ident{(*ast.Ident)(0xc042050460)})
			//     Type:    go/ast.Expr(&ast.Ident{NamePos:54, Name:"int", Obj:(*ast.Object)(nil)})
			//     Tag:     <nil>
			//     Comment: <nil>
			//    }
			//   ]
			//   Closing: go/token.Pos(92)
			//  }
			//  Results: <nil>
			// }
			// Body: &go/ast.BlockStmt{
			//  Lbrace: go/token.Pos(94)
			//  List:   slice[ ]
			//  Rbrace: go/token.Pos(97)
			// }
			//}
		}
	}
}
@agnivade
Copy link
Contributor

Indeed, looks like a corner case. @griesemer

@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jun 11, 2018
@agnivade agnivade added this to the Go1.12 milestone Jun 11, 2018
@agnivade agnivade changed the title go/parser: Comments to parameters are not processed go/parser: comments to parameters are not processed Jun 11, 2018
@agnivade
Copy link
Contributor

agnivade commented Jun 11, 2018

Wait sorry, if I set the mode to parser.ParseComments and print the ast, it indeed prints all the comments.

func main() {
	fset := token.NewFileSet()
	f, err := parser.ParseFile(fset, "_", f, parser.ParseComments)
	if err != nil {
		ffmt.Mark(err)
		return
	}
	ast.Print(fset, f)
}

The comments are part of f.Comments, not f.Decls, as clarified here https://godoc.org/go/ast#File.

for _, v := range f.Comments {
	ffmt.P(v)
}

I am not sure whether an ast.Field is supposed to have comments in the Comment section. Leaving this open until someone can clarify that.

@wzshiming
Copy link
Author

wzshiming commented Jun 11, 2018

It only exists in File.Comments , not in ast.field
I don't think that's reasonable
I'm trying to fix it #25817

@gopherbot
Copy link

Change https://golang.org/cl/117795 mentions this issue: go/parser: Fixes #25814

@griesemer
Copy link
Contributor

@wzshiming Unfortunately this is by design (these comments are not shown in godoc, after all). I wouldn't call it "not reasonable", but I agree that it's unfortunate.

Keep in mind that go/ast, go/parser, and friends are among the very first larger Go packages ever written and they show their age. Also, and I'm taking full responsibility for this, only doc comments are directly attached to AST nodes; all the other ones are collected in the File.Comments list and must be "interspersed" by position when printing. It works ok for gofmt but it's a real pain elsewhere. In retrospect that was a bad design decision. We know all about it.

Closing as unfortunate.

@griesemer griesemer added Unfortunate and removed NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. labels Jun 11, 2018
@griesemer griesemer self-assigned this Jun 11, 2018
@golang golang locked and limited conversation to collaborators Jun 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants