From 169fae2bcd381bcb9f98b258bfa0a534a534149b Mon Sep 17 00:00:00 2001 From: Aofei Sheng Date: Wed, 15 Jan 2025 08:53:16 +0800 Subject: [PATCH] fix(tools/spxls): support array resource references and handle EOF Updates #1199 - https://github.com/goplus/builder/issues/1199#issuecomment-2588558346 - https://github.com/goplus/builder/issues/1199#issuecomment-2588560243 Signed-off-by: Aofei Sheng --- tools/spxls/go.sum | 2 - tools/spxls/internal/server/command.go | 6 +-- tools/spxls/internal/server/command_test.go | 54 ++++++++++++++++++--- tools/spxls/internal/server/compile.go | 52 ++++++++++++++------ tools/spxls/internal/server/completion.go | 4 +- tools/spxls/internal/server/hover_test.go | 20 ++++++++ tools/spxls/internal/server/server.go | 2 +- tools/spxls/internal/server/util.go | 9 ++++ 8 files changed, 115 insertions(+), 34 deletions(-) diff --git a/tools/spxls/go.sum b/tools/spxls/go.sum index 29e8d3b91..f4e1680fc 100644 --- a/tools/spxls/go.sum +++ b/tools/spxls/go.sum @@ -26,8 +26,6 @@ github.com/goplus/gop v1.2.0-pre.1.0.20250112163018-5fb12b1b2972 h1:wA4+I+DDfoNw github.com/goplus/gop v1.2.0-pre.1.0.20250112163018-5fb12b1b2972/go.mod h1:lcW75c0a5v361jId1Vxs4lRrsasWsQDH0k3dG3Z7wH0= github.com/goplus/mod v0.13.15 h1:IyneSjwm1VpwvHGz6hSHnFxZCuO6ULHcu74IAZcW9nw= github.com/goplus/mod v0.13.15/go.mod h1:invR72Rz2+qpOOsXqxz830MX8/aR2GDR2EAow/WgfHI= -github.com/goplus/spx v1.1.0 h1:rke7F7YpbjeI583HkuNiUGmsbYdrloNT1ABYLfgv9i8= -github.com/goplus/spx v1.1.0/go.mod h1:1hkX9vPdDHrsA6LG8itiLMp83KIDJ0KlJI9CmpCoiCA= github.com/goplus/spx v1.1.1-0.20241231062359-381fc67db3e1 h1:JpAWB4m+LYjEIZwKMVvIeKqqvIjbcLYTXkkvU92Vbkk= github.com/goplus/spx v1.1.1-0.20241231062359-381fc67db3e1/go.mod h1:1hkX9vPdDHrsA6LG8itiLMp83KIDJ0KlJI9CmpCoiCA= github.com/hajimehoshi/ebiten/v2 v2.7.9 h1:DYH/usAa9dMHcGkBIIEApJsVqDekrJBxYHmsBuly8Iw= diff --git a/tools/spxls/internal/server/command.go b/tools/spxls/internal/server/command.go index 8e5f054f3..162bb8b79 100644 --- a/tools/spxls/internal/server/command.go +++ b/tools/spxls/internal/server/command.go @@ -123,11 +123,7 @@ func (s *Server) spxGetDefinitions(params []SpxGetDefinitionsParams) ([]SpxDefin // Find the innermost scope contains the position. tokenFile := result.fset.File(astFile.Pos()) - // Gop compiler may truncate the last empty line of the file, so we need to adjust the line number to avoid panic. - // TODO: Check if this should be fixed by gop compiler. - line := min(int(param.Position.Line)+1, tokenFile.LineCount()) - lineStart := tokenFile.LineStart(line) - pos := tokenFile.Pos(tokenFile.Offset(lineStart) + int(param.Position.Character)) + pos := posAt(tokenFile, param.Position) if !pos.IsValid() { return nil, nil } diff --git a/tools/spxls/internal/server/command_test.go b/tools/spxls/internal/server/command_test.go index e0957dba2..0c47ef9d5 100644 --- a/tools/spxls/internal/server/command_test.go +++ b/tools/spxls/internal/server/command_test.go @@ -198,7 +198,7 @@ onStart => { "assets/sprites/MySprite/index.json": []byte(`{}`), }), nil) - mainSpxFileScopeParams := []SpxGetDefinitionsParams{ + mySpriteSpxFileScopeParams := []SpxGetDefinitionsParams{ { TextDocumentPositionParams: TextDocumentPositionParams{ TextDocument: TextDocumentIdentifier{URI: "file:///MySprite.spx"}, @@ -206,27 +206,67 @@ onStart => { }, }, } - mainSpxFileScopeDefs, err := s.spxGetDefinitions(mainSpxFileScopeParams) + mySpriteSpxFileScopeDefs, err := s.spxGetDefinitions(mySpriteSpxFileScopeParams) require.NoError(t, err) - require.NotNil(t, mainSpxFileScopeDefs) - assert.True(t, spxDefinitionIdentifierSliceContains(mainSpxFileScopeDefs, SpxDefinitionIdentifier{ + require.NotNil(t, mySpriteSpxFileScopeDefs) + assert.True(t, spxDefinitionIdentifierSliceContains(mySpriteSpxFileScopeDefs, SpxDefinitionIdentifier{ Package: util.ToPtr(spxPkgPath), Name: util.ToPtr("Game.play"), OverloadID: util.ToPtr("1"), })) - assert.False(t, spxDefinitionIdentifierSliceContains(mainSpxFileScopeDefs, SpxDefinitionIdentifier{ + assert.False(t, spxDefinitionIdentifierSliceContains(mySpriteSpxFileScopeDefs, SpxDefinitionIdentifier{ Package: util.ToPtr(spxPkgPath), Name: util.ToPtr("Game.onStart"), })) - assert.True(t, spxDefinitionIdentifierSliceContains(mainSpxFileScopeDefs, SpxDefinitionIdentifier{ + assert.True(t, spxDefinitionIdentifierSliceContains(mySpriteSpxFileScopeDefs, SpxDefinitionIdentifier{ Package: util.ToPtr(spxPkgPath), Name: util.ToPtr("Sprite.onStart"), })) - assert.True(t, spxDefinitionIdentifierSliceContains(mainSpxFileScopeDefs, SpxDefinitionIdentifier{ + assert.True(t, spxDefinitionIdentifierSliceContains(mySpriteSpxFileScopeDefs, SpxDefinitionIdentifier{ Package: util.ToPtr(spxPkgPath), Name: util.ToPtr("Sprite.onClick"), })) }) + + t.Run("EOF", func(t *testing.T) { + s := New(newMapFSWithoutModTime(map[string][]byte{ + "main.spx": []byte(` +onStart => {} +`), + }), nil) + + mainSpxOnStartScopeParams := []SpxGetDefinitionsParams{ + { + TextDocumentPositionParams: TextDocumentPositionParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + Position: Position{Line: 1, Character: 0}, + }, + }, + } + mainSpxOnStartScopeDefs, err := s.spxGetDefinitions(mainSpxOnStartScopeParams) + require.NoError(t, err) + require.NotNil(t, mainSpxOnStartScopeDefs) + assert.False(t, spxDefinitionIdentifierSliceContains(mainSpxOnStartScopeDefs, SpxDefinitionIdentifier{ + Package: util.ToPtr(spxPkgPath), + Name: util.ToPtr("Game.onStart"), + })) + + mainSpxFileScopeParams := []SpxGetDefinitionsParams{ + { + TextDocumentPositionParams: TextDocumentPositionParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + Position: Position{Line: 2, Character: 0}, + }, + }, + } + mainSpxFileScopeDefs, err := s.spxGetDefinitions(mainSpxFileScopeParams) + require.NoError(t, err) + require.NotNil(t, mainSpxFileScopeDefs) + assert.True(t, spxDefinitionIdentifierSliceContains(mainSpxFileScopeDefs, SpxDefinitionIdentifier{ + Package: util.ToPtr(spxPkgPath), + Name: util.ToPtr("Game.onStart"), + })) + }) } // spxDefinitionIdentifierSliceContains reports whether a slice of [SpxDefinitionIdentifier] diff --git a/tools/spxls/internal/server/compile.go b/tools/spxls/internal/server/compile.go index 46f98124b..6d6cdbff5 100644 --- a/tools/spxls/internal/server/compile.go +++ b/tools/spxls/internal/server/compile.go @@ -918,23 +918,20 @@ func (s *Server) inspectForSpxResourceRefs(result *compileResult) { // Use the last parameter type for variadic functions. paramType = lastParamType } - switch paramType.String() { - case spxBackdropNameTypeFullName: - s.inspectSpxBackdropResourceRefAtExpr(result, arg, paramType) - case spxSpriteNameTypeFullName, spxSpriteTypeFullName: - s.inspectSpxSpriteResourceRefAtExpr(result, arg, paramType) - case spxSpriteCostumeNameTypeFullName: - if spxSpriteResource != nil { - s.inspectSpxSpriteCostumeResourceRefAtExpr(result, spxSpriteResource, arg, paramType) - } - case spxSpriteAnimationNameTypeFullName: - if spxSpriteResource != nil { - s.inspectSpxSpriteAnimationResourceRefAtExpr(result, spxSpriteResource, arg, paramType) + + // Handle slice/array parameter types. + if sliceType, ok := paramType.(*types.Slice); ok { + paramType = unwrapPointerType(sliceType.Elem()) + } else if arrayType, ok := paramType.(*types.Array); ok { + paramType = unwrapPointerType(arrayType.Elem()) + } + + if sliceLit, ok := arg.(*gopast.SliceLit); ok { + for _, elt := range sliceLit.Elts { + s.inspectSpxResourceRefForTypeAtExpr(result, elt, paramType, spxSpriteResource) } - case spxSoundNameTypeFullName, spxSoundTypeFullName: - s.inspectSpxSoundResourceRefAtExpr(result, arg, paramType) - case spxWidgetNameTypeFullName: - s.inspectSpxWidgetResourceRefAtExpr(result, arg, paramType) + } else { + s.inspectSpxResourceRefForTypeAtExpr(result, arg, paramType, spxSpriteResource) } } default: @@ -998,6 +995,29 @@ func (s *Server) inspectForSpxResourceRefs(result *compileResult) { } } +// inspectSpxResourceRefForTypeAtExpr inspects a spx resource reference for a +// given type at an expression. +func (s *Server) inspectSpxResourceRefForTypeAtExpr(result *compileResult, expr gopast.Expr, typ types.Type, spxSpriteResource *SpxSpriteResource) { + switch typ.String() { + case spxBackdropNameTypeFullName: + s.inspectSpxBackdropResourceRefAtExpr(result, expr, typ) + case spxSpriteNameTypeFullName, spxSpriteTypeFullName: + s.inspectSpxSpriteResourceRefAtExpr(result, expr, typ) + case spxSpriteCostumeNameTypeFullName: + if spxSpriteResource != nil { + s.inspectSpxSpriteCostumeResourceRefAtExpr(result, spxSpriteResource, expr, typ) + } + case spxSpriteAnimationNameTypeFullName: + if spxSpriteResource != nil { + s.inspectSpxSpriteAnimationResourceRefAtExpr(result, spxSpriteResource, expr, typ) + } + case spxSoundNameTypeFullName, spxSoundTypeFullName: + s.inspectSpxSoundResourceRefAtExpr(result, expr, typ) + case spxWidgetNameTypeFullName: + s.inspectSpxWidgetResourceRefAtExpr(result, expr, typ) + } +} + // inspectSpxBackdropResourceRefAtExpr inspects a spx backdrop resource // reference at an expression. It returns the spx backdrop resource if it was // successfully retrieved. diff --git a/tools/spxls/internal/server/completion.go b/tools/spxls/internal/server/completion.go index 975aebf31..ab3de18db 100644 --- a/tools/spxls/internal/server/completion.go +++ b/tools/spxls/internal/server/completion.go @@ -28,9 +28,7 @@ func (s *Server) textDocumentCompletion(params *CompletionParams) ([]CompletionI } tokenFile := result.fset.File(astFile.Pos()) - line := min(int(params.Position.Line)+1, tokenFile.LineCount()) - lineStart := tokenFile.LineStart(line) - pos := tokenFile.Pos(tokenFile.Offset(lineStart) + int(params.Position.Character)) + pos := posAt(tokenFile, params.Position) if !pos.IsValid() { return nil, nil } diff --git a/tools/spxls/internal/server/hover_test.go b/tools/spxls/internal/server/hover_test.go index 34f1b01da..5fd4c6211 100644 --- a/tools/spxls/internal/server/hover_test.go +++ b/tools/spxls/internal/server/hover_test.go @@ -61,6 +61,7 @@ onStart => { clone imagePoint.X = 100 } +onTouchStart ["MySprite"], => {} `), "assets/index.json": []byte(`{}`), "assets/sprites/MySprite/index.json": []byte(`{"costumes":[{"name":"costume1"}]}`), @@ -442,6 +443,25 @@ onStart => { End: Position{Line: 6, Character: 13}, }, }, imagePointFieldHover) + + onTouchStartFirstArgHover, err := s.textDocumentHover(&HoverParams{ + TextDocumentPositionParams: TextDocumentPositionParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///MySprite.spx"}, + Position: Position{Line: 8, Character: 14}, + }, + }) + require.NoError(t, err) + require.NotNil(t, onTouchStartFirstArgHover) + assert.Equal(t, &Hover{ + Contents: MarkupContent{ + Kind: Markdown, + Value: "\n", + }, + Range: Range{ + Start: Position{Line: 8, Character: 14}, + End: Position{Line: 8, Character: 24}, + }, + }, onTouchStartFirstArgHover) }) t.Run("InvalidPosition", func(t *testing.T) { diff --git a/tools/spxls/internal/server/server.go b/tools/spxls/internal/server/server.go index 39b819a1a..1d4d56387 100644 --- a/tools/spxls/internal/server/server.go +++ b/tools/spxls/internal/server/server.go @@ -22,7 +22,7 @@ type MessageReplier interface { ReplyMessage(m jsonrpc2.Message) error } -// Server is the core language server implementation that handles LSP messages +// Server is the core language server implementation that handles LSP messages. type Server struct { workspaceRootURI DocumentURI workspaceRootFS *vfs.MapFS diff --git a/tools/spxls/internal/server/util.go b/tools/spxls/internal/server/util.go index 2af59d895..80df9e52e 100644 --- a/tools/spxls/internal/server/util.go +++ b/tools/spxls/internal/server/util.go @@ -68,6 +68,15 @@ func getStringLitOrConstValue(expr gopast.Expr, tv types.TypeAndValue) (string, } } +// posAt returns the [goptoken.Pos] of the given position in the given token file. +func posAt(tokenFile *goptoken.File, position Position) goptoken.Pos { + line := int(position.Line) + 1 + if line > tokenFile.LineCount() { + return goptoken.Pos(tokenFile.Base() + tokenFile.Size()) // EOF + } + return tokenFile.LineStart(line) + goptoken.Pos(int(position.Character)) +} + // deduplicateLocations deduplicates locations. func deduplicateLocations(locations []Location) []Location { result := make([]Location, 0, len(locations))