qbit/go
1
0
Fork 0
mirror of https://github.com/golang/go synced 2024-11-18 13:04:46 -07:00
Code Releases Activity

internal/lsp/cache: refactor go mod tidy error logic

Browse Source 
This CL is mostly a refactoring of the logic to compute errors for
`go mod tidy` diagnostics. It had been getting a little confusing, so
hopefully this makes things easier to read.

I made a few other small changes, such as slightly changing a few error
messages and showing diagnostics in the go.mod file for all missing
modules.

Change-Id: Ia8cf580731b997248591a2d64dff133accd9c5aa
Reviewed-on: https://go-review.googlesource.com/c/tools/+/244610
Run-TryBot: Rebecca Stambler <rstambler@golang.org>
TryBot-Result: Gobot Gobot <gobot@golang.org>
Reviewed-by: Heschi Kreinick <heschi@google.com>
This commit is contained in:
Rebecca Stambler 2020-07-26 02:55:12 -04:00
parent 9221131678
commit c05a0f5be4
3 changed files with 238 additions and 287 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

514
internal/lsp/cache/mod_tidy.go vendored
View File

@ -16,6 +16,7 @@ import (
"golang.org/x/mod/modfile"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/lsp/debug/tag"
"golang.org/x/tools/internal/lsp/diff"
"golang.org/x/tools/internal/lsp/protocol"
"golang.org/x/tools/internal/lsp/source"
"golang.org/x/tools/internal/memoize"
@ -78,7 +79,6 @@ func (s *snapshot) ModTidy(ctx context.Context) (*source.TidiedModule, error) {
var (
modURI = s.view.modURI
cfg = s.config(ctx)
options = s.view.Options()
)
key := modTidyKey{
sessionID: s.view.session.id,
@ -127,46 +127,17 @@ func (s *snapshot) ModTidy(ctx context.Context) (*source.TidiedModule, error) {
// since it has been "tidied".
return &modTidyData{err: err}
}
// Get the dependencies that are different between the original and
// ideal go.mod files.
unusedDeps := make(map[string]*modfile.Require, len(pm.File.Require))
missingDeps := make(map[string]*modfile.Require, len(ideal.Require))
for _, req := range pm.File.Require {
unusedDeps[req.Mod.Path] = req
}
for _, req := range ideal.Require {
origDep := unusedDeps[req.Mod.Path]
if origDep != nil && origDep.Indirect == req.Indirect {
delete(unusedDeps, req.Mod.Path)
} else {
missingDeps[req.Mod.Path] = req
}
}
// First, compute any errors specific to the go.mod file. These include
// unused dependencies and modules with incorrect // indirect comments.
/// Both the diagnostic and the fix will appear on the go.mod file.
modRequireErrs, err := modRequireErrors(pm.Mapper, missingDeps, unusedDeps, options)
if err != nil {
return &modTidyData{err: err}
}
for _, req := range missingDeps {
if unusedDeps[req.Mod.Path] != nil {
delete(missingDeps, req.Mod.Path)
}
}
// Next, compute any diagnostics for modules that are missing from the
// go.mod file. The fixes will be for the go.mod file, but the
// diagnostics should appear on the import statements in the Go or
// go.mod files.
missingModuleErrs, err := missingModuleErrors(ctx, snapshot, pm.Mapper, workspacePkgs, ideal.Require, missingDeps, pm.File, options)
// Compare the original and tidied go.mod files to compute errors and
// suggested fixes.
errors, err := modTidyErrors(ctx, snapshot, pm, ideal, workspacePkgs)
if err != nil {
return &modTidyData{err: err}
}
return &modTidyData{
tidied: &source.TidiedModule{
Errors: errors,
Parsed: pm,
TidiedContent: tempContents,
Errors: append(modRequireErrs, missingModuleErrs...),
},
}
})
@ -196,58 +167,164 @@ func hashImports(ctx context.Context, wsPackages []source.Package) (string, erro
return hashContents([]byte(hashed)), nil
}
// modRequireErrors extracts the errors that occur on the require directives.
// It checks for directness issues and unused dependencies.
func modRequireErrors(m *protocol.ColumnMapper, missingDeps, unusedDeps map[string]*modfile.Require, options source.Options) ([]source.Error, error) {
var errors []source.Error
for dep, req := range unusedDeps {
if req.Syntax == nil {
// modTidyErrors computes the differences between the original and tidied
// go.mod files to produce diagnostic and suggested fixes. Some diagnostics
// may appear on the Go files that import packages from missing modules.
func modTidyErrors(ctx context.Context, snapshot source.Snapshot, pm *source.ParsedModule, ideal *modfile.File, workspacePkgs []source.Package) (errors []source.Error, err error) {
// First, determine which modules are unused and which are missing from the
// original go.mod file.
var (
unused = make(map[string]*modfile.Require, len(pm.File.Require))
missing = make(map[string]*modfile.Require, len(ideal.Require))
wrongDirectness = make(map[string]*modfile.Require, len(pm.File.Require))
)
for _, req := range pm.File.Require {
unused[req.Mod.Path] = req
}
for _, req := range ideal.Require {
origReq := unused[req.Mod.Path]
if origReq == nil {
missing[req.Mod.Path] = req
continue
} else if origReq.Indirect != req.Indirect {
wrongDirectness[req.Mod.Path] = origReq
}
delete(unused, req.Mod.Path)
}
for _, req := range unused {
srcErr, err := unusedError(pm.Mapper, req, snapshot.View().Options().ComputeEdits)
if err != nil {
return nil, err
}
errors = append(errors, srcErr)
}
for _, req := range wrongDirectness {
// Handle dependencies that are incorrectly labeled indirect and
// vice versa.
srcErr, err := directnessError(pm.Mapper, req, snapshot.View().Options().ComputeEdits)
if err != nil {
return nil, err
}
errors = append(errors, srcErr)
}
// Next, compute any diagnostics for modules that are missing from the
// go.mod file. The fixes will be for the go.mod file, but the
// diagnostics should also appear in both the go.mod file and the import
// statements in the Go files in which the dependencies are used.
missingModuleFixes := map[*modfile.Require][]source.SuggestedFix{}
for _, req := range missing {
srcErr, err := missingModuleError(snapshot, pm, req)
if err != nil {
return nil, err
}
missingModuleFixes[req] = srcErr.SuggestedFixes
errors = append(errors, srcErr)
}
// Add diagnostics for missing modules anywhere they are imported in the
// workspace.
for _, pkg := range workspacePkgs {
missingImports := map[string]*modfile.Require{}
for _, imp := range pkg.Imports() {
if req, ok := missing[imp.PkgPath()]; ok {
missingImports[imp.PkgPath()] = req
break
}
// If the import is a package of the dependency, then add the
// package to the map, this will eliminate the need to do this
// prefix package search on each import for each file.
// Example:
//
// import (
// "golang.org/x/tools/go/expect"
// "golang.org/x/tools/go/packages"
// )
// They both are related to the same module: "golang.org/x/tools".
var match string
for _, req := range ideal.Require {
if strings.HasPrefix(imp.PkgPath(), req.Mod.Path) && len(req.Mod.Path) > len(match) {
match = req.Mod.Path
}
}
if req, ok := missing[match]; ok {
missingImports[imp.PkgPath()] = req
}
}
// None of this package's imports are from missing modules.
if len(missingImports) == 0 {
continue
}
// Handle dependencies that are incorrectly labeled indirect and vice versa.
if missingDeps[dep] != nil && req.Indirect != missingDeps[dep].Indirect {
directErr, err := modDirectnessErrors(m, req, options)
for _, pgf := range pkg.CompiledGoFiles() {
file, m := pgf.File, pgf.Mapper
if file == nil || m == nil {
continue
}
imports := make(map[string]*ast.ImportSpec)
for _, imp := range file.Imports {
if imp.Path == nil {
continue
}
if target, err := strconv.Unquote(imp.Path.Value); err == nil {
imports[target] = imp
}
}
if len(imports) == 0 {
continue
}
for importPath, req := range missingImports {
imp, ok := imports[importPath]
if !ok {
continue
}
fixes, ok := missingModuleFixes[req]
if !ok {
return nil, fmt.Errorf("no missing module fix for %q (%q)", importPath, req.Mod.Path)
}
srcErr, err := missingModuleForImport(snapshot, m, imp, req, fixes)
if err != nil {
return nil, err
}
errors = append(errors, directErr)
errors = append(errors, srcErr)
}
// Handle unused dependencies.
if missingDeps[dep] == nil {
rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
if err != nil {
return nil, err
}
edits, err := dropDependencyEdits(m, req, options)
if err != nil {
return nil, err
}
errors = append(errors, source.Error{
Category: ModTidyError,
Message: fmt.Sprintf("%s is not used in this module.", dep),
Range: rng,
URI: m.URI,
SuggestedFixes: []source.SuggestedFix{{
Title: fmt.Sprintf("Remove dependency: %s", dep),
Edits: map[span.URI][]protocol.TextEdit{
m.URI: edits,
},
}},
})
}
}
return errors, nil
}
const ModTidyError = "go mod tidy"
// modDirectnessErrors extracts errors when a dependency is labeled indirect when it should be direct and vice versa.
func modDirectnessErrors(m *protocol.ColumnMapper, req *modfile.Require, options source.Options) (source.Error, error) {
// unusedError returns a source.Error for an unused require.
func unusedError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) (source.Error, error) {
rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
if err != nil {
return source.Error{}, err
}
edits, err := dropDependency(req, m, computeEdits)
if err != nil {
return source.Error{}, err
}
return source.Error{
Category: source.GoModTidy,
Message: fmt.Sprintf("%s is not used in this module", req.Mod.Path),
Range: rng,
URI: m.URI,
SuggestedFixes: []source.SuggestedFix{{
Title: fmt.Sprintf("Remove dependency: %s", req.Mod.Path),
Edits: map[span.URI][]protocol.TextEdit{
m.URI: edits,
},
}},
}, nil
}
// directnessError extracts errors when a dependency is labeled indirect when
// it should be direct and vice versa.
func directnessError(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) (source.Error, error) {
rng, err := rangeFromPositions(m, req.Syntax.Start, req.Syntax.End)
if err != nil {
return source.Error{}, err
}
direction := "indirect"
if req.Indirect {
direction = "direct"
// If the dependency should be direct, just highlight the // indirect.
if comments := req.Syntax.Comment(); comments != nil && len(comments.Suffix) > 0 {
end := comments.Suffix[0].Start
@ -258,35 +335,19 @@ func modDirectnessErrors(m *protocol.ColumnMapper, req *modfile.Require, options
return source.Error{}, err
}
}
edits, err := changeDirectnessEdits(m, req, false, options)
if err != nil {
return source.Error{}, err
}
return source.Error{
Category: ModTidyError,
Message: fmt.Sprintf("%s should be a direct dependency", req.Mod.Path),
Range: rng,
URI: m.URI,
SuggestedFixes: []source.SuggestedFix{{
Title: fmt.Sprintf("Make %s direct", req.Mod.Path),
Edits: map[span.URI][]protocol.TextEdit{
m.URI: edits,
},
}},
}, nil
}
// If the dependency should be indirect, add the // indirect.
edits, err := changeDirectnessEdits(m, req, true, options)
edits, err := switchDirectness(req, m, computeEdits)
if err != nil {
return source.Error{}, err
}
return source.Error{
Category: ModTidyError,
Message: fmt.Sprintf("%s should be an indirect dependency", req.Mod.Path),
Message: fmt.Sprintf("%s should be %s", req.Mod.Path, direction),
Range: rng,
URI: m.URI,
Category: source.GoModTidy,
SuggestedFixes: []source.SuggestedFix{{
Title: fmt.Sprintf("Make %s indirect", req.Mod.Path),
Title: fmt.Sprintf("Change %s to %s", req.Mod.Path, direction),
Edits: map[span.URI][]protocol.TextEdit{
m.URI: edits,
},
@ -294,19 +355,35 @@ func modDirectnessErrors(m *protocol.ColumnMapper, req *modfile.Require, options
}, nil
}
// dropDependencyEdits gets the edits needed to remove the dependency from the go.mod file.
// As an example, this function will codify the edits needed to convert the before go.mod file to the after.
// Before:
// module t
//
// go 1.11
//
// require golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee
// After:
// module t
//
// go 1.11
func dropDependencyEdits(m *protocol.ColumnMapper, req *modfile.Require, options source.Options) ([]protocol.TextEdit, error) {
func missingModuleError(snapshot source.Snapshot, pm *source.ParsedModule, req *modfile.Require) (source.Error, error) {
start, end := pm.File.Module.Syntax.Span()
rng, err := rangeFromPositions(pm.Mapper, start, end)
if err != nil {
return source.Error{}, err
}
edits, err := addRequireFix(pm.Mapper, req, snapshot.View().Options().ComputeEdits)
if err != nil {
return source.Error{}, err
}
fix := &source.SuggestedFix{
Title: "Add %s to your go.mod file",
Edits: map[span.URI][]protocol.TextEdit{
pm.Mapper.URI: edits,
},
}
return source.Error{
URI: pm.Mapper.URI,
Range: rng,
Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
Category: source.GoModTidy,
Kind: source.ModTidyError,
SuggestedFixes: []source.SuggestedFix{*fix},
}, nil
}
// dropDependency returns the edits to remove the given require from the go.mod
// file.
func dropDependency(req *modfile.Require, m *protocol.ColumnMapper, computeEdits diff.ComputeEdits) ([]protocol.TextEdit, error) {
// We need a private copy of the parsed go.mod file, since we're going to
// modify it.
copied, err := modfile.Parse("", m.Content, nil)
@ -322,29 +399,13 @@ func dropDependencyEdits(m *protocol.ColumnMapper, req *modfile.Require, options
return nil, err
}
// Calculate the edits to be made due to the change.
diff := options.ComputeEdits(m.URI, string(m.Content), string(newContent))
edits, err := source.ToProtocolEdits(m, diff)
if err != nil {
return nil, err
}
return edits, nil
diff := computeEdits(m.URI, string(m.Content), string(newContent))
return source.ToProtocolEdits(m, diff)
}
// changeDirectnessEdits gets the edits needed to change an indirect dependency to direct and vice versa.
// As an example, this function will codify the edits needed to convert the before go.mod file to the after.
// Before:
// module t
//
// go 1.11
//
// require golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee
// After:
// module t
//
// go 1.11
//
// require golang.org/x/mod v0.1.1-0.20191105210325-c90efee705ee // indirect
func changeDirectnessEdits(m *protocol.ColumnMapper, req *modfile.Require, indirect bool, options source.Options) ([]protocol.TextEdit, error) {
// switchDirectness gets the edits needed to change an indirect dependency to
// direct and vice versa.
func switchDirectness(req *modfile.Require, m *protocol.ColumnMapper, computeEdits diff.ComputeEdits) ([]protocol.TextEdit, error) {
// We need a private copy of the parsed go.mod file, since we're going to
// modify it.
copied, err := modfile.Parse("", m.Content, nil)
@ -359,7 +420,7 @@ func changeDirectnessEdits(m *protocol.ColumnMapper, req *modfile.Require, indir
requires = append(requires, &modfile.Require{
Mod: r.Mod,
Syntax: r.Syntax,
Indirect: indirect,
Indirect: !r.Indirect,
})
continue
}
@ -371,12 +432,54 @@ func changeDirectnessEdits(m *protocol.ColumnMapper, req *modfile.Require, indir
return nil, err
}
// Calculate the edits to be made due to the change.
diff := options.ComputeEdits(m.URI, string(m.Content), string(newContent))
edits, err := source.ToProtocolEdits(m, diff)
diff := computeEdits(m.URI, string(m.Content), string(newContent))
return source.ToProtocolEdits(m, diff)
}
// missingModuleForImport creates an error for a given import path that comes
// from a missing module.
func missingModuleForImport(snapshot source.Snapshot, m *protocol.ColumnMapper, imp *ast.ImportSpec, req *modfile.Require, fixes []source.SuggestedFix) (source.Error, error) {
if req.Syntax == nil {
return source.Error{}, fmt.Errorf("no syntax for %v", req)
}
spn, err := span.NewRange(snapshot.FileSet(), imp.Path.Pos(), imp.Path.End()).Span()
if err != nil {
return source.Error{}, err
}
rng, err := m.Range(spn)
if err != nil {
return source.Error{}, err
}
return source.Error{
Category: source.GoModTidy,
URI: m.URI,
Range: rng,
Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
Kind: source.ModTidyError,
SuggestedFixes: fixes,
}, nil
}
// addRequireFix creates edits for adding a given require to a go.mod file.
func addRequireFix(m *protocol.ColumnMapper, req *modfile.Require, computeEdits diff.ComputeEdits) ([]protocol.TextEdit, error) {
// We need a private copy of the parsed go.mod file, since we're going to
// modify it.
copied, err := modfile.Parse("", m.Content, nil)
if err != nil {
return nil, err
}
return edits, nil
// Calculate the quick fix edits that need to be made to the go.mod file.
if err := copied.AddRequire(req.Mod.Path, req.Mod.Version); err != nil {
return nil, err
}
copied.SortBlocks()
newContents, err := copied.Format()
if err != nil {
return nil, err
}
// Calculate the edits to be made due to the change.
diff := computeEdits(m.URI, string(m.Content), string(newContents))
return source.ToProtocolEdits(m, diff)
}
func rangeFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (protocol.Range, error) {
@ -397,158 +500,3 @@ func rangeFromPositions(m *protocol.ColumnMapper, s, e modfile.Position) (protoc
}
return m.Range(span.New(m.URI, start, end))
}
// missingModuleErrors returns diagnostics for each file in each workspace
// package that has dependencies that are not reflected in the go.mod file.
func missingModuleErrors(ctx context.Context, snapshot *snapshot, modMapper *protocol.ColumnMapper, pkgs []source.Package, modules []*modfile.Require, missingMods map[string]*modfile.Require, original *modfile.File, options source.Options) ([]source.Error, error) {
var moduleErrs []source.Error
matchedMissingMods := make(map[*modfile.Require]struct{})
for _, pkg := range pkgs {
missingPkgs := map[string]*modfile.Require{}
for _, imp := range pkg.Imports() {
found := func(req *modfile.Require) {
missingPkgs[imp.PkgPath()] = req
matchedMissingMods[req] = struct{}{}
}
if req, ok := missingMods[imp.PkgPath()]; ok {
found(req)
break
}
// If the import is a package of the dependency, then add the
// package to the map, this will eliminate the need to do this
// prefix package search on each import for each file.
// Example:
//
// import (
// "golang.org/x/tools/go/expect"
// "golang.org/x/tools/go/packages"
// )
// They both are related to the same module: "golang.org/x/tools".
var match string
for _, mod := range modules {
if strings.HasPrefix(imp.PkgPath(), mod.Mod.Path) && len(mod.Mod.Path) > len(match) {
match = mod.Mod.Path
}
}
if req, ok := missingMods[match]; ok {
found(req)
}
}
if len(missingPkgs) > 0 {
errs, err := missingModules(ctx, snapshot, modMapper, pkg, missingPkgs, options)
if err != nil {
return nil, err
}
moduleErrs = append(moduleErrs, errs...)
}
}
for _, req := range missingMods {
if _, ok := matchedMissingMods[req]; ok {
continue
}
s, e := original.Module.Syntax.Span()
rng, err := rangeFromPositions(modMapper, s, e)
if err != nil {
return nil, err
}
edits, err := addRequireFix(modMapper, req, options)
if err != nil {
return nil, err
}
moduleErrs = append(moduleErrs, source.Error{
URI: modMapper.URI,
Range: rng,
Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
Category: "go mod tidy",
Kind: source.ModTidyError,
SuggestedFixes: []source.SuggestedFix{
{
Title: "Add %s to your go.mod file",
Edits: edits,
},
},
})
}
return moduleErrs, nil
}
func missingModules(ctx context.Context, snapshot *snapshot, modMapper *protocol.ColumnMapper, pkg source.Package, missing map[string]*modfile.Require, options source.Options) ([]source.Error, error) {
var errors []source.Error
for _, pgf := range pkg.CompiledGoFiles() {
imports := make(map[string]*ast.ImportSpec)
for _, imp := range pgf.File.Imports {
if imp.Path == nil {
continue
}
if target, err := strconv.Unquote(imp.Path.Value); err == nil {
imports[target] = imp
}
}
if len(imports) == 0 {
continue
}
for mod, req := range missing {
if req.Syntax == nil {
continue
}
imp, ok := imports[mod]
if !ok {
continue
}
spn, err := span.NewRange(snapshot.view.session.cache.fset, imp.Path.Pos(), imp.Path.End()).Span()
if err != nil {
return nil, err
}
rng, err := pgf.Mapper.Range(spn)
if err != nil {
return nil, err
}
edits, err := addRequireFix(modMapper, req, options)
if err != nil {
return nil, err
}
errors = append(errors, source.Error{
URI: pgf.URI,
Range: rng,
Message: fmt.Sprintf("%s is not in your go.mod file", req.Mod.Path),
Category: "go mod tidy",
Kind: source.ModTidyError,
SuggestedFixes: []source.SuggestedFix{
{
Title: "Add %s to your go.mod file",
Edits: edits,
},
},
})
}
}
return errors, nil
}
// addRequireFix creates edits for adding a given require to a go.mod file.
func addRequireFix(m *protocol.ColumnMapper, req *modfile.Require, options source.Options) (map[span.URI][]protocol.TextEdit, error) {
// We need a private copy of the parsed go.mod file, since we're going to
// modify it.
copied, err := modfile.Parse("", m.Content, nil)
if err != nil {
return nil, err
}
// Calculate the quick fix edits that need to be made to the go.mod file.
if err := copied.AddRequire(req.Mod.Path, req.Mod.Version); err != nil {
return nil, err
}
copied.SortBlocks()
newContents, err := copied.Format()
if err != nil {
return nil, err
}
// Calculate the edits to be made due to the change.
diff := options.ComputeEdits(m.URI, string(m.Content), string(newContents))
edits, err := source.ToProtocolEdits(m, diff)
if err != nil {
return nil, err
}
return map[span.URI][]protocol.TextEdit{
m.URI: edits,
}, nil
}

3
internal/lsp/source/view.go
View File

@ -459,6 +459,9 @@ type Error struct {
Related []RelatedInformation
}
// GoModTidy is the source for a diagnostic computed by running `go mod tidy`.
const GoModTidy = "go mod tidy"
type ErrorKind int
const (

2
internal/lsp/testdata/unused/primarymod/go.mod vendored
View File

@ -2,4 +2,4 @@ module unused
go 1.12
require example.com/extramodule v1.0.0 //@diag("require example.com/extramodule v1.0.0", "go mod tidy", "example.com/extramodule is not used in this module.", "warning"),suggestedfix("require example.com/extramodule v1.0.0", "quickfix")
require example.com/extramodule v1.0.0 //@diag("require example.com/extramodule v1.0.0", "go mod tidy", "example.com/extramodule is not used in this module", "warning"),suggestedfix("require example.com/extramodule v1.0.0", "quickfix")