diff --git a/internal/lsp/code_action.go b/internal/lsp/code_action.go index 51dac8ae3e..e125375568 100644 --- a/internal/lsp/code_action.go +++ b/internal/lsp/code_action.go @@ -7,15 +7,14 @@ package lsp import ( "context" "fmt" + "regexp" "sort" "strings" "golang.org/x/tools/internal/imports" - "golang.org/x/tools/internal/lsp/debug/tag" "golang.org/x/tools/internal/lsp/mod" "golang.org/x/tools/internal/lsp/protocol" "golang.org/x/tools/internal/lsp/source" - "golang.org/x/tools/internal/telemetry/event" ) func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionParams) ([]protocol.CodeAction, error) { @@ -52,7 +51,7 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara if diagnostics := params.Context.Diagnostics; len(diagnostics) > 0 { codeActions = append(codeActions, mod.SuggestedFixes(ctx, snapshot, fh, diagnostics)...) } - if !wanted[protocol.SourceOrganizeImports] { + if wanted[protocol.SourceOrganizeImports] { codeActions = append(codeActions, protocol.CodeAction{ Title: "Tidy", Kind: protocol.SourceOrganizeImports, @@ -66,75 +65,74 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara case source.Go: diagnostics := params.Context.Diagnostics - var importEdits []protocol.TextEdit - var importEditsPerFix []*source.ImportFix - var analysisQuickFixes []protocol.CodeAction - var highConfidenceEdits []protocol.TextDocumentEdit - - // Retrieve any necessary import edits or fixes. - if wanted[protocol.QuickFix] && len(diagnostics) > 0 || wanted[protocol.SourceOrganizeImports] { - importEdits, importEditsPerFix, err = source.AllImportsFixes(ctx, snapshot, fh) + // First, process any missing imports and pair them with the + // diagnostics they fix. + if wantQuickFixes := wanted[protocol.QuickFix] && len(diagnostics) > 0; wantQuickFixes || wanted[protocol.SourceOrganizeImports] { + importEdits, importEditsPerFix, err := source.AllImportsFixes(ctx, snapshot, fh) if err != nil { return nil, err } + // Separate this into a set of codeActions per diagnostic, where + // each action is the addition, removal, or renaming of one import. + if wantQuickFixes { + for _, importFix := range importEditsPerFix { + fixes := importDiagnostics(importFix.Fix, diagnostics) + if len(fixes) == 0 { + continue + } + codeActions = append(codeActions, protocol.CodeAction{ + Title: importFixTitle(importFix.Fix), + Kind: protocol.QuickFix, + Edit: protocol.WorkspaceEdit{ + DocumentChanges: documentChanges(fh, importFix.Edits), + }, + Diagnostics: fixes, + }) + } + } + // Send all of the import edits as one code action if the file is + // being organized. + if wanted[protocol.SourceOrganizeImports] && len(importEdits) > 0 { + codeActions = append(codeActions, protocol.CodeAction{ + Title: "Organize Imports", + Kind: protocol.SourceOrganizeImports, + Edit: protocol.WorkspaceEdit{ + DocumentChanges: documentChanges(fh, importEdits), + }, + }) + } + } + // Check for context cancellation before processing analysis fixes. + if ctx.Err() != nil { + return nil, ctx.Err() } // Retrieve any necessary analysis fixes or edits. if (wanted[protocol.QuickFix] || wanted[protocol.SourceFixAll]) && len(diagnostics) > 0 { - analysisQuickFixes, highConfidenceEdits, err = analysisFixes(ctx, snapshot, fh, diagnostics) + analysisQuickFixes, highConfidenceEdits, err := analysisFixes(ctx, snapshot, fh, diagnostics) if err != nil { - event.Error(ctx, "analysis fixes failed", err, tag.URI.Of(uri)) + return nil, err } - } + if wanted[protocol.QuickFix] { + // Add the quick fixes reported by go/analysis. + codeActions = append(codeActions, analysisQuickFixes...) - if wanted[protocol.QuickFix] && len(diagnostics) > 0 { - // First, add the quick fixes reported by go/analysis. - codeActions = append(codeActions, analysisQuickFixes...) - - // If we also have diagnostics for missing imports, we can associate them with quick fixes. - if findImportErrors(diagnostics) { - // Separate this into a set of codeActions per diagnostic, where - // each action is the addition, removal, or renaming of one import. - for _, importFix := range importEditsPerFix { - // Get the diagnostics this fix would affect. - if fixDiagnostics := importDiagnostics(importFix.Fix, diagnostics); len(fixDiagnostics) > 0 { - codeActions = append(codeActions, protocol.CodeAction{ - Title: importFixTitle(importFix.Fix), - Kind: protocol.QuickFix, - Edit: protocol.WorkspaceEdit{ - DocumentChanges: documentChanges(fh, importFix.Edits), - }, - Diagnostics: fixDiagnostics, - }) - } + // If there are any diagnostics relating to the go.mod file, + // add their corresponding quick fixes. + moduleQuickFixes, err := getModuleQuickFixes(ctx, snapshot, diagnostics) + if err != nil { + return nil, err } + codeActions = append(codeActions, moduleQuickFixes...) } - - // Get any actions that might be attributed to missing modules in the go.mod file. - actions, err := mod.SuggestedGoFixes(ctx, snapshot, diagnostics) - if err != nil { - event.Error(ctx, "quick fixes failed", err, tag.URI.Of(uri)) + if wanted[protocol.SourceFixAll] && len(highConfidenceEdits) > 0 { + codeActions = append(codeActions, protocol.CodeAction{ + Title: "Simplifications", + Kind: protocol.SourceFixAll, + Edit: protocol.WorkspaceEdit{ + DocumentChanges: highConfidenceEdits, + }, + }) } - if len(actions) > 0 { - codeActions = append(codeActions, actions...) - } - } - if wanted[protocol.SourceOrganizeImports] && len(importEdits) > 0 { - codeActions = append(codeActions, protocol.CodeAction{ - Title: "Organize Imports", - Kind: protocol.SourceOrganizeImports, - Edit: protocol.WorkspaceEdit{ - DocumentChanges: documentChanges(fh, importEdits), - }, - }) - } - if wanted[protocol.SourceFixAll] && len(highConfidenceEdits) > 0 { - codeActions = append(codeActions, protocol.CodeAction{ - Title: "Simplifications", - Kind: protocol.SourceFixAll, - Edit: protocol.WorkspaceEdit{ - DocumentChanges: highConfidenceEdits, - }, - }) } default: // Unsupported file kind for a code action. @@ -143,6 +141,47 @@ func (s *Server) codeAction(ctx context.Context, params *protocol.CodeActionPara return codeActions, nil } +var missingRequirementRe = regexp.MustCompile(`(.+) is not in your go.mod file`) + +func getModuleQuickFixes(ctx context.Context, snapshot source.Snapshot, diagnostics []protocol.Diagnostic) ([]protocol.CodeAction, error) { + // Don't bother getting quick fixes if we have no relevant diagnostics. + var missingDeps map[string]protocol.Diagnostic + for _, diagnostic := range diagnostics { + matches := missingRequirementRe.FindStringSubmatch(diagnostic.Message) + if len(matches) != 2 { + continue + } + if missingDeps == nil { + missingDeps = make(map[string]protocol.Diagnostic) + } + missingDeps[matches[1]] = diagnostic + } + if len(missingDeps) == 0 { + return nil, nil + } + // Get suggested fixes for each missing dependency. + edits, err := mod.SuggestedGoFixes(ctx, snapshot) + if err != nil { + return nil, err + } + var codeActions []protocol.CodeAction + for dep, diagnostic := range missingDeps { + edit, ok := edits[dep] + if !ok { + continue + } + codeActions = append(codeActions, protocol.CodeAction{ + Title: fmt.Sprintf("Add %s to go.mod", dep), + Diagnostics: []protocol.Diagnostic{diagnostic}, + Edit: protocol.WorkspaceEdit{ + DocumentChanges: []protocol.TextDocumentEdit{edit}, + }, + Kind: protocol.QuickFix, + }) + } + return codeActions, nil +} + func (s *Server) getSupportedCodeActions() []protocol.CodeActionKind { allCodeActionKinds := make(map[protocol.CodeActionKind]struct{}) for _, kinds := range s.session.Options().SupportedCodeActions { @@ -160,28 +199,6 @@ func (s *Server) getSupportedCodeActions() []protocol.CodeActionKind { return result } -// findImports determines if a given diagnostic represents an error that could -// be fixed by organizing imports. -// TODO(rstambler): We need a better way to check this than string matching. -func findImportErrors(diagnostics []protocol.Diagnostic) bool { - for _, diagnostic := range diagnostics { - // "undeclared name: X" may be an unresolved import. - if strings.HasPrefix(diagnostic.Message, "undeclared name: ") { - return true - } - // "could not import: X" may be an invalid import. - if strings.HasPrefix(diagnostic.Message, "could not import: ") { - return true - } - // "X imported but not used" is an unused import. - // "X imported but not used as Y" is an unused import. - if strings.Contains(diagnostic.Message, " imported but not used") { - return true - } - } - return false -} - func importFixTitle(fix *imports.ImportFix) string { var str string switch fix.FixType { @@ -258,8 +275,7 @@ func analysisFixes(ctx context.Context, snapshot source.Snapshot, fh source.File for uri, edits := range fix.Edits { fh, err := snapshot.GetFile(uri) if err != nil { - event.Error(ctx, "no file", err, tag.URI.Of(uri)) - continue + return nil, nil, err } docChanges := documentChanges(fh, edits) if analyzer.HighConfidence { diff --git a/internal/lsp/mod/diagnostics.go b/internal/lsp/mod/diagnostics.go index a4c62acf65..e36db7397c 100644 --- a/internal/lsp/mod/diagnostics.go +++ b/internal/lsp/mod/diagnostics.go @@ -8,8 +8,6 @@ package mod import ( "context" - "fmt" - "regexp" "golang.org/x/mod/modfile" "golang.org/x/tools/internal/lsp/debug/tag" @@ -115,11 +113,10 @@ func SuggestedFixes(ctx context.Context, snapshot source.Snapshot, realfh source return actions } -func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot, diags []protocol.Diagnostic) ([]protocol.CodeAction, error) { - // TODO: We will want to support diagnostics for go.mod files even when the -modfile flag is turned off. +func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot) (map[string]protocol.TextDocumentEdit, error) { + // TODO(rstambler): Support diagnostics for go.mod files even when the + // -modfile flag is turned off. realURI, tempURI := snapshot.View().ModFiles() - - // Check the case when the tempModfile flag is turned off. if realURI == "" || tempURI == "" { return nil, nil } @@ -139,23 +136,16 @@ func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot, diags []pro if err != nil { return nil, err } + if len(missingDeps) == 0 { + return nil, nil + } // Get the contents of the go.mod file before we make any changes. oldContents, _, err := realfh.Read(ctx) if err != nil { return nil, err } - - var actions []protocol.CodeAction - for _, diag := range diags { - re := regexp.MustCompile(`(.+) is not in your go.mod file`) - matches := re.FindStringSubmatch(diag.Message) - if len(matches) != 2 { - continue - } - req := missingDeps[matches[1]] - if req == nil { - continue - } + textDocumentEdits := make(map[string]protocol.TextDocumentEdit) + for dep, req := range missingDeps { // Calculate the quick fix edits that need to be made to the go.mod file. if err := realFile.AddRequire(req.Mod.Path, req.Mod.Version); err != nil { return nil, err @@ -175,27 +165,17 @@ func SuggestedGoFixes(ctx context.Context, snapshot source.Snapshot, diags []pro if err != nil { return nil, err } - action := protocol.CodeAction{ - Title: fmt.Sprintf("Add %s to go.mod", req.Mod.Path), - Kind: protocol.QuickFix, - Diagnostics: []protocol.Diagnostic{diag}, - Edit: protocol.WorkspaceEdit{ - DocumentChanges: []protocol.TextDocumentEdit{ - { - TextDocument: protocol.VersionedTextDocumentIdentifier{ - Version: realfh.Identity().Version, - TextDocumentIdentifier: protocol.TextDocumentIdentifier{ - URI: protocol.URIFromSpanURI(realfh.Identity().URI), - }, - }, - Edits: edits, - }, + textDocumentEdits[dep] = protocol.TextDocumentEdit{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + Version: realfh.Identity().Version, + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: protocol.URIFromSpanURI(realfh.Identity().URI), }, }, + Edits: edits, } - actions = append(actions, action) } - return actions, nil + return textDocumentEdits, nil } func sameDiagnostic(d protocol.Diagnostic, e source.Error) bool { diff --git a/internal/lsp/source/format.go b/internal/lsp/source/format.go index d55ed28ff4..06d0f7bf65 100644 --- a/internal/lsp/source/format.go +++ b/internal/lsp/source/format.go @@ -81,14 +81,12 @@ func AllImportsFixes(ctx context.Context, snapshot Snapshot, fh FileHandle) (all defer done() pgh := snapshot.View().Session().Cache().ParseGoHandle(fh, ParseFull) - err = snapshot.View().RunProcessEnvFunc(ctx, func(opts *imports.Options) error { + if err := snapshot.View().RunProcessEnvFunc(ctx, func(opts *imports.Options) error { allFixEdits, editsPerFix, err = computeImportEdits(ctx, snapshot.View(), pgh, opts) return err - }) - if err != nil { + }); err != nil { return nil, nil, errors.Errorf("computing fix edits: %v", err) } - return allFixEdits, editsPerFix, nil }