From 4358dcc584a5ccfffb3ff4a13ad981c7c90e65f6 Mon Sep 17 00:00:00 2001 From: Aofei Sheng <aofei@aofeisheng.com> Date: Mon, 17 Mar 2025 15:00:01 +0800 Subject: [PATCH] fix: prevent duplicate references for auto-binding resources Fixes goplus/builder#1471 Signed-off-by: Aofei Sheng <aofei@aofeisheng.com> --- internal/server/compile.go | 25 +++-- internal/server/diagnostic_test.go | 14 ++- internal/server/document_test.go | 171 ++++++++++++++++++++++++++--- 3 files changed, 178 insertions(+), 32 deletions(-) diff --git a/internal/server/compile.go b/internal/server/compile.go index 4c928fd..7190692 100644 --- a/internal/server/compile.go +++ b/internal/server/compile.go @@ -1003,20 +1003,11 @@ func (s *Server) inspectForSpxResourceRefs(result *compileResult) { 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{}{} } + s.inspectSpxResourceRefForTypeAtExpr(result, ident, unwrapPointerType(obj.Type()), nil) } // Check all identifier uses. @@ -1246,7 +1237,12 @@ func (s *Server) inspectSpxSpriteResourceRefAtExpr(result *compileResult, expr g return nil } spxSpriteName = obj.Name() - spxResourceRefKind = SpxResourceRefKindAutoBindingReference + defIdent := result.defIdentFor(obj) + if defIdent == ident { + spxResourceRefKind = SpxResourceRefKindAutoBinding + } else { + spxResourceRefKind = SpxResourceRefKindAutoBindingReference + } } if spxSpriteName == "" { result.addDiagnostics(exprDocumentURI, Diagnostic{ @@ -1419,7 +1415,12 @@ func (s *Server) inspectSpxSoundResourceRefAtExpr(result *compileResult, expr go return nil } spxSoundName = obj.Name() - spxResourceRefKind = SpxResourceRefKindAutoBindingReference + defIdent := result.defIdentFor(obj) + if defIdent == ident { + spxResourceRefKind = SpxResourceRefKindAutoBinding + } else { + spxResourceRefKind = SpxResourceRefKindAutoBindingReference + } default: return nil } diff --git a/internal/server/diagnostic_test.go b/internal/server/diagnostic_test.go index 9e6a5b6..eaf35ba 100644 --- a/internal/server/diagnostic_test.go +++ b/internal/server/diagnostic_test.go @@ -195,8 +195,9 @@ func TestServerWorkspaceDiagnostic(t *testing.T) { var ( MyAircraft MyAircraft `), - "MyAircraft.spx": []byte(`var x int`), - "assets/index.json": []byte(`{}`), + "MyAircraft.spx": []byte(`var x int`), + "assets/index.json": []byte(`{}`), + "assets/sprites/MyAircraft/index.json": []byte(`{}`), } s := New(newMapFSWithoutModTime(m), nil, fileMapGetter(m)) @@ -413,6 +414,15 @@ onStart => { fullReport := item.Value.(WorkspaceFullDocumentDiagnosticReport) assert.Equal(t, string(DiagnosticFull), fullReport.Kind) switch fullReport.URI { + case "file:///main.spx": + 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}, + }, + }) case "file:///MySprite1.spx": assert.Contains(t, fullReport.Items, Diagnostic{ Severity: SeverityError, diff --git a/internal/server/document_test.go b/internal/server/document_test.go index b0a242c..1754239 100644 --- a/internal/server/document_test.go +++ b/internal/server/document_test.go @@ -36,10 +36,9 @@ onStart => { } s := New(newMapFSWithoutModTime(m), nil, fileMapGetter(m)) - paramsForMainSpx := &DocumentLinkParams{ + linksForMainSpx, err := s.textDocumentDocumentLink(&DocumentLinkParams{ TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, - } - linksForMainSpx, err := s.textDocumentDocumentLink(paramsForMainSpx) + }) require.NoError(t, err) require.Len(t, linksForMainSpx, 14) assert.Contains(t, linksForMainSpx, DocumentLink{ @@ -153,10 +152,9 @@ onStart => { Target: toURI("gop:github.com/goplus/spx?Game.Title"), }) - paramsForMySpriteSpx := &DocumentLinkParams{ + linksForMySpriteSpx, err := s.textDocumentDocumentLink(&DocumentLinkParams{ TextDocument: TextDocumentIdentifier{URI: "file:///MySprite.spx"}, - } - linksForMySpriteSpx, err := s.textDocumentDocumentLink(paramsForMySpriteSpx) + }) require.NoError(t, err) require.Len(t, linksForMySpriteSpx, 21) assert.Contains(t, linksForMySpriteSpx, DocumentLink{ @@ -263,22 +261,20 @@ onStart => { "main.gop": []byte(`echo "Hello, Go+!"`), } s := New(newMapFSWithoutModTime(m), nil, fileMapGetter(m)) - params := &DocumentLinkParams{ - TextDocument: TextDocumentIdentifier{URI: "file:///main.gop"}, - } - links, err := s.textDocumentDocumentLink(params) + links, err := s.textDocumentDocumentLink(&DocumentLinkParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.gop"}, + }) assert.EqualError(t, err, `file "main.gop" does not have .spx extension`) assert.Nil(t, links) }) t.Run("FileNotFound", func(t *testing.T) { s := New(newMapFSWithoutModTime(map[string][]byte{}), nil, fileMapGetter(map[string][]byte{})) - params := &DocumentLinkParams{ - TextDocument: TextDocumentIdentifier{URI: "file:///notexist.spx"}, - } - links, err := s.textDocumentDocumentLink(params) + links, err := s.textDocumentDocumentLink(&DocumentLinkParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///notexist.spx"}, + }) assert.ErrorIs(t, err, errNoMainSpxFile) assert.Nil(t, links) }) @@ -294,11 +290,10 @@ var ( "assets/sounds/MySound/index.json": []byte(`{}`), } s := New(newMapFSWithoutModTime(m), nil, fileMapGetter(m)) - params := &DocumentLinkParams{ - TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, - } - links, err := s.textDocumentDocumentLink(params) + links, err := s.textDocumentDocumentLink(&DocumentLinkParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + }) require.NoError(t, err) require.Len(t, links, 3) assert.Contains(t, links, DocumentLink{ @@ -326,4 +321,144 @@ var ( Target: toURI("gop:github.com/goplus/spx?Sound"), }) }) + + t.Run("AutoBindingSprite", func(t *testing.T) { + m := map[string][]byte{ + "main.spx": []byte(` +var ( + MySprite Sprite +) +`), + "MySprite.spx": []byte(``), + "assets/index.json": []byte(`{}`), + "assets/sprites/MySprite/index.json": []byte(`{}`), + } + s := New(newMapFSWithoutModTime(m), nil, fileMapGetter(m)) + + links, err := s.textDocumentDocumentLink(&DocumentLinkParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + }) + require.NoError(t, err) + require.Len(t, links, 3) + assert.Contains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 9}, + }, + Target: toURI("spx://resources/sprites/MySprite"), + Data: SpxResourceRefDocumentLinkData{ + Kind: SpxResourceRefKindAutoBinding, + }, + }) + assert.NotContains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 9}, + }, + Target: toURI("spx://resources/sprites/MySprite"), + Data: SpxResourceRefDocumentLinkData{ + Kind: SpxResourceRefKindAutoBindingReference, + }, + }) + assert.Contains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 9}, + }, + Target: toURI("gop:main?Game.MySprite"), + }) + }) + + t.Run("AutoBindingSpriteAsEmbeddedField", func(t *testing.T) { + m := map[string][]byte{ + "main.spx": []byte(` +var ( + MySprite +) +`), + "MySprite.spx": []byte(``), + "assets/index.json": []byte(`{}`), + "assets/sprites/MySprite/index.json": []byte(`{}`), + } + s := New(newMapFSWithoutModTime(m), nil, fileMapGetter(m)) + + links, err := s.textDocumentDocumentLink(&DocumentLinkParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + }) + require.NoError(t, err) + require.Len(t, links, 3) + assert.Contains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 9}, + }, + Target: toURI("spx://resources/sprites/MySprite"), + Data: SpxResourceRefDocumentLinkData{ + Kind: SpxResourceRefKindAutoBinding, + }, + }) + assert.NotContains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 9}, + }, + Target: toURI("spx://resources/sprites/MySprite"), + Data: SpxResourceRefDocumentLinkData{ + Kind: SpxResourceRefKindAutoBindingReference, + }, + }) + assert.Contains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 9}, + }, + Target: toURI("gop:main?Game.MySprite"), + }) + }) + + t.Run("AutoBindingSound", func(t *testing.T) { + m := map[string][]byte{ + "main.spx": []byte(` +var ( + MySound Sound +) +`), + "assets/index.json": []byte(`{}`), + "assets/sounds/MySound/index.json": []byte(`{}`), + } + s := New(newMapFSWithoutModTime(m), nil, fileMapGetter(m)) + + links, err := s.textDocumentDocumentLink(&DocumentLinkParams{ + TextDocument: TextDocumentIdentifier{URI: "file:///main.spx"}, + }) + require.NoError(t, err) + require.Len(t, links, 3) + assert.Contains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 8}, + }, + Target: toURI("spx://resources/sounds/MySound"), + Data: SpxResourceRefDocumentLinkData{ + Kind: SpxResourceRefKindAutoBinding, + }, + }) + assert.NotContains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 8}, + }, + Target: toURI("spx://resources/sounds/MySound"), + Data: SpxResourceRefDocumentLinkData{ + Kind: SpxResourceRefKindAutoBindingReference, + }, + }) + assert.Contains(t, links, DocumentLink{ + Range: Range{ + Start: Position{Line: 2, Character: 1}, + End: Position{Line: 2, Character: 8}, + }, + Target: toURI("gop:main?Game.MySound"), + }) + }) }