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

feat(spxls): add support for retrieving package documentation in hover #1373

Open
wants to merge 3 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
37 changes: 37 additions & 0 deletions tools/spxls/internal/server/compile.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"maps"
"path"
"slices"
"strconv"
"strings"
"sync"
"time"
Expand Down Expand Up @@ -510,6 +511,42 @@ func (r *compileResult) spxResourceRefAtASTFilePosition(astFile *gopast.File, po
return bestRef
}

// spxImportsAtASTFilePosition returns the import at the given position in the given AST file.
func (r *compileResult) spxImportsAtASTFilePosition(astFile *gopast.File, position Position) *SpxReferencePkg {
spxFile := r.nodeFilename(astFile)
line := int(position.Line) + 1
column := int(position.Character) + 1

var rpkg *SpxReferencePkg

for _, imp := range astFile.Imports {
nodePos := r.fset.Position(imp.Pos())
nodeEnd := r.fset.Position(imp.End())
if nodePos.Filename != spxFile ||
line != nodePos.Line ||
column < nodePos.Column ||
column > nodeEnd.Column {
continue
}

pkg := imp.Path.Value
unquoted, err := strconv.Unquote(pkg)
if err != nil {
continue
}
pkgDoc, err := pkgdata.GetPkgDoc(unquoted)
if err == nil {
rpkg = &SpxReferencePkg{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

话说这里是不是就该 return 了?感觉没必要再继续遍历下去了

Pkg: pkgDoc,
PkgPath: pkg,
Node: imp,
}
}

}
return rpkg
}

// addSpxResourceRef adds an spx resource reference to the compile result.
func (r *compileResult) addSpxResourceRef(ref SpxResourceRef) {
if r.seenSpxResourceRefs == nil {
Expand Down
18 changes: 17 additions & 1 deletion tools/spxls/internal/server/hover.go
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

哦对了,可能需要补个单测,往 hover_test.go 里的 TestServerTextDocumentHover 加个 subtest 就好

Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package server

import "strings"
import (
"go/doc"
"strings"
)

// See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification#textDocument_hover
func (s *Server) textDocumentHover(params *HoverParams) (*Hover, error) {
Expand All @@ -22,6 +25,19 @@ func (s *Server) textDocumentHover(params *HoverParams) (*Hover, error) {
}, nil
}

// Check if the position is within an import declaration.
// If so, return the package documentation.
rpkg := result.spxImportsAtASTFilePosition(astFile, params.Position)
if rpkg != nil {
return &Hover{
Contents: MarkupContent{
Kind: Markdown,
Value: doc.Synopsis(rpkg.Pkg.Doc),
},
Range: result.rangeForNode(rpkg.Node),
}, nil
}

ident := result.identAtASTFilePosition(astFile, params.Position)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

其实我倾向于认为这种对于在 import "fmt" 语句中包名的位置拿不到 ast.Ident 的情况属于是 go+ 那边的 bug。换句话讲按照我们已有的实现,它应该出现在 typesutil.InfoDefs 字段中,且对应的 types.Object 是一个 types.PkgName 才合理。如果那样的话,我们应该可以尝试在 go+ 那边修复这个 bug。

当然不排除 go 那边的处理方式也是类似的,如果是那样就不算 go+ 的 bug 了。

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

我最早也是考虑在 typesutil 里面处理,typestuil 里有一个 Implicits,ast.ImportSpec 在这个字段中,不过 ImportSpec 具体的pkg 是一个BasicLit 不是 ident,所以就没在这里处理, 看了 gopls 也是单独分支处理,所以就单独放出来

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

好的。不过感觉这个逻辑是不是可以放进 ident == nil 里去?我的理解它算是在找不到 ident 时的特殊处理。

if ident == nil {
return nil, nil
Expand Down
13 changes: 13 additions & 0 deletions tools/spxls/internal/server/spx_reference_pkg.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
package server

import (
"github.com/goplus/builder/tools/spxls/internal/pkgdoc"
gopast "github.com/goplus/gop/ast"
)

// SpxReferencePkg is a reference to an imported package.
type SpxReferencePkg struct {
PkgPath string
Pkg *pkgdoc.PkgDoc
Node *gopast.ImportSpec
}
Loading