diff --git a/gopls/doc/analyzers.md b/gopls/doc/analyzers.md index e79f01a1ce..121fa4964d 100644 --- a/gopls/doc/analyzers.md +++ b/gopls/doc/analyzers.md @@ -653,6 +653,15 @@ expensive to compute, callers should compute it separately, using the SuggestedFix function below. +**Enabled by default.** + +## **stubmethods** + +stub methods analyzer + +This analyzer generates method stubs for concrete types +in order to implement a target interface + **Enabled by default.** diff --git a/internal/lsp/analysis/stubmethods/stubmethods.go b/internal/lsp/analysis/stubmethods/stubmethods.go new file mode 100644 index 0000000000..ba0a343320 --- /dev/null +++ b/internal/lsp/analysis/stubmethods/stubmethods.go @@ -0,0 +1,351 @@ +// 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.Obj().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 is the interface that the client wants to implement. + // When the interface is defined, the underlying object will be a TypeName. + // Note that we keep track of types.Object instead of types.Type in order + // to keep a reference to the declaring object's package and the ast file + // in the case where the concrete type file requires a new import that happens to be renamed + // in the interface file. + // TODO(marwan-at-work): implement interface literals. + Interface types.Object + Concrete *types.Named + 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: + // An error here may not indicate a real error the user should know about, but it may. + // Therefore, it would be best to log it out for debugging/reporting purposes instead of ignoring + // it. However, event.Log takes a context which is not passed via the analysis package. + // TODO(marwan-at-work): properly log this error. + si, _ := fromReturnStmt(ti, pos, path, n, pkg) + return si + case *ast.AssignStmt: + return fromAssignStmt(ti, n, pkg, pos) + } + } + return nil +} + +// fromReturnStmt 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 := concreteType(rs.Results[returnIdx], ti) + if concObj == nil || concObj.Obj().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 := ifaceType(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 := concreteType(valueNode, ti) + if concObj == nil || concObj.Obj().Pkg() == nil { + return nil + } + ifaceObj := ifaceType(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 := ifaceType(lhs, ti) + if ifaceObj == nil { + return nil + } + concType, pointer := concreteType(rhs, ti) + if concType == nil || concType.Obj().Pkg() == nil { + return nil + } + return &StubInfo{ + Concrete: concType, + Interface: ifaceObj, + Pointer: pointer, + } +} + +// 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 + } +} + +// ifaceType will try to extract the types.Object that defines +// the interface given the ast.Expr where the "missing method" +// or "conversion" errors happen. +func ifaceType(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() +} + +// concreteType tries to extract the *types.Named 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.Named. The second return parameter +// is a boolean that indicates whether the concreteType was defined as a +// pointer or value. +func concreteType(n ast.Expr, ti *types.Info) (*types.Named, bool) { + tv, ok := ti.Types[n] + if !ok { + return nil, false + } + typ := tv.Type + ptr, isPtr := typ.(*types.Pointer) + if isPtr { + typ = ptr.Elem() + } + named, ok := typ.(*types.Named) + if !ok { + return nil, false + } + return named, 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/api_json.go b/internal/lsp/source/api_json.go index 4e8e9df14a..4742fb11f4 100755 --- a/internal/lsp/source/api_json.go +++ b/internal/lsp/source/api_json.go @@ -433,6 +433,11 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "note incomplete struct initializations\n\nThis analyzer provides diagnostics for any struct literals that do not have\nany fields initialized. Because the suggested fix for this analysis is\nexpensive to compute, callers should compute it separately, using the\nSuggestedFix function below.\n", Default: "true", }, + { + Name: "\"stubmethods\"", + Doc: "stub methods analyzer\n\nThis analyzer generates method stubs for concrete types\nin order to implement a target interface", + Default: "true", + }, }, }, Default: "{}", @@ -946,5 +951,10 @@ var GeneratedAPIJSON = &APIJSON{ Doc: "note incomplete struct initializations\n\nThis analyzer provides diagnostics for any struct literals that do not have\nany fields initialized. Because the suggested fix for this analysis is\nexpensive to compute, callers should compute it separately, using the\nSuggestedFix function below.\n", Default: true, }, + { + Name: "stubmethods", + Doc: "stub methods analyzer\n\nThis analyzer generates method stubs for concrete types\nin order to implement a target interface", + Default: true, + }, }, } diff --git a/internal/lsp/source/fix.go b/internal/lsp/source/fix.go index e0046ee589..2f921ad0ca 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 5c34656f49..139ae7004d 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" @@ -1217,6 +1218,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 0000000000..276bec1c46 --- /dev/null +++ b/internal/lsp/source/stub.go @@ -0,0 +1,328 @@ +// 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, rng 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, rng) + 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.Obj(), snapshot) + if err != nil { + return nil, fmt.Errorf("getFile(concrete): %w", err) + } + var ( + methodsSrc []byte + stubImports []*stubImport // additional imports needed for method stubs + ) + if si.Interface.Pkg() == nil && si.Interface.Name() == "error" && si.Interface.Parent() == types.Universe { + methodsSrc = stubErr(ctx, parsedConcreteFile.File, si, snapshot) + } else { + methodsSrc, 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.Obj().Pos(), si.Concrete.Obj().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(methodsSrc) + 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, []*stubImport, error) { + ifacePkg, err := deducePkgFromTypes(ctx, snapshot, si.Interface) + if err != nil { + return nil, nil, err + } + si.Concrete.Obj().Type() + concMS := types.NewMethodSet(types.NewPointer(si.Concrete.Obj().Type())) + missing, err := missingMethods(ctx, snapshot, concMS, si.Concrete.Obj().Pkg(), 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 ( + stubImports []*stubImport + 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(si.Concrete.Obj().Pkg(), concreteFile, mi.file, func(name, path string) { + for _, imp := range stubImports { + if imp.Name == name && imp.Path == path { + return + } + } + stubImports = append(stubImports, &stubImport{name, path}) + })) + _, err = methodsBuffer.Write(printStubMethod(methodData{ + Method: m.Name(), + Concrete: getStubReceiver(si), + Interface: deduceIfaceName(si.Concrete.Obj().Pkg(), si.Interface.Pkg(), si.Concrete.Obj(), 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(), 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 { + return printStubMethod(methodData{ + Method: "Error", + Interface: "error", + Concrete: getStubReceiver(si), + Signature: "() string", + }) +} + +// getStubReceiver returns the concrete type's name as a method receiver. +// TODO(marwan-at-work): add type parameters to the receiver when the concrete type +// is a generic one. +func getStubReceiver(si *stubmethods.StubInfo) string { + concrete := si.Concrete.Obj().Name() + if si.Pointer { + concrete = "*" + concrete + } + return concrete +} + +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, ifaceObj types.Object) (Package, error) { + pkgs, err := snapshot.KnownPackages(ctx) + if err != nil { + return nil, err + } + for _, p := range pkgs { + if p.PkgPath() == ifaceObj.Pkg().Path() { + return p, nil + } + } + return nil, fmt.Errorf("pkg %q not found", ifaceObj.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, concMS *types.MethodSet, concPkg *types.Package, 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, concMS, concPkg, 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 the concrete type does not have the interface method + if concMS.Lookup(concPkg, method.Name()) == nil { + if _, ok := visited[method.Name()]; !ok { + mi.missing = append(mi.missing, method) + visited[method.Name()] = struct{}{} + } + } + if sel := concMS.Lookup(concPkg, method.Name()); sel != nil { + implSig := sel.Type().(*types.Signature) + ifaceSig := method.Type().(*types.Signature) + if !types.Identical(ifaceSig, implSig) { + return nil, fmt.Errorf("mimsatched %q function signatures:\nhave: %s\nwant: %s", method.Name(), implSig, 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 +} + +// stubImport 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 stubImport struct{ Name, Path string } diff --git a/internal/lsp/testdata/stub/other/other.go b/internal/lsp/testdata/stub/other/other.go new file mode 100644 index 0000000000..ba3c1747ab --- /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 0000000000..a15afd7c24 --- /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 0000000000..e885483eaa --- /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 0000000000..9336361d00 --- /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 0000000000..a52a823679 --- /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 0000000000..01b330fda5 --- /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 0000000000..e1e71adbd5 --- /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 0000000000..6d6a986bf2 --- /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 0000000000..c258ebaf46 --- /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 0000000000..908c7d3152 --- /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 0000000000..717aed8629 --- /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 0000000000..bbf05885af --- /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 0000000000..f80874d2b9 --- /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 0000000000..f0abcc5345 --- /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 0000000000..4f34d79916 --- /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 0000000000..4276b79942 --- /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 0000000000..b9ac423676 --- /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 0000000000..2b3681b835 --- /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 0000000000..c4133d7a44 --- /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 0000000000..eaebe25101 --- /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 0000000000..48ff4f1537 --- /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 0000000000..96caf540d6 --- /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 0000000000..9ba2cb440e --- /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 0000000000..0d54a6daad --- /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 0000000000..8636cead41 --- /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 bfe40b3a6d..29493920f7 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 02c674252c..86b7b1e6c0 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