Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

x/tools/gopls: implement struct field generation quickfix #544

Open
wants to merge 9 commits into
base: master
Choose a base branch
from
30 changes: 29 additions & 1 deletion gopls/doc/features/diagnostics.md
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,6 @@ There is an optional third source of diagnostics:
are transitively free from errors, so optimization diagnostics
will not be shown on packages that do not build.


## Recomputation of diagnostics

By default, diagnostics are automatically recomputed each time the source files
Expand Down Expand Up @@ -272,6 +271,35 @@ func doSomething(i int) string {
panic("unimplemented")
}
```

### `StubMissingStructField`: Declare missing field T.f

When you attempt to access a field on a type that does not have the field,
the compiler will report an error such as "type X has no field or method Y".
In this scenario, gopls now offers a quick fix to generate a stub declaration of
the missing field, inferring its type from the context in which it is used.

Consider the following code where `Foo` does not have a field `bar`:

```go
type Foo struct{}

func main() {
var s string
f := Foo{}
s = f.bar // error: f.bar undefined (type Foo has no field or method bar)
}
```

Gopls will offer a quick fix, "Declare missing field Foo.bar".
When invoked, it creates the following declaration:

```go
type Foo struct{
bar string
}
```

<!--

dorky details and deletia:
Expand Down
21 changes: 21 additions & 0 deletions gopls/doc/release/v0.17.0.md
Original file line number Diff line number Diff line change
Expand Up @@ -196,3 +196,24 @@ causing `Add` to race with `Wait`.
(This check is equivalent to
[staticcheck's SA2000](https://staticcheck.dev/docs/checks#SA2000),
but is enabled by default.)

## Add test for function or method

If the selected chunk of code is part of a function or method declaration F,
gopls will offer the "Add test for F" code action, which adds a new test for the
selected function in the corresponding `_test.go` file. The generated test takes
into account its signature, including input parameters and results.

Since this feature is implemented by the server (gopls), it is compatible with
all LSP-compliant editors. VS Code users may continue to use the client-side
`Go: Generate Unit Tests For file/function/package` command which uses the
[gotests](https://github.com/cweill/gotests) tool.

## Generate missing struct field from access

When you attempt to access a field on a type that does not have the field,
the compiler will report an error like “type X has no field or method Y”.
Gopls now offers a new code action, “Declare missing field of T.f”,
where T is the concrete type and f is the undefined field.
The stub field's signature is inferred
from the context of the access.
15 changes: 12 additions & 3 deletions gopls/internal/golang/codeaction.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ import (
//
// See ../protocol/codeactionkind.go for some code action theory.
func CodeActions(ctx context.Context, snapshot *cache.Snapshot, fh file.Handle, rng protocol.Range, diagnostics []protocol.Diagnostic, enabled func(protocol.CodeActionKind) bool, trigger protocol.CodeActionTriggerKind) (actions []protocol.CodeAction, _ error) {

loc := protocol.Location{URI: fh.URI(), Range: rng}

pgf, err := snapshot.ParseGo(ctx, fh, parsego.Full)
Expand Down Expand Up @@ -335,13 +334,23 @@ func quickFix(ctx context.Context, req *codeActionsRequest) error {
}

// "type X has no field or method Y" compiler error.
// Offer a "Declare missing method T.f" code action.
// See [stubMissingCalledFunctionFixer] for command implementation.
case strings.Contains(msg, "has no field or method"):
// Offer a "Declare missing method T.f" code action.
// See [stubMissingCalledFunctionFixer] for command implementation.
si := stubmethods.GetCallStubInfo(req.pkg.FileSet(), info, req.pgf, start, end)
if si != nil {
msg := fmt.Sprintf("Declare missing method %s.%s", si.Receiver.Obj().Name(), si.MethodName)
req.addApplyFixAction(msg, fixMissingCalledFunction, req.loc)
} else {
fi := stubmethods.GetFieldStubInfo(req.pkg.FileSet(), info, req.pgf, start, end)
if fi != nil {
msg := fmt.Sprintf("Declare missing struct field %s.%s", fi.Named.Obj().Name(), fi.Expr.Sel.Name)
req.addApplyFixAction(msg, fixMissingStructField, req.loc)

// undeclared field might be a method
// msg = fmt.Sprintf("Declare missing method %s.%s", fi.Named.Obj().Name(), fi.Expr.Sel.Name)
// req.addApplyFixAction(msg, fixMissingCalledFunction, req.loc)
}
}

// "undeclared name: X" or "undefined: X" compiler error.
Expand Down
2 changes: 2 additions & 0 deletions gopls/internal/golang/fix.go
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ const (
fixCreateUndeclared = "create_undeclared"
fixMissingInterfaceMethods = "stub_missing_interface_method"
fixMissingCalledFunction = "stub_missing_called_function"
fixMissingStructField = "stub_missing_struct_field"
)

// ApplyFix applies the specified kind of suggested fix to the given
Expand Down Expand Up @@ -109,6 +110,7 @@ func ApplyFix(ctx context.Context, fix string, snapshot *cache.Snapshot, fh file
fixCreateUndeclared: singleFile(createUndeclared),
fixMissingInterfaceMethods: stubMissingInterfaceMethodsFixer,
fixMissingCalledFunction: stubMissingCalledFunctionFixer,
fixMissingStructField: stubMissingStructFieldFixer,
}
fixer, ok := fixers[fix]
if !ok {
Expand Down
228 changes: 158 additions & 70 deletions gopls/internal/golang/stub.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"bytes"
"context"
"fmt"
"go/ast"
"go/format"
"go/parser"
"go/token"
Expand Down Expand Up @@ -49,6 +50,17 @@ func stubMissingCalledFunctionFixer(ctx context.Context, snapshot *cache.Snapsho
return insertDeclsAfter(ctx, snapshot, pkg.Metadata(), si.Fset, si.After, si.Emit)
}

// stubMissingStructFieldFixer returns a suggested fix to declare the missing
// field that the user may want to generate based on SelectorExpr
// at the cursor position.
func stubMissingStructFieldFixer(ctx context.Context, snapshot *cache.Snapshot, pkg *cache.Package, pgf *parsego.File, start, end token.Pos) (*token.FileSet, *analysis.SuggestedFix, error) {
fi := stubmethods.GetFieldStubInfo(pkg.FileSet(), pkg.TypesInfo(), pgf, start, end)
if fi == nil {
return nil, nil, fmt.Errorf("invalid type request")
}
return insertStructField(ctx, snapshot, pkg.Metadata(), fi)
}

// An emitter writes new top-level declarations into an existing
// file. References to symbols should be qualified using qual, which
// respects the local import environment.
Expand Down Expand Up @@ -81,76 +93,10 @@ func insertDeclsAfter(ctx context.Context, snapshot *cache.Snapshot, mp *metadat
return nil, nil, bug.Errorf("can't find metadata for file %s among dependencies of %s", declPGF.URI, mp)
}

// Build import environment for the declaring file.
// (typesinternal.FileQualifier works only for complete
// import mappings, and requires types.)
importEnv := make(map[ImportPath]string) // value is local name
for _, imp := range declPGF.File.Imports {
importPath := metadata.UnquoteImportPath(imp)
var name string
if imp.Name != nil {
name = imp.Name.Name
if name == "_" {
continue
} else if name == "." {
name = "" // see types.Qualifier
}
} else {
// Use the correct name from the metadata of the imported
// package---not a guess based on the import path.
mp := snapshot.Metadata(declMeta.DepsByImpPath[importPath])
if mp == nil {
continue // can't happen?
}
name = string(mp.Name)
}
importEnv[importPath] = name // latest alias wins
}

// Create a package name qualifier that uses the
// locally appropriate imported package name.
// It records any needed new imports.
// TODO(adonovan): factor with golang.FormatVarType?
//
// Prior to CL 469155 this logic preserved any renaming
// imports from the file that declares the interface
// method--ostensibly the preferred name for imports of
// frequently renamed packages such as protobufs.
// Now we use the package's declared name. If this turns out
// to be a mistake, then use parseHeader(si.iface.Pos()).
//
type newImport struct{ name, importPath string }
var newImports []newImport // for AddNamedImport
qual := func(pkg *types.Package) string {
// TODO(adonovan): don't ignore vendor prefix.
//
// Ignore the current package import.
if pkg.Path() == sym.Pkg().Path() {
return ""
}

importPath := ImportPath(pkg.Path())
name, ok := importEnv[importPath]
if !ok {
// Insert new import using package's declared name.
//
// TODO(adonovan): resolve conflict between declared
// name and existing file-level (declPGF.File.Imports)
// or package-level (sym.Pkg.Scope) decls by
// generating a fresh name.
name = pkg.Name()
importEnv[importPath] = name
new := newImport{importPath: string(importPath)}
// For clarity, use a renaming import whenever the
// local name does not match the path's last segment.
if name != pathpkg.Base(trimVersionSuffix(new.importPath)) {
new.name = name
}
newImports = append(newImports, new)
}
return name
}

newImports := make([]newImport, 0, len(declPGF.File.Imports))
qual := newNamedImportQual(declPGF, snapshot, declMeta, sym, func(imp newImport) {
newImports = append(newImports, imp)
})
// Compute insertion point for new declarations:
// after the top-level declaration enclosing the (package-level) type.
insertOffset, err := safetoken.Offset(declPGF.Tok, declPGF.File.End())
Expand Down Expand Up @@ -236,3 +182,145 @@ func trimVersionSuffix(path string) string {
}
return path
}

func insertStructField(ctx context.Context, snapshot *cache.Snapshot, mp *metadata.Package, fieldInfo *stubmethods.StructFieldInfo) (*token.FileSet, *analysis.SuggestedFix, error) {
if fieldInfo == nil {
return nil, nil, fmt.Errorf("no field info provided")
}

// get the file containing the struct definition using the position
declPGF, _, err := parseFull(ctx, snapshot, fieldInfo.Fset, fieldInfo.Named.Obj().Pos())
if err != nil {
return nil, nil, fmt.Errorf("failed to parse file declaring struct: %w", err)
}
if declPGF.Fixed() {
return nil, nil, fmt.Errorf("file contains parse errors: %s", declPGF.URI)
}

// find the struct type declaration
pos := fieldInfo.Named.Obj().Pos()
endPos := pos + token.Pos(len(fieldInfo.Named.Obj().Name()))
curIdent, ok := declPGF.Cursor.FindPos(pos, endPos)
if !ok {
return nil, nil, fmt.Errorf("could not find identifier at position %v-%v", pos, endPos)
}

// Rest of the code remains the same
typeNode, ok := curIdent.NextSibling()
if !ok {
return nil, nil, fmt.Errorf("could not find type specification")
}

structType, ok := typeNode.Node().(*ast.StructType)
if !ok {
return nil, nil, fmt.Errorf("type at position %v is not a struct type", pos)
}
// Find metadata for the symbol's declaring package
// as we'll need its import mapping.
declMeta := findFileInDeps(snapshot, mp, declPGF.URI)
if declMeta == nil {
return nil, nil, bug.Errorf("can't find metadata for file %s among dependencies of %s", declPGF.URI, mp)
}

qual := newNamedImportQual(declPGF, snapshot, declMeta, fieldInfo.Named.Obj(), func(imp newImport) { /* discard */ })

// find the position to insert the new field (end of struct fields)
insertPos := structType.Fields.Closing - 1
if insertPos == structType.Fields.Opening {
// struct has no fields yet
insertPos = structType.Fields.Closing
_, err = declPGF.Mapper.PosRange(declPGF.Tok, insertPos, insertPos)
if err != nil {
return nil, nil, err
}
}

var buf bytes.Buffer
if err := fieldInfo.Emit(&buf, qual); err != nil {
return nil, nil, err
}

return fieldInfo.Fset, &analysis.SuggestedFix{
Message: fmt.Sprintf("Add field %s to struct %s", fieldInfo.Expr.Sel.Name, fieldInfo.Named.Obj().Name()),
TextEdits: []analysis.TextEdit{{
Pos: insertPos,
End: insertPos,
NewText: buf.Bytes(),
}},
}, nil
}

type newImport struct {
name string
importPath string
}

func newNamedImportQual(declPGF *parsego.File, snapshot *cache.Snapshot, declMeta *metadata.Package, sym types.Object, newImportHandler func(imp newImport)) func(*types.Package) string {
// Build import environment for the declaring file.
// (typesinternal.FileQualifier works only for complete
// import mappings, and requires types.)
importEnv := make(map[ImportPath]string) // value is local name
for _, imp := range declPGF.File.Imports {
importPath := metadata.UnquoteImportPath(imp)
var name string
if imp.Name != nil {
name = imp.Name.Name
if name == "_" {
continue
} else if name == "." {
name = "" // see types.Qualifier
}
} else {
// Use the correct name from the metadata of the imported
// package---not a guess based on the import path.
mp := snapshot.Metadata(declMeta.DepsByImpPath[importPath])
if mp == nil {
continue // can't happen?
}
name = string(mp.Name)
}
importEnv[importPath] = name // latest alias wins
}

// Create a package name qualifier that uses the
// locally appropriate imported package name.
// It records any needed new imports.
// TODO(adonovan): factor with golang.FormatVarType?
//
// Prior to CL 469155 this logic preserved any renaming
// imports from the file that declares the interface
// method--ostensibly the preferred name for imports of
// frequently renamed packages such as protobufs.
// Now we use the package's declared name. If this turns out
// to be a mistake, then use parseHeader(si.iface.Pos()).
//
return func(pkg *types.Package) string {
// TODO(adonovan): don't ignore vendor prefix.
//
// Ignore the current package import.
if pkg.Path() == sym.Pkg().Path() {
return ""
}

importPath := ImportPath(pkg.Path())
name, ok := importEnv[importPath]
if !ok {
// Insert new import using package's declared name.
//
// TODO(adonovan): resolve conflict between declared
// name and existing file-level (declPGF.File.Imports)
// or package-level (sym.Pkg.Scope) decls by
// generating a fresh name.
name = pkg.Name()
importEnv[importPath] = name
new := newImport{importPath: string(importPath)}
// For clarity, use a renaming import whenever the
// local name does not match the path's last segment.
if name != pathpkg.Base(trimVersionSuffix(new.importPath)) {
new.name = name
}
newImportHandler(new)
}
return name
}
}
2 changes: 1 addition & 1 deletion gopls/internal/golang/stubmethods/stubcalledfunc.go
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ func GetCallStubInfo(fset *token.FileSet, info *types.Info, pgf *parsego.File, s
// If recvExpr is a package name, compiler error would be
// e.g., "undefined: http.bar", thus will not hit this code path.
recvExpr := s.X
recvType, pointer := concreteType(recvExpr, info)
recvType, pointer := concreteType(info, recvExpr)

if recvType == nil || recvType.Obj().Pkg() == nil {
return nil
Expand Down
Loading