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

x/tools/go/astutil: PathEnclosingInterval omits FuncType for intervals in the function signature #68202

Closed
adonovan opened this issue Jun 26, 2024 · 3 comments
Assignees
Labels
Tools This label describes issues relating to any tools in the x/tools repository.
Milestone

Comments

@adonovan
Copy link
Member

adonovan commented Jun 26, 2024

Invoking PathEnclosingInterval on the extent of x in this program:

package p

func f(x int)

gives this result:

Path enclosing interval #18-19 [exact=true]:
[0] *ast.Ident @ 3:8-3:9 (#18-19)
[1] *ast.Field @ 3:8-3:13 (#18-23)
[2] *ast.FieldList @ 3:7-3:14 (#17-24)
[3] *ast.FuncDecl @ 3:1-3:14 (#11-24) Type.Scope={x}
[4] *ast.File @ 1:1-3:14 (#0-24) Scope={}

The FuncType--FuncDecl.Type, which holds the Params FieldList--is nowhere to be seen.

I'm sure this is because the invariant that each node contains its children has an edge case for FuncType: the FuncDecl for func f(x int) has a FuncType that includes the func token and the (x int) signature but not the name f (FuncDecl.Name), or the receiver for a method.

I'm amazed we haven't noticed this before.

@gopherbot gopherbot added the Tools This label describes issues relating to any tools in the x/tools repository. label Jun 26, 2024
@gopherbot gopherbot added this to the Unreleased milestone Jun 26, 2024
@adonovan adonovan self-assigned this Jun 26, 2024
@gopherbot
Copy link
Contributor

Change https://go.dev/cl/595195 mentions this issue: go/ast/astutil: PathEnclosingInterval: add missing FuncType nodes

@gopherbot
Copy link
Contributor

Change https://go.dev/cl/595196 mentions this issue: gopls/internal/golang: splitlines: remove workaround for golang/go#68202

@findleyr
Copy link
Member

I'm not surprised we've missed this. We typically care about idents, fields, and decls only. FuncType is a somewhat arbitrary nesting.

gopherbot pushed a commit to golang/tools that referenced this issue Jun 27, 2024


There's no need for splitlines to handle FuncDecl since it
handles FuncType... now that the bug in PathEnclosingInterval
is fixed.

Updates golang/go#68202

Change-Id: I3c96535b87c62e5d2a5b68ec66fed7df50b5d6c4
Reviewed-on: https://go-review.googlesource.com/c/tools/+/595196
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
Reviewed-by: Robert Findley <rfindley@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Tools This label describes issues relating to any tools in the x/tools repository.
Projects
None yet
Development

No branches or pull requests

3 participants