From f657c39f43e11e1d05449589b687db0c18524c23 Mon Sep 17 00:00:00 2001 From: Marwan Sulaiman Date: Thu, 16 Sep 2021 17:26:44 -0400 Subject: [PATCH] gopls,internal/lsp: Implement method stubbing via CodeAction This CL adds a quickfix CodeAction that detects "missing method" compiler errors and suggests adding method stubs to the concrete type that would implement the interface. There are many ways that a user might indicate a concrete type is meant to be used as an interface. This PR detects two types of those errors: variable declaration and function returns. For variable declarations, things like the following should be detected: 1. var _ SomeInterface = SomeType{} 2. var _ = SomeInterface(SomeType{}) 3. var _ SomeInterface = (*SomeType)(nil) For function returns, the following example is the primary detection: func newIface() SomeInterface { return &SomeType{} } More detections can be added in the future of course. Fixes golang/go#37537 Change-Id: Ibb7784622184c9885eff2ccc786767682876b4d3 --- .../lsp/analysis/stubmethods/stubmethods.go | 338 ++++++++++++++++ internal/lsp/source/fix.go | 2 + internal/lsp/source/options.go | 7 + internal/lsp/source/stub.go | 362 ++++++++++++++++++ internal/lsp/testdata/stub/other/other.go | 10 + .../lsp/testdata/stub/stub_add_selector.go | 12 + .../testdata/stub/stub_add_selector.go.golden | 19 + internal/lsp/testdata/stub/stub_assign.go | 10 + .../lsp/testdata/stub/stub_assign.go.golden | 17 + .../testdata/stub/stub_assign_multivars.go | 11 + .../stub/stub_assign_multivars.go.golden | 18 + internal/lsp/testdata/stub/stub_embedded.go | 15 + .../lsp/testdata/stub/stub_embedded.go.golden | 37 ++ internal/lsp/testdata/stub/stub_err.go | 7 + internal/lsp/testdata/stub/stub_err.go.golden | 14 + .../lsp/testdata/stub/stub_function_return.go | 11 + .../stub/stub_function_return.go.golden | 18 + .../lsp/testdata/stub/stub_ignored_imports.go | 18 + .../stub/stub_ignored_imports.go.golden | 26 ++ internal/lsp/testdata/stub/stub_multi_var.go | 11 + .../testdata/stub/stub_multi_var.go.golden | 18 + internal/lsp/testdata/stub/stub_pointer.go | 9 + .../lsp/testdata/stub/stub_pointer.go.golden | 16 + .../lsp/testdata/stub/stub_renamed_import.go | 11 + .../stub/stub_renamed_import.go.golden | 18 + .../stub/stub_renamed_import_iface.go | 13 + .../stub/stub_renamed_import_iface.go.golden | 22 ++ internal/lsp/testdata/stub/stub_stdlib.go | 9 + .../lsp/testdata/stub/stub_stdlib.go.golden | 16 + internal/lsp/testdata/summary.txt.golden | 2 +- .../lsp/testdata/summary_go1.18.txt.golden | 2 +- 31 files changed, 1097 insertions(+), 2 deletions(-) create mode 100644 internal/lsp/analysis/stubmethods/stubmethods.go create mode 100644 internal/lsp/source/stub.go create mode 100644 internal/lsp/testdata/stub/other/other.go create mode 100644 internal/lsp/testdata/stub/stub_add_selector.go create mode 100644 internal/lsp/testdata/stub/stub_add_selector.go.golden create mode 100644 internal/lsp/testdata/stub/stub_assign.go create mode 100644 internal/lsp/testdata/stub/stub_assign.go.golden create mode 100644 internal/lsp/testdata/stub/stub_assign_multivars.go create mode 100644 internal/lsp/testdata/stub/stub_assign_multivars.go.golden create mode 100644 internal/lsp/testdata/stub/stub_embedded.go create mode 100644 internal/lsp/testdata/stub/stub_embedded.go.golden create mode 100644 internal/lsp/testdata/stub/stub_err.go create mode 100644 internal/lsp/testdata/stub/stub_err.go.golden create mode 100644 internal/lsp/testdata/stub/stub_function_return.go create mode 100644 internal/lsp/testdata/stub/stub_function_return.go.golden create mode 100644 internal/lsp/testdata/stub/stub_ignored_imports.go create mode 100644 internal/lsp/testdata/stub/stub_ignored_imports.go.golden create mode 100644 internal/lsp/testdata/stub/stub_multi_var.go create mode 100644 internal/lsp/testdata/stub/stub_multi_var.go.golden create mode 100644 internal/lsp/testdata/stub/stub_pointer.go create mode 100644 internal/lsp/testdata/stub/stub_pointer.go.golden create mode 100644 internal/lsp/testdata/stub/stub_renamed_import.go create mode 100644 internal/lsp/testdata/stub/stub_renamed_import.go.golden create mode 100644 internal/lsp/testdata/stub/stub_renamed_import_iface.go create mode 100644 internal/lsp/testdata/stub/stub_renamed_import_iface.go.golden create mode 100644 internal/lsp/testdata/stub/stub_stdlib.go create mode 100644 internal/lsp/testdata/stub/stub_stdlib.go.golden diff --git a/internal/lsp/analysis/stubmethods/stubmethods.go b/internal/lsp/analysis/stubmethods/stubmethods.go new file mode 100644 index 00000000000..ff6b5020267 --- /dev/null +++ b/internal/lsp/analysis/stubmethods/stubmethods.go @@ -0,0 +1,338 @@ +// Copyright 2022 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 + +import ( + "bytes" + "fmt" + "go/ast" + "go/format" + "go/token" + "go/types" + "strconv" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/analysis/passes/inspect" + "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/analysisinternal" + "golang.org/x/tools/internal/typesinternal" +) + +const Doc = `stub methods analyzer + +This analyzer generates method stubs for concrete types +in order to implement a target interface` + +var Analyzer = &analysis.Analyzer{ + Name: "stubmethods", + Doc: Doc, + Requires: []*analysis.Analyzer{inspect.Analyzer}, + Run: run, + RunDespiteErrors: true, +} + +func run(pass *analysis.Pass) (interface{}, error) { + for _, err := range analysisinternal.GetTypeErrors(pass) { + ifaceErr := strings.Contains(err.Msg, "missing method") || strings.HasPrefix(err.Msg, "cannot convert") + if !ifaceErr { + continue + } + var file *ast.File + for _, f := range pass.Files { + if f.Pos() <= err.Pos && err.Pos < f.End() { + file = f + break + } + } + if file == nil { + continue + } + // Get the end position of the error. + _, _, endPos, ok := typesinternal.ReadGo116ErrorData(err) + if !ok { + var buf bytes.Buffer + if err := format.Node(&buf, pass.Fset, file); err != nil { + continue + } + endPos = analysisinternal.TypeErrorEndPos(pass.Fset, buf.Bytes(), err.Pos) + } + path, _ := astutil.PathEnclosingInterval(file, err.Pos, endPos) + si := GetStubInfo(pass.TypesInfo, path, pass.Pkg, err.Pos) + if si == nil { + continue + } + qf := RelativeToFiles(si.Concrete.Pkg(), file, nil, nil) + pass.Report(analysis.Diagnostic{ + Pos: err.Pos, + End: endPos, + Message: fmt.Sprintf("Implement %s", types.TypeString(si.Interface.Type(), qf)), + }) + } + return nil, nil +} + +// StubInfo represents a concrete type +// that wants to stub out an interface type +type StubInfo struct { + Interface types.Object + Concrete types.Object + Pointer bool +} + +// GetStubInfo determines whether the "missing method error" +// can be used to deduced what the concrete and interface types are. +func GetStubInfo(ti *types.Info, path []ast.Node, pkg *types.Package, pos token.Pos) *StubInfo { + for _, n := range path { + switch n := n.(type) { + case *ast.ValueSpec: + return fromValueSpec(ti, n, pkg, pos) + case *ast.ReturnStmt: + si, _ := fromReturnStmt(ti, pos, path, n, pkg) + return si + case *ast.AssignStmt: + return fromAssignStmt(ti, n, pkg, pos) + } + } + return nil +} + +// RelativeToFiles returns a types.Qualifier that formats package names +// according to the files where the concrete and interface types are defined. +// +// This is similar to types.RelativeTo except if a file imports the package with a different name, +// then it will use it. And if the file does import the package but it is ignored, +// then it will return the original name. It also prefers package names in ifaceFile in case +// an import is missing from concFile but is present in ifaceFile. +// +// Additionally, if missingImport is not nil, the function will be called whenever the concFile +// is presented with a package that is not imported. This is useful so that as types.TypeString is +// formatting a function signature, it is identifying packages that will need to be imported when +// stubbing an interface. +func RelativeToFiles(concPkg *types.Package, concFile, ifaceFile *ast.File, missingImport func(name, path string)) types.Qualifier { + return func(other *types.Package) string { + if other == concPkg { + return "" + } + + // Check if the concrete file already has the given import, + // if so return the default package name or the renamed import statement. + for _, imp := range concFile.Imports { + impPath, _ := strconv.Unquote(imp.Path.Value) + isIgnored := imp.Name != nil && (imp.Name.Name == "." || imp.Name.Name == "_") + if impPath == other.Path() && !isIgnored { + importName := other.Name() + if imp.Name != nil { + importName = imp.Name.Name + } + return importName + } + } + + // If the concrete file does not have the import, check if the package + // is renamed in the interface file and prefer that. + var importName string + if ifaceFile != nil { + for _, imp := range ifaceFile.Imports { + impPath, _ := strconv.Unquote(imp.Path.Value) + isIgnored := imp.Name != nil && (imp.Name.Name == "." || imp.Name.Name == "_") + if impPath == other.Path() && !isIgnored { + if imp.Name != nil && imp.Name.Name != concPkg.Name() { + importName = imp.Name.Name + } + break + } + } + } + + if missingImport != nil { + missingImport(importName, other.Path()) + } + + // Up until this point, importName must stay empty when calling missingImport, + // otherwise we'd end up with `import time "time"` which doesn't look idiomatic. + if importName == "" { + importName = other.Name() + } + return importName + } +} + +// getStubInfoFromReturns analyzes a "return" statement to extract +// a concrete type that is trying to be returned as an interface type. +// +// For example, func() io.Writer { return myType{} } +// would return StubInfo with the interface being io.Writer and the concrete type being myType{}. +func fromReturnStmt(ti *types.Info, pos token.Pos, path []ast.Node, rs *ast.ReturnStmt, pkg *types.Package) (*StubInfo, error) { + returnIdx := -1 + for i, r := range rs.Results { + if pos >= r.Pos() && pos <= r.End() { + returnIdx = i + } + } + if returnIdx == -1 { + return nil, fmt.Errorf("pos %d not within return statement bounds: [%d-%d]", pos, rs.Pos(), rs.End()) + } + concObj, pointer := getConcreteType(rs.Results[returnIdx], ti) + if concObj == nil || concObj.Pkg() == nil { + return nil, nil + } + ef := enclosingFunction(path, ti) + if ef == nil { + return nil, fmt.Errorf("could not find the enclosing function of the return statement") + } + iface := getIfaceType(ef.Results.List[returnIdx].Type, ti) + if iface == nil { + return nil, nil + } + return &StubInfo{ + Concrete: concObj, + Pointer: pointer, + Interface: iface, + }, nil +} + +// fromValueSpec returns *StubInfo from a variable declaration such as +// var x io.Writer = &T{} +func fromValueSpec(ti *types.Info, vs *ast.ValueSpec, pkg *types.Package, pos token.Pos) *StubInfo { + var idx int + for i, vs := range vs.Values { + if pos >= vs.Pos() && pos <= vs.End() { + idx = i + break + } + } + + valueNode := vs.Values[idx] + ifaceNode := vs.Type + callExp, ok := valueNode.(*ast.CallExpr) + // if the ValueSpec is `var _ = myInterface(...)` + // as opposed to `var _ myInterface = ...` + if ifaceNode == nil && ok && len(callExp.Args) == 1 { + ifaceNode = callExp.Fun + valueNode = callExp.Args[0] + } + concObj, pointer := getConcreteType(valueNode, ti) + if concObj == nil || concObj.Pkg() == nil { + return nil + } + ifaceObj := getIfaceType(ifaceNode, ti) + if ifaceObj == nil { + return nil + } + return &StubInfo{ + Concrete: concObj, + Interface: ifaceObj, + Pointer: pointer, + } +} + +// fromAssignStmt returns *StubInfo from a variable re-assignment such as +// var x io.Writer +// x = &T{} +func fromAssignStmt(ti *types.Info, as *ast.AssignStmt, pkg *types.Package, pos token.Pos) *StubInfo { + idx := -1 + var lhs, rhs ast.Expr + // Given a re-assignment interface conversion error, + // the compiler error shows up on the right hand side of the expression. + // For example, x = &T{} where x is io.Writer highlights the error + // under "&T{}" and not "x". + for i, hs := range as.Rhs { + if pos >= hs.Pos() && pos <= hs.End() { + idx = i + break + } + } + if idx == -1 { + return nil + } + // Technically, this should never happen as + // we would get a "cannot assign N values to M variables" + // before we get an interface conversion error. Nonetheless, + // guard against out of range index errors. + if idx >= len(as.Lhs) { + return nil + } + lhs, rhs = as.Lhs[idx], as.Rhs[idx] + ifaceObj := getIfaceType(lhs, ti) + if ifaceObj == nil { + return nil + } + concObj, pointer := getConcreteType(rhs, ti) + if concObj == nil || concObj.Pkg() == nil { + return nil + } + return &StubInfo{ + Concrete: concObj, + Interface: ifaceObj, + Pointer: pointer, + } +} + +// getIfaceType will try to extract the types.Object that defines +// the interface given the ast.Expr where the "missing method" +// or "conversion" errors happen. +func getIfaceType(n ast.Expr, ti *types.Info) types.Object { + tv, ok := ti.Types[n] + if !ok { + return nil + } + typ := tv.Type + named, ok := typ.(*types.Named) + if !ok { + return nil + } + _, ok = named.Underlying().(*types.Interface) + if !ok { + return nil + } + // Interfaces defined in the "builtin" package return nil a Pkg(). + // But they are still real interfaces that we need to make a special case for. + // Therefore, protect gopls from panicking if a new interface type was added in the future. + if named.Obj().Pkg() == nil && named.Obj().Name() != "error" { + return nil + } + return named.Obj() +} + +// getConcreteType will try to extract the types.Object that defines +// the concrete type given the ast.Expr where the "missing method" +// or "conversion" errors happened. If the concrete type is something +// that cannot have methods defined on it (such as basic types), this +// method will return a nil types.Object. +func getConcreteType(n ast.Expr, ti *types.Info) (types.Object, bool) { + tv, ok := ti.Types[n] + if !ok { + return nil, false + } + typ := tv.Type + ptr, isPtr := typ.(*types.Pointer) + if isPtr { + typ = ptr.Elem() + } + nd, ok := typ.(*types.Named) + if !ok { + return nil, false + } + return nd.Obj(), isPtr +} + +// enclosingFunction returns the signature and type of the function +// enclosing the given position. +func enclosingFunction(path []ast.Node, info *types.Info) *ast.FuncType { + for _, node := range path { + switch t := node.(type) { + case *ast.FuncDecl: + if _, ok := info.Defs[t.Name]; ok { + return t.Type + } + case *ast.FuncLit: + if _, ok := info.Types[t]; ok { + return t.Type + } + } + } + return nil +} diff --git a/internal/lsp/source/fix.go b/internal/lsp/source/fix.go index e0046ee589e..2f921ad0caa 100644 --- a/internal/lsp/source/fix.go +++ b/internal/lsp/source/fix.go @@ -32,6 +32,7 @@ type ( const ( FillStruct = "fill_struct" + StubMethods = "stub_methods" UndeclaredName = "undeclared_name" ExtractVariable = "extract_variable" ExtractFunction = "extract_function" @@ -45,6 +46,7 @@ var suggestedFixes = map[string]SuggestedFixFunc{ ExtractVariable: singleFile(extractVariable), ExtractFunction: singleFile(extractFunction), ExtractMethod: singleFile(extractMethod), + StubMethods: stubSuggestedFixFunc, } // singleFile calls analyzers that expect inputs for a single file diff --git a/internal/lsp/source/options.go b/internal/lsp/source/options.go index 98ef2aeaa43..0c3233a950a 100644 --- a/internal/lsp/source/options.go +++ b/internal/lsp/source/options.go @@ -55,6 +55,7 @@ import ( "golang.org/x/tools/internal/lsp/analysis/simplifycompositelit" "golang.org/x/tools/internal/lsp/analysis/simplifyrange" "golang.org/x/tools/internal/lsp/analysis/simplifyslice" + "golang.org/x/tools/internal/lsp/analysis/stubmethods" "golang.org/x/tools/internal/lsp/analysis/undeclaredname" "golang.org/x/tools/internal/lsp/analysis/unusedparams" "golang.org/x/tools/internal/lsp/analysis/useany" @@ -1216,6 +1217,12 @@ func convenienceAnalyzers() map[string]*Analyzer { Enabled: true, ActionKind: []protocol.CodeActionKind{protocol.RefactorRewrite}, }, + stubmethods.Analyzer.Name: { + Analyzer: stubmethods.Analyzer, + ActionKind: []protocol.CodeActionKind{protocol.RefactorRewrite}, + Fix: StubMethods, + Enabled: true, + }, } } diff --git a/internal/lsp/source/stub.go b/internal/lsp/source/stub.go new file mode 100644 index 00000000000..889330fc0ce --- /dev/null +++ b/internal/lsp/source/stub.go @@ -0,0 +1,362 @@ +// Copyright 2022 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 source + +import ( + "bytes" + "context" + "fmt" + "go/ast" + "go/format" + "go/parser" + "go/token" + "go/types" + "strings" + + "golang.org/x/tools/go/analysis" + "golang.org/x/tools/go/ast/astutil" + "golang.org/x/tools/internal/lsp/analysis/stubmethods" + "golang.org/x/tools/internal/lsp/protocol" + "golang.org/x/tools/internal/span" +) + +func stubSuggestedFixFunc(ctx context.Context, snapshot Snapshot, fh VersionedFileHandle, pRng protocol.Range) (*analysis.SuggestedFix, error) { + pkg, pgf, err := GetParsedFile(ctx, snapshot, fh, NarrowestPackage) + if err != nil { + return nil, fmt.Errorf("GetParsedFile: %w", err) + } + nodes, pos, err := getStubNodes(pgf, pRng) + if err != nil { + return nil, fmt.Errorf("getNodes: %w", err) + } + si := stubmethods.GetStubInfo(pkg.GetTypesInfo(), nodes, pkg.GetTypes(), pos) + if si == nil { + return nil, fmt.Errorf("nil interface request") + } + parsedConcreteFile, concreteFH, err := getStubFile(ctx, si.Concrete, snapshot) + if err != nil { + return nil, fmt.Errorf("getFile(concrete): %w", err) + } + var ( + methodsBytes []byte + stubImports []*stubImports + ) + if si.Interface.Pkg() == nil { + methodsBytes = stubErr(ctx, parsedConcreteFile.File, si, snapshot) + } else { + methodsBytes, stubImports, err = stubMethods(ctx, parsedConcreteFile.File, si, snapshot) + } + if err != nil { + return nil, fmt.Errorf("stubMethods: %w", err) + } + nodes, _ = astutil.PathEnclosingInterval(parsedConcreteFile.File, si.Concrete.Pos(), si.Concrete.Pos()) + concreteSrc, err := concreteFH.Read() + if err != nil { + return nil, fmt.Errorf("error reading concrete file source: %w", err) + } + insertPos := snapshot.FileSet().Position(nodes[1].End()).Offset + if insertPos >= len(concreteSrc) { + return nil, fmt.Errorf("insertion position is past the end of the file") + } + var buf bytes.Buffer + buf.Write(concreteSrc[:insertPos]) + buf.WriteByte('\n') + buf.Write(methodsBytes) + buf.Write(concreteSrc[insertPos:]) + fset := token.NewFileSet() + newF, err := parser.ParseFile(fset, parsedConcreteFile.File.Name.Name, buf.Bytes(), parser.ParseComments) + if err != nil { + return nil, fmt.Errorf("could not reparse file: %w", err) + } + for _, imp := range stubImports { + astutil.AddNamedImport(fset, newF, imp.Name, imp.Path) + } + var source bytes.Buffer + err = format.Node(&source, fset, newF) + if err != nil { + return nil, fmt.Errorf("format.Node: %w", err) + } + diffEdits, err := snapshot.View().Options().ComputeEdits(parsedConcreteFile.URI, string(parsedConcreteFile.Src), source.String()) + if err != nil { + return nil, err + } + var edits []analysis.TextEdit + for _, edit := range diffEdits { + rng, err := edit.Span.Range(parsedConcreteFile.Mapper.Converter) + if err != nil { + return nil, err + } + edits = append(edits, analysis.TextEdit{ + Pos: rng.Start, + End: rng.End, + NewText: []byte(edit.NewText), + }) + } + return &analysis.SuggestedFix{ + TextEdits: edits, + }, nil +} + +// stubMethods returns the Go code of all methods +// that implement the given interface +func stubMethods(ctx context.Context, concreteFile *ast.File, si *stubmethods.StubInfo, snapshot Snapshot) ([]byte, []*stubImports, error) { + ct := &concreteType{ + pkg: si.Concrete.Pkg(), + fset: snapshot.FileSet(), + file: concreteFile, + tms: types.NewMethodSet(si.Concrete.Type()), + pms: types.NewMethodSet(types.NewPointer(si.Concrete.Type())), + } + ifacePkg, err := deducePkgFromTypes(ctx, snapshot, si.Interface.Pkg()) + if err != nil { + return nil, nil, err + } + missing, err := missingMethods(ctx, snapshot, ct, si.Interface, ifacePkg, map[string]struct{}{}) + if err != nil { + return nil, nil, fmt.Errorf("missingMethods: %w", err) + } + if len(missing) == 0 { + return nil, nil, fmt.Errorf("no missing methods found") + } + var methodsBuffer bytes.Buffer + for _, mi := range missing { + for _, m := range mi.missing { + // TODO(marwan-at-work): this should share the same logic with source.FormatVarType + // as it also accounts for type aliases. + sig := types.TypeString(m.Type(), stubmethods.RelativeToFiles(ct.pkg, concreteFile, mi.file, func(name, path string) { + for _, imp := range ct.stubImports { + if imp.Name == name && imp.Path == path { + return + } + } + ct.stubImports = append(ct.stubImports, &stubImports{name, path}) + })) + concrete := si.Concrete.Name() + if si.Pointer { + concrete = "*" + concrete + } + _, err = methodsBuffer.Write(printStubMethod(methodData{ + Method: m.Name(), + Concrete: concrete, + Interface: deduceIfaceName(si.Concrete.Pkg(), si.Interface.Pkg(), si.Concrete, si.Interface), + Signature: strings.TrimPrefix(sig, "func"), + })) + if err != nil { + return nil, nil, fmt.Errorf("error printing method: %w", err) + } + methodsBuffer.WriteRune('\n') + } + } + return methodsBuffer.Bytes(), ct.stubImports, nil +} + +// stubErr reurns the Go code implementation +// of an error interface relevant to the +// concrete type +func stubErr(ctx context.Context, concreteFile *ast.File, si *stubmethods.StubInfo, snapshot Snapshot) []byte { + concrete := si.Concrete.Name() + if si.Pointer { + concrete = "*" + concrete + } + return printStubMethod(methodData{ + Method: "Error", + Interface: "error", + Concrete: concrete, + Signature: "() string", + }) +} + +type methodData struct { + Method string + Interface string + Concrete string + Signature string +} + +// printStubMethod takes methodData and returns Go code that represents the given method such as: +/* + // {{ .Method }} implements {{ .Interface }} + func ({{ .Concrete }}) {{ .Method }}{{ .Signature }} { + panic("unimplemented") + } +*/ +func printStubMethod(md methodData) []byte { + var b bytes.Buffer + fmt.Fprintf(&b, "// %s implements %s\n", md.Method, md.Interface) + fmt.Fprintf(&b, "func (%s) %s%s {\n\t", md.Concrete, md.Method, md.Signature) + fmt.Fprintln(&b, `panic("unimplemented")`) + fmt.Fprintln(&b, "}") + return b.Bytes() +} + +func deducePkgFromTypes(ctx context.Context, snapshot Snapshot, pkg *types.Package) (Package, error) { + pkgs, err := snapshot.KnownPackages(ctx) + if err != nil { + return nil, err + } + for _, p := range pkgs { + if p.PkgPath() == pkg.Path() { + return p, nil + } + } + return nil, fmt.Errorf("pkg %q not found", pkg.Path()) +} + +func deduceIfaceName(concretePkg, ifacePkg *types.Package, concreteObj, ifaceObj types.Object) string { + if concretePkg.Path() == ifacePkg.Path() { + return ifaceObj.Name() + } + return fmt.Sprintf("%s.%s", ifacePkg.Name(), ifaceObj.Name()) +} + +func getStubNodes(pgf *ParsedGoFile, pRng protocol.Range) ([]ast.Node, token.Pos, error) { + spn, err := pgf.Mapper.RangeSpan(pRng) + if err != nil { + return nil, 0, err + } + rng, err := spn.Range(pgf.Mapper.Converter) + if err != nil { + return nil, 0, err + } + nodes, _ := astutil.PathEnclosingInterval(pgf.File, rng.Start, rng.End) + return nodes, rng.Start, nil +} + +/* +missingMethods takes a concrete type and returns any missing methods for the given interface as well as +any missing interface that might have been embedded to its parent. For example: + +type I interface { + io.Writer + Hello() +} +returns []*missingInterface{ + { + iface: *types.Interface (io.Writer), + file: *ast.File: io.go, + missing []*types.Func{Write}, + }, + { + iface: *types.Interface (I), + file: *ast.File: myfile.go, + missing: []*types.Func{Hello} + }, +} +*/ +func missingMethods(ctx context.Context, snapshot Snapshot, ct *concreteType, ifaceObj types.Object, ifacePkg Package, visited map[string]struct{}) ([]*missingInterface, error) { + iface, ok := ifaceObj.Type().Underlying().(*types.Interface) + if !ok { + return nil, fmt.Errorf("expected %v to be an interface but got %T", iface, ifaceObj.Type().Underlying()) + } + missing := []*missingInterface{} + for i := 0; i < iface.NumEmbeddeds(); i++ { + eiface := iface.Embedded(i).Obj() + depPkg := ifacePkg + if eiface.Pkg().Path() != ifacePkg.PkgPath() { + var err error + depPkg, err = ifacePkg.GetImport(eiface.Pkg().Path()) + if err != nil { + return nil, err + } + } + em, err := missingMethods(ctx, snapshot, ct, eiface, depPkg, visited) + if err != nil { + return nil, err + } + missing = append(missing, em...) + } + parsedFile, _, err := getStubFile(ctx, ifaceObj, snapshot) + if err != nil { + return nil, fmt.Errorf("error getting iface file: %w", err) + } + mi := &missingInterface{ + pkg: ifacePkg, + iface: iface, + file: parsedFile.File, + } + if mi.file == nil { + return nil, fmt.Errorf("could not find ast.File for %v", ifaceObj.Name()) + } + for i := 0; i < iface.NumExplicitMethods(); i++ { + method := iface.ExplicitMethod(i) + if ct.doesNotHaveMethod(method.Name()) { + if _, ok := visited[method.Name()]; !ok { + mi.missing = append(mi.missing, method) + visited[method.Name()] = struct{}{} + } + } + if sel := ct.getMethodSelection(method.Name()); sel != nil { + implSig := sel.Type().(*types.Signature) + ifaceSig := method.Type().(*types.Signature) + if !types.Identical(ifaceSig, implSig) { + return nil, &mismatchError{ + name: method.Name(), + have: implSig, + want: ifaceSig, + } + } + } + } + if len(mi.missing) > 0 { + missing = append(missing, mi) + } + return missing, nil +} + +func getStubFile(ctx context.Context, obj types.Object, snapshot Snapshot) (*ParsedGoFile, VersionedFileHandle, error) { + objPos := snapshot.FileSet().Position(obj.Pos()) + objFile := span.URIFromPath(objPos.Filename) + objectFH := snapshot.FindFile(objFile) + _, goFile, err := GetParsedFile(ctx, snapshot, objectFH, WidestPackage) + if err != nil { + return nil, nil, fmt.Errorf("GetParsedFile: %w", err) + } + return goFile, objectFH, nil +} + +// missingInterface represents an interface +// that has all or some of its methods missing +// from the destination concrete type +type missingInterface struct { + iface *types.Interface + file *ast.File + pkg Package + missing []*types.Func +} + +// concreteType is the destination type +// that will implement the interface methods +type concreteType struct { + pkg *types.Package + fset *token.FileSet + file *ast.File + tms, pms *types.MethodSet + stubImports []*stubImports +} + +func (ct *concreteType) doesNotHaveMethod(name string) bool { + return ct.tms.Lookup(ct.pkg, name) == nil && ct.pms.Lookup(ct.pkg, name) == nil +} + +func (ct *concreteType) getMethodSelection(name string) *types.Selection { + if sel := ct.tms.Lookup(ct.pkg, name); sel != nil { + return sel + } + return ct.pms.Lookup(ct.pkg, name) +} + +// stubImports represents a newly added import +// statement to the concrete type. If name is not +// empty, then that import is required to have that name. +type stubImports struct{ Name, Path string } + +type mismatchError struct { + name string + have, want *types.Signature +} + +func (me *mismatchError) Error() string { + return fmt.Sprintf("mimsatched %q function signatures:\nhave: %s\nwant: %s", me.name, me.have, me.want) +} diff --git a/internal/lsp/testdata/stub/other/other.go b/internal/lsp/testdata/stub/other/other.go new file mode 100644 index 00000000000..ba3c1747ab7 --- /dev/null +++ b/internal/lsp/testdata/stub/other/other.go @@ -0,0 +1,10 @@ +package other + +import ( + "bytes" + renamed_context "context" +) + +type Interface interface { + Get(renamed_context.Context) *bytes.Buffer +} diff --git a/internal/lsp/testdata/stub/stub_add_selector.go b/internal/lsp/testdata/stub/stub_add_selector.go new file mode 100644 index 00000000000..a15afd7c244 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_add_selector.go @@ -0,0 +1,12 @@ +package stub + +import "io" + +// This file tests that if an interface +// method references a type from its own package +// then our implementation must add the import/package selector +// in the concrete method if the concrete type is outside of the interface +// package +var _ io.ReaderFrom = &readerFrom{} //@suggestedfix("&readerFrom", "refactor.rewrite") + +type readerFrom struct{} diff --git a/internal/lsp/testdata/stub/stub_add_selector.go.golden b/internal/lsp/testdata/stub/stub_add_selector.go.golden new file mode 100644 index 00000000000..e885483eaaf --- /dev/null +++ b/internal/lsp/testdata/stub/stub_add_selector.go.golden @@ -0,0 +1,19 @@ +-- suggestedfix_stub_add_selector_10_23 -- +package stub + +import "io" + +// This file tests that if an interface +// method references a type from its own package +// then our implementation must add the import/package selector +// in the concrete method if the concrete type is outside of the interface +// package +var _ io.ReaderFrom = &readerFrom{} //@suggestedfix("&readerFrom", "refactor.rewrite") + +type readerFrom struct{} + +// ReadFrom implements io.ReaderFrom +func (*readerFrom) ReadFrom(r io.Reader) (n int64, err error) { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_assign.go b/internal/lsp/testdata/stub/stub_assign.go new file mode 100644 index 00000000000..9336361d009 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_assign.go @@ -0,0 +1,10 @@ +package stub + +import "io" + +func main() { + var br io.ByteWriter + br = &byteWriter{} //@suggestedfix("&", "refactor.rewrite") +} + +type byteWriter struct{} diff --git a/internal/lsp/testdata/stub/stub_assign.go.golden b/internal/lsp/testdata/stub/stub_assign.go.golden new file mode 100644 index 00000000000..a52a8236798 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_assign.go.golden @@ -0,0 +1,17 @@ +-- suggestedfix_stub_assign_7_7 -- +package stub + +import "io" + +func main() { + var br io.ByteWriter + br = &byteWriter{} //@suggestedfix("&", "refactor.rewrite") +} + +type byteWriter struct{} + +// WriteByte implements io.ByteWriter +func (*byteWriter) WriteByte(c byte) error { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_assign_multivars.go b/internal/lsp/testdata/stub/stub_assign_multivars.go new file mode 100644 index 00000000000..01b330fda54 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_assign_multivars.go @@ -0,0 +1,11 @@ +package stub + +import "io" + +func main() { + var br io.ByteWriter + var i int + i, br = 1, &multiByteWriter{} //@suggestedfix("&", "refactor.rewrite") +} + +type multiByteWriter struct{} diff --git a/internal/lsp/testdata/stub/stub_assign_multivars.go.golden b/internal/lsp/testdata/stub/stub_assign_multivars.go.golden new file mode 100644 index 00000000000..e1e71adbd50 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_assign_multivars.go.golden @@ -0,0 +1,18 @@ +-- suggestedfix_stub_assign_multivars_8_13 -- +package stub + +import "io" + +func main() { + var br io.ByteWriter + var i int + i, br = 1, &multiByteWriter{} //@suggestedfix("&", "refactor.rewrite") +} + +type multiByteWriter struct{} + +// WriteByte implements io.ByteWriter +func (*multiByteWriter) WriteByte(c byte) error { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_embedded.go b/internal/lsp/testdata/stub/stub_embedded.go new file mode 100644 index 00000000000..6d6a986bf24 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_embedded.go @@ -0,0 +1,15 @@ +package stub + +import ( + "io" + "sort" +) + +var _ embeddedInterface = (*embeddedConcrete)(nil) //@suggestedfix("(", "refactor.rewrite") + +type embeddedConcrete struct{} + +type embeddedInterface interface { + sort.Interface + io.Reader +} diff --git a/internal/lsp/testdata/stub/stub_embedded.go.golden b/internal/lsp/testdata/stub/stub_embedded.go.golden new file mode 100644 index 00000000000..c258ebaf46c --- /dev/null +++ b/internal/lsp/testdata/stub/stub_embedded.go.golden @@ -0,0 +1,37 @@ +-- suggestedfix_stub_embedded_8_27 -- +package stub + +import ( + "io" + "sort" +) + +var _ embeddedInterface = (*embeddedConcrete)(nil) //@suggestedfix("(", "refactor.rewrite") + +type embeddedConcrete struct{} + +// Len implements embeddedInterface +func (*embeddedConcrete) Len() int { + panic("unimplemented") +} + +// Less implements embeddedInterface +func (*embeddedConcrete) Less(i int, j int) bool { + panic("unimplemented") +} + +// Swap implements embeddedInterface +func (*embeddedConcrete) Swap(i int, j int) { + panic("unimplemented") +} + +// Read implements embeddedInterface +func (*embeddedConcrete) Read(p []byte) (n int, err error) { + panic("unimplemented") +} + +type embeddedInterface interface { + sort.Interface + io.Reader +} + diff --git a/internal/lsp/testdata/stub/stub_err.go b/internal/lsp/testdata/stub/stub_err.go new file mode 100644 index 00000000000..908c7d3152f --- /dev/null +++ b/internal/lsp/testdata/stub/stub_err.go @@ -0,0 +1,7 @@ +package stub + +func main() { + var br error = &customErr{} //@suggestedfix("&", "refactor.rewrite") +} + +type customErr struct{} diff --git a/internal/lsp/testdata/stub/stub_err.go.golden b/internal/lsp/testdata/stub/stub_err.go.golden new file mode 100644 index 00000000000..717aed86293 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_err.go.golden @@ -0,0 +1,14 @@ +-- suggestedfix_stub_err_4_17 -- +package stub + +func main() { + var br error = &customErr{} //@suggestedfix("&", "refactor.rewrite") +} + +type customErr struct{} + +// Error implements error +func (*customErr) Error() string { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_function_return.go b/internal/lsp/testdata/stub/stub_function_return.go new file mode 100644 index 00000000000..bbf05885af2 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_function_return.go @@ -0,0 +1,11 @@ +package stub + +import ( + "io" +) + +func newCloser() io.Closer { + return closer{} //@suggestedfix("c", "refactor.rewrite") +} + +type closer struct{} diff --git a/internal/lsp/testdata/stub/stub_function_return.go.golden b/internal/lsp/testdata/stub/stub_function_return.go.golden new file mode 100644 index 00000000000..f80874d2b94 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_function_return.go.golden @@ -0,0 +1,18 @@ +-- suggestedfix_stub_function_return_8_9 -- +package stub + +import ( + "io" +) + +func newCloser() io.Closer { + return closer{} //@suggestedfix("c", "refactor.rewrite") +} + +type closer struct{} + +// Close implements io.Closer +func (closer) Close() error { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_ignored_imports.go b/internal/lsp/testdata/stub/stub_ignored_imports.go new file mode 100644 index 00000000000..f0abcc53453 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_ignored_imports.go @@ -0,0 +1,18 @@ +package stub + +import ( + "compress/zlib" + . "io" + _ "io" +) + +// This file tests that dot-imports and underscore imports +// are properly ignored and that a new import is added to +// refernece method types + +var ( + _ Reader + _ zlib.Resetter = (*ignoredResetter)(nil) //@suggestedfix("(", "refactor.rewrite") +) + +type ignoredResetter struct{} diff --git a/internal/lsp/testdata/stub/stub_ignored_imports.go.golden b/internal/lsp/testdata/stub/stub_ignored_imports.go.golden new file mode 100644 index 00000000000..4f34d799162 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_ignored_imports.go.golden @@ -0,0 +1,26 @@ +-- suggestedfix_stub_ignored_imports_15_20 -- +package stub + +import ( + "compress/zlib" + "io" + . "io" + _ "io" +) + +// This file tests that dot-imports and underscore imports +// are properly ignored and that a new import is added to +// refernece method types + +var ( + _ Reader + _ zlib.Resetter = (*ignoredResetter)(nil) //@suggestedfix("(", "refactor.rewrite") +) + +type ignoredResetter struct{} + +// Reset implements zlib.Resetter +func (*ignoredResetter) Reset(r io.Reader, dict []byte) error { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_multi_var.go b/internal/lsp/testdata/stub/stub_multi_var.go new file mode 100644 index 00000000000..4276b799429 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_multi_var.go @@ -0,0 +1,11 @@ +package stub + +import "io" + +// This test ensures that a variable declaration that +// has multiple values on the same line can still be +// analyzed correctly to target the interface implementation +// diagnostic. +var one, two, three io.Reader = nil, &multiVar{}, nil //@suggestedfix("&", "refactor.rewrite") + +type multiVar struct{} diff --git a/internal/lsp/testdata/stub/stub_multi_var.go.golden b/internal/lsp/testdata/stub/stub_multi_var.go.golden new file mode 100644 index 00000000000..b9ac4236766 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_multi_var.go.golden @@ -0,0 +1,18 @@ +-- suggestedfix_stub_multi_var_9_38 -- +package stub + +import "io" + +// This test ensures that a variable declaration that +// has multiple values on the same line can still be +// analyzed correctly to target the interface implementation +// diagnostic. +var one, two, three io.Reader = nil, &multiVar{}, nil //@suggestedfix("&", "refactor.rewrite") + +type multiVar struct{} + +// Read implements io.Reader +func (*multiVar) Read(p []byte) (n int, err error) { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_pointer.go b/internal/lsp/testdata/stub/stub_pointer.go new file mode 100644 index 00000000000..2b3681b8357 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_pointer.go @@ -0,0 +1,9 @@ +package stub + +import "io" + +func getReaderFrom() io.ReaderFrom { + return &pointerImpl{} //@suggestedfix("&", "refactor.rewrite") +} + +type pointerImpl struct{} diff --git a/internal/lsp/testdata/stub/stub_pointer.go.golden b/internal/lsp/testdata/stub/stub_pointer.go.golden new file mode 100644 index 00000000000..c4133d7a44d --- /dev/null +++ b/internal/lsp/testdata/stub/stub_pointer.go.golden @@ -0,0 +1,16 @@ +-- suggestedfix_stub_pointer_6_9 -- +package stub + +import "io" + +func getReaderFrom() io.ReaderFrom { + return &pointerImpl{} //@suggestedfix("&", "refactor.rewrite") +} + +type pointerImpl struct{} + +// ReadFrom implements io.ReaderFrom +func (*pointerImpl) ReadFrom(r io.Reader) (n int64, err error) { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_renamed_import.go b/internal/lsp/testdata/stub/stub_renamed_import.go new file mode 100644 index 00000000000..eaebe251018 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_renamed_import.go @@ -0,0 +1,11 @@ +package stub + +import ( + "compress/zlib" + myio "io" +) + +var _ zlib.Resetter = &myIO{} //@suggestedfix("&", "refactor.rewrite") +var _ myio.Reader + +type myIO struct{} diff --git a/internal/lsp/testdata/stub/stub_renamed_import.go.golden b/internal/lsp/testdata/stub/stub_renamed_import.go.golden new file mode 100644 index 00000000000..48ff4f1537f --- /dev/null +++ b/internal/lsp/testdata/stub/stub_renamed_import.go.golden @@ -0,0 +1,18 @@ +-- suggestedfix_stub_renamed_import_8_23 -- +package stub + +import ( + "compress/zlib" + myio "io" +) + +var _ zlib.Resetter = &myIO{} //@suggestedfix("&", "refactor.rewrite") +var _ myio.Reader + +type myIO struct{} + +// Reset implements zlib.Resetter +func (*myIO) Reset(r myio.Reader, dict []byte) error { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_renamed_import_iface.go b/internal/lsp/testdata/stub/stub_renamed_import_iface.go new file mode 100644 index 00000000000..96caf540d60 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_renamed_import_iface.go @@ -0,0 +1,13 @@ +package stub + +import ( + "golang.org/x/tools/internal/lsp/stub/other" +) + +// This file tests that if an interface +// method references an import from its own package +// that the concrete type does not yet import, and that import happens +// to be renamed, then we prefer the renaming of the interface. +var _ other.Interface = &otherInterfaceImpl{} //@suggestedfix("&otherInterfaceImpl", "refactor.rewrite") + +type otherInterfaceImpl struct{} diff --git a/internal/lsp/testdata/stub/stub_renamed_import_iface.go.golden b/internal/lsp/testdata/stub/stub_renamed_import_iface.go.golden new file mode 100644 index 00000000000..9ba2cb440e8 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_renamed_import_iface.go.golden @@ -0,0 +1,22 @@ +-- suggestedfix_stub_renamed_import_iface_11_25 -- +package stub + +import ( + "bytes" + renamed_context "context" + "golang.org/x/tools/internal/lsp/stub/other" +) + +// This file tests that if an interface +// method references an import from its own package +// that the concrete type does not yet import, and that import happens +// to be renamed, then we prefer the renaming of the interface. +var _ other.Interface = &otherInterfaceImpl{} //@suggestedfix("&otherInterfaceImpl", "refactor.rewrite") + +type otherInterfaceImpl struct{} + +// Get implements other.Interface +func (*otherInterfaceImpl) Get(renamed_context.Context) *bytes.Buffer { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/stub/stub_stdlib.go b/internal/lsp/testdata/stub/stub_stdlib.go new file mode 100644 index 00000000000..0d54a6daadf --- /dev/null +++ b/internal/lsp/testdata/stub/stub_stdlib.go @@ -0,0 +1,9 @@ +package stub + +import ( + "io" +) + +var _ io.Writer = writer{} //@suggestedfix("w", "refactor.rewrite") + +type writer struct{} diff --git a/internal/lsp/testdata/stub/stub_stdlib.go.golden b/internal/lsp/testdata/stub/stub_stdlib.go.golden new file mode 100644 index 00000000000..8636cead414 --- /dev/null +++ b/internal/lsp/testdata/stub/stub_stdlib.go.golden @@ -0,0 +1,16 @@ +-- suggestedfix_stub_stdlib_7_19 -- +package stub + +import ( + "io" +) + +var _ io.Writer = writer{} //@suggestedfix("w", "refactor.rewrite") + +type writer struct{} + +// Write implements io.Writer +func (writer) Write(p []byte) (n int, err error) { + panic("unimplemented") +} + diff --git a/internal/lsp/testdata/summary.txt.golden b/internal/lsp/testdata/summary.txt.golden index bfe40b3a6db..29493920f75 100644 --- a/internal/lsp/testdata/summary.txt.golden +++ b/internal/lsp/testdata/summary.txt.golden @@ -13,7 +13,7 @@ FoldingRangesCount = 2 FormatCount = 6 ImportCount = 8 SemanticTokenCount = 3 -SuggestedFixCount = 49 +SuggestedFixCount = 61 FunctionExtractionCount = 25 MethodExtractionCount = 6 DefinitionsCount = 95 diff --git a/internal/lsp/testdata/summary_go1.18.txt.golden b/internal/lsp/testdata/summary_go1.18.txt.golden index 02c674252c2..86b7b1e6c0b 100644 --- a/internal/lsp/testdata/summary_go1.18.txt.golden +++ b/internal/lsp/testdata/summary_go1.18.txt.golden @@ -13,7 +13,7 @@ FoldingRangesCount = 2 FormatCount = 6 ImportCount = 8 SemanticTokenCount = 3 -SuggestedFixCount = 49 +SuggestedFixCount = 61 FunctionExtractionCount = 25 MethodExtractionCount = 6 DefinitionsCount = 99