Skip to content

Commit

Permalink
fix(tools/spxls): correct spx method receiver type name and simplify …
Browse files Browse the repository at this point in the history 
…type display (#1163)

Signed-off-by: Aofei Sheng <[email protected]>
  • Loading branch information
@aofei
aofei authored Dec 23, 2024
1 parent 303dc12 commit b7639b1
Show file tree
Hide file tree
Showing 8 changed files with 183 additions and 106 deletions.
32 changes: 0 additions & 32 deletions tools/spxls/internal/server/compile.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,38 +212,6 @@ func (r *compileResult) isInMainSpxFirstVarBlock(pos goptoken.Pos) bool {
pos <= r.mainSpxFirstVarBlock.End()
}

// isSpxAutoGenMethodDecl reports whether the given function declaration is a
// spx auto-generated method declaration.
func (r *compileResult) isSpxAutoGenMethodDecl(decl *gopast.FuncDecl) bool {
if decl.Recv == nil || len(decl.Recv.List) != 1 {
return false
}
starExpr, ok := decl.Recv.List[0].Type.(*gopast.StarExpr)
if !ok {
return false
}
typ, ok := starExpr.X.(*gopast.Ident)
if !ok {
return false
}
if !isMainPkgObject(r.typeInfo.ObjectOf(typ)) {
return false
}
if typ.Name == "Game" {
switch decl.Name.Name {
case "Main", "MainEntry":
return true
}
}
if slices.Contains(r.spxSpriteNames, typ.Name) {
switch decl.Name.Name {
case "Main", "Classfname":
return true
}
}
return false
}

// spxResourceRefAtASTFilePosition returns the spx resource reference at the
// given position in the given AST file.
func (r *compileResult) spxResourceRefAtASTFilePosition(astFile *gopast.File, position Position) (SpxResourceRefKey, SpxResourceRef) {
Expand Down
32 changes: 16 additions & 16 deletions tools/spxls/internal/server/diagnostic_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,8 +64,8 @@ func TestServerTextDocumentDiagnostic(t *testing.T) {
require.NoError(t, err)
require.NotNil(t, report)

fullReport, ok := report.Value.(*RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected *RelatedFullDocumentDiagnosticReport")
fullReport, ok := report.Value.(RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected RelatedFullDocumentDiagnosticReport")
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
assert.Empty(t, fullReport.Items)
})
Expand All @@ -88,8 +88,8 @@ var (
require.NoError(t, err)
require.NotNil(t, report)

fullReport, ok := report.Value.(*RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected *RelatedFullDocumentDiagnosticReport")
fullReport, ok := report.Value.(RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected RelatedFullDocumentDiagnosticReport")
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
require.Len(t, fullReport.Items, 2)
assert.Contains(t, fullReport.Items, Diagnostic{
Expand Down Expand Up @@ -124,8 +124,8 @@ var (
require.NoError(t, err)
require.NotNil(t, report)

fullReport, ok := report.Value.(*RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected *RelatedFullDocumentDiagnosticReport")
fullReport, ok := report.Value.(RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected RelatedFullDocumentDiagnosticReport")
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
assert.Empty(t, fullReport.Items)
})
Expand All @@ -142,8 +142,8 @@ var (
require.NoError(t, err)
require.NotNil(t, report)

fullReport, ok := report.Value.(*RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected *RelatedFullDocumentDiagnosticReport")
fullReport, ok := report.Value.(RelatedFullDocumentDiagnosticReport)
assert.True(t, ok, "expected RelatedFullDocumentDiagnosticReport")
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
assert.Empty(t, fullReport.Items)
})
Expand All @@ -161,7 +161,7 @@ func TestServerWorkspaceDiagnostic(t *testing.T) {
assert.Len(t, report.Items, 3)
foundFiles := make(map[string]bool)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
relPath, err := s.fromDocumentURI(fullReport.URI)
require.NoError(t, err)
foundFiles[relPath] = true
Expand Down Expand Up @@ -190,7 +190,7 @@ var (
require.NotNil(t, report)
assert.Len(t, report.Items, 2)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
if fullReport.URI == "file:///main.spx" {
require.Len(t, fullReport.Items, 2)
assert.Contains(t, fullReport.Items, Diagnostic{
Expand Down Expand Up @@ -262,7 +262,7 @@ onStart => {
require.NotNil(t, report)
assert.Len(t, report.Items, 2)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
switch fullReport.URI {
case "file:///main.spx":
Expand Down Expand Up @@ -373,7 +373,7 @@ onStart => {
require.NotNil(t, report)
assert.Len(t, report.Items, 2)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
switch fullReport.URI {
case "file:///main.spx":
Expand Down Expand Up @@ -451,7 +451,7 @@ onStart => {
require.NotNil(t, report)
assert.Len(t, report.Items, 3)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
switch fullReport.URI {
case "file:///main.spx":
Expand Down Expand Up @@ -559,7 +559,7 @@ onStart => {
require.NotNil(t, report)
assert.Len(t, report.Items, 2)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
switch fullReport.URI {
case "file:///MySprite.spx":
Expand Down Expand Up @@ -607,7 +607,7 @@ onStart => {
require.NotNil(t, report)
assert.Len(t, report.Items, 2)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
switch fullReport.URI {
case "file:///MySprite.spx":
Expand Down Expand Up @@ -660,7 +660,7 @@ onStart => {
require.NotNil(t, report)
assert.Len(t, report.Items, 2)
for _, item := range report.Items {
fullReport := item.Value.(*WorkspaceFullDocumentDiagnosticReport)
fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport)
assert.Equal(t, string(DiagnosticFull), fullReport.Kind)
switch fullReport.URI {
case "file:///MySprite.spx":
Expand Down
4 changes: 2 additions & 2 deletions tools/spxls/internal/server/highlight.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package server

import (
"errors"
"slices"

"github.com/goplus/builder/tools/spxls/internal/util"
gopast "github.com/goplus/gop/ast"
Expand Down Expand Up @@ -46,8 +47,7 @@ func (s *Server) textDocumentDocumentHighlight(params *DocumentHighlightParams)

kind := Text

for i := len(path) - 2; i >= 0; i-- {
parent := path[i]
for _, parent := range slices.Backward(path[:len(path)-1]) {
switch p := parent.(type) {
case *gopast.ValueSpec:
for _, name := range p.Names {
Expand Down
102 changes: 62 additions & 40 deletions tools/spxls/internal/server/hover.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import (

// See https://microsoft.github.io/language-server-protocol/specifications/lsp/3.18/specification#textDocument_hover
func (s *Server) textDocumentHover(params *HoverParams) (*Hover, error) {
result, _, astFile, err := s.compileAndGetASTFileForDocumentURI(params.TextDocument.URI)
result, spxFile, astFile, err := s.compileAndGetASTFileForDocumentURI(params.TextDocument.URI)
if err != nil {
if errors.Is(err, errNoValidSpxFiles) || errors.Is(err, errNoMainSpxFile) {
return nil, nil
Expand Down Expand Up @@ -89,7 +89,7 @@ func (s *Server) textDocumentHover(params *HoverParams) (*Hover, error) {
case *types.Const:
defs = append(defs, makeConstHoverDefinition(result, obj))
case *types.Func:
defs = append(defs, makeFuncHoverDefinition(result, obj)...)
defs = append(defs, makeFuncHoverDefinition(result, spxFile, astFile, ident, obj)...)
case *types.TypeName:
defs = append(defs, makeTypeHoverDefinition(result, obj))
case *types.PkgName:
Expand All @@ -112,12 +112,12 @@ func (s *Server) textDocumentHover(params *HoverParams) (*Hover, error) {
// functions and types.
var builtinDefinitionOverviews = map[string]string{
// Variables.
"nil": "var nil",
"nil": "var nil Type",

// Constants.
"false": "const false bool",
"iota": "const iota untyped int",
"true": "const true bool",
"false": "const false = 0 != 0",
"iota": "const iota = 0",
"true": "const true = 0 == 0",

// Types.
"any": "type any interface{}",
Expand Down Expand Up @@ -207,7 +207,7 @@ func makeVarHoverDefinition(result *compileResult, v *types.Var) hoverDefinition
}
overview.WriteString(v.Name())
overview.WriteString(" ")
overview.WriteString(v.Type().String())
overview.WriteString(getSimplifiedTypeString(v.Type()))

var detail, typeName string
if pkgPath == "main" {
Expand Down Expand Up @@ -324,21 +324,48 @@ func makeConstHoverDefinition(result *compileResult, c *types.Const) hoverDefini
}

// makeFuncHoverDefinition makes hover definition for functions.
func makeFuncHoverDefinition(result *compileResult, fun *types.Func) []hoverDefinition {
func makeFuncHoverDefinition(result *compileResult, spxFile string, astFile *gopast.File, ident *gopast.Ident, fun *types.Func) []hoverDefinition {
pkg := fun.Pkg()
pkgPath := pkg.Path()
defIdent := result.defIdentOf(fun)

overview, parsedName, recvTypeNamed, overloadID := makeFuncHoverDefinitionOverview(fun)
var recvTypeNamedName string
if recvTypeNamed != nil {
recvTypeNamedName = recvTypeNamed.Obj().Name()
if pkgPath == spxPkgPath {
switch recvTypeNamedName {
case "Sprite":
recvTypeNamedName = "SpriteImpl"
overview, parsedName, recvTypeName, overloadID := makeFuncHoverDefinitionOverview(fun)
if recvTypeName != "" {
var actualRecvTypeName string
if path, _ := util.PathEnclosingInterval(astFile, ident.Pos(), ident.End()); len(path) > 0 {
for _, node := range slices.Backward(path) {
sel, ok := node.(*gopast.SelectorExpr)
if !ok {
continue
}
tv, ok := result.typeInfo.Types[sel.X]
if !ok {
continue
}
if named, ok := unwrapPointerType(tv.Type).(*types.Named); ok {
actualRecvTypeName = named.Obj().Name()
break
}
}
}
if actualRecvTypeName == "" && pkgPath == spxPkgPath {
astFileScope := result.typeInfo.Scopes[astFile]
innermostScope := result.innermostScopeAt(ident.Pos())
if innermostScope == astFileScope {
if spxFile == result.mainSpxFile {
actualRecvTypeName = "Game"
} else {
actualRecvTypeName = "SpriteImpl"
}
}
}
if actualRecvTypeName != "" {
recvTypeName = actualRecvTypeName
}

if pkgPath == spxPkgPath && recvTypeName == "Sprite" {
recvTypeName = "SpriteImpl"
}
}

var detail string
Expand All @@ -348,20 +375,20 @@ func makeFuncHoverDefinition(result *compileResult, fun *types.Func) []hoverDefi
detail = decl.Doc.Text()
}
} else if pkgDoc, err := pkgdata.GetPkgDoc(pkgPath); err == nil {
if recvTypeNamed == nil {
if recvTypeName == "" {
detail = pkgDoc.Funcs[fun.Name()]
} else if spxRecvTypeDoc, ok := pkgDoc.Types[recvTypeNamedName]; ok {
} else if spxRecvTypeDoc, ok := pkgDoc.Types[recvTypeName]; ok {
detail = spxRecvTypeDoc.Methods[fun.Name()]
}
}

idName := parsedName
if recvTypeNamed != nil {
recvTypeNamedNameToDisplay := recvTypeNamedName
if pkgPath == spxPkgPath && recvTypeNamedName == "SpriteImpl" {
recvTypeNamedNameToDisplay = "Sprite"
if recvTypeName != "" {
recvTypeDisplayName := recvTypeName
if pkgPath == spxPkgPath && recvTypeDisplayName == "SpriteImpl" {
recvTypeDisplayName = "Sprite"
}
idName = recvTypeNamedNameToDisplay + "." + idName
idName = recvTypeDisplayName + "." + idName
}
defs := []hoverDefinition{
{
Expand All @@ -381,16 +408,16 @@ func makeFuncHoverDefinition(result *compileResult, fun *types.Func) []hoverDefi
if err != nil {
return defs
}
if recvTypeNamed != nil {
recvType := pkg.Scope().Lookup(recvTypeNamedName).Type()
if recvTypeName != "" {
recvType := pkg.Scope().Lookup(recvTypeName).Type()
if recvType == nil {
return defs
}
if _, ok := recvType.Underlying().(*types.Struct); !ok {
return defs
}

recvTypeDoc, ok := pkgDoc.Types[recvTypeNamedName]
recvTypeDoc, ok := pkgDoc.Types[recvTypeName]
if !ok {
return defs
}
Expand Down Expand Up @@ -468,7 +495,7 @@ func makeFuncHoverDefinition(result *compileResult, fun *types.Func) []hoverDefi

// makeFuncHoverDefinitionOverview makes hover definition overview for the
// provided function.
func makeFuncHoverDefinitionOverview(fun *types.Func) (overview, parsedName string, recvTypeNamed *types.Named, overloadID *string) {
func makeFuncHoverDefinitionOverview(fun *types.Func) (overview, parsedName, recvTypeName string, overloadID *string) {
pkg := fun.Pkg()
pkgPath := pkg.Path()
isGopPkg := pkg.Scope().Lookup(util.GopPackage) != nil
Expand All @@ -482,23 +509,18 @@ func makeFuncHoverDefinitionOverview(fun *types.Func) (overview, parsedName stri
if recv := sig.Recv(); recv != nil {
recvType := unwrapPointerType(recv.Type())
if named, ok := recvType.(*types.Named); ok {
recvTypeNamed = named
recvTypeName = named.Obj().Name()
}
} else if isGopPkg {
switch {
case strings.HasPrefix(name, util.GoptPrefix):
recvTypeName, methodName, ok := util.SplitGoptMethod(name)
splitRecvTypeName, methodName, ok := util.SplitGoptMethod(name)
if !ok {
break
}
name = methodName
recvTypeName = splitRecvTypeName
isGoptMethod = true

recvTypeObj := pkg.Scope().Lookup(recvTypeName)
if recvTypeObj == nil {
break
}
recvTypeNamed = recvTypeObj.Type().(*types.Named)
}
}

Expand All @@ -523,14 +545,14 @@ func makeFuncHoverDefinitionOverview(fun *types.Func) (overview, parsedName stri
param := sig.Params().At(i)
sb.WriteString(param.Name())
sb.WriteString(" ")
sb.WriteString(param.Type().String())
sb.WriteString(getSimplifiedTypeString(param.Type()))
}
sb.WriteString(")")

if results := sig.Results(); results.Len() > 0 {
if results.Len() == 1 {
sb.WriteString(" ")
sb.WriteString(results.At(0).Type().String())
sb.WriteString(getSimplifiedTypeString(results.At(0).Type()))
} else {
sb.WriteString(" (")
for i := range results.Len() {
Expand All @@ -542,7 +564,7 @@ func makeFuncHoverDefinitionOverview(fun *types.Func) (overview, parsedName stri
sb.WriteString(name)
sb.WriteString(" ")
}
sb.WriteString(result.Type().String())
sb.WriteString(getSimplifiedTypeString(result.Type()))
}
sb.WriteString(")")
}
Expand All @@ -563,9 +585,9 @@ func makeTypeHoverDefinition(result *compileResult, typeName *types.TypeName) ho
overview.WriteString(typeName.Name())
overview.WriteString(" ")
if named, ok := typeName.Type().(*types.Named); ok {
overview.WriteString(named.Underlying().String())
overview.WriteString(getSimplifiedTypeString(named.Underlying()))
} else {
overview.WriteString(typeName.Type().String())
overview.WriteString(getSimplifiedTypeString(typeName.Type()))
}

var detail string
Expand Down
Loading

0 comments on commit b7639b1

Please sign in to comment.