diff --git a/internal/lsp/cache/file.go b/internal/lsp/cache/file.go index d0683ebe856..f71a15e570e 100644 --- a/internal/lsp/cache/file.go +++ b/internal/lsp/cache/file.go @@ -6,7 +6,6 @@ package cache import ( "context" - "go/ast" "go/token" "path/filepath" "strings" @@ -18,6 +17,7 @@ import ( // viewFile extends source.File with helper methods for the view package. type viewFile interface { source.File + filename() string addURI(uri span.URI) int } @@ -33,16 +33,6 @@ type fileBase struct { token *token.File } -// goFile holds all the information we know about a go file. -type goFile struct { - fileBase - - ast *ast.File - pkg *pkg - meta *metadata - imports []*ast.ImportSpec -} - func basename(filename string) string { return strings.ToLower(filepath.Base(filename)) } @@ -72,50 +62,7 @@ func (f *fileBase) FileSet() *token.FileSet { return f.view.Session().Cache().FileSet() } -func (f *goFile) GetToken(ctx context.Context) *token.File { - f.view.mu.Lock() - defer f.view.mu.Unlock() - if f.token == nil || len(f.view.contentChanges) > 0 { - if _, err := f.view.parse(ctx, f); err != nil { - f.View().Session().Logger().Errorf(ctx, "unable to check package for %s: %v", f.URI(), err) - return nil - } - } - return f.token -} - -func (f *goFile) GetAST(ctx context.Context) *ast.File { - f.view.mu.Lock() - defer f.view.mu.Unlock() - - if f.ast == nil || len(f.view.contentChanges) > 0 { - if _, err := f.view.parse(ctx, f); err != nil { - f.View().Session().Logger().Errorf(ctx, "unable to check package for %s: %v", f.URI(), err) - return nil - } - } - return f.ast -} - -func (f *goFile) GetPackage(ctx context.Context) source.Package { - f.view.mu.Lock() - defer f.view.mu.Unlock() - - if f.pkg == nil || len(f.view.contentChanges) > 0 { - if errs, err := f.view.parse(ctx, f); err != nil { - f.View().Session().Logger().Errorf(ctx, "unable to check package for %s: %v", f.URI(), err) - - // Create diagnostics for errors if we are able to. - if len(errs) > 0 { - return &pkg{errors: errs} - } - return nil - } - } - return f.pkg -} - -// read is the internal part of Content. It assumes that the caller is +// read is the internal part of GetContent. It assumes that the caller is // holding the mutex of the file's view. func (f *fileBase) read(ctx context.Context) { if err := ctx.Err(); err != nil { @@ -144,53 +91,3 @@ func (f *fileBase) read(ctx context.Context) { func (f *goFile) isPopulated() bool { return f.ast != nil && f.token != nil && f.pkg != nil && f.meta != nil && f.imports != nil } - -func (f *goFile) GetActiveReverseDeps(ctx context.Context) []source.GoFile { - pkg := f.GetPackage(ctx) - if pkg == nil { - return nil - } - - f.view.mu.Lock() - defer f.view.mu.Unlock() - - f.view.mcache.mu.Lock() - defer f.view.mcache.mu.Unlock() - - seen := make(map[string]struct{}) // visited packages - results := make(map[*goFile]struct{}) - f.view.reverseDeps(ctx, seen, results, pkg.PkgPath()) - - var files []source.GoFile - for rd := range results { - if rd == nil { - continue - } - // Don't return any of the active files in this package. - if rd.pkg != nil && rd.pkg == pkg { - continue - } - files = append(files, rd) - } - return files -} - -func (v *view) reverseDeps(ctx context.Context, seen map[string]struct{}, results map[*goFile]struct{}, pkgPath string) { - if _, ok := seen[pkgPath]; ok { - return - } - seen[pkgPath] = struct{}{} - m, ok := v.mcache.packages[pkgPath] - if !ok { - return - } - for _, filename := range m.files { - uri := span.FileURI(filename) - if f, err := v.getFile(uri); err == nil && v.session.IsOpen(uri) { - results[f.(*goFile)] = struct{}{} - } - } - for parentPkgPath := range m.parents { - v.reverseDeps(ctx, seen, results, parentPkgPath) - } -} diff --git a/internal/lsp/cache/gofile.go b/internal/lsp/cache/gofile.go new file mode 100644 index 00000000000..aab3d829005 --- /dev/null +++ b/internal/lsp/cache/gofile.go @@ -0,0 +1,113 @@ +package cache + +import ( + "context" + "go/ast" + "go/token" + + "golang.org/x/tools/internal/lsp/source" + "golang.org/x/tools/internal/span" +) + +// goFile holds all of the information we know about a Go file. +type goFile struct { + fileBase + + ast *ast.File + pkg *pkg + meta *metadata + imports []*ast.ImportSpec +} + +func (f *goFile) GetToken(ctx context.Context) *token.File { + f.view.mu.Lock() + defer f.view.mu.Unlock() + if f.token == nil || len(f.view.contentChanges) > 0 { + if _, err := f.view.parse(ctx, f); err != nil { + f.View().Session().Logger().Errorf(ctx, "unable to check package for %s: %v", f.URI(), err) + return nil + } + } + return f.token +} + +func (f *goFile) GetAST(ctx context.Context) *ast.File { + f.view.mu.Lock() + defer f.view.mu.Unlock() + + if f.ast == nil || len(f.view.contentChanges) > 0 { + if _, err := f.view.parse(ctx, f); err != nil { + f.View().Session().Logger().Errorf(ctx, "unable to check package for %s: %v", f.URI(), err) + return nil + } + } + return f.ast +} + +func (f *goFile) GetPackage(ctx context.Context) source.Package { + f.view.mu.Lock() + defer f.view.mu.Unlock() + + if f.pkg == nil || len(f.view.contentChanges) > 0 { + if errs, err := f.view.parse(ctx, f); err != nil { + f.View().Session().Logger().Errorf(ctx, "unable to check package for %s: %v", f.URI(), err) + + // Create diagnostics for errors if we are able to. + if len(errs) > 0 { + return &pkg{errors: errs} + } + return nil + } + } + return f.pkg +} + +func (f *goFile) GetActiveReverseDeps(ctx context.Context) []source.GoFile { + pkg := f.GetPackage(ctx) + if pkg == nil { + return nil + } + + f.view.mu.Lock() + defer f.view.mu.Unlock() + + f.view.mcache.mu.Lock() + defer f.view.mcache.mu.Unlock() + + seen := make(map[string]struct{}) // visited packages + results := make(map[*goFile]struct{}) + f.view.reverseDeps(ctx, seen, results, pkg.PkgPath()) + + var files []source.GoFile + for rd := range results { + if rd == nil { + continue + } + // Don't return any of the active files in this package. + if rd.pkg != nil && rd.pkg == pkg { + continue + } + files = append(files, rd) + } + return files +} + +func (v *view) reverseDeps(ctx context.Context, seen map[string]struct{}, results map[*goFile]struct{}, pkgPath string) { + if _, ok := seen[pkgPath]; ok { + return + } + seen[pkgPath] = struct{}{} + m, ok := v.mcache.packages[pkgPath] + if !ok { + return + } + for _, filename := range m.files { + uri := span.FileURI(filename) + if f, err := v.getFile(uri); err == nil && v.session.IsOpen(uri) { + results[f.(*goFile)] = struct{}{} + } + } + for parentPkgPath := range m.parents { + v.reverseDeps(ctx, seen, results, parentPkgPath) + } +} diff --git a/internal/lsp/cache/modfile.go b/internal/lsp/cache/modfile.go new file mode 100644 index 00000000000..b912a6cc241 --- /dev/null +++ b/internal/lsp/cache/modfile.go @@ -0,0 +1,16 @@ +package cache + +import ( + "context" + "go/token" +) + +// modFile holds all of the information we know about a mod file. +type modFile struct { + fileBase +} + +func (*modFile) GetToken(context.Context) *token.File { return nil } +func (*modFile) setContent(content []byte) {} +func (*modFile) filename() string { return "" } +func (*modFile) isActive() bool { return false } diff --git a/internal/lsp/cache/sumfile.go b/internal/lsp/cache/sumfile.go new file mode 100644 index 00000000000..03c11a00a6e --- /dev/null +++ b/internal/lsp/cache/sumfile.go @@ -0,0 +1,16 @@ +package cache + +import ( + "context" + "go/token" +) + +// sumFile holds all of the information we know about a sum file. +type sumFile struct { + fileBase +} + +func (*sumFile) GetToken(context.Context) *token.File { return nil } +func (*sumFile) setContent(content []byte) {} +func (*sumFile) filename() string { return "" } +func (*sumFile) isActive() bool { return false } diff --git a/internal/lsp/cache/view.go b/internal/lsp/cache/view.go index 7908f0b6250..96983b1cebd 100644 --- a/internal/lsp/cache/view.go +++ b/internal/lsp/cache/view.go @@ -159,6 +159,13 @@ func (v *view) shutdown(context.Context) { } } +// Ignore checks if the given URI is a URI we ignore. +// As of right now, we only ignore files in the "builtin" package. +func (v *view) Ignore(uri span.URI) bool { + _, ok := v.ignoredURIs[uri] + return ok +} + func (v *view) BackgroundContext() context.Context { v.mu.Lock() defer v.mu.Unlock() @@ -306,7 +313,7 @@ func (v *view) getFile(uri span.URI) (viewFile, error) { if err != nil { return nil, err } - var f *goFile + var f viewFile switch ext := filepath.Ext(filename); ext { case ".go": f = &goFile{ @@ -315,25 +322,30 @@ func (v *view) getFile(uri span.URI) (viewFile, error) { fname: filename, }, } + v.session.filesWatchMap.Watch(uri, func() { + f.(*goFile).invalidate() + }) case ".mod": - return nil, fmt.Errorf("mod files are not yet supported") + f = &modFile{ + fileBase: fileBase{ + view: v, + fname: filename, + }, + } + case ".sum": + f = &sumFile{ + fileBase: fileBase{ + view: v, + fname: filename, + }, + } default: return nil, fmt.Errorf("unsupported file extension: %s", ext) } - v.session.filesWatchMap.Watch(uri, func() { - f.invalidate() - }) v.mapFile(uri, f) return f, nil } -// Ignore checks if the given URI is a URI we ignore. -// As of right now, we only ignore files in the "builtin" package. -func (v *view) Ignore(uri span.URI) bool { - _, ok := v.ignoredURIs[uri] - return ok -} - // findFile checks the cache for any file matching the given uri. // // An error is only returned for an irreparable failure, for example, if the diff --git a/internal/lsp/diagnostics.go b/internal/lsp/diagnostics.go index 4e4f7fc34ef..ffc7c82babf 100644 --- a/internal/lsp/diagnostics.go +++ b/internal/lsp/diagnostics.go @@ -13,14 +13,24 @@ import ( "golang.org/x/tools/internal/span" ) -func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI) { +func (s *Server) Diagnostics(ctx context.Context, v source.View, uri span.URI) { if ctx.Err() != nil { s.session.Logger().Errorf(ctx, "canceling diagnostics for %s: %v", uri, ctx.Err()) return } - reports, err := source.Diagnostics(ctx, view, uri) + f, err := v.GetFile(ctx, uri) if err != nil { - s.session.Logger().Errorf(ctx, "failed to compute diagnostics for %s: %v", uri, err) + s.session.Logger().Errorf(ctx, "no file for %s: %v", uri, err) + return + } + // For non-Go files, don't return any diagnostics. + gof, ok := f.(source.GoFile) + if !ok { + return + } + reports, err := source.Diagnostics(ctx, v, gof) + if err != nil { + s.session.Logger().Errorf(ctx, "failed to compute diagnostics for %s: %v", gof.URI(), err) return } @@ -28,7 +38,7 @@ func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI defer s.undeliveredMu.Unlock() for uri, diagnostics := range reports { - if err := s.publishDiagnostics(ctx, view, uri, diagnostics); err != nil { + if err := s.publishDiagnostics(ctx, v, uri, diagnostics); err != nil { if s.undelivered == nil { s.undelivered = make(map[span.URI][]source.Diagnostic) } @@ -41,7 +51,7 @@ func (s *Server) Diagnostics(ctx context.Context, view source.View, uri span.URI // Anytime we compute diagnostics, make sure to also send along any // undelivered ones (only for remaining URIs). for uri, diagnostics := range s.undelivered { - err := s.publishDiagnostics(ctx, view, uri, diagnostics) + err := s.publishDiagnostics(ctx, v, uri, diagnostics) if err != nil { s.session.Logger().Errorf(ctx, "failed to deliver diagnostic for %s: %v", uri, err) } diff --git a/internal/lsp/lsp_test.go b/internal/lsp/lsp_test.go index 7d20a5d300b..61156f25bfc 100644 --- a/internal/lsp/lsp_test.go +++ b/internal/lsp/lsp_test.go @@ -61,7 +61,15 @@ func testLSP(t *testing.T, exporter packagestest.Exporter) { func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) { v := r.server.session.View(viewName) for uri, want := range data { - results, err := source.Diagnostics(context.Background(), v, uri) + f, err := v.GetFile(context.Background(), uri) + if err != nil { + t.Fatalf("no file for %s: %v", f, err) + } + gof, ok := f.(source.GoFile) + if !ok { + t.Fatalf("%s is not a Go file: %v", uri, err) + } + results, err := source.Diagnostics(context.Background(), v, gof) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/source/analysis.go b/internal/lsp/source/analysis.go index f7021b20b14..46e64b41228 100644 --- a/internal/lsp/source/analysis.go +++ b/internal/lsp/source/analysis.go @@ -80,7 +80,7 @@ type packageFactKey struct { } func (act *Action) String() string { - return fmt.Sprintf("%s@%s", act.Analyzer, act.Pkg) + return fmt.Sprintf("%s@%s", act.Analyzer, act.Pkg.PkgPath()) } func execAll(ctx context.Context, fset *token.FileSet, actions []*Action) error { @@ -112,7 +112,7 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { var failed []string for _, dep := range act.Deps { if dep.err != nil { - failed = append(failed, dep.String()) + failed = append(failed, fmt.Sprintf("%s: %v", dep.String(), dep.err)) } } if failed != nil { @@ -159,7 +159,7 @@ func (act *Action) execOnce(ctx context.Context, fset *token.FileSet) error { act.pass = pass if len(act.Pkg.GetErrors()) > 0 && !pass.Analyzer.RunDespiteErrors { - act.err = fmt.Errorf("analysis skipped due to errors in package") + act.err = fmt.Errorf("analysis skipped due to errors in package: %v", act.Pkg.GetErrors()) } else { act.result, act.err = pass.Analyzer.Run(pass) if act.err == nil { diff --git a/internal/lsp/source/diagnostics.go b/internal/lsp/source/diagnostics.go index b3d78689afe..922840f1940 100644 --- a/internal/lsp/source/diagnostics.go +++ b/internal/lsp/source/diagnostics.go @@ -51,19 +51,10 @@ const ( SeverityError ) -func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diagnostic, error) { - f, err := v.GetFile(ctx, uri) - if err != nil { - return singleDiagnostic(uri, "no file found for %s", uri), nil - } - // For non-Go files, don't return any diagnostics. - gof, ok := f.(GoFile) - if !ok { - return nil, nil - } - pkg := gof.GetPackage(ctx) +func Diagnostics(ctx context.Context, v View, f GoFile) (map[span.URI][]Diagnostic, error) { + pkg := f.GetPackage(ctx) if pkg == nil { - return singleDiagnostic(uri, "%s is not part of a package", uri), nil + return singleDiagnostic(f.URI(), "%s is not part of a package", f.URI()), nil } // Prepare the reports we will send for this package. reports := make(map[span.URI][]Diagnostic) @@ -84,11 +75,11 @@ func Diagnostics(ctx context.Context, v View, uri span.URI) (map[span.URI][]Diag if !diagnostics(ctx, v, pkg, reports) { // If we don't have any list, parse, or type errors, run analyses. if err := analyses(ctx, v, pkg, reports); err != nil { - return singleDiagnostic(uri, "failed to run analyses for %s: %v", uri, err), nil + v.Session().Logger().Errorf(ctx, "failed to run analyses for %s: %v", f.URI(), err) } } // Updates to the diagnostics for this package may need to be propagated. - for _, f := range gof.GetActiveReverseDeps(ctx) { + for _, f := range f.GetActiveReverseDeps(ctx) { pkg := f.GetPackage(ctx) if pkg == nil { continue @@ -184,7 +175,6 @@ func packageErrorSpan(pkgErr packages.Error) span.Span { if pkgErr.Pos == "" { return parseDiagnosticMessage(pkgErr.Msg) } - return span.Parse(pkgErr.Pos) } diff --git a/internal/lsp/source/source_test.go b/internal/lsp/source/source_test.go index f89ec0c63e7..fa2209188e0 100644 --- a/internal/lsp/source/source_test.go +++ b/internal/lsp/source/source_test.go @@ -52,7 +52,11 @@ func testSource(t *testing.T, exporter packagestest.Exporter) { func (r *runner) Diagnostics(t *testing.T, data tests.Diagnostics) { for uri, want := range data { - results, err := source.Diagnostics(context.Background(), r.view, uri) + f, err := r.view.GetFile(context.Background(), uri) + if err != nil { + t.Fatal(err) + } + results, err := source.Diagnostics(context.Background(), r.view, f.(source.GoFile)) if err != nil { t.Fatal(err) } diff --git a/internal/lsp/source/view.go b/internal/lsp/source/view.go index b4380317210..e00cff87aa5 100644 --- a/internal/lsp/source/view.go +++ b/internal/lsp/source/view.go @@ -156,6 +156,14 @@ type GoFile interface { GetActiveReverseDeps(ctx context.Context) []GoFile } +type ModFile interface { + File +} + +type SumFile interface { + File +} + // Package represents a Go package that has been type-checked. It maintains // only the relevant fields of a *go/packages.Package. type Package interface {