From 6ca1ec848a730d9ce972f46fa1b10a19f647d26b Mon Sep 17 00:00:00 2001 From: Aofei Sheng Date: Fri, 10 Jan 2025 09:05:10 +0800 Subject: [PATCH] fix(tools/spxls): resolve issues with LS functionality Updates #1199 - https://github.com/goplus/builder/issues/1199#issuecomment-2579274294 - https://github.com/goplus/builder/issues/1199#issuecomment-2579281650 - https://github.com/goplus/builder/issues/1199#issuecomment-2579290223 - https://github.com/goplus/builder/issues/1199#issuecomment-2579313694 - https://github.com/goplus/builder/issues/1199#issuecomment-2586158276 Signed-off-by: Aofei Sheng --- tools/spxls/internal/server/compile.go | 160 +++++++++--------- tools/spxls/internal/server/definition.go | 4 +- .../spxls/internal/server/definition_test.go | 17 ++ .../spxls/internal/server/diagnostic_test.go | 148 ++++------------ tools/spxls/internal/server/document_test.go | 10 +- tools/spxls/internal/server/format.go | 43 +++-- tools/spxls/internal/server/format_test.go | 51 +++++- tools/spxls/internal/server/reference.go | 2 +- tools/spxls/internal/server/rename.go | 9 +- tools/spxls/internal/server/rename_test.go | 70 ++++++++ tools/spxls/internal/server/spx_definition.go | 8 +- tools/spxls/internal/server/util.go | 9 - 12 files changed, 291 insertions(+), 240 deletions(-) diff --git a/tools/spxls/internal/server/compile.go b/tools/spxls/internal/server/compile.go index 34225d1f9..46f98124b 100644 --- a/tools/spxls/internal/server/compile.go +++ b/tools/spxls/internal/server/compile.go @@ -56,6 +56,12 @@ type compileResult struct { // mainPkg is the main package. mainPkg *types.Package + // mainPkgGameType is the Game type in the main package. + mainPkgGameType *types.Named + + // mainPkgSpriteTypes stores sprite types in the main package. + mainPkgSpriteTypes []*types.Named + // mainASTPkg is the main package AST. mainASTPkg *gopast.Package @@ -80,9 +86,6 @@ type compileResult struct { // process. typeInfo *goptypesutil.Info - // spxSpriteNames stores spx sprite names. - spxSpriteNames []string - // spxResourceSet is the set of spx resources. spxResourceSet SpxResourceSet @@ -134,6 +137,11 @@ type astFileLine struct { line int } +// isInFset reports whether the given position exists in the file set. +func (r *compileResult) isInFset(pos goptoken.Pos) bool { + return r.fset.File(pos) != nil +} + // innermostScopeAt returns the innermost scope that contains the given // position. It returns nil if not found. func (r *compileResult) innermostScopeAt(pos goptoken.Pos) *types.Scope { @@ -591,7 +599,10 @@ func (s *Server) compileUncached(snapshot *vfs.MapFS, spxFiles []string) (*compi diagnostics: make(map[DocumentURI][]Diagnostic, len(spxFiles)), } - gpfs := vfs.NewGopParserFS(snapshot) + var ( + gpfs = vfs.NewGopParserFS(snapshot) + spriteNames = make([]string, 0, len(spxFiles)-1) + ) for _, spxFile := range spxFiles { documentURI := s.toDocumentURI(spxFile) result.diagnostics[documentURI] = []Diagnostic{} @@ -644,7 +655,7 @@ func (s *Server) compileUncached(snapshot *vfs.MapFS, spxFiles []string) (*compi if spxFileBaseName := path.Base(spxFile); spxFileBaseName == "main.spx" { result.mainSpxFile = spxFile } else { - result.spxSpriteNames = append(result.spxSpriteNames, strings.TrimSuffix(spxFileBaseName, ".spx")) + spriteNames = append(spriteNames, strings.TrimSuffix(spxFileBaseName, ".spx")) } for _, decl := range astFile.Decls { @@ -707,12 +718,23 @@ func (s *Server) compileUncached(snapshot *vfs.MapFS, spxFiles []string) (*compi }, nil, result.typeInfo, - ).Files(nil, gopASTFileMapToSlice(result.mainASTPkg.Files)); err != nil { + ).Files(nil, slices.Collect(maps.Values(result.mainASTPkg.Files))); err != nil { // Errors should be handled by the type checker. } + if obj := result.mainPkg.Scope().Lookup("Game"); obj != nil { + result.mainPkgGameType = obj.Type().(*types.Named) + } + + result.mainPkgSpriteTypes = make([]*types.Named, 0, len(spriteNames)) + for _, spxSpriteName := range spriteNames { + if obj := result.mainPkg.Scope().Lookup(spxSpriteName); obj != nil { + result.mainPkgSpriteTypes = append(result.mainPkgSpriteTypes, obj.Type().(*types.Named)) + } + } + s.inspectForSpxResourceSet(snapshot, result) - s.inspectForSpxResourceAutoBindingsAndRefsAtDecls(result) + s.inspectForSpxResourceAutoBindings(result) s.inspectForSpxResourceRefs(result) return result, nil @@ -791,99 +813,69 @@ func (s *Server) inspectForSpxResourceSet(snapshot *vfs.MapFS, result *compileRe result.spxResourceSet = *spxResourceSet } -// inspectForSpxResourceAutoBindingsAndRefsAtDecls inspects for spx resource -// auto-bindings and references at variable or constant declarations. -func (s *Server) inspectForSpxResourceAutoBindingsAndRefsAtDecls(result *compileResult) { +// inspectForSpxResourceAutoBindings inspects for spx resource auto-bindings and +// references at variable or constant declarations. +func (s *Server) inspectForSpxResourceAutoBindings(result *compileResult) { + mainSpxFileScope := result.typeInfo.Scopes[result.mainASTPkg.Files[result.mainSpxFile]] for ident, obj := range result.typeInfo.Defs { - objType, ok := obj.Type().(*types.Named) + v, ok := obj.(*types.Var) + if !ok { + continue + } + varType, ok := v.Type().(*types.Named) if !ok { continue } spxFile := result.fset.Position(ident.Pos()).Filename - isInMainSpx := spxFile == result.mainSpxFile - documentURI := s.toDocumentURI(spxFile) - - identRange := Range{ - Start: FromGopTokenPosition(result.fset.Position(ident.Pos())), - End: FromGopTokenPosition(result.fset.Position(ident.End())), + if spxFile != result.mainSpxFile || result.innermostScopeAt(ident.Pos()) != mainSpxFileScope { + continue } var ( isSpxSoundResourceAutoBinding bool isSpxSpriteResourceAutoBinding bool ) - switch objTypeName := objType.String(); objTypeName { + switch varType.String() { case spxSoundTypeFullName: - isSpxSoundResourceAutoBinding = true + isSpxSoundResourceAutoBinding = result.spxResourceSet.Sound(v.Name()) != nil case spxSpriteTypeFullName: - isSpxSpriteResourceAutoBinding = true + isSpxSpriteResourceAutoBinding = result.spxResourceSet.Sprite(v.Name()) != nil default: - for _, spxSpriteName := range result.spxSpriteNames { - if objTypeName != "main."+spxSpriteName { - continue - } - if ident.Name != spxSpriteName { - result.addDiagnostics(documentURI, Diagnostic{ - Severity: SeverityError, - Range: identRange, - Message: "sprite resource name must match type name for explicit auto-binding to work", - }) - continue - } - isSpxSpriteResourceAutoBinding = true - break - } + isSpxSpriteResourceAutoBinding = v.Name() == varType.Obj().Name() && slices.Contains(result.mainPkgSpriteTypes, varType) } - if isSpxSoundResourceAutoBinding || isSpxSpriteResourceAutoBinding { - var diags []Diagnostic - if isInMainSpx { - if result.isDefinedInFirstVarBlock(obj) { - switch { - case isSpxSoundResourceAutoBinding: - result.addSpxResourceRef(SpxResourceRef{ - ID: SpxSoundResourceID{SoundName: ident.Name}, - Kind: SpxResourceRefKindAutoBinding, - Node: ident, - }) - result.spxSoundResourceAutoBindings[obj] = struct{}{} - if result.spxResourceSet.Sound(ident.Name) == nil { - diags = []Diagnostic{{ - Severity: SeverityError, - Range: identRange, - Message: fmt.Sprintf("sound resource %q not found", ident.Name), - }} - } - case isSpxSpriteResourceAutoBinding: - result.addSpxResourceRef(SpxResourceRef{ - ID: SpxSpriteResourceID{SpriteName: ident.Name}, - Kind: SpxResourceRefKindAutoBinding, - Node: ident, - }) - result.spxSpriteResourceAutoBindings[obj] = struct{}{} - if result.spxResourceSet.Sprite(ident.Name) == nil { - diags = []Diagnostic{{ - Severity: SeverityError, - Range: identRange, - Message: fmt.Sprintf("sprite resource %q not found", ident.Name), - }} - } - } - } else { - diags = []Diagnostic{{ - Severity: SeverityWarning, - Range: identRange, - Message: "resources must be defined in the first var block for auto-binding", - }} - } - } else { - diags = []Diagnostic{{ - Severity: SeverityWarning, - Range: identRange, - Message: "auto-binding of resources can only happen in main.spx", - }} - } - result.addDiagnostics(documentURI, diags...) + if !isSpxSoundResourceAutoBinding && !isSpxSpriteResourceAutoBinding { + continue + } + + if !result.isDefinedInFirstVarBlock(obj) { + documentURI := s.toDocumentURI(spxFile) + result.addDiagnostics(documentURI, Diagnostic{ + Severity: SeverityWarning, + Range: Range{ + Start: FromGopTokenPosition(result.fset.Position(ident.Pos())), + End: FromGopTokenPosition(result.fset.Position(ident.End())), + }, + Message: "resources must be defined in the first var block for auto-binding", + }) + continue + } + + switch { + case isSpxSoundResourceAutoBinding: + result.addSpxResourceRef(SpxResourceRef{ + ID: SpxSoundResourceID{SoundName: ident.Name}, + Kind: SpxResourceRefKindAutoBinding, + Node: ident, + }) + result.spxSoundResourceAutoBindings[obj] = struct{}{} + case isSpxSpriteResourceAutoBinding: + result.addSpxResourceRef(SpxResourceRef{ + ID: SpxSpriteResourceID{SpriteName: ident.Name}, + Kind: SpxResourceRefKindAutoBinding, + Node: ident, + }) + result.spxSpriteResourceAutoBindings[obj] = struct{}{} } } } diff --git a/tools/spxls/internal/server/definition.go b/tools/spxls/internal/server/definition.go index 72cc04144..7f1df4210 100644 --- a/tools/spxls/internal/server/definition.go +++ b/tools/spxls/internal/server/definition.go @@ -33,7 +33,7 @@ func (s *Server) textDocumentDefinition(params *DefinitionParams) (any, error) { } defIdent := result.defIdentFor(obj) - if defIdent == nil { + if defIdent == nil || !result.isInFset(defIdent.Pos()) { return nil, nil } return s.createLocationFromIdent(result.fset, defIdent), nil @@ -64,7 +64,7 @@ func (s *Server) textDocumentTypeDefinition(params *TypeDefinitionParams) (any, } objPos := named.Obj().Pos() - if !objPos.IsValid() { + if !result.isInFset(objPos) { return nil, nil } return s.createLocationFromPos(result.fset, objPos), nil diff --git a/tools/spxls/internal/server/definition_test.go b/tools/spxls/internal/server/definition_test.go index 6796dd66c..b454ae49a 100644 --- a/tools/spxls/internal/server/definition_test.go +++ b/tools/spxls/internal/server/definition_test.go @@ -87,6 +87,23 @@ var x int require.Nil(t, def) }) + t.Run("ThisPtr", func(t *testing.T) { + s := New(newMapFSWithoutModTime(map[string][]byte{ + "main.spx": []byte(` +this.run "assets", {Title: "My Game"} +`), + }), nil) + + def, err := s.textDocumentDefinition(&DefinitionParams{ + TextDocumentPositionParams: TextDocumentPositionParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + Position: Position{Line: 1, Character: 0}, + }, + }) + require.NoError(t, err) + require.Nil(t, def) + }) + t.Run("InvalidPosition", func(t *testing.T) { s := New(newMapFSWithoutModTime(map[string][]byte{ "main.spx": []byte(` diff --git a/tools/spxls/internal/server/diagnostic_test.go b/tools/spxls/internal/server/diagnostic_test.go index ec519af44..5001a8f03 100644 --- a/tools/spxls/internal/server/diagnostic_test.go +++ b/tools/spxls/internal/server/diagnostic_test.go @@ -180,7 +180,7 @@ var ( for _, item := range report.Items { fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport) if fullReport.URI == "file:///main.spx" { - require.Len(t, fullReport.Items, 3) + require.Len(t, fullReport.Items, 2) assert.Contains(t, fullReport.Items, Diagnostic{ Severity: SeverityError, Message: "expected ')', found 'EOF'", @@ -197,14 +197,6 @@ var ( End: Position{Line: 3, Character: 23}, }, }) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: `sprite resource "MyAircraft" not found`, - Range: Range{ - Start: Position{Line: 3, Character: 1}, - End: Position{Line: 3, Character: 11}, - }, - }) } else { assert.Empty(t, fullReport.Items) } @@ -223,19 +215,16 @@ var ( s := New(newMapFSWithoutModTime(map[string][]byte{ "main.spx": []byte(` var ( - AutoBindingSoundName Sound -) -play AutoBindingSoundName -var ( - InvalidAutoBindingSoundName Sound + Sound1 Sound ) +play Sound1 run "assets", {Title: "My Game"} `), "MySprite.spx": []byte(` const ConstSoundName = "ConstSoundName" var ( - VarSoundName string - AutoBindingSoundName2 Sound + VarSoundName string + Sound2 Sound ) VarSoundName = "VarSoundName" onStart => { @@ -243,8 +232,8 @@ onStart => { play ConstSoundName play "LiteralSoundName" play VarSoundName - play AutoBindingSoundName - play AutoBindingSoundName2 + play Sound1 + play Sound2 } `), "assets/index.json": []byte(`{}`), @@ -258,42 +247,8 @@ onStart => { fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport) assert.Equal(t, string(DiagnosticFull), fullReport.Kind) switch fullReport.URI { - case "file:///main.spx": - require.Len(t, fullReport.Items, 3) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: `sound resource "AutoBindingSoundName" not found`, - Range: Range{ - Start: Position{Line: 2, Character: 1}, - End: Position{Line: 2, Character: 21}, - }, - }) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: `sound resource "AutoBindingSoundName" not found`, - Range: Range{ - Start: Position{Line: 4, Character: 5}, - End: Position{Line: 4, Character: 25}, - }, - }) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityWarning, - Message: "resources must be defined in the first var block for auto-binding", - Range: Range{ - Start: Position{Line: 6, Character: 1}, - End: Position{Line: 6, Character: 28}, - }, - }) case "file:///MySprite.spx": - require.Len(t, fullReport.Items, 5) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityWarning, - Message: "auto-binding of resources can only happen in main.spx", - Range: Range{ - Start: Position{Line: 4, Character: 1}, - End: Position{Line: 4, Character: 22}, - }, - }) + require.Len(t, fullReport.Items, 3) assert.Contains(t, fullReport.Items, Diagnostic{ Severity: SeverityError, Message: "sound resource name cannot be empty", @@ -318,14 +273,6 @@ onStart => { End: Position{Line: 10, Character: 24}, }, }) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: `sound resource "AutoBindingSoundName" not found`, - Range: Range{ - Start: Position{Line: 12, Character: 6}, - End: Position{Line: 12, Character: 26}, - }, - }) default: assert.Empty(t, fullReport.Items) } @@ -406,9 +353,8 @@ onStart => { s := New(newMapFSWithoutModTime(map[string][]byte{ "main.spx": []byte(` var ( - MySprite1 Sprite - MySprite2 MySprite2 - MySprite2a MySprite2 + MySprite1 Sprite + MySprite2 MySprite2 ) run "assets", {Title: "My Game"} `), @@ -437,42 +383,7 @@ onStart => { fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport) assert.Equal(t, string(DiagnosticFull), fullReport.Kind) switch fullReport.URI { - case "file:///main.spx": - require.Len(t, fullReport.Items, 3) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: `sprite resource "MySprite1" not found`, - Range: Range{ - Start: Position{Line: 2, Character: 1}, - End: Position{Line: 2, Character: 10}, - }, - }) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: `sprite resource "MySprite2" not found`, - Range: Range{ - Start: Position{Line: 3, Character: 1}, - End: Position{Line: 3, Character: 10}, - }, - }) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: "sprite resource name must match type name for explicit auto-binding to work", - Range: Range{ - Start: Position{Line: 4, Character: 1}, - End: Position{Line: 4, Character: 11}, - }, - }) case "file:///MySprite1.spx": - require.Len(t, fullReport.Items, 3) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityWarning, - Message: "auto-binding of resources can only happen in main.spx", - Range: Range{ - Start: Position{Line: 1, Character: 4}, - End: Position{Line: 1, Character: 13}, - }, - }) assert.Contains(t, fullReport.Items, Diagnostic{ Severity: SeverityError, Message: `sprite resource "MySprite1" not found`, @@ -490,15 +401,6 @@ onStart => { }, }) case "file:///MySprite2.spx": - require.Len(t, fullReport.Items, 3) - assert.Contains(t, fullReport.Items, Diagnostic{ - Severity: SeverityError, - Message: `sprite resource "MySprite1" not found`, - Range: Range{ - Start: Position{Line: 2, Character: 1}, - End: Position{Line: 2, Character: 10}, - }, - }) assert.Contains(t, fullReport.Items, Diagnostic{ Severity: SeverityError, Message: `sprite resource "MySprite2" not found`, @@ -706,9 +608,7 @@ onStart => { "main.spx": []byte(` onKey KeyLeft, => {} -// FIXME: Multi-key bindings currently fail because goplus/gogen lacks support for types.Alias. -// See https://github.com/goplus/gogen/issues/457 for details. -// onKey [KeyRight, KeyUp, KeyDown], => {} +onKey [KeyRight, KeyUp, KeyDown], => {} run "assets", {Title: "My Game"} `), @@ -725,4 +625,30 @@ run "assets", {Title: "My Game"} assert.Empty(t, fullReport.Items) } }) + + t.Run("NoTypeSpriteVarDeclaration", func(t *testing.T) { + s := New(newMapFSWithoutModTime(map[string][]byte{ + "main.spx": []byte(`// A spx game. + +var ( + MySprite +) + +run "assets", {Title: "My Game"} +`), + "MySprite.spx": []byte(``), + "assets/index.json": []byte(`{}`), + "assets/sprites/MySprite/index.json": []byte(`{}`), + }), nil) + + report, err := s.workspaceDiagnostic(&WorkspaceDiagnosticParams{}) + require.NoError(t, err) + require.NotNil(t, report) + assert.Len(t, report.Items, 2) + for _, item := range report.Items { + fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport) + assert.Equal(t, string(DiagnosticFull), fullReport.Kind) + assert.Empty(t, fullReport.Items) + } + }) } diff --git a/tools/spxls/internal/server/document_test.go b/tools/spxls/internal/server/document_test.go index 4120e30c5..242e04893 100644 --- a/tools/spxls/internal/server/document_test.go +++ b/tools/spxls/internal/server/document_test.go @@ -19,7 +19,7 @@ run "assets", {Title: "Bullet (by Go+)"} `), "MySprite.spx": []byte(` onStart => { - play "sound1" + play "MySound" onBackdrop "backdrop1", func() {} MySprite.setCostume "costume1" MySprite.animate "anim1" @@ -28,7 +28,7 @@ onStart => { `), "assets/index.json": []byte(`{"backdrops":[{"name":"backdrop1"}],"zorder":[{"name":"widget1","type":"monitor"}]}`), "assets/sprites/MySprite/index.json": []byte(`{"costumes":[{"name":"costume1"}],"fAnimations":{"anim1":{}}}`), - "assets/sounds/sound1/index.json": []byte(`{}`), + "assets/sounds/MySound/index.json": []byte(`{}`), }), nil) paramsForMainSpx := &DocumentLinkParams{ @@ -119,9 +119,9 @@ onStart => { assert.Contains(t, linksForMySpriteSpx, DocumentLink{ Range: Range{ Start: Position{Line: 2, Character: 6}, - End: Position{Line: 2, Character: 14}, + End: Position{Line: 2, Character: 15}, }, - Target: toURI("spx://resources/sounds/sound1"), + Target: toURI("spx://resources/sounds/MySound"), Data: SpxResourceRefDocumentLinkData{ Kind: SpxResourceRefKindStringLiteral, }, @@ -216,6 +216,8 @@ onStart => { var ( MySound Sound `), + "assets/index.json": []byte(`{}`), + "assets/sounds/MySound/index.json": []byte(`{}`), }), nil) params := &DocumentLinkParams{ TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, diff --git a/tools/spxls/internal/server/format.go b/tools/spxls/internal/server/format.go index 5dda9d74d..6275c3e29 100644 --- a/tools/spxls/internal/server/format.go +++ b/tools/spxls/internal/server/format.go @@ -9,6 +9,7 @@ import ( gopast "github.com/goplus/gop/ast" gopfmt "github.com/goplus/gop/format" gopparser "github.com/goplus/gop/parser" + "github.com/goplus/gop/parser/fsx/memfs" goptoken "github.com/goplus/gop/token" ) @@ -58,25 +59,37 @@ func (s *Server) textDocumentFormatting(params *DocumentFormattingParams) ([]Tex func formatSpx(src []byte) ([]byte, error) { // Parse the source into AST. fset := goptoken.NewFileSet() - f, err := gopparser.ParseFile(fset, "main.spx", src, gopparser.ParseComments|gopparser.ParseGoPlusClass) - if err != nil { + astFile, err := gopparser.ParseFSEntry(fset, memfs.SingleFile("", "main.spx", string(src)), "main.spx", nil, gopparser.Config{ + Mode: gopparser.ParseComments | gopparser.ParseGoPlusClass, + }) + if astFile == nil { + // Return error only if parsing completely failed. For partial parsing + // failures, we proceed with formatting. return nil, err } - gopast.SortImports(fset, f) + // Sort import statements first. + gopast.SortImports(fset, astFile) - // Find all var blocks and function declarations. + // Collect all declarations. var ( - varBlocks []*gopast.GenDecl - funcDecls []gopast.Decl - otherDecls []gopast.Decl + importDecls []gopast.Decl + constDecls []gopast.Decl + varBlocks []*gopast.GenDecl + funcDecls []gopast.Decl + otherDecls []gopast.Decl ) - for _, decl := range f.Decls { + for _, decl := range astFile.Decls { switch d := decl.(type) { case *gopast.GenDecl: - if d.Tok == goptoken.VAR { + switch d.Tok { + case goptoken.IMPORT: + importDecls = append(importDecls, d) + case goptoken.CONST: + constDecls = append(constDecls, d) + case goptoken.VAR: varBlocks = append(varBlocks, d) - } else { + default: otherDecls = append(otherDecls, d) } case *gopast.FuncDecl: @@ -86,10 +99,12 @@ func formatSpx(src []byte) ([]byte, error) { } } - // Reorder declarations: vars -> funcs -> others. + // Reorder declarations: imports -> consts -> vars -> funcs -> others. // // See https://github.com/goplus/builder/issues/591 and https://github.com/goplus/builder/issues/752. - newDecls := make([]gopast.Decl, 0, len(f.Decls)) + newDecls := make([]gopast.Decl, 0, len(astFile.Decls)) + newDecls = append(newDecls, importDecls...) + newDecls = append(newDecls, constDecls...) if len(varBlocks) > 0 { // Merge multiple var blocks into a single one. firstVarBlock := varBlocks[0] @@ -112,11 +127,11 @@ func formatSpx(src []byte) ([]byte, error) { } newDecls = append(newDecls, funcDecls...) newDecls = append(newDecls, otherDecls...) - f.Decls = newDecls + astFile.Decls = newDecls // Format the modified AST. var buf bytes.Buffer - if err := gopfmt.Node(&buf, fset, f); err != nil { + if err := gopfmt.Node(&buf, fset, astFile); err != nil { return nil, err } return buf.Bytes(), nil diff --git a/tools/spxls/internal/server/format_test.go b/tools/spxls/internal/server/format_test.go index a6820d7a5..2bab7c4ae 100644 --- a/tools/spxls/internal/server/format_test.go +++ b/tools/spxls/internal/server/format_test.go @@ -81,18 +81,35 @@ run "assets", {Title: "Bullet (by Go+)"} require.Nil(t, edits) }) - t.Run("FormatError", func(t *testing.T) { + t.Run("AcceptableFormatError", func(t *testing.T) { s := New(newMapFSWithoutModTime(map[string][]byte{ - "main.spx": []byte("vbr Foobar string"), + "main.spx": []byte(`// A spx game. + +var MyAircraft MyAircraft +!InvalidSyntax +`), }), nil) params := &DocumentFormattingParams{ TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, } edits, err := s.textDocumentFormatting(params) - require.Error(t, err) - require.Contains(t, err.Error(), "failed to format spx source file") - require.Nil(t, edits) + require.NoError(t, err) + require.Len(t, edits, 1) + assert.Contains(t, edits, TextEdit{ + Range: Range{ + Start: Position{Line: 0, Character: 0}, + End: Position{Line: 4, Character: 0}, + }, + NewText: `// A spx game. + +var ( + MyAircraft MyAircraft +) + +!InvalidSyntax +`, + }) }) t.Run("WithFormatSpx", func(t *testing.T) { @@ -179,9 +196,27 @@ var ( run "assets", {Title: "My Game"} `), - "MySprite.spx": []byte(``), - "assets/index.json": []byte(`{}`), - "assets/sprites/MySprite/index.json": []byte(`{}`), + }), nil) + params := &DocumentFormattingParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + } + + edits, err := s.textDocumentFormatting(params) + require.NoError(t, err) + require.Nil(t, edits) + }) + + t.Run("WithImportStmt", func(t *testing.T) { + s := New(newMapFSWithoutModTime(map[string][]byte{ + "main.spx": []byte(`// A spx game. +import "math" + +onClick => { + println math.floor(2.5) +} + +run "assets", {Title: "My Game"} +`), }), nil) params := &DocumentFormattingParams{ TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, diff --git a/tools/spxls/internal/server/reference.go b/tools/spxls/internal/server/reference.go index fe870112e..1179fe05f 100644 --- a/tools/spxls/internal/server/reference.go +++ b/tools/spxls/internal/server/reference.go @@ -36,7 +36,7 @@ func (s *Server) textDocumentReferences(params *ReferenceParams) ([]Location, er if params.Context.IncludeDeclaration { defIdent := result.defIdentFor(obj) - if defIdent != nil { + if defIdent != nil && result.isInFset(defIdent.Pos()) { locations = append(locations, s.createLocationFromIdent(result.fset, defIdent)) } } diff --git a/tools/spxls/internal/server/rename.go b/tools/spxls/internal/server/rename.go index 1f4e0a4f8..8161072f8 100644 --- a/tools/spxls/internal/server/rename.go +++ b/tools/spxls/internal/server/rename.go @@ -30,6 +30,10 @@ func (s *Server) textDocumentPrepareRename(params *PrepareRenameParams) (*Range, if !isRenameableObject(obj) { return nil, nil } + defIdent := result.defIdentFor(obj) + if defIdent == nil || !result.isInFset(defIdent.Pos()) { + return nil, nil + } return &Range{ Start: FromGopTokenPosition(result.fset.Position(ident.Pos())), @@ -63,9 +67,8 @@ func (s *Server) textDocumentRename(params *RenameParams) (*WorkspaceEdit, error if !isRenameableObject(obj) { return nil, nil } - defIdent := result.defIdentFor(obj) - if defIdent == nil { + if defIdent == nil || !result.isInFset(defIdent.Pos()) { return nil, fmt.Errorf("failed to find definition of object %q", obj.Name()) } @@ -111,7 +114,7 @@ func (s *Server) spxRenameResourceAtRefs(result *compileResult, id SpxResourceID // It has to be a constant. So we must find its declaration site and // use the position of its value instead. defIdent := result.defIdentFor(result.typeInfo.ObjectOf(ident)) - if defIdent != nil { + if defIdent != nil && result.isInFset(defIdent.Pos()) { parent, ok := defIdent.Obj.Decl.(*gopast.ValueSpec) if ok && slices.Contains(parent.Names, defIdent) && len(parent.Values) > 0 { nodePos = result.fset.Position(parent.Values[0].Pos()) diff --git a/tools/spxls/internal/server/rename_test.go b/tools/spxls/internal/server/rename_test.go index 36730f77a..d9180276c 100644 --- a/tools/spxls/internal/server/rename_test.go +++ b/tools/spxls/internal/server/rename_test.go @@ -61,6 +61,42 @@ onStart => { require.NoError(t, err) require.Nil(t, range3) }) + + t.Run("ThisPtr", func(t *testing.T) { + s := New(newMapFSWithoutModTime(map[string][]byte{ + "main.spx": []byte(` +onClick => { + _ = this +} +run "assets", {Title: "My Game"} +`), + "MySprite.spx": []byte(` +onClick => { + _ = this +} +`), + "assets/index.json": []byte(`{}`), + "assets/sprites/MySprite/index.json": []byte(`{}`), + }), nil) + + range1, err := s.textDocumentPrepareRename(&PrepareRenameParams{ + TextDocumentPositionParams: TextDocumentPositionParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + Position: Position{Line: 2, Character: 5}, + }, + }) + require.NoError(t, err) + require.Nil(t, range1) + + range2, err := s.textDocumentPrepareRename(&PrepareRenameParams{ + TextDocumentPositionParams: TextDocumentPositionParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///MySprite.spx"}, + Position: Position{Line: 2, Character: 5}, + }, + }) + require.NoError(t, err) + require.Nil(t, range2) + }) } func TestServerTextDocumentRename(t *testing.T) { @@ -218,6 +254,40 @@ onStart => { NewText: "NewSprite", }) }) + + t.Run("ThisPtr", func(t *testing.T) { + s := New(newMapFSWithoutModTime(map[string][]byte{ + "main.spx": []byte(` +onClick => { + _ = this +} +run "assets", {Title: "My Game"} +`), + "MySprite.spx": []byte(` +onClick => { + _ = this +} +`), + "assets/index.json": []byte(`{}`), + "assets/sprites/MySprite/index.json": []byte(`{}`), + }), nil) + + mainSpxWorkspaceEdit, err := s.textDocumentRename(&RenameParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + Position: Position{Line: 2, Character: 5}, + NewName: "that", + }) + require.EqualError(t, err, `failed to find definition of object "this"`) + require.Nil(t, mainSpxWorkspaceEdit) + + mySpriteSpxWorkspaceEdit, err := s.textDocumentRename(&RenameParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///MySprite.spx"}, + Position: Position{Line: 2, Character: 5}, + NewName: "that", + }) + require.EqualError(t, err, `failed to find definition of object "this"`) + require.Nil(t, mySpriteSpxWorkspaceEdit) + }) } func TestServerSpxRenameBackdropResource(t *testing.T) { diff --git a/tools/spxls/internal/server/spx_definition.go b/tools/spxls/internal/server/spx_definition.go index c4ad4ede9..af80b6e67 100644 --- a/tools/spxls/internal/server/spx_definition.go +++ b/tools/spxls/internal/server/spx_definition.go @@ -260,15 +260,15 @@ var GetSpxPkg = sync.OnceValue(func() *types.Package { }) // GetSpxGameType returns the [spx.Game] type. -var GetSpxGameType = sync.OnceValue(func() types.Type { +var GetSpxGameType = sync.OnceValue(func() *types.Named { spxPkg := GetSpxPkg() - return spxPkg.Scope().Lookup("Game").Type() + return spxPkg.Scope().Lookup("Game").Type().(*types.Named) }) // GetSpxSpriteImplType returns the [spx.SpriteImpl] type. -var GetSpxSpriteImplType = sync.OnceValue(func() types.Type { +var GetSpxSpriteImplType = sync.OnceValue(func() *types.Named { spxPkg := GetSpxPkg() - return spxPkg.Scope().Lookup("SpriteImpl").Type() + return spxPkg.Scope().Lookup("SpriteImpl").Type().(*types.Named) }) // GetSpxPkgDefinitions returns the spx definitions for the spx package. diff --git a/tools/spxls/internal/server/util.go b/tools/spxls/internal/server/util.go index 3a3ee08fb..2af59d895 100644 --- a/tools/spxls/internal/server/util.go +++ b/tools/spxls/internal/server/util.go @@ -32,15 +32,6 @@ func listSpxFiles(rootFS fs.ReadDirFS) ([]string, error) { return files, nil } -// gopASTFileMapToSlice converts a map of [gopast.File] to a slice of [gopast.File]. -func gopASTFileMapToSlice(fileMap map[string]*gopast.File) []*gopast.File { - files := make([]*gopast.File, 0, len(fileMap)) - for _, file := range fileMap { - files = append(files, file) - } - return files -} - // unwrapPointerType returns the underlying type of t. For pointer types, it // returns the element type that the pointer points to. For non-pointer types, // it returns the type unchanged.