gopls: use safetoken.Line to compute line numbers

Replace the use of *tokenFile.Line with safetoken.Line. The former adjusts for line directives, which gopls is ignoring. Fixes: golang.go/#59124 Change-Id: Ida3e7520669bcca998d4f696d728fc5f82e2db46 Reviewed-on: https://go-review.googlesource.com/c/tools/+/480515 Run-TryBot: Peter Weinberger <[email protected]> TryBot-Result: Gopher Robot <[email protected]> gopls-CI: kokoro <[email protected]> Reviewed-by: Robert Findley <[email protected]>
golang · Mar 31, 2023 · 0a3e5f0 · 0a3e5f0
1 parent f4e613e
commit 0a3e5f0
Show file tree

Hide file tree

Showing 12 changed files with 57 additions and 20 deletions.
diff --git a/gopls/internal/lsp/cache/parse.go b/gopls/internal/lsp/cache/parse.go
@@ -298,7 +298,7 @@ func fixMissingCurlies(f *ast.File, b *ast.BlockStmt, parent ast.Node, tok *toke
 		}
 	}
 
-	parentLine := tok.Line(parent.Pos())
+	parentLine := safetoken.Line(tok, parent.Pos())
 
 	if parentLine >= tok.LineCount() {
 		// If we are the last line in the file, no need to fix anything.
@@ -411,7 +411,7 @@ func fixEmptySwitch(body *ast.BlockStmt, tok *token.File, src []byte) bool {
 		return false
 	}
 
-	braceLine := tok.Line(body.Rbrace)
+	braceLine := safetoken.Line(tok, body.Rbrace)
 	if braceLine >= tok.LineCount() {
 		// If we are the last line in the file, no need to fix anything.
 		return false
@@ -748,7 +748,7 @@ FindTo:
 		// the period is likely a dangling selector and needs a phantom
 		// "_". Likewise if the current token is on a different line than
 		// the period, the period is likely a dangling selector.
-		if lastToken == token.PERIOD && (tkn == token.RBRACE || tok.Line(to) > tok.Line(last)) {
+		if lastToken == token.PERIOD && (tkn == token.RBRACE || safetoken.Line(tok, to) > safetoken.Line(tok, last)) {
 			// Insert phantom "_" selector after the dangling ".".
 			phantomSelectors = append(phantomSelectors, last+1)
 			// If we aren't in a block then end the expression after the ".".

diff --git a/gopls/internal/lsp/safetoken/safetoken.go b/gopls/internal/lsp/safetoken/safetoken.go
@@ -91,6 +91,11 @@ func Position(f *token.File, pos token.Pos) token.Position {
 	return f.PositionFor(pos, false)
 }
 
+// Line returns the line number for the given offset in the given file.
+func Line(f *token.File, pos token.Pos) int {
+	return Position(f, pos).Line
+}
+
 // StartPosition converts a start Pos in the FileSet into a Position.
 //
 // Call this function only if start represents the start of a token or

diff --git a/gopls/internal/lsp/safetoken/safetoken_test.go b/gopls/internal/lsp/safetoken/safetoken_test.go
@@ -98,6 +98,7 @@ func TestGoplsSourceDoesNotCallTokenFileMethods(t *testing.T) {
 		oldMethod, _, _ := types.LookupFieldOrMethod(recv.Type(), true, recv.Pkg(), old)
 		alternative[oldMethod] = new
 	}
+	setAlternative(File, "Line", "safetoken.Line")
 	setAlternative(File, "Offset", "safetoken.Offset")
 	setAlternative(File, "Position", "safetoken.Position")
 	setAlternative(File, "PositionFor", "safetoken.Position")

diff --git a/gopls/internal/lsp/semantic.go b/gopls/internal/lsp/semantic.go
@@ -258,7 +258,7 @@ func (e *encoded) strStack() string {
 // find the line in the source
 func (e *encoded) srcLine(x ast.Node) string {
 	file := e.pgf.Tok
-	line := file.Line(x.Pos())
+	line := safetoken.Line(file, x.Pos())
 	start, err := safetoken.Offset(file, file.LineStart(line))
 	if err != nil {
 		return ""

diff --git a/gopls/internal/lsp/source/completion/completion.go b/gopls/internal/lsp/source/completion/completion.go
@@ -916,14 +916,14 @@ func (c *completer) populateCommentCompletions(ctx context.Context, comment *ast
 	// comment itself.
 	c.opts.documentation = false
 
-	commentLine := file.Line(comment.End())
+	commentLine := safetoken.Line(file, comment.End())
 
 	// comment is valid, set surrounding as word boundaries around cursor
 	c.setSurroundingForComment(comment)
 
 	// Using the next line pos, grab and parse the exported symbol on that line
 	for _, n := range c.file.Decls {
-		declLine := file.Line(n.Pos())
+		declLine := safetoken.Line(file, n.Pos())
 		// if the comment is not in, directly above or on the same line as a declaration
 		if declLine != commentLine && declLine != commentLine+1 &&
 			!(n.Pos() <= comment.Pos() && comment.End() <= n.End()) {

diff --git a/gopls/internal/lsp/source/completion/package.go b/gopls/internal/lsp/source/completion/package.go
@@ -108,7 +108,7 @@ func packageCompletionSurrounding(pgf *source.ParsedGoFile, offset int) (*Select
 	// appear on any line of the file as long as it's the first code expression
 	// in the file.
 	lines := strings.Split(string(pgf.Src), "\n")
-	cursorLine := tok.Line(cursor)
+	cursorLine := safetoken.Line(tok, cursor)
 	if cursorLine <= 0 || cursorLine > len(lines) {
 		return nil, fmt.Errorf("invalid line number")
 	}

diff --git a/gopls/internal/lsp/source/completion/postfix_snippets.go b/gopls/internal/lsp/source/completion/postfix_snippets.go
@@ -17,6 +17,7 @@ import (
 	"text/template"
 
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
+	"golang.org/x/tools/gopls/internal/lsp/safetoken"
 	"golang.org/x/tools/gopls/internal/lsp/snippet"
 	"golang.org/x/tools/gopls/internal/lsp/source"
 	"golang.org/x/tools/internal/event"
@@ -340,7 +341,7 @@ func (c *completer) addPostfixSnippetCandidates(ctx context.Context, sel *ast.Se
 				//
 				// detect that "foo." makes up the entire statement since the
 				// apparent selector spans lines.
-				stmtOK = tokFile.Line(c.pos) < tokFile.Line(p.TokPos)
+				stmtOK = safetoken.Line(tokFile, c.pos) < safetoken.Line(tokFile, p.TokPos)
 			}
 			break
 		}
@@ -362,8 +363,8 @@ func (c *completer) addPostfixSnippetCandidates(ctx context.Context, sel *ast.Se
 	//
 	// and adjust afterDot so that we don't mistakenly delete the
 	// newline thinking "bar" is part of our selector.
-	if startLine := tokFile.Line(sel.Pos()); startLine != tokFile.Line(afterDot) {
-		if tokFile.Line(c.pos) != startLine {
+	if startLine := safetoken.Line(tokFile, sel.Pos()); startLine != safetoken.Line(tokFile, afterDot) {
+		if safetoken.Line(tokFile, c.pos) != startLine {
 			return
 		}
 		afterDot = c.pos

diff --git a/gopls/internal/lsp/source/extract.go b/gopls/internal/lsp/source/extract.go
@@ -132,7 +132,7 @@ func CanExtractVariable(start, end token.Pos, file *ast.File) (ast.Expr, []ast.N
 // formatting (i.e. the proper indentation). To do so, we observe the indentation on the
 // line of code on which the insertion occurs.
 func calculateIndentation(content []byte, tok *token.File, insertBeforeStmt ast.Node) (string, error) {
-	line := tok.Line(insertBeforeStmt.Pos())
+	line := safetoken.Line(tok, insertBeforeStmt.Pos())
 	lineOffset, stmtOffset, err := safetoken.Offsets(tok, tok.LineStart(line), insertBeforeStmt.Pos())
 	if err != nil {
 		return "", err

diff --git a/gopls/internal/lsp/source/folding_range.go b/gopls/internal/lsp/source/folding_range.go
@@ -12,6 +12,7 @@ import (
 	"strings"
 
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
+	"golang.org/x/tools/gopls/internal/lsp/safetoken"
 	"golang.org/x/tools/internal/bug"
 )
 
@@ -124,7 +125,7 @@ func foldingRangeFunc(pgf *ParsedGoFile, n ast.Node, lineFoldingOnly bool) *Fold
 		return nil
 	}
 	// in line folding mode, do not fold if the start and end lines are the same.
-	if lineFoldingOnly && pgf.Tok.Line(start) == pgf.Tok.Line(end) {
+	if lineFoldingOnly && safetoken.Line(pgf.Tok, start) == safetoken.Line(pgf.Tok, end) {
 		return nil
 	}
 	mrng, err := pgf.PosMappedRange(start, end)
@@ -149,8 +150,8 @@ func validLineFoldingRange(tokFile *token.File, open, close, start, end token.Po
 		// as an example, the example below should *not* fold:
 		// var x = [2]string{"d",
 		// "e" }
-		if tokFile.Line(open) == tokFile.Line(start) ||
-			tokFile.Line(close) == tokFile.Line(end) {
+		if safetoken.Line(tokFile, open) == safetoken.Line(tokFile, start) ||
+			safetoken.Line(tokFile, close) == safetoken.Line(tokFile, end) {
 			return token.NoPos, token.NoPos
 		}
 
@@ -165,15 +166,15 @@ func validLineFoldingRange(tokFile *token.File, open, close, start, end token.Po
 func commentsFoldingRange(pgf *ParsedGoFile) (comments []*FoldingRangeInfo) {
 	tokFile := pgf.Tok
 	for _, commentGrp := range pgf.File.Comments {
-		startGrpLine, endGrpLine := tokFile.Line(commentGrp.Pos()), tokFile.Line(commentGrp.End())
+		startGrpLine, endGrpLine := safetoken.Line(tokFile, commentGrp.Pos()), safetoken.Line(tokFile, commentGrp.End())
 		if startGrpLine == endGrpLine {
 			// Don't fold single line comments.
 			continue
 		}
 
 		firstComment := commentGrp.List[0]
 		startPos, endLinePos := firstComment.Pos(), firstComment.End()
-		startCmmntLine, endCmmntLine := tokFile.Line(startPos), tokFile.Line(endLinePos)
+		startCmmntLine, endCmmntLine := safetoken.Line(tokFile, startPos), safetoken.Line(tokFile, endLinePos)
 		if startCmmntLine != endCmmntLine {
 			// If the first comment spans multiple lines, then we want to have the
 			// folding range start at the end of the first line.

diff --git a/gopls/internal/lsp/source/format.go b/gopls/internal/lsp/source/format.go
@@ -238,7 +238,7 @@ func importPrefix(src []byte) (string, error) {
 		// specifically, in the text of a comment, it will strip out \r\n line
 		// endings in favor of \n. To account for these differences, we try to
 		// return a position on the next line whenever possible.
-		switch line := tok.Line(tok.Pos(offset)); {
+		switch line := safetoken.Line(tok, tok.Pos(offset)); {
 		case line < tok.LineCount():
 			nextLineOffset, err := safetoken.Offset(tok, tok.LineStart(line+1))
 			if err != nil {

diff --git a/gopls/internal/lsp/source/rename.go b/gopls/internal/lsp/source/rename.go
@@ -1113,7 +1113,7 @@ func (r *renamer) update() (map[span.URI][]diff.Edit, error) {
 			// Just run the loop body once over the entire multiline comment.
 			lines := strings.Split(comment.Text, "\n")
 			tokFile := pgf.Tok
-			commentLine := tokFile.Line(comment.Pos())
+			commentLine := safetoken.Line(tokFile, comment.Pos())
 			uri := span.URIFromPath(tokFile.Name())
 			for i, line := range lines {
 				lineStart := comment.Pos()
@@ -1165,14 +1165,14 @@ func docComment(pgf *ParsedGoFile, id *ast.Ident) *ast.CommentGroup {
 				return nil
 			}
 
-			identLine := pgf.Tok.Line(id.Pos())
+			identLine := safetoken.Line(pgf.Tok, id.Pos())
 			for _, comment := range nodes[len(nodes)-1].(*ast.File).Comments {
 				if comment.Pos() > id.Pos() {
 					// Comment is after the identifier.
 					continue
 				}
 
-				lastCommentLine := pgf.Tok.Line(comment.End())
+				lastCommentLine := safetoken.Line(pgf.Tok, comment.End())
 				if lastCommentLine+1 == identLine {
 					return comment
 				}

diff --git a/gopls/internal/regtest/misc/imports_test.go b/gopls/internal/regtest/misc/imports_test.go
@@ -12,6 +12,7 @@ import (
 	"testing"
 
 	. "golang.org/x/tools/gopls/internal/lsp/regtest"
+	"golang.org/x/tools/gopls/internal/lsp/tests/compare"
 
 	"golang.org/x/tools/gopls/internal/lsp/protocol"
 	"golang.org/x/tools/internal/testenv"
@@ -54,6 +55,34 @@ func TestZ(t *testing.T) {
 	})
 }
 
+func TestIssue59124(t *testing.T) {
+	const stuff = `
+-- go.mod --
+module foo
+go 1.29
+-- a.go --
+//line foo.y:102
+package main
+
+import "fmt"
+
+//this comment is necessary for failure
+func a() {
+	fmt.Println("hello")
+}
+`
+	Run(t, stuff, func(t *testing.T, env *Env) {
+		env.OpenFile("a.go")
+		was := env.BufferText("a.go")
+		env.Await(NoDiagnostics())
+		env.OrganizeImports("a.go")
+		is := env.BufferText("a.go")
+		if diff := compare.Text(was, is); diff != "" {
+			t.Errorf("unexpected diff after organizeImports:\n%s", diff)
+		}
+	})
+}
+
 func TestVim1(t *testing.T) {
 	const vim1 = `package main