diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md
index f78f1bdf732..ec2b6316374 100644
--- a/gopls/doc/analyzers.md
+++ b/gopls/doc/analyzers.md
@@ -796,42 +796,6 @@ Default: on.
Package documentation: [structtag](https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/structtag)
-
-## `stubmethods`: detect missing methods and fix with stub implementations
-
-
-This analyzer detects type-checking errors due to missing methods
-in assignments from concrete types to interface types, and offers
-a suggested fix that will create a set of stub methods so that
-the concrete type satisfies the interface.
-
-For example, this function will not compile because the value
-NegativeErr{} does not implement the "error" interface:
-
- func sqrt(x float64) (float64, error) {
- if x < 0 {
- return 0, NegativeErr{} // error: missing method
- }
- ...
- }
-
- type NegativeErr struct{}
-
-This analyzer will suggest a fix to declare this method:
-
- // Error implements error.Error.
- func (NegativeErr) Error() string {
- panic("unimplemented")
- }
-
-(At least, it appears to behave that way, but technically it
-doesn't use the SuggestedFix mechanism and the stub is created by
-logic in gopls's golang.stub function.)
-
-Default: on.
-
-Package documentation: [stubmethods](https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/stubmethods)
-
## `testinggoroutine`: report calls to (*testing.T).Fatal from goroutines started by a test
diff --git a/gopls/doc/features/diagnostics.md b/gopls/doc/features/diagnostics.md
index f58a6465d1d..b667f69a080 100644
--- a/gopls/doc/features/diagnostics.md
+++ b/gopls/doc/features/diagnostics.md
@@ -119,6 +119,52 @@ Client support:
- **Vim + coc.nvim**: ??
- **CLI**: `gopls check file.go`
+
+
+
+### `stubMethods`: Declare missing methods of type
+
+When a value of a concrete type is assigned to a variable of an
+interface type, but the concrete type does not possess all the
+necessary methods, the type checker will report a "missing method"
+error.
+
+In this situation, gopls offers a quick fix to add stub declarations
+of all the missing methods to the concrete type so that it implements
+the interface.
+
+For example, this function will not compile because the value
+`NegativeErr{}` does not implement the "error" interface:
+
+```go
+func sqrt(x float64) (float64, error) {
+ if x < 0 {
+ return 0, NegativeErr{} // error: missing method
+ }
+ ...
+}
+
+type NegativeErr struct{}
+```
+
+Gopls will offer a quick fix to declare this method:
+
+```go
+
+// Error implements error.Error.
+func (NegativeErr) Error() string {
+ panic("unimplemented")
+}
+```
+
+Beware that the new declarations appear alongside the concrete type,
+which may be in a different file or even package from the cursor
+position.
+(Perhaps gopls should send a `showDocument` request to navigate the
+client there, or a progress notification indicating that something
+happened.)
+
-
- **`refactor.extract.function`** replaces one or more complete statements by a
call to a new function named `newFunction` whose body contains the
statements. The selection must enclose fewer statements than the
diff --git a/gopls/internal/analysis/stubmethods/doc.go b/gopls/internal/analysis/stubmethods/doc.go
deleted file mode 100644
index e1383cfc7e7..00000000000
--- a/gopls/internal/analysis/stubmethods/doc.go
+++ /dev/null
@@ -1,38 +0,0 @@
-// Copyright 2023 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-// Package stubmethods defines a code action for missing interface methods.
-//
-// # Analyzer stubmethods
-//
-// stubmethods: detect missing methods and fix with stub implementations
-//
-// This analyzer detects type-checking errors due to missing methods
-// in assignments from concrete types to interface types, and offers
-// a suggested fix that will create a set of stub methods so that
-// the concrete type satisfies the interface.
-//
-// For example, this function will not compile because the value
-// NegativeErr{} does not implement the "error" interface:
-//
-// func sqrt(x float64) (float64, error) {
-// if x < 0 {
-// return 0, NegativeErr{} // error: missing method
-// }
-// ...
-// }
-//
-// type NegativeErr struct{}
-//
-// This analyzer will suggest a fix to declare this method:
-//
-// // Error implements error.Error.
-// func (NegativeErr) Error() string {
-// panic("unimplemented")
-// }
-//
-// (At least, it appears to behave that way, but technically it
-// doesn't use the SuggestedFix mechanism and the stub is created by
-// logic in gopls's golang.stub function.)
-package stubmethods
diff --git a/gopls/internal/analysis/stubmethods/stubmethods.go b/gopls/internal/analysis/stubmethods/stubmethods.go
index c79a50d51e7..bfb68a44753 100644
--- a/gopls/internal/analysis/stubmethods/stubmethods.go
+++ b/gopls/internal/analysis/stubmethods/stubmethods.go
@@ -4,101 +4,16 @@
package stubmethods
+// TODO(adonovan): rename package to golang/stubmethods or move
+// functions to golang/stub.go.
+
import (
- "bytes"
- _ "embed"
"fmt"
"go/ast"
- "go/format"
"go/token"
"go/types"
- "strings"
-
- "golang.org/x/tools/go/analysis"
- "golang.org/x/tools/go/ast/astutil"
- "golang.org/x/tools/gopls/internal/util/typesutil"
- "golang.org/x/tools/internal/analysisinternal"
- "golang.org/x/tools/internal/typesinternal"
)
-//go:embed doc.go
-var doc string
-
-var Analyzer = &analysis.Analyzer{
- Name: "stubmethods",
- Doc: analysisinternal.MustExtractDoc(doc, "stubmethods"),
- Run: run,
- RunDespiteErrors: true,
- URL: "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/stubmethods",
-}
-
-// TODO(rfindley): remove this thin wrapper around the stubmethods refactoring,
-// and eliminate the stubmethods analyzer.
-//
-// Previous iterations used the analysis framework for computing refactorings,
-// which proved inefficient.
-func run(pass *analysis.Pass) (interface{}, error) {
- for _, err := range pass.TypeErrors {
- var file *ast.File
- for _, f := range pass.Files {
- if f.Pos() <= err.Pos && err.Pos < f.End() {
- file = f
- break
- }
- }
- // Get the end position of the error.
- _, _, end, ok := typesinternal.ReadGo116ErrorData(err)
- if !ok {
- var buf bytes.Buffer
- if err := format.Node(&buf, pass.Fset, file); err != nil {
- continue
- }
- end = analysisinternal.TypeErrorEndPos(pass.Fset, buf.Bytes(), err.Pos)
- }
- if diag, ok := DiagnosticForError(pass.Fset, file, err.Pos, end, err.Msg, pass.TypesInfo); ok {
- pass.Report(diag)
- }
- }
-
- return nil, nil
-}
-
-// MatchesMessage reports whether msg matches the error message sought after by
-// the stubmethods fix.
-func MatchesMessage(msg string) bool {
- return strings.Contains(msg, "missing method") || strings.HasPrefix(msg, "cannot convert") || strings.Contains(msg, "not implement")
-}
-
-// DiagnosticForError computes a diagnostic suggesting to implement an
-// interface to fix the type checking error defined by (start, end, msg).
-//
-// If no such fix is possible, the second result is false.
-func DiagnosticForError(fset *token.FileSet, file *ast.File, start, end token.Pos, msg string, info *types.Info) (analysis.Diagnostic, bool) {
- if !MatchesMessage(msg) {
- return analysis.Diagnostic{}, false
- }
-
- path, _ := astutil.PathEnclosingInterval(file, start, end)
- si := GetStubInfo(fset, info, path, start)
- if si == nil {
- return analysis.Diagnostic{}, false
- }
- qf := typesutil.FileQualifier(file, si.Concrete.Obj().Pkg(), info)
- iface := types.TypeString(si.Interface.Type(), qf)
- return analysis.Diagnostic{
- Pos: start,
- End: end,
- Message: msg,
- Category: FixCategory,
- SuggestedFixes: []analysis.SuggestedFix{{
- Message: fmt.Sprintf("Declare missing methods of %s", iface),
- // No TextEdits => computed later by gopls.
- }},
- }, true
-}
-
-const FixCategory = "stubmethods" // recognized by gopls ApplyFix
-
// StubInfo represents a concrete type
// that wants to stub out an interface type
type StubInfo struct {
diff --git a/gopls/internal/analysis/stubmethods/stubmethods_test.go b/gopls/internal/analysis/stubmethods/stubmethods_test.go
deleted file mode 100644
index 9c744c9b7a3..00000000000
--- a/gopls/internal/analysis/stubmethods/stubmethods_test.go
+++ /dev/null
@@ -1,17 +0,0 @@
-// Copyright 2023 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package stubmethods_test
-
-import (
- "testing"
-
- "golang.org/x/tools/go/analysis/analysistest"
- "golang.org/x/tools/gopls/internal/analysis/stubmethods"
-)
-
-func Test(t *testing.T) {
- testdata := analysistest.TestData()
- analysistest.Run(t, testdata, stubmethods.Analyzer, "typeparams")
-}
diff --git a/gopls/internal/analysis/stubmethods/testdata/src/typeparams/implement.go b/gopls/internal/analysis/stubmethods/testdata/src/typeparams/implement.go
deleted file mode 100644
index 7b6f2911ea9..00000000000
--- a/gopls/internal/analysis/stubmethods/testdata/src/typeparams/implement.go
+++ /dev/null
@@ -1,15 +0,0 @@
-// Copyright 2023 The Go Authors. All rights reserved.
-// Use of this source code is governed by a BSD-style
-// license that can be found in the LICENSE file.
-
-package stubmethods
-
-var _ I = Y{} // want "does not implement I"
-
-type I interface{ F() }
-
-type X struct{}
-
-func (X) F(string) {}
-
-type Y struct{ X }
diff --git a/gopls/internal/doc/api.json b/gopls/internal/doc/api.json
index d294ea0197d..b076abd26b0 100644
--- a/gopls/internal/doc/api.json
+++ b/gopls/internal/doc/api.json
@@ -567,11 +567,6 @@
"Doc": "check that struct field tags conform to reflect.StructTag.Get\n\nAlso report certain struct tags (json, xml) used with unexported fields.",
"Default": "true"
},
- {
- "Name": "\"stubmethods\"",
- "Doc": "detect missing methods and fix with stub implementations\n\nThis analyzer detects type-checking errors due to missing methods\nin assignments from concrete types to interface types, and offers\na suggested fix that will create a set of stub methods so that\nthe concrete type satisfies the interface.\n\nFor example, this function will not compile because the value\nNegativeErr{} does not implement the \"error\" interface:\n\n\tfunc sqrt(x float64) (float64, error) {\n\t\tif x \u003c 0 {\n\t\t\treturn 0, NegativeErr{} // error: missing method\n\t\t}\n\t\t...\n\t}\n\n\ttype NegativeErr struct{}\n\nThis analyzer will suggest a fix to declare this method:\n\n\t// Error implements error.Error.\n\tfunc (NegativeErr) Error() string {\n\t\tpanic(\"unimplemented\")\n\t}\n\n(At least, it appears to behave that way, but technically it\ndoesn't use the SuggestedFix mechanism and the stub is created by\nlogic in gopls's golang.stub function.)",
- "Default": "true"
- },
{
"Name": "\"testinggoroutine\"",
"Doc": "report calls to (*testing.T).Fatal from goroutines started by a test\n\nFunctions that abruptly terminate a test, such as the Fatal, Fatalf, FailNow, and\nSkip{,f,Now} methods of *testing.T, must be called from the test goroutine itself.\nThis checker detects calls to these functions that occur within a goroutine\nstarted by the test. For example:\n\n\tfunc TestFoo(t *testing.T) {\n\t go func() {\n\t t.Fatal(\"oops\") // error: (*T).Fatal called from non-test goroutine\n\t }()\n\t}",
@@ -1238,12 +1233,6 @@
"URL": "https://pkg.go.dev/golang.org/x/tools/go/analysis/passes/structtag",
"Default": true
},
- {
- "Name": "stubmethods",
- "Doc": "detect missing methods and fix with stub implementations\n\nThis analyzer detects type-checking errors due to missing methods\nin assignments from concrete types to interface types, and offers\na suggested fix that will create a set of stub methods so that\nthe concrete type satisfies the interface.\n\nFor example, this function will not compile because the value\nNegativeErr{} does not implement the \"error\" interface:\n\n\tfunc sqrt(x float64) (float64, error) {\n\t\tif x \u003c 0 {\n\t\t\treturn 0, NegativeErr{} // error: missing method\n\t\t}\n\t\t...\n\t}\n\n\ttype NegativeErr struct{}\n\nThis analyzer will suggest a fix to declare this method:\n\n\t// Error implements error.Error.\n\tfunc (NegativeErr) Error() string {\n\t\tpanic(\"unimplemented\")\n\t}\n\n(At least, it appears to behave that way, but technically it\ndoesn't use the SuggestedFix mechanism and the stub is created by\nlogic in gopls's golang.stub function.)",
- "URL": "https://pkg.go.dev/golang.org/x/tools/gopls/internal/analysis/stubmethods",
- "Default": true
- },
{
"Name": "testinggoroutine",
"Doc": "report calls to (*testing.T).Fatal from goroutines started by a test\n\nFunctions that abruptly terminate a test, such as the Fatal, Fatalf, FailNow, and\nSkip{,f,Now} methods of *testing.T, must be called from the test goroutine itself.\nThis checker detects calls to these functions that occur within a goroutine\nstarted by the test. For example:\n\n\tfunc TestFoo(t *testing.T) {\n\t go func() {\n\t t.Fatal(\"oops\") // error: (*T).Fatal called from non-test goroutine\n\t }()\n\t}",
diff --git a/gopls/internal/golang/change_quote.go b/gopls/internal/golang/change_quote.go
index e793ec72c89..67f29430700 100644
--- a/gopls/internal/golang/change_quote.go
+++ b/gopls/internal/golang/change_quote.go
@@ -68,5 +68,5 @@ func convertStringLiteral(req *codeActionsRequest) {
bug.Reportf("failed to convert diff.Edit to protocol.TextEdit:%v", err)
return
}
- req.addEditAction(title, protocol.DocumentChangeEdit(req.fh, textedits))
+ req.addEditAction(title, nil, protocol.DocumentChangeEdit(req.fh, textedits))
}
diff --git a/gopls/internal/golang/codeaction.go b/gopls/internal/golang/codeaction.go
index f82abc6ce0d..b68b317b6db 100644
--- a/gopls/internal/golang/codeaction.go
+++ b/gopls/internal/golang/codeaction.go
@@ -19,6 +19,7 @@ import (
"golang.org/x/tools/go/ast/astutil"
"golang.org/x/tools/gopls/internal/analysis/fillstruct"
"golang.org/x/tools/gopls/internal/analysis/fillswitch"
+ "golang.org/x/tools/gopls/internal/analysis/stubmethods"
"golang.org/x/tools/gopls/internal/cache"
"golang.org/x/tools/gopls/internal/cache/parsego"
"golang.org/x/tools/gopls/internal/file"
@@ -26,6 +27,7 @@ import (
"golang.org/x/tools/gopls/internal/protocol"
"golang.org/x/tools/gopls/internal/protocol/command"
"golang.org/x/tools/gopls/internal/settings"
+ "golang.org/x/tools/gopls/internal/util/typesutil"
"golang.org/x/tools/internal/event"
"golang.org/x/tools/internal/imports"
"golang.org/x/tools/internal/typesinternal"
@@ -135,13 +137,13 @@ type codeActionsRequest struct {
}
// addApplyFixAction adds an ApplyFix command-based CodeAction to the result.
-func (req *codeActionsRequest) addApplyFixAction(title, fix string, loc protocol.Location) *protocol.CodeAction {
+func (req *codeActionsRequest) addApplyFixAction(title, fix string, loc protocol.Location) {
cmd := command.NewApplyFixCommand(title, command.ApplyFixArgs{
Fix: fix,
Location: loc,
ResolveEdits: req.resolveEdits(),
})
- return req.addCommandAction(cmd, true)
+ req.addCommandAction(cmd, true)
}
// addCommandAction adds a CodeAction to the result based on the provided command.
@@ -153,7 +155,7 @@ func (req *codeActionsRequest) addApplyFixAction(title, fix string, loc protocol
// Set allowResolveEdits only for actions that generate edits.
//
// Otherwise, the command is set as the code action operation.
-func (req *codeActionsRequest) addCommandAction(cmd *protocol.Command, allowResolveEdits bool) *protocol.CodeAction {
+func (req *codeActionsRequest) addCommandAction(cmd *protocol.Command, allowResolveEdits bool) {
act := protocol.CodeAction{
Title: cmd.Title,
Kind: req.kind,
@@ -168,24 +170,22 @@ func (req *codeActionsRequest) addCommandAction(cmd *protocol.Command, allowReso
} else {
act.Command = cmd
}
- return req.addAction(act)
+ req.addAction(act)
}
// addCommandAction adds an edit-based CodeAction to the result.
-func (req *codeActionsRequest) addEditAction(title string, changes ...protocol.DocumentChange) *protocol.CodeAction {
- return req.addAction(protocol.CodeAction{
- Title: title,
- Kind: req.kind,
- Edit: protocol.NewWorkspaceEdit(changes...),
+func (req *codeActionsRequest) addEditAction(title string, fixedDiagnostics []protocol.Diagnostic, changes ...protocol.DocumentChange) {
+ req.addAction(protocol.CodeAction{
+ Title: title,
+ Kind: req.kind,
+ Diagnostics: fixedDiagnostics,
+ Edit: protocol.NewWorkspaceEdit(changes...),
})
}
// addAction adds a code action to the response.
-// It returns an ephemeral pointer to the action in situ.
-// which may be used (but only immediately) for further mutation.
-func (req *codeActionsRequest) addAction(act protocol.CodeAction) *protocol.CodeAction {
+func (req *codeActionsRequest) addAction(act protocol.CodeAction) {
*req.actions = append(*req.actions, act)
- return &(*req.actions)[len(*req.actions)-1]
}
// resolveEdits reports whether the client can resolve edits lazily.
@@ -225,7 +225,7 @@ type codeActionProducer struct {
}
var codeActionProducers = [...]codeActionProducer{
- {kind: protocol.QuickFix, fn: quickFix},
+ {kind: protocol.QuickFix, fn: quickFix, needPkg: true},
{kind: protocol.SourceOrganizeImports, fn: sourceOrganizeImports},
{kind: settings.GoAssembly, fn: goAssembly, needPkg: true},
{kind: settings.GoDoc, fn: goDoc, needPkg: true},
@@ -257,13 +257,15 @@ func sourceOrganizeImports(ctx context.Context, req *codeActionsRequest) error {
// Send all of the import edits as one code action
// if the file is being organized.
if len(res.allFixEdits) > 0 {
- req.addEditAction("Organize Imports", protocol.DocumentChangeEdit(req.fh, res.allFixEdits))
+ req.addEditAction("Organize Imports", nil, protocol.DocumentChangeEdit(req.fh, res.allFixEdits))
}
return nil
}
-// quickFix produces code actions that add/delete/rename imports to fix type errors.
+// quickFix produces code actions that fix errors,
+// for example by adding/deleting/renaming imports,
+// or declaring the missing methods of a type.
func quickFix(ctx context.Context, req *codeActionsRequest) error {
// Only compute quick fixes if there are any diagnostics to fix.
if len(req.diagnostics) == 0 {
@@ -279,14 +281,42 @@ func quickFix(ctx context.Context, req *codeActionsRequest) error {
// Separate this into a set of codeActions per diagnostic, where
// each action is the addition, removal, or renaming of one import.
for _, importFix := range res.editsPerFix {
- fixed := fixedByImportFix(importFix.fix, req.diagnostics)
- if len(fixed) == 0 {
+ fixedDiags := fixedByImportFix(importFix.fix, req.diagnostics)
+ if len(fixedDiags) == 0 {
continue
}
- act := req.addEditAction(
- importFixTitle(importFix.fix),
- protocol.DocumentChangeEdit(req.fh, importFix.edits))
- act.Diagnostics = fixed
+ req.addEditAction(importFixTitle(importFix.fix), fixedDiags, protocol.DocumentChangeEdit(req.fh, importFix.edits))
+ }
+
+ // Quick fixes for type errors.
+ info := req.pkg.TypesInfo()
+ for _, typeError := range req.pkg.TypeErrors() {
+ // Does type error overlap with CodeAction range?
+ start, end := typeError.Pos, typeError.Pos
+ if _, _, endPos, ok := typesinternal.ReadGo116ErrorData(typeError); ok {
+ end = endPos
+ }
+ typeErrorRange, err := req.pgf.PosRange(start, end)
+ if err != nil || !protocol.Intersect(typeErrorRange, req.loc.Range) {
+ continue
+ }
+
+ // "Missing method" error? (stubmethods)
+ // Offer a "Declare missing methods of INTERFACE" code action.
+ // See [stubMethodsFixer] for command implementation.
+ msg := typeError.Error()
+ if strings.Contains(msg, "missing method") ||
+ strings.HasPrefix(msg, "cannot convert") ||
+ strings.Contains(msg, "not implement") {
+ path, _ := astutil.PathEnclosingInterval(req.pgf.File, start, end)
+ si := stubmethods.GetStubInfo(req.pkg.FileSet(), info, path, start)
+ if si != nil {
+ qf := typesutil.FileQualifier(req.pgf.File, si.Concrete.Obj().Pkg(), info)
+ iface := types.TypeString(si.Interface.Type(), qf)
+ msg := fmt.Sprintf("Declare missing methods of %s", iface)
+ req.addApplyFixAction(msg, fixStubMethods, req.loc)
+ }
+ }
}
return nil
@@ -473,7 +503,7 @@ func refactorRewriteJoinLines(ctx context.Context, req *codeActionsRequest) erro
return nil
}
-// refactorRewriteFillStruct produces "Join ITEMS into one line" code actions.
+// refactorRewriteFillStruct produces "Fill STRUCT" code actions.
// See [fillstruct.SuggestedFix] for command implementation.
func refactorRewriteFillStruct(ctx context.Context, req *codeActionsRequest) error {
// fillstruct.Diagnose is a lazy analyzer: all it gives us is
@@ -498,7 +528,7 @@ func refactorRewriteFillSwitch(ctx context.Context, req *codeActionsRequest) err
if err != nil {
return err
}
- req.addEditAction(diag.Message, changes...)
+ req.addEditAction(diag.Message, nil, changes...)
}
return nil
@@ -560,6 +590,7 @@ func canRemoveParameter(pkg *cache.Package, pgf *parsego.File, rng protocol.Rang
func refactorInlineCall(ctx context.Context, req *codeActionsRequest) error {
// To avoid distraction (e.g. VS Code lightbulb), offer "inline"
// only after a selection or explicit menu operation.
+ // TODO(adonovan): remove this (and req.trigger); see comment at TestVSCodeIssue65167.
if req.trigger == protocol.CodeActionAutomatic && req.loc.Empty() {
return nil
}
diff --git a/gopls/internal/golang/fix.go b/gopls/internal/golang/fix.go
index 3844fc0d65c..7c44aa4d273 100644
--- a/gopls/internal/golang/fix.go
+++ b/gopls/internal/golang/fix.go
@@ -14,7 +14,6 @@ import (
"golang.org/x/tools/go/analysis"
"golang.org/x/tools/gopls/internal/analysis/embeddirective"
"golang.org/x/tools/gopls/internal/analysis/fillstruct"
- "golang.org/x/tools/gopls/internal/analysis/stubmethods"
"golang.org/x/tools/gopls/internal/analysis/undeclaredname"
"golang.org/x/tools/gopls/internal/analysis/unusedparams"
"golang.org/x/tools/gopls/internal/cache"
@@ -66,6 +65,7 @@ const (
fixInvertIfCondition = "invert_if_condition"
fixSplitLines = "split_lines"
fixJoinLines = "join_lines"
+ fixStubMethods = "stub_methods"
)
// ApplyFix applies the specified kind of suggested fix to the given
@@ -98,7 +98,6 @@ func ApplyFix(ctx context.Context, fix string, snapshot *cache.Snapshot, fh file
// These match the Diagnostic.Category.
embeddirective.FixCategory: addEmbedImport,
fillstruct.FixCategory: singleFile(fillstruct.SuggestedFix),
- stubmethods.FixCategory: stubMethodsFixer,
undeclaredname.FixCategory: singleFile(undeclaredname.SuggestedFix),
// Ad-hoc fixers: these are used when the command is
@@ -110,6 +109,7 @@ func ApplyFix(ctx context.Context, fix string, snapshot *cache.Snapshot, fh file
fixInvertIfCondition: singleFile(invertIfCondition),
fixSplitLines: singleFile(splitLines),
fixJoinLines: singleFile(joinLines),
+ fixStubMethods: stubMethodsFixer,
}
fixer, ok := fixers[fix]
if !ok {
diff --git a/gopls/internal/golang/stub.go b/gopls/internal/golang/stub.go
index 84299171fe4..c8a47b609c4 100644
--- a/gopls/internal/golang/stub.go
+++ b/gopls/internal/golang/stub.go
@@ -40,6 +40,7 @@ func stubMethodsFixer(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.
// A function-local type cannot be stubbed
// since there's nowhere to put the methods.
+ // TODO(adonovan): move this check into GetStubInfo instead of offering a bad fix.
conc := si.Concrete.Obj()
if conc.Parent() != conc.Pkg().Scope() {
return nil, nil, fmt.Errorf("local type %q cannot be stubbed", conc.Name())
diff --git a/gopls/internal/protocol/span.go b/gopls/internal/protocol/span.go
index 213251b1adb..2911d4aa29b 100644
--- a/gopls/internal/protocol/span.go
+++ b/gopls/internal/protocol/span.go
@@ -58,12 +58,37 @@ func ComparePosition(a, b Position) int {
return 0
}
-func Intersect(a, b Range) bool {
- if a.Start.Line > b.End.Line || a.End.Line < b.Start.Line {
- return false
+// Intersect reports whether x and y intersect.
+//
+// Two non-empty half-open integer intervals intersect iff:
+//
+// y.start < x.end && x.start < y.end
+//
+// Mathematical conventional views an interval as a set of integers.
+// An empty interval is the empty set, so its intersection with any
+// other interval is empty, and thus an empty interval does not
+// intersect any other interval.
+//
+// However, this function uses a looser definition appropriate for
+// text selections: if either x or y is empty, it uses <= operators
+// instead, so an empty range within or abutting a non-empty range is
+// considered to overlap it, and an empty range overlaps itself.
+//
+// This handles the common case in which there is no selection, but
+// the cursor is at the start or end of an expression and the caller
+// wants to know whether the cursor intersects the range of the
+// expression. The answer in this case should be yes, even though the
+// selection is empty. Similarly the answer should also be yes if the
+// cursor is properly within the range of the expression. But a
+// non-empty selection abutting the expression should not be
+// considered to intersect it.
+func Intersect(x, y Range) bool {
+ r1 := ComparePosition(x.Start, y.End)
+ r2 := ComparePosition(y.Start, x.End)
+ if r1 < 0 && r2 < 0 {
+ return true // mathematical intersection
}
- return !((a.Start.Line == b.End.Line) && a.Start.Character > b.End.Character ||
- (a.End.Line == b.Start.Line) && a.End.Character < b.Start.Character)
+ return (x.Empty() || y.Empty()) && r1 <= 0 && r2 <= 0
}
// Format implements fmt.Formatter.
diff --git a/gopls/internal/settings/analysis.go b/gopls/internal/settings/analysis.go
index 65ecb215c02..86fa4766b51 100644
--- a/gopls/internal/settings/analysis.go
+++ b/gopls/internal/settings/analysis.go
@@ -55,7 +55,6 @@ import (
"golang.org/x/tools/gopls/internal/analysis/simplifycompositelit"
"golang.org/x/tools/gopls/internal/analysis/simplifyrange"
"golang.org/x/tools/gopls/internal/analysis/simplifyslice"
- "golang.org/x/tools/gopls/internal/analysis/stubmethods"
"golang.org/x/tools/gopls/internal/analysis/undeclaredname"
"golang.org/x/tools/gopls/internal/analysis/unusedparams"
"golang.org/x/tools/gopls/internal/analysis/unusedvariable"
@@ -202,7 +201,6 @@ func init() {
{analyzer: fillreturns.Analyzer, enabled: true},
{analyzer: nonewvars.Analyzer, enabled: true},
{analyzer: noresultvalues.Analyzer, enabled: true},
- {analyzer: stubmethods.Analyzer, enabled: true},
{analyzer: undeclaredname.Analyzer, enabled: true},
// TODO(rfindley): why isn't the 'unusedvariable' analyzer enabled, if it
// is only enhancing type errors with suggested fixes?
diff --git a/gopls/internal/test/integration/misc/codeactions_test.go b/gopls/internal/test/integration/misc/codeactions_test.go
index 354921afc01..7e5ac9aba62 100644
--- a/gopls/internal/test/integration/misc/codeactions_test.go
+++ b/gopls/internal/test/integration/misc/codeactions_test.go
@@ -80,6 +80,16 @@ func g() {}
// Test refactor.inline.call is not included in automatically triggered code action
// unless users want refactoring.
+//
+// (The mechanism behind this behavior has changed. It was added when
+// we used to interpret CodeAction(Only=[]) as "all kinds", which was
+// a distracting nuisance (too many lightbulbs); this was fixed by
+// adding special logic to refactor.inline.call to respect the trigger
+// kind; but now we do this for all actions (for similar reasons) and
+// interpret Only=[] as Only=[quickfix] unless triggerKind=invoked;
+// except that the test client always requests CodeAction(Only=[""]).
+// So, we should remove the special logic from refactorInlineCall
+// and vary the Only parameter used by the test client.)
func TestVSCodeIssue65167(t *testing.T) {
const vim1 = `package main
diff --git a/gopls/internal/test/marker/doc.go b/gopls/internal/test/marker/doc.go
index 5bcb31b46de..bd23a4f12ef 100644
--- a/gopls/internal/test/marker/doc.go
+++ b/gopls/internal/test/marker/doc.go
@@ -220,12 +220,12 @@ The following markers are supported within marker tests:
the active parameter (an index) highlighted.
- suggestedfix(location, regexp, golden): like diag, the location and
- regexp identify an expected diagnostic. This diagnostic must
- to have exactly one associated code action of the specified kind.
+ regexp identify an expected diagnostic, which must have exactly one
+ associated "quickfix" code action.
This action is executed for its editing effects on the source files.
Like rename, the golden directory contains the expected transformed files.
- - suggestedfixerr(location, regexp, kind, wantError): specifies that the
+ - suggestedfixerr(location, regexp, wantError): specifies that the
suggestedfix operation should fail with an error that matches the expectation.
(Failures in the computation to offer a fix do not generally result
in LSP errors, so this marker is not appropriate for testing them.)
diff --git a/gopls/internal/test/marker/marker_test.go b/gopls/internal/test/marker/marker_test.go
index a128bcb7f18..1478fe631c7 100644
--- a/gopls/internal/test/marker/marker_test.go
+++ b/gopls/internal/test/marker/marker_test.go
@@ -2028,8 +2028,10 @@ func (mark marker) consumeExtraNotes(name string, f func(marker)) {
// suggestedfixMarker implements the @suggestedfix(location, regexp,
// kind, golden) marker. It acts like @diag(location, regexp), to set
-// the expectation of a diagnostic, but then it applies the first code
-// action of the specified kind suggested by the matched diagnostic.
+// the expectation of a diagnostic, but then it applies the "quickfix"
+// code action (which must be unique) suggested by the matched diagnostic.
+//
+// TODO(adonovan): rename to @quickfix, since that's the LSP term.
func suggestedfixMarker(mark marker, loc protocol.Location, re *regexp.Regexp, golden *Golden) {
loc.Range.End = loc.Range.Start // diagnostics ignore end position.
// Find and remove the matching diagnostic.
diff --git a/gopls/internal/test/marker/testdata/suggestedfix/stub.txt b/gopls/internal/test/marker/testdata/suggestedfix/stub.txt
index e31494ae461..fc10d8e58ad 100644
--- a/gopls/internal/test/marker/testdata/suggestedfix/stub.txt
+++ b/gopls/internal/test/marker/testdata/suggestedfix/stub.txt
@@ -347,9 +347,10 @@ type (
func _() {
// Local types can't be stubbed as there's nowhere to put the methods.
- // The suggestedfix assertion can't express this yet. TODO(adonovan): support it.
+ // Check that executing the code action causes an error, not file corruption.
+ // TODO(adonovan): it would be better not to offer the quick fix in this case.
type local struct{}
- var _ io.ReadCloser = local{} //@diag("local", re"does not implement")
+ var _ io.ReadCloser = local{} //@suggestedfixerr("local", re"does not implement", "local type \"local\" cannot be stubbed")
}
-- @typedecl_group/typedecl_group.go --
@@ -18 +18,10 @@