x/tools/go/astutil/inspector: add Cursor.ParentEdge{Kind,Index} convenience accessors

[adonovan]

Update: see #77195 (comment) for latest.

Background: the inspector.Cursor.ParentEdge method returns both the kind and index of the edge to the cursor relative to its parent (e.g "c is the third argument to a function call"), since it is more efficient to compute both at once. However, it is common to need only the kind, resulting in statements like this one that discards the second component:

if ek, _ := cur.ParentEdge(); ek == edge.CallExpr_Args { ... }

Since these often appear as one clause in a long conjunction a && ... && z, we have an internal helper function IsChildOf:

if astutil.IsChildOf(cur, edge.SelectorExpr_Sel) { ... }

It is frequently used: there are currently 23 calls in x/tools.

Proposal: For convenience, we propose to publish it as a method of Cursor:

package inspector // golang.org/x/tools/go/ast/inspector

type Cursor ...

// IsChildOf(k) reports whether the first component of ParentEdge equals k.
func (Cursor) IsChildOf(edge.Kind) bool

[gabyhelp]

Related Issues

• proposal: x/tools/go/ast/inspector: add (*Cursor).Valid() bool method #76361
• x/tools/go/ast/inspector: add Cursor, to enable partial and multi-level traversals #70859 (closed)
• x/tools/go/ast/inspector: add Inspector.PreorderSeq and top-level func All #67795 (closed)
• proposal: x/tools/go/ast/astutil: PathContainingPos #74367 (closed)

_{(Emoji vote if this was helpful or unhelpful; more detailed feedback welcome in this discussion.)}

[DanielMorsing]

In favor of the method being moved to Cursor, but the name seems less specific than it could be. "Child of" implies that it tests against a node type, rather than an edge. Maybe the edge kind as a parameter disambiguates such that people read the meaning intended, but I find it a weird name.

Maybe ParentEdgeIs? Barring that, BikeshedColour. It's not a very strong objection :)

[adonovan]

Fair point. ParentEdge returns a pair (kind, index) whereas this method tests only the kind, so perhaps HasParentEdgeKind(k) would be more accurate, albeit less concise.

[aclements]

The "Has" part is just a == check. Does it make more sense to have a ParentEdgeKind() edge.Kind and let the caller do the == or whatever makes the most sense in context?

[adonovan]

Agreed; that is more flexible. So let's just provide both separate accessors:

package inspector // golang.org/x/tools/go/ast/inspector

type Cursor ...

// These convenience methods return the first and second components of the result of [ParentEdge].
func (Cursor) ParentEdgeKind() edge.Kind
func (Cursor) ParentEdgeIndex() int

Example: c.ParentEdgeKind() == edge.CallExpr_Args

[aclements]

This proposal has been added to the active column of the proposals project
and will now be reviewed at the weekly proposal review meetings.
— aclements for the proposal review group

[aclements]

Based on the discussion above, this proposal seems like a likely accept.
— aclements for the proposal review group

The proposal is given in #77195 (comment)