Skip to content

Commit

Permalink
gopls/internal/golang: don't lose ... when split/joining variadics
Browse files Browse the repository at this point in the history
We wrap the last argument f(x...) in an explicit Ellipsis
to record that it was a variadic call, and add the "..."
back with corresponding logic when creating the edits.

Fixes golang/go#70519

Change-Id: I1fdfa5f3ccb000c9622f856ed7703b31d7911620
Reviewed-on: https://go-review.googlesource.com/c/tools/+/631335
Auto-Submit: Alan Donovan <adonovan@google.com>
Reviewed-by: Robert Findley <rfindley@google.com>
LUCI-TryBot-Result: Go LUCI <golang-scoped@luci-project-accounts.iam.gserviceaccount.com>
  • Loading branch information
adonovan committed Nov 22, 2024
1 parent 1e0d4ee commit 68caf84
Show file tree
Hide file tree
Showing 3 changed files with 78 additions and 1 deletion.
3 changes: 2 additions & 1 deletion gopls/internal/golang/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,8 @@ import (
// The supplied token positions (start, end) must belong to
// pkg.FileSet(), and the returned positions
// (SuggestedFix.TextEdits[*].{Pos,End}) must belong to the returned
// FileSet.
// FileSet, which is not necessarily the same.
// (See [insertDeclsAfter] for explanation.)
//
// A fixer may return (nil, nil) if no fix is available.
type fixer func(ctx context.Context, s *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error)
Expand Down
21 changes: 21 additions & 0 deletions gopls/internal/golang/lines.go
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,15 @@ func processLines(fset *token.FileSet, items []ast.Node, comments []*ast.Comment
}

edits = append(edits, analysis.TextEdit{Pos: pos, End: end, NewText: []byte(sep + indent)})

// Print the Ellipsis if we synthesized one earlier.
if is[*ast.Ellipsis](nodes[i]) {
edits = append(edits, analysis.TextEdit{
Pos: nodes[i].End(),
End: nodes[i].End(),
NewText: []byte("..."),
})
}
}

return &analysis.SuggestedFix{TextEdits: edits}
Expand Down Expand Up @@ -205,6 +214,18 @@ func findSplitJoinTarget(fset *token.FileSet, file *ast.File, src []byte, start,
for _, arg := range node.Args {
items = append(items, arg)
}

// Preserve "..." by wrapping the last
// argument in an Ellipsis node
// with the same Pos/End as the argument.
// See corresponding logic in processLines.
if node.Ellipsis.IsValid() {
last := &items[len(items)-1]
*last = &ast.Ellipsis{
Ellipsis: (*last).Pos(), // determines Ellipsis.Pos()
Elt: (*last).(ast.Expr), // determines Ellipsis.End()
}
}
case *ast.CompositeLit:
for _, arg := range node.Elts {
items = append(items, arg)
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
This is a regression test for #70519, in which the ellipsis
of a variadic call would go missing after split/join lines.

-- go.mod --
module example.com
go 1.18

-- a/a.go --
package a

var a, b, c []any
func f(any, any, ...any)

func _() {
f(a, b, c...) //@codeaction("a", "refactor.rewrite.splitLines", result=split)

f(
a,
b,
c..., /*@codeaction("c", "refactor.rewrite.joinLines", result=joined)*/
)
}

-- @split/a/a.go --
package a

var a, b, c []any
func f(any, any, ...any)

func _() {
f(
a,
b,
c...,
) //@codeaction("a", "refactor.rewrite.splitLines", result=split)

f(
a,
b,
c..., /*@codeaction("c", "refactor.rewrite.joinLines", result=joined)*/
)
}

-- @joined/a/a.go --
package a

var a, b, c []any
func f(any, any, ...any)

func _() {
f(a, b, c...) //@codeaction("a", "refactor.rewrite.splitLines", result=split)

f(a, b, c..., /*@codeaction("c", "refactor.rewrite.joinLines", result=joined)*/)
}

0 comments on commit 68caf84

Please sign in to comment.