1
0
mirror of https://github.com/golang/go synced 2024-11-18 19:54:44 -07:00

internal/lsp, internal/imports: use the internal goimports library

This change modifies gopls to use the internal goimports library, which
allows us to manually configure the ProcessEnv. We also add a logger to
the ProcessEnv to allow this change not to conflict with gopls's logging
mechanism.

Fixes golang/go#32585

Change-Id: Ic9aae69c7cfbc9b1f2e66aa8d812175dbc0065ce
Reviewed-on: https://go-review.googlesource.com/c/tools/+/184198
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Ian Cottrell <iancottrell@google.com>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2019-06-28 16:21:07 -04:00
parent e47c3d98c3
commit 38ae2c8f64
10 changed files with 81 additions and 22 deletions

View File

@ -13,7 +13,6 @@ import (
"go/parser" "go/parser"
"go/token" "go/token"
"io/ioutil" "io/ioutil"
"log"
"os" "os"
"os/exec" "os/exec"
"path" "path"
@ -246,6 +245,12 @@ type pass struct {
// loadPackageNames saves the package names for everything referenced by imports. // loadPackageNames saves the package names for everything referenced by imports.
func (p *pass) loadPackageNames(imports []*importInfo) error { func (p *pass) loadPackageNames(imports []*importInfo) error {
if p.env.Debug {
p.env.Logf("loading package names for %v packages", len(imports))
defer func() {
p.env.Logf("done loading package names for %v packages", len(imports))
}()
}
var unknown []string var unknown []string
for _, imp := range imports { for _, imp := range imports {
if _, ok := p.knownPackages[imp.importPath]; ok { if _, ok := p.knownPackages[imp.importPath]; ok {
@ -313,7 +318,7 @@ func (p *pass) load() bool {
err := p.loadPackageNames(append(imports, p.candidates...)) err := p.loadPackageNames(append(imports, p.candidates...))
if err != nil { if err != nil {
if p.env.Debug { if p.env.Debug {
log.Printf("loading package names: %v", err) p.env.Logf("loading package names: %v", err)
} }
return false return false
} }
@ -443,7 +448,7 @@ func fixImportsDefault(fset *token.FileSet, f *ast.File, filename string, env *P
} }
srcDir := filepath.Dir(abs) srcDir := filepath.Dir(abs)
if env.Debug { if env.Debug {
log.Printf("fixImports(filename=%q), abs=%q, srcDir=%q ...", filename, abs, srcDir) env.Logf("fixImports(filename=%q), abs=%q, srcDir=%q ...", filename, abs, srcDir)
} }
// First pass: looking only at f, and using the naive algorithm to // First pass: looking only at f, and using the naive algorithm to
@ -512,6 +517,9 @@ type ProcessEnv struct {
// If true, use go/packages regardless of the environment. // If true, use go/packages regardless of the environment.
ForceGoPackages bool ForceGoPackages bool
// Logf is the default logger for the ProcessEnv.
Logf func(format string, args ...interface{})
resolver resolver resolver resolver
} }
@ -577,7 +585,7 @@ func (e *ProcessEnv) invokeGo(args ...string) (*bytes.Buffer, error) {
cmd.Dir = e.WorkingDir cmd.Dir = e.WorkingDir
if e.Debug { if e.Debug {
defer func(start time.Time) { log.Printf("%s for %v", time.Since(start), cmdDebugStr(cmd)) }(time.Now()) defer func(start time.Time) { e.Logf("%s for %v", time.Since(start), cmdDebugStr(cmd)) }(time.Now())
} }
if err := cmd.Run(); err != nil { if err := cmd.Run(); err != nil {
return nil, fmt.Errorf("running go: %v (stderr:\n%s)", err, stderr) return nil, fmt.Errorf("running go: %v (stderr:\n%s)", err, stderr)
@ -943,7 +951,7 @@ func VendorlessPath(ipath string) string {
// It returns nil on error or if the package name in dir does not match expectPackage. // It returns nil on error or if the package name in dir does not match expectPackage.
func loadExports(ctx context.Context, env *ProcessEnv, expectPackage string, pkg *pkg) (map[string]bool, error) { func loadExports(ctx context.Context, env *ProcessEnv, expectPackage string, pkg *pkg) (map[string]bool, error) {
if env.Debug { if env.Debug {
log.Printf("loading exports in dir %s (seeking package %s)", pkg.dir, expectPackage) env.Logf("loading exports in dir %s (seeking package %s)", pkg.dir, expectPackage)
} }
if pkg.goPackage != nil { if pkg.goPackage != nil {
exports := map[string]bool{} exports := map[string]bool{}
@ -1021,7 +1029,7 @@ func loadExports(ctx context.Context, env *ProcessEnv, expectPackage string, pkg
exportList = append(exportList, k) exportList = append(exportList, k)
} }
sort.Strings(exportList) sort.Strings(exportList)
log.Printf("loaded exports in dir %v (package %v): %v", pkg.dir, expectPackage, strings.Join(exportList, ", ")) env.Logf("loaded exports in dir %v (package %v): %v", pkg.dir, expectPackage, strings.Join(exportList, ", "))
} }
return exports, nil return exports, nil
} }
@ -1058,7 +1066,7 @@ func findImport(ctx context.Context, pass *pass, dirScan []*pkg, pkgName string,
sort.Sort(byDistanceOrImportPathShortLength(candidates)) sort.Sort(byDistanceOrImportPathShortLength(candidates))
if pass.env.Debug { if pass.env.Debug {
for i, c := range candidates { for i, c := range candidates {
log.Printf("%s candidate %d/%d: %v in %v", pkgName, i+1, len(candidates), c.pkg.importPathShort, c.pkg.dir) pass.env.Logf("%s candidate %d/%d: %v in %v", pkgName, i+1, len(candidates), c.pkg.importPathShort, c.pkg.dir)
} }
} }
@ -1098,7 +1106,7 @@ func findImport(ctx context.Context, pass *pass, dirScan []*pkg, pkgName string,
exports, err := loadExports(ctx, pass.env, pkgName, c.pkg) exports, err := loadExports(ctx, pass.env, pkgName, c.pkg)
if err != nil { if err != nil {
if pass.env.Debug { if pass.env.Debug {
log.Printf("loading exports in dir %s (seeking package %s): %v", c.pkg.dir, pkgName, err) pass.env.Logf("loading exports in dir %s (seeking package %s): %v", c.pkg.dir, pkgName, err)
} }
resc <- nil resc <- nil
return return

View File

@ -19,6 +19,7 @@ import (
"go/token" "go/token"
"io" "io"
"io/ioutil" "io/ioutil"
"log"
"regexp" "regexp"
"strconv" "strconv"
"strings" "strings"
@ -50,6 +51,11 @@ func Process(filename string, src []byte, opt *Options) ([]byte, error) {
src = b src = b
} }
// Set the logger if the user has not provided it.
if opt.Env.Logf == nil {
opt.Env.Logf = log.Printf
}
fileSet := token.NewFileSet() fileSet := token.NewFileSet()
file, adjust, err := parse(fileSet, filename, src, opt) file, adjust, err := parse(fileSet, filename, src, opt)
if err != nil { if err != nil {

View File

@ -4,7 +4,6 @@ import (
"bytes" "bytes"
"encoding/json" "encoding/json"
"io/ioutil" "io/ioutil"
"log"
"os" "os"
"path" "path"
"path/filepath" "path/filepath"
@ -63,7 +62,7 @@ func (r *moduleResolver) init() error {
} }
if mod.Dir == "" { if mod.Dir == "" {
if r.env.Debug { if r.env.Debug {
log.Printf("module %v has not been downloaded and will be ignored", mod.Path) r.env.Logf("module %v has not been downloaded and will be ignored", mod.Path)
} }
// Can't do anything with a module that's not downloaded. // Can't do anything with a module that's not downloaded.
continue continue
@ -254,7 +253,7 @@ func (r *moduleResolver) scan(_ references) ([]*pkg, error) {
modPath, err := module.DecodePath(filepath.ToSlash(matches[1])) modPath, err := module.DecodePath(filepath.ToSlash(matches[1]))
if err != nil { if err != nil {
if r.env.Debug { if r.env.Debug {
log.Printf("decoding module cache path %q: %v", subdir, err) r.env.Logf("decoding module cache path %q: %v", subdir, err)
} }
return return
} }

View File

@ -84,7 +84,7 @@ func (v *view) checkMetadata(ctx context.Context, f *goFile) (map[packageID]*met
return nil, nil, ctx.Err() return nil, nil, ctx.Err()
} }
pkgs, err := packages.Load(v.buildConfig(), fmt.Sprintf("file=%s", f.filename())) pkgs, err := packages.Load(v.Config(), fmt.Sprintf("file=%s", f.filename()))
if len(pkgs) == 0 { if len(pkgs) == 0 {
if err == nil { if err == nil {
err = fmt.Errorf("go/packages.Load: no packages found for %s", f.filename()) err = fmt.Errorf("go/packages.Load: no packages found for %s", f.filename())

View File

@ -111,7 +111,7 @@ func (v *view) Folder() span.URI {
// Config returns the configuration used for the view's interaction with the // Config returns the configuration used for the view's interaction with the
// go/packages API. It is shared across all views. // go/packages API. It is shared across all views.
func (v *view) buildConfig() *packages.Config { func (v *view) Config() *packages.Config {
// TODO: Should we cache the config and/or overlay somewhere? // TODO: Should we cache the config and/or overlay somewhere?
return &packages.Config{ return &packages.Config{
Dir: v.folder.Filename(), Dir: v.folder.Filename(),
@ -187,7 +187,7 @@ func (v *view) BuiltinPackage() *ast.Package {
// It assumes that the view is not active yet, // It assumes that the view is not active yet,
// i.e. it has not been added to the session's list of views. // i.e. it has not been added to the session's list of views.
func (v *view) buildBuiltinPkg() { func (v *view) buildBuiltinPkg() {
cfg := *v.buildConfig() cfg := *v.Config()
pkgs, _ := packages.Load(&cfg, "builtin") pkgs, _ := packages.Load(&cfg, "builtin")
if len(pkgs) != 1 { if len(pkgs) != 1 {
v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil) v.builtinPkg, _ = ast.NewPackage(cfg.Fset, nil, nil, nil)

View File

@ -100,7 +100,7 @@ func organizeImports(ctx context.Context, view source.View, s span.Span) ([]prot
if err != nil { if err != nil {
return nil, err return nil, err
} }
edits, err := source.Imports(ctx, f, rng) edits, err := source.Imports(ctx, view, f, rng)
if err != nil { if err != nil {
return nil, err return nil, err
} }

View File

@ -10,10 +10,11 @@ import (
"context" "context"
"fmt" "fmt"
"go/format" "go/format"
"strings"
"golang.org/x/tools/go/ast/astutil" "golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/go/packages" "golang.org/x/tools/go/packages"
"golang.org/x/tools/imports" "golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/lsp/diff" "golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/span" "golang.org/x/tools/internal/span"
) )
@ -33,12 +34,14 @@ func Format(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error) {
return nil, fmt.Errorf("no exact AST node matching the specified range") return nil, fmt.Errorf("no exact AST node matching the specified range")
} }
node := path[0] node := path[0]
fset := f.FileSet()
buf := &bytes.Buffer{}
// format.Node changes slightly from one release to another, so the version // format.Node changes slightly from one release to another, so the version
// of Go used to build the LSP server will determine how it formats code. // of Go used to build the LSP server will determine how it formats code.
// This should be acceptable for all users, who likely be prompted to rebuild // This should be acceptable for all users, who likely be prompted to rebuild
// the LSP server on each Go release. // the LSP server on each Go release.
fset := f.FileSet()
buf := &bytes.Buffer{}
if err := format.Node(buf, fset, node); err != nil { if err := format.Node(buf, fset, node); err != nil {
return nil, err return nil, err
} }
@ -46,7 +49,7 @@ func Format(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error) {
} }
// Imports formats a file using the goimports tool. // Imports formats a file using the goimports tool.
func Imports(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error) { func Imports(ctx context.Context, view View, f GoFile, rng span.Range) ([]TextEdit, error) {
data, _, err := f.Handle(ctx).Read(ctx) data, _, err := f.Handle(ctx).Read(ctx)
if err != nil { if err != nil {
return nil, err return nil, err
@ -58,7 +61,17 @@ func Imports(ctx context.Context, f GoFile, rng span.Range) ([]TextEdit, error)
if hasListErrors(pkg.GetErrors()) { if hasListErrors(pkg.GetErrors()) {
return nil, fmt.Errorf("%s has list errors, not running goimports", f.URI()) return nil, fmt.Errorf("%s has list errors, not running goimports", f.URI())
} }
formatted, err := imports.Process(f.URI().Filename(), data, nil) options := &imports.Options{
Env: buildProcessEnv(ctx, view),
// Defaults.
AllErrors: true,
Comments: true,
Fragment: true,
FormatOnly: false,
TabIndent: true,
TabWidth: 8,
}
formatted, err := imports.Process(f.URI().Filename(), data, options)
if err != nil { if err != nil {
return nil, err return nil, err
} }
@ -83,6 +96,37 @@ func hasListErrors(errors []packages.Error) bool {
return false return false
} }
func buildProcessEnv(ctx context.Context, view View) *imports.ProcessEnv {
cfg := view.Config()
env := &imports.ProcessEnv{
WorkingDir: cfg.Dir,
Logf: func(format string, v ...interface{}) {
view.Session().Logger().Infof(ctx, format, v...)
},
}
for _, kv := range cfg.Env {
split := strings.Split(kv, "=")
if len(split) < 2 {
continue
}
switch split[0] {
case "GOPATH":
env.GOPATH = split[1]
case "GOROOT":
env.GOROOT = split[1]
case "GO111MODULE":
env.GO111MODULE = split[1]
case "GOPROXY":
env.GOROOT = split[1]
case "GOFLAGS":
env.GOFLAGS = split[1]
case "GOSUMDB":
env.GOSUMDB = split[1]
}
}
return env
}
func computeTextEdits(ctx context.Context, file File, formatted string) (edits []TextEdit) { func computeTextEdits(ctx context.Context, file File, formatted string) (edits []TextEdit) {
data, _, err := file.Handle(ctx).Read(ctx) data, _, err := file.Handle(ctx).Read(ctx)
if err != nil { if err != nil {

View File

@ -343,7 +343,7 @@ func (r *runner) Import(t *testing.T, data tests.Imports) {
if err != nil { if err != nil {
t.Fatalf("failed for %v: %v", spn, err) t.Fatalf("failed for %v: %v", spn, err)
} }
edits, err := source.Imports(ctx, f.(source.GoFile), rng) edits, err := source.Imports(ctx, r.view, f.(source.GoFile), rng)
if err != nil { if err != nil {
if goimported != "" { if goimported != "" {
t.Error(err) t.Error(err)

View File

@ -209,6 +209,8 @@ type View interface {
// Ignore returns true if this file should be ignored by this view. // Ignore returns true if this file should be ignored by this view.
Ignore(span.URI) bool Ignore(span.URI) bool
Config() *packages.Config
} }
// File represents a source file of any type. // File represents a source file of any type.

View File

@ -25,7 +25,7 @@ import (
// We hardcode the expected number of test cases to ensure that all tests // We hardcode the expected number of test cases to ensure that all tests
// are being executed. If a test is added, this number must be changed. // are being executed. If a test is added, this number must be changed.
const ( const (
ExpectedCompletionsCount = 137 ExpectedCompletionsCount = 138
ExpectedCompletionSnippetCount = 15 ExpectedCompletionSnippetCount = 15
ExpectedDiagnosticsCount = 17 ExpectedDiagnosticsCount = 17
ExpectedFormatCount = 5 ExpectedFormatCount = 5