mirror
/
go
mirror of https://github.com/golang/go.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

internal/lsp: prefer inserting imports into existing blocks 

Insert imports into existing multiline import declarations when
possible.

Logic for choosing where to insert taken from
golang.org/x/tools/go/ast/astutil.

Change-Id: Ie5ad96d5e3d5db2e92a2c05a6104d14a4a192ed3
Reviewed-on: https://go-review.googlesource.com/c/tools/+/190598
Run-TryBot: Suzy Mueller <suzmue@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
This commit is contained in:
Suzy Mueller 2019-08-16 15:02:11 -04:00
parent 6889da9d54
commit d308a98e2e
3 changed files with 156 additions and 39 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
internal/lsp/source/completion_format.go
View File

@ -89,7 +89,7 @@ func (c *completer) item(cand candidate) (CompletionItem, error) {
// If this candidate needs an additional import statement,
// add the additional text edits needed.
if cand.imp != nil {
edit, err := AddNamedImport(c.view.Session().Cache().FileSet(), c.file, cand.imp.Name, cand.imp.ImportPath)
edit, err := addNamedImport(c.view.Session().Cache().FileSet(), c.file, cand.imp.Name, cand.imp.ImportPath)
if err != nil {
return CompletionItem{}, err
}

183
internal/lsp/source/imports.go
View File

@ -6,10 +6,12 @@ package source
import (
"bytes"
"fmt"
"go/ast"
"go/format"
"go/token"
"strconv"
"strings"
"golang.org/x/tools/internal/span"
)
@ -24,16 +26,16 @@ import (
// astutil.AddNamedImport makes it hard to determine what changes need to be made
// to the source document from the ast, as astutil.AddNamedImport includes a merging pass.
// AddNamedImport adds the import with the given name and path to the file f, if absent.
// addNamedImport adds the import with the given name and path to the file f, if absent.
// If name is not empty, it is used to rename the import.
//
// For example, calling
// AddNamedImport(fset, f, "pathpkg", "path")
// addNamedImport(fset, f, "pathpkg", "path")
// adds
// import pathpkg "path"
//
// AddNamedImport only returns edits that affect the import declarations.
func AddNamedImport(fset *token.FileSet, f *ast.File, name, path string) (edits []TextEdit, err error) {
// addNamedImport only returns edits that affect the import declarations.
func addNamedImport(fset *token.FileSet, f *ast.File, name, path string) (edits []TextEdit, err error) {
if alreadyImports(f, name, path) {
return nil, nil
}
@ -48,48 +50,128 @@ func AddNamedImport(fset *token.FileSet, f *ast.File, name, path string) (edits
newImport.Name = &ast.Ident{Name: name}
}
// TODO(suzmue): insert the import statement in the location that would be chosen
// by astutil.AddNamedImport
// Find the last import decl.
var lastImport = -1 // index in f.Decls of the file's final import decl
// Find an import decl to add to.
// The goal is to find an existing import
// whose import path has the longest shared
// prefix with path.
var (
bestMatch = -1 // length of longest shared prefix
lastImport = -1 // index in f.Decls of the file's final import decl
impDecl *ast.GenDecl // import decl containing the best match
impIndex = -1 // spec index in impDecl containing the best match
isThirdPartyPath = isThirdParty(path)
)
for i, decl := range f.Decls {
gen, ok := decl.(*ast.GenDecl)
if ok && gen.Tok == token.IMPORT {
lastImport = i
// Do not add to import "C", to avoid disrupting the
// association with its doc comment, breaking cgo.
if declImports(gen, "C") {
continue
}
// Do not add to single imports.
if !gen.Lparen.IsValid() {
continue
}
// Match an empty import decl if that's all that is available.
if len(gen.Specs) == 0 && bestMatch == -1 {
impDecl = gen
}
// Compute longest shared prefix with imports in this group and find best
// matched import spec.
// 1. Always prefer import spec with longest shared prefix.
// 2. While match length is 0,
// - for stdlib package: prefer first import spec.
// - for third party package: prefer first third party import spec.
// We cannot use last import spec as best match for third party package
// because grouped imports are usually placed last by goimports -local
// flag.
// See issue #19190.
seenAnyThirdParty := false
for j, spec := range gen.Specs {
impspec := spec.(*ast.ImportSpec)
p := importPath(impspec)
n := matchLen(p, path)
if n > bestMatch || (bestMatch == 0 && !seenAnyThirdParty && isThirdPartyPath) {
bestMatch = n
impDecl = gen
impIndex = j
}
seenAnyThirdParty = seenAnyThirdParty || isThirdParty(p)
}
}
}
// Add an import decl after the last import.
impDecl := &ast.GenDecl{
Tok: token.IMPORT,
}
impDecl.Specs = append(impDecl.Specs, newImport)
var insertPos token.Pos
if lastImport >= 0 {
insertPos = f.Decls[lastImport].End()
} else {
// There are no existing imports.
// Our new import, preceded by a blank line, goes after the package declaration
// and after the comment, if any, that starts on the same line as the
// package declaration.
insertPos = f.Name.End()
var newText string
// If no import decl found, add one after the last import.
if impDecl == nil {
// Add an import decl after the last import.
impDecl = &ast.GenDecl{
Tok: token.IMPORT,
}
impDecl.Specs = append(impDecl.Specs, newImport)
file := fset.File(f.Package)
pkgLine := file.Line(f.Package)
for _, c := range f.Comments {
if file.Line(c.Pos()) > pkgLine {
break
if lastImport >= 0 {
insertPos = f.Decls[lastImport].End()
} else {
// There are no existing imports.
// Our new import goes after the package declaration.
insertPos = f.Name.End()
}
// Print the whole import declaration.
newText = fmt.Sprintf("\n\nimport %s", printImportSpec(fset, newImport))
} else {
// Insert new import at insertAt.
insertAt := 0
if impIndex >= 0 {
// insert after the found import
insertAt = impIndex + 1
}
insertPos = impDecl.Lparen + 1 // insert after the parenthesis
if len(impDecl.Specs) > 0 {
insertPos = impDecl.Specs[0].Pos() // insert at the beginning
}
if insertAt > 0 {
// If there is a comment after an existing import, preserve the comment
// position by adding the new import after the comment.
if spec, ok := impDecl.Specs[insertAt-1].(*ast.ImportSpec); ok && spec.Comment != nil {
insertPos = spec.Comment.End()
} else {
// Assign same position as the previous import,
// so that the sorter sees it as being in the same block.
insertPos = impDecl.Specs[insertAt-1].End()
}
}
// Print this import declaration.
newText = fmt.Sprintf("\n\t%s", printImportSpec(fset, newImport))
}
// If we didn't find a valid insert position, return no edits.
if !insertPos.IsValid() {
return edits, nil
}
// Make sure that we are printed after any comments that start on the same line.
file := fset.File(insertPos)
pkgLine := file.Line(insertPos)
for _, c := range f.Comments {
if file.Line(c.Pos()) > pkgLine {
break
}
if c.End() > insertPos {
insertPos = c.End()
}
}
// Print this import declaration.
var buf bytes.Buffer
format.Node(&buf, fset, impDecl)
newText := "\n\n" + buf.String() + "\n"
rng := span.NewRange(fset, insertPos, insertPos)
spn, err := rng.Span()
if err != nil {
@ -103,6 +185,18 @@ func AddNamedImport(fset *token.FileSet, f *ast.File, name, path string) (edits
return edits, nil
}
func printImportSpec(fset *token.FileSet, spec *ast.ImportSpec) string {
var buf bytes.Buffer
format.Node(&buf, fset, spec)
return buf.String()
}
func isThirdParty(importPath string) bool {
// Third party package import path usually contains "." (".com", ".org", ...)
// This logic is taken from golang.org/x/tools/imports package.
return strings.Contains(importPath, ".")
}
// alreadyImports reports whether f has an import with the specified name and path.
func alreadyImports(f *ast.File, name, path string) bool {
for _, s := range f.Imports {
@ -131,3 +225,28 @@ func importPath(s *ast.ImportSpec) string {
}
return t
}
// declImports reports whether gen contains an import of path.
func declImports(gen *ast.GenDecl, path string) bool {
if gen.Tok != token.IMPORT {
return false
}
for _, spec := range gen.Specs {
impspec := spec.(*ast.ImportSpec)
if importPath(impspec) == path {
return true
}
}
return false
}
// matchLen returns the length of the longest path segment prefix shared by x and y.
func matchLen(x, y string) int {
n := 0
for i := 0; i < len(x) && i < len(y) && x[i] == y[i]; i++ {
if x[i] == '/' {
n++
}
}
return n
}

10
internal/lsp/source/imports_test.go
View File

@ -7,7 +7,6 @@ import (
"go/format"
"go/parser"
"go/token"
"log"
"testing"
)
@ -203,7 +202,7 @@ func TestAddImport(t *testing.T) {
file := parse(t, test.name, test.in)
var before bytes.Buffer
ast.Fprint(&before, fset, file, nil)
edits, err := AddNamedImport(fset, file, test.renamedPkg, test.pkg)
edits, err := addNamedImport(fset, file, test.renamedPkg, test.pkg)
if err != nil && !test.unchanged {
t.Errorf("error adding import: %s", err)
continue
@ -217,7 +216,7 @@ func TestAddImport(t *testing.T) {
// AddNamedImport should be idempotent. Verify that by calling it again,
// expecting no change to the AST, and the returned added value to always be false.
edits, err = AddNamedImport(fset, gotFile, test.renamedPkg, test.pkg)
edits, err = addNamedImport(fset, gotFile, test.renamedPkg, test.pkg)
if err != nil && !test.unchanged {
t.Errorf("error adding import: %s", err)
continue
@ -236,17 +235,16 @@ func TestDoubleAddNamedImport(t *testing.T) {
in := "package main\n"
file := parse(t, name, in)
// Add a named import
edits, err := AddNamedImport(fset, file, "o", "os")
edits, err := addNamedImport(fset, file, "o", "os")
if err != nil {
t.Errorf("error adding import: %s", err)
return
}
got := applyEdits(in, edits)
log.Println(got)
gotFile := parse(t, name, got)
// Add a second named import
edits, err = AddNamedImport(fset, gotFile, "i", "io")
edits, err = addNamedImport(fset, gotFile, "i", "io")
if err != nil {
t.Errorf("error adding import: %s", err)
return