From 54ca6afb8c8c060471ce6d4081c2084673df2782 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Wed, 12 Mar 2025 22:02:22 +0800 Subject: [PATCH 01/15] feat(server): implement file synchronization methods for document notifications --- gop/proj.go | 29 + gop/proj_test.go | 127 ++++ internal/server/server.go | 9 +- internal/server/text_syncronization.go | 126 ++++ internal/server/text_syncronization_test.go | 755 ++++++++++++++++++++ 5 files changed, 1042 insertions(+), 4 deletions(-) create mode 100644 internal/server/text_syncronization.go create mode 100644 internal/server/text_syncronization_test.go diff --git a/gop/proj.go b/gop/proj.go index d4bff4c..84c9749 100644 --- a/gop/proj.go +++ b/gop/proj.go @@ -68,6 +68,13 @@ type FileImpl struct { ModTime time.Time } +// FileChange represents a file change. +type FileChange struct { + Path string + Content []byte + Version int // Version is timestamp in milliseconds +} + // Project represents a project. type Project struct { files sync.Map // path => File @@ -195,6 +202,28 @@ func (p *Project) PutFile(path string, file File) { p.deleteCache(path) } +func (p *Project) ModifyFiles(changes []FileChange) { + // Process all changes in a batch + for _, change := range changes { + // Create new file with updated content + file := &FileImpl{ + Content: change.Content, + ModTime: time.UnixMilli(int64(change.Version)), + } + + // Check if file exists + if oldFile, ok := p.File(change.Path); ok { + // Only update if version is newer + if change.Version > int(oldFile.ModTime.UnixMilli()) { + p.PutFile(change.Path, file) + } + } else { + // New file, always add + p.PutFile(change.Path, file) + } + } +} + // UpdateFiles updates all files in the project with the provided map of files. // This will remove existing files not present in the new map and add/update files from the new map. func (p *Project) UpdateFiles(newFiles map[string]File) { diff --git a/gop/proj_test.go b/gop/proj_test.go index 043c7cc..90e48d3 100644 --- a/gop/proj_test.go +++ b/gop/proj_test.go @@ -258,3 +258,130 @@ func TestUpdateFiles(t *testing.T) { t.Fatal("Cache should be invalidated when ModTime changes") } } + +func TestModifyFiles(t *testing.T) { + tests := []struct { + name string + initial map[string]File + changes []FileChange + want map[string]string // path -> expected content + }{ + { + name: "add new files", + initial: map[string]File{}, + changes: []FileChange{ + { + Path: "new.go", + Content: []byte("package main"), + Version: 100, + }, + }, + want: map[string]string{ + "new.go": "package main", + }, + }, + { + name: "update existing file with newer version", + initial: map[string]File{ + "main.go": &FileImpl{ + Content: []byte("old content"), + ModTime: time.UnixMilli(100), + }, + }, + changes: []FileChange{ + { + Path: "main.go", + Content: []byte("new content"), + Version: 200, + }, + }, + want: map[string]string{ + "main.go": "new content", + }, + }, + { + name: "ignore older version update", + initial: map[string]File{ + "main.go": &FileImpl{ + Content: []byte("current content"), + ModTime: time.UnixMilli(200), + }, + }, + changes: []FileChange{ + { + Path: "main.go", + Content: []byte("old content"), + Version: 100, + }, + }, + want: map[string]string{ + "main.go": "current content", + }, + }, + { + name: "multiple file changes", + initial: map[string]File{ + "file1.go": &FileImpl{ + Content: []byte("content1"), + ModTime: time.UnixMilli(100), + }, + "file2.go": &FileImpl{ + Content: []byte("content2"), + ModTime: time.UnixMilli(100), + }, + }, + changes: []FileChange{ + { + Path: "file1.go", + Content: []byte("new content1"), + Version: 200, + }, + { + Path: "file3.go", + Content: []byte("content3"), + Version: 200, + }, + }, + want: map[string]string{ + "file1.go": "new content1", + "file2.go": "content2", + "file3.go": "content3", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create new project with initial files + proj := NewProject(nil, tt.initial, FeatAll) + + // Apply changes + proj.ModifyFiles(tt.changes) + + // Verify results + for path, wantContent := range tt.want { + file, ok := proj.File(path) + if !ok { + t.Errorf("file %s not found", path) + continue + } + if got := string(file.Content); got != wantContent { + t.Errorf("file %s content = %q, want %q", path, got, wantContent) + } + } + + // Verify no extra files exist + count := 0 + proj.RangeFiles(func(path string) bool { + count++ + if _, ok := tt.want[path]; !ok { + t.Errorf("unexpected file: %s", path) + } + return true + }) + if count != len(tt.want) { + t.Errorf("got %d files, want %d", count, len(tt.want)) + } + }) + } +} diff --git a/internal/server/server.go b/internal/server/server.go index 3e40613..e6dda35 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -242,25 +242,26 @@ func (s *Server) handleNotification(n *jsonrpc2.Notification) error { if err := UnmarshalJSON(n.Params(), ¶ms); err != nil { return fmt.Errorf("failed to parse didOpen params: %w", err) } - return errors.New("TODO") + + return s.didOpen(¶ms) case "textDocument/didChange": var params DidChangeTextDocumentParams if err := UnmarshalJSON(n.Params(), ¶ms); err != nil { return fmt.Errorf("failed to parse didChange params: %w", err) } - return errors.New("TODO") + return s.didChange(¶ms) case "textDocument/didSave": var params DidSaveTextDocumentParams if err := UnmarshalJSON(n.Params(), ¶ms); err != nil { return fmt.Errorf("failed to parse didSave params: %w", err) } - return errors.New("TODO") + return s.didSave(¶ms) case "textDocument/didClose": var params DidCloseTextDocumentParams if err := UnmarshalJSON(n.Params(), ¶ms); err != nil { return fmt.Errorf("failed to parse didClose params: %w", err) } - return errors.New("TODO") + return s.didClose(¶ms) } return nil } diff --git a/internal/server/text_syncronization.go b/internal/server/text_syncronization.go new file mode 100644 index 0000000..f77533f --- /dev/null +++ b/internal/server/text_syncronization.go @@ -0,0 +1,126 @@ +package server + +import ( + "github.com/goplus/goxlsw/gop" +) + +// didOpen updates the file when a open notification is received. +func (s *Server) didOpen(params *DidOpenTextDocumentParams) error { + path, err := s.fromDocumentURI(params.TextDocument.URI) + if err != nil { + return err + } + + return s.didModifyFile([]gop.FileChange{{ + Path: path, + Content: []byte(params.TextDocument.Text), + Version: int(params.TextDocument.Version), + }}) +} + +// didChange updates the file when a change notification is received. +func (s *Server) didChange(params *DidChangeTextDocumentParams) error { + path, err := s.fromDocumentURI(params.TextDocument.URI) + if err != nil { + return err + } + + // Convert all changes to FileChange + changes := []gop.FileChange{{ + Path: path, + Content: []byte(params.ContentChanges[len(params.ContentChanges)-1].Text), // Use latest content + Version: int(params.TextDocument.Version), + }} + + return s.didModifyFile(changes) +} + +// didSave updates the file when a save notification is received. +func (s *Server) didSave(params *DidSaveTextDocumentParams) error { + // If text is included in save notification, update the file + if params.Text != nil { + path, err := s.fromDocumentURI(params.TextDocument.URI) + if err != nil { + return err + } + + return s.didModifyFile([]gop.FileChange{{ + Path: path, + Content: []byte(*params.Text), + Version: 0, // Save notifications don't include versions + }}) + } + return nil +} + +// didClose clears diagnostics when a file is closed. +func (s *Server) didClose(params *DidCloseTextDocumentParams) error { + // Clear diagnostics when file is closed + return s.publishDiagnostics(params.TextDocument.URI, nil) +} + +// didModifyFile updates the project with the changes and publishes diagnostics. +func (s *Server) didModifyFile(changes []gop.FileChange) error { + // 1. Update files + s.getProj().ModifyFiles(changes) + + // 2. Asynchronously generate and publish diagnostics + go func() { + for _, change := range changes { + // Convert path to URI for diagnostics + uri := s.toDocumentURI(change.Path) + + // Get diagnostics from AST and type checking + diagnostics, err := s.getDiagnostics(change.Path) + if err != nil { + // Log error but continue processing other files + continue + } + + // Publish diagnostics + if err := s.publishDiagnostics(uri, diagnostics); err != nil { + // Log error but continue + continue + } + } + }() + + return nil +} + +func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { + var diagnostics []Diagnostic + + proj := s.getProj() + // Get AST diagnostics + _, err := proj.AST(path) + if err != nil { + // Convert syntax errors to diagnostics + return []Diagnostic{{ + Range: Range{ + Start: Position{Line: 0, Character: 0}, + End: Position{Line: 0, Character: 0}, + }, + Severity: SeverityError, + Source: "goxlsw", + Message: err.Error(), + }}, nil + } + + // Get type checking diagnostics + _, _, err, _ = proj.TypeInfo() + if err != nil { + // Add type checking errors to diagnostics + diagnostics = append(diagnostics, Diagnostic{ + Range: Range{ + Start: Position{Line: 0, Character: 0}, + End: Position{Line: 0, Character: 0}, + }, + Severity: SeverityError, + Source: "goxlsw", + Message: err.Error(), + }) + } + + return diagnostics, nil +} diff --git a/internal/server/text_syncronization_test.go b/internal/server/text_syncronization_test.go new file mode 100644 index 0000000..9efceeb --- /dev/null +++ b/internal/server/text_syncronization_test.go @@ -0,0 +1,755 @@ +package server + +import ( + "encoding/json" + "errors" + "go/types" + "testing" + + "github.com/goplus/gop/ast" + "github.com/goplus/gop/x/typesutil" + "github.com/goplus/goxlsw/gop" + "github.com/goplus/goxlsw/jsonrpc2" + "github.com/goplus/goxlsw/protocol" +) + +// MockProject 实现模拟的项目接口 +type MockProject struct { + files map[string]gop.File + astError error + typeError error + updatedPaths []string +} + +func (m *MockProject) AST(path string) (*ast.File, error) { + if m.astError != nil { + return nil, m.astError + } + // 创建一个最小的 ast.File 实例 + return &ast.File{ + Name: &ast.Ident{Name: "main"}, + }, nil +} + +func (m *MockProject) TypeInfo() (*types.Package, *typesutil.Info, error, error) { + if m.typeError != nil { + return nil, nil, m.typeError, nil + } + + // 创建最小的类型信息实例 + pkg := types.NewPackage("main", "main") + info := &typesutil.Info{ + Types: make(map[ast.Expr]types.TypeAndValue), + Defs: make(map[*ast.Ident]types.Object), + Uses: make(map[*ast.Ident]types.Object), + } + + return pkg, info, nil, nil +} + +func (m *MockProject) ModifyFiles(changes []gop.FileChange) { + for _, change := range changes { + m.files[change.Path] = &gop.FileImpl{ + Content: change.Content, + } + m.updatedPaths = append(m.updatedPaths, change.Path) + } +} + +func (m *MockProject) File(path string) (gop.File, bool) { + file, ok := m.files[path] + return file, ok +} + +// MockReplier 实现模拟的消息回复接口 +type MockReplier struct { + notifications []*jsonrpc2.Notification +} + +func (m *MockReplier) ReplyMessage(msg jsonrpc2.Message) error { + if n, ok := msg.(*jsonrpc2.Notification); ok { + m.notifications = append(m.notifications, n) + } + return nil +} + +// TestServer 实现测试用的服务器 +type TestServer struct { + proj *MockProject + replier *MockReplier + convertError error +} + +func (s *TestServer) getProj() *MockProject { + return s.proj +} + +func (s *TestServer) fromDocumentURI(uri protocol.DocumentURI) (string, error) { + if s.convertError != nil { + return "", s.convertError + } + // 简单地将URI转换为路径，移除file://前缀 + path := string(uri) + if len(path) > 7 && path[:7] == "file://" { + path = path[7:] + } + return path, nil +} + +func (s *TestServer) toDocumentURI(path string) protocol.DocumentURI { + return protocol.DocumentURI("file://" + path) +} + +func (s *TestServer) publishDiagnostics(uri protocol.DocumentURI, diagnostics []protocol.Diagnostic) error { + params := &protocol.PublishDiagnosticsParams{ + URI: uri, + Diagnostics: diagnostics, + } + notification, _ := jsonrpc2.NewNotification("textDocument/publishDiagnostics", params) + return s.replier.ReplyMessage(notification) +} + +func (s *TestServer) didOpen(params *protocol.DidOpenTextDocumentParams) error { + path, err := s.fromDocumentURI(params.TextDocument.URI) + if err != nil { + return err + } + + return s.didModifyFile([]gop.FileChange{{ + Path: path, + Content: []byte(params.TextDocument.Text), + Version: int(params.TextDocument.Version), + }}) +} + +func (s *TestServer) didChange(params *protocol.DidChangeTextDocumentParams) error { + path, err := s.fromDocumentURI(params.TextDocument.URI) + if err != nil { + return err + } + + changes := []gop.FileChange{{ + Path: path, + Content: []byte(params.ContentChanges[len(params.ContentChanges)-1].Text), + Version: int(params.TextDocument.Version), + }} + + return s.didModifyFile(changes) +} + +func (s *TestServer) didSave(params *protocol.DidSaveTextDocumentParams) error { + if params.Text == nil { + return nil + } + + path, err := s.fromDocumentURI(params.TextDocument.URI) + if err != nil { + return err + } + + return s.didModifyFile([]gop.FileChange{{ + Path: path, + Content: []byte(*params.Text), + Version: 0, + }}) +} + +func (s *TestServer) didClose(params *protocol.DidCloseTextDocumentParams) error { + return s.publishDiagnostics(params.TextDocument.URI, nil) +} + +func (s *TestServer) didModifyFile(changes []gop.FileChange) error { + s.proj.ModifyFiles(changes) + + // 同步处理诊断，简化测试 + for _, change := range changes { + uri := s.toDocumentURI(change.Path) + diagnostics, _ := s.getDiagnostics(change.Path) + s.publishDiagnostics(uri, diagnostics) + } + + return nil +} + +func (s *TestServer) getDiagnostics(path string) ([]protocol.Diagnostic, error) { + var diagnostics []protocol.Diagnostic + + // 检查 AST 错误 + if s.proj.astError != nil { + return []protocol.Diagnostic{{ + Range: protocol.Range{ + Start: protocol.Position{Line: 0, Character: 0}, + End: protocol.Position{Line: 0, Character: 0}, + }, + Severity: protocol.SeverityError, + Source: "goxlsw", + Message: s.proj.astError.Error(), + }}, nil + } + + // 检查类型错误 + if s.proj.typeError != nil { + diagnostics = append(diagnostics, protocol.Diagnostic{ + Range: protocol.Range{ + Start: protocol.Position{Line: 0, Character: 0}, + End: protocol.Position{Line: 0, Character: 0}, + }, + Severity: protocol.SeverityError, + Source: "goxlsw", + Message: s.proj.typeError.Error(), + }) + } + + return diagnostics, nil +} + +// 以下是测试函数 + +func TestDidOpen(t *testing.T) { + tests := []struct { + name string + params *protocol.DidOpenTextDocumentParams + convertError error + expectedPath string + expectedContent string + wantErr bool + }{ + { + name: "basic open", + params: &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file:///test.gop", + Version: 1, + Text: "package main", + }, + }, + expectedPath: "/test.gop", + expectedContent: "package main", + wantErr: false, + }, + { + name: "URI conversion error", + params: &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file:///invalid.gop", + Version: 1, + Text: "package main", + }, + }, + convertError: errors.New("URI conversion failed"), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // 准备测试环境 + mockProj := &MockProject{ + files: make(map[string]gop.File), + } + mockReplier := &MockReplier{} + + server := &TestServer{ + proj: mockProj, + replier: mockReplier, + convertError: tt.convertError, + } + + // 执行测试 + err := server.didOpen(tt.params) + + // 验证结果 + if (err != nil) != tt.wantErr { + t.Errorf("didOpen() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !tt.wantErr { + // 验证文件是否被正确更新 + if len(mockProj.updatedPaths) == 0 { + t.Errorf("No files were updated") + return + } + + if mockProj.updatedPaths[0] != tt.expectedPath { + t.Errorf("Updated wrong path: got %s, want %s", mockProj.updatedPaths[0], tt.expectedPath) + } + + file, ok := mockProj.files[tt.expectedPath] + if !ok { + t.Errorf("File not found in project: %s", tt.expectedPath) + return + } + + if string(file.Content) != tt.expectedContent { + t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) + } + + // 验证是否发送了诊断通知 + if len(mockReplier.notifications) == 0 { + t.Errorf("No diagnostics notifications were sent") + } else if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { + t.Errorf("Wrong notification method: %s", mockReplier.notifications[0].Method()) + } + } + }) + } +} + +func TestDidChange(t *testing.T) { + tests := []struct { + name string + params *protocol.DidChangeTextDocumentParams + convertError error + expectedPath string + expectedContent string + wantErr bool + }{ + { + name: "basic change", + params: &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: "file:///test.gop", + }, + Version: 2, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + {Text: "package main\n\nfunc main() {}"}, + }, + }, + expectedPath: "/test.gop", + expectedContent: "package main\n\nfunc main() {}", + wantErr: false, + }, + { + name: "URI conversion error", + params: &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: "file:///invalid.gop", + }, + Version: 2, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + {Text: "package main"}, + }, + }, + convertError: errors.New("URI conversion failed"), + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // 准备测试环境 + mockProj := &MockProject{ + files: make(map[string]gop.File), + } + mockReplier := &MockReplier{} + + server := &TestServer{ + proj: mockProj, + replier: mockReplier, + convertError: tt.convertError, + } + + // 执行测试 + err := server.didChange(tt.params) + + // 验证结果 + if (err != nil) != tt.wantErr { + t.Errorf("didChange() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !tt.wantErr { + // 验证文件是否被正确更新 + if len(mockProj.updatedPaths) == 0 { + t.Errorf("No files were updated") + return + } + + if mockProj.updatedPaths[0] != tt.expectedPath { + t.Errorf("Updated wrong path: got %s, want %s", mockProj.updatedPaths[0], tt.expectedPath) + } + + file, ok := mockProj.files[tt.expectedPath] + if !ok { + t.Errorf("File not found in project: %s", tt.expectedPath) + return + } + + if string(file.Content) != tt.expectedContent { + t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) + } + + // 验证是否发送了诊断通知 + if len(mockReplier.notifications) == 0 { + t.Errorf("No diagnostics notifications were sent") + } else if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { + t.Errorf("Wrong notification method: %s", mockReplier.notifications[0].Method()) + } + } + }) + } +} + +func TestDidSave(t *testing.T) { + content := "package main\n\nfunc main() {}" + tests := []struct { + name string + params *protocol.DidSaveTextDocumentParams + convertError error + expectedPath string + expectedContent string + wantUpdate bool + wantErr bool + }{ + { + name: "save with content", + params: &protocol.DidSaveTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file:///test.gop", + }, + Text: &content, + }, + expectedPath: "/test.gop", + expectedContent: content, + wantUpdate: true, + wantErr: false, + }, + { + name: "save without content", + params: &protocol.DidSaveTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file:///test.gop", + }, + }, + wantUpdate: false, + wantErr: false, + }, + { + name: "URI conversion error", + params: &protocol.DidSaveTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file:///invalid.gop", + }, + Text: &content, + }, + convertError: errors.New("URI conversion failed"), + wantUpdate: false, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // 准备测试环境 + mockProj := &MockProject{ + files: make(map[string]gop.File), + } + mockReplier := &MockReplier{} + + server := &TestServer{ + proj: mockProj, + replier: mockReplier, + convertError: tt.convertError, + } + + // 执行测试 + err := server.didSave(tt.params) + + // 验证结果 + if (err != nil) != tt.wantErr { + t.Errorf("didSave() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !tt.wantErr && tt.wantUpdate { + // 验证文件是否被正确更新 + if len(mockProj.updatedPaths) == 0 { + t.Errorf("No files were updated") + return + } + + if mockProj.updatedPaths[0] != tt.expectedPath { + t.Errorf("Updated wrong path: got %s, want %s", mockProj.updatedPaths[0], tt.expectedPath) + } + + file, ok := mockProj.files[tt.expectedPath] + if !ok { + t.Errorf("File not found in project: %s", tt.expectedPath) + return + } + + if string(file.Content) != tt.expectedContent { + t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) + } + + // 验证是否发送了诊断通知 + if len(mockReplier.notifications) == 0 { + t.Errorf("No diagnostics notifications were sent") + } + } + + if !tt.wantErr && !tt.wantUpdate { + if len(mockProj.updatedPaths) > 0 { + t.Errorf("File was updated but shouldn't be") + } + } + }) + } +} + +func TestDidClose(t *testing.T) { + tests := []struct { + name string + params *protocol.DidCloseTextDocumentParams + wantErr bool + }{ + { + name: "basic close", + params: &protocol.DidCloseTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file:///test.gop", + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // 准备测试环境 + mockReplier := &MockReplier{} + + server := &TestServer{ + replier: mockReplier, + } + + // 执行测试 + err := server.didClose(tt.params) + + // 验证结果 + if (err != nil) != tt.wantErr { + t.Errorf("didClose() error = %v, wantErr %v", err, tt.wantErr) + return + } + + // 验证是否发送了清空诊断的通知 + if len(mockReplier.notifications) == 0 { + t.Errorf("No diagnostics notifications were sent") + return + } + + if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { + t.Errorf("Wrong notification method: %s", mockReplier.notifications[0].Method()) + } + // 检查诊断是否为空 + var params protocol.PublishDiagnosticsParams + if err := json.Unmarshal(mockReplier.notifications[0].Params(), ¶ms); err != nil { + t.Errorf("Failed to unmarshal notification params: %v", err) + return + } + + if len(params.Diagnostics) != 0 { + t.Errorf("Diagnostics not cleared, got %d items", len(params.Diagnostics)) + } + }) + } +} + +func TestGetDiagnostics(t *testing.T) { + tests := []struct { + name string + path string + astError error + typeError error + wantCount int + }{ + { + name: "no errors", + path: "/test.gop", + wantCount: 0, + }, + { + name: "AST error", + path: "/test.gop", + astError: errors.New("syntax error"), + wantCount: 1, + }, + { + name: "Type error", + path: "/test.gop", + typeError: errors.New("type error"), + wantCount: 1, + }, + { + name: "Both errors", + path: "/test.gop", + astError: errors.New("syntax error"), + typeError: errors.New("type error"), + wantCount: 1, // AST errors take precedence + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // 准备测试环境 + mockProj := &MockProject{ + files: make(map[string]gop.File), + astError: tt.astError, + typeError: tt.typeError, + } + + server := &TestServer{ + proj: mockProj, + } + + // 执行测试 + diagnostics, err := server.getDiagnostics(tt.path) + + // 验证结果 + if err != nil { + t.Errorf("getDiagnostics() error = %v", err) + return + } + + if len(diagnostics) != tt.wantCount { + t.Errorf("getDiagnostics() returned %d diagnostics, want %d", len(diagnostics), tt.wantCount) + } + + if tt.astError != nil && len(diagnostics) > 0 { + if diagnostics[0].Message != tt.astError.Error() { + t.Errorf("Diagnostic message = %q, want %q", diagnostics[0].Message, tt.astError.Error()) + } + } else if tt.typeError != nil && len(diagnostics) > 0 { + if diagnostics[0].Message != tt.typeError.Error() { + t.Errorf("Diagnostic message = %q, want %q", diagnostics[0].Message, tt.typeError.Error()) + } + } + }) + } +} + +func TestDidModifyFile(t *testing.T) { + tests := []struct { + name string + changes []gop.FileChange + astError error + typeError error + wantDiags bool + }{ + { + name: "single file no errors", + changes: []gop.FileChange{ + { + Path: "/test.gop", + Content: []byte("package main"), + Version: 1, + }, + }, + wantDiags: false, + }, + { + name: "single file with AST error", + changes: []gop.FileChange{ + { + Path: "/test.gop", + Content: []byte("package main"), + Version: 1, + }, + }, + astError: errors.New("syntax error"), + wantDiags: true, + }, + { + name: "multiple files", + changes: []gop.FileChange{ + { + Path: "/test1.gop", + Content: []byte("package main"), + Version: 1, + }, + { + Path: "/test2.gop", + Content: []byte("package main"), + Version: 1, + }, + }, + wantDiags: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // 准备测试环境 + mockProj := &MockProject{ + files: make(map[string]gop.File), + astError: tt.astError, + typeError: tt.typeError, + } + mockReplier := &MockReplier{} + + server := &TestServer{ + proj: mockProj, + replier: mockReplier, + } + + // 执行测试 + err := server.didModifyFile(tt.changes) + + // 验证结果 + if err != nil { + t.Errorf("didModifyFile() error = %v", err) + return + } + + // 验证文件是否被更新 + if len(mockProj.updatedPaths) != len(tt.changes) { + t.Errorf("Updated %d files, want %d", len(mockProj.updatedPaths), len(tt.changes)) + } + + for i, change := range tt.changes { + if i < len(mockProj.updatedPaths) && mockProj.updatedPaths[i] != change.Path { + t.Errorf("Updated wrong path at index %d: got %s, want %s", + i, mockProj.updatedPaths[i], change.Path) + } + + file, ok := mockProj.files[change.Path] + if !ok { + t.Errorf("File not found in project: %s", change.Path) + continue + } + + if string(file.Content) != string(change.Content) { + t.Errorf("File content = %q, want %q", string(file.Content), string(change.Content)) + } + } + + // 验证是否发送了诊断通知 + if len(mockReplier.notifications) != len(tt.changes) { + t.Errorf("Sent %d notifications, want %d", len(mockReplier.notifications), len(tt.changes)) + } + + for _, notification := range mockReplier.notifications { + if notification.Method() != "textDocument/publishDiagnostics" { + t.Errorf("Wrong notification method: %s", notification.Method()) + } + + // 检查诊断内容 + var params protocol.PublishDiagnosticsParams + if err := json.Unmarshal(notification.Params(), ¶ms); err != nil { + t.Errorf("Failed to unmarshal notification params: %v", err) + continue + } + + hasDiags := len(params.Diagnostics) > 0 + if hasDiags != tt.wantDiags { + t.Errorf("Diagnostics present = %v, want %v", hasDiags, tt.wantDiags) + } + } + }) + } +} From 16e16c08aa3afd889c3d9f099a0c0272a96e2db7 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Wed, 12 Mar 2025 22:05:33 +0800 Subject: [PATCH 02/15] feat(server): enhance LSP notification handlers with detailed comments --- internal/server/text_syncronization.go | 50 ++++++++++++++++++++------ 1 file changed, 40 insertions(+), 10 deletions(-) diff --git a/internal/server/text_syncronization.go b/internal/server/text_syncronization.go index f77533f..49871e1 100644 --- a/internal/server/text_syncronization.go +++ b/internal/server/text_syncronization.go @@ -4,7 +4,10 @@ import ( "github.com/goplus/goxlsw/gop" ) -// didOpen updates the file when a open notification is received. +// didOpen handles the textDocument/didOpen notification from the LSP client. +// It updates the project with the new file content and publishes diagnostics. +// The document URI is converted to a filesystem path, and a file change is created +// with the document's content and version number. func (s *Server) didOpen(params *DidOpenTextDocumentParams) error { path, err := s.fromDocumentURI(params.TextDocument.URI) if err != nil { @@ -18,7 +21,10 @@ func (s *Server) didOpen(params *DidOpenTextDocumentParams) error { }}) } -// didChange updates the file when a change notification is received. +// didChange handles the textDocument/didChange notification from the LSP client. +// It applies document changes to the project and publishes updated diagnostics. +// For simplicity, this implementation only uses the latest content change +// rather than applying incremental changes. func (s *Server) didChange(params *DidChangeTextDocumentParams) error { path, err := s.fromDocumentURI(params.TextDocument.URI) if err != nil { @@ -26,16 +32,21 @@ func (s *Server) didChange(params *DidChangeTextDocumentParams) error { } // Convert all changes to FileChange + // Note: We currently take only the final state of the document + // rather than applying incremental changes changes := []gop.FileChange{{ Path: path, - Content: []byte(params.ContentChanges[len(params.ContentChanges)-1].Text), // Use latest content + Content: []byte(params.ContentChanges[len(params.ContentChanges)-1].Text), Version: int(params.TextDocument.Version), }} return s.didModifyFile(changes) } -// didSave updates the file when a save notification is received. +// didSave handles the textDocument/didSave notification from the LSP client. +// If the notification includes the document text, the project is updated. +// Otherwise, no change is made since the document content hasn't changed. +// Save notifications typically don't include version numbers, so 0 is used. func (s *Server) didSave(params *DidSaveTextDocumentParams) error { // If text is included in save notification, update the file if params.Text != nil { @@ -53,18 +64,26 @@ func (s *Server) didSave(params *DidSaveTextDocumentParams) error { return nil } -// didClose clears diagnostics when a file is closed. +// didClose handles the textDocument/didClose notification from the LSP client. +// When a document is closed, its diagnostics are cleared by sending an empty +// diagnostics array to the client. func (s *Server) didClose(params *DidCloseTextDocumentParams) error { // Clear diagnostics when file is closed return s.publishDiagnostics(params.TextDocument.URI, nil) } -// didModifyFile updates the project with the changes and publishes diagnostics. +// didModifyFile is a shared implementation for handling document modifications. +// It updates the project with file changes and asynchronously publishes diagnostics. +// The function: +// 1. Updates the project's files with the provided changes +// 2. Starts a goroutine to generate and publish diagnostics for each changed file +// 3. Returns immediately after updating files for better responsiveness func (s *Server) didModifyFile(changes []gop.FileChange) error { - // 1. Update files + // 1. Update files synchronously s.getProj().ModifyFiles(changes) // 2. Asynchronously generate and publish diagnostics + // This allows for quick response while diagnostics computation happens in background go func() { for _, change := range changes { // Convert path to URI for diagnostics @@ -88,14 +107,24 @@ func (s *Server) didModifyFile(changes []gop.FileChange) error { return nil } +// getDiagnostics generates diagnostic information for a specific file. +// It performs two checks: +// 1. AST parsing - reports syntax errors +// 2. Type checking - reports type errors +// +// If AST parsing fails, only syntax errors are returned as diagnostics. +// If AST parsing succeeds but type checking fails, type errors are returned. +// Returns a slice of diagnostics and an error (if diagnostic generation failed). func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { var diagnostics []Diagnostic proj := s.getProj() - // Get AST diagnostics + + // 1. Get AST diagnostics + // Parse the file and check for syntax errors _, err := proj.AST(path) if err != nil { - // Convert syntax errors to diagnostics + // Convert syntax errors to diagnostics with position at the start of file return []Diagnostic{{ Range: Range{ Start: Position{Line: 0, Character: 0}, @@ -107,7 +136,8 @@ func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { }}, nil } - // Get type checking diagnostics + // 2. Get type checking diagnostics + // Perform type checking on the file _, _, err, _ = proj.TypeInfo() if err != nil { // Add type checking errors to diagnostics From b91d0342ef43ea474c50c3293e25d9f364dca067 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Wed, 12 Mar 2025 22:23:59 +0800 Subject: [PATCH 03/15] chore: update documentation for file synchronization methods --- internal/server/text_syncronization_test.go | 101 ++++++++++++-------- 1 file changed, 62 insertions(+), 39 deletions(-) diff --git a/internal/server/text_syncronization_test.go b/internal/server/text_syncronization_test.go index 9efceeb..6fde4f9 100644 --- a/internal/server/text_syncronization_test.go +++ b/internal/server/text_syncronization_test.go @@ -13,7 +13,7 @@ import ( "github.com/goplus/goxlsw/protocol" ) -// MockProject 实现模拟的项目接口 +// MockProject implements a mock Project interface for testing type MockProject struct { files map[string]gop.File astError error @@ -21,22 +21,24 @@ type MockProject struct { updatedPaths []string } +// AST returns a mock AST file or an error if astError is set func (m *MockProject) AST(path string) (*ast.File, error) { if m.astError != nil { return nil, m.astError } - // 创建一个最小的 ast.File 实例 + // Create a minimal ast.File instance return &ast.File{ Name: &ast.Ident{Name: "main"}, }, nil } +// TypeInfo returns mock type information or an error if typeError is set func (m *MockProject) TypeInfo() (*types.Package, *typesutil.Info, error, error) { if m.typeError != nil { return nil, nil, m.typeError, nil } - // 创建最小的类型信息实例 + // Create minimal type information instances pkg := types.NewPackage("main", "main") info := &typesutil.Info{ Types: make(map[ast.Expr]types.TypeAndValue), @@ -47,6 +49,7 @@ func (m *MockProject) TypeInfo() (*types.Package, *typesutil.Info, error, error) return pkg, info, nil, nil } +// ModifyFiles tracks file modifications for testing func (m *MockProject) ModifyFiles(changes []gop.FileChange) { for _, change := range changes { m.files[change.Path] = &gop.FileImpl{ @@ -56,16 +59,18 @@ func (m *MockProject) ModifyFiles(changes []gop.FileChange) { } } +// File returns a file by path if it exists in the mock project func (m *MockProject) File(path string) (gop.File, bool) { file, ok := m.files[path] return file, ok } -// MockReplier 实现模拟的消息回复接口 +// MockReplier implements a message replier for testing type MockReplier struct { notifications []*jsonrpc2.Notification } +// ReplyMessage records notifications for later verification func (m *MockReplier) ReplyMessage(msg jsonrpc2.Message) error { if n, ok := msg.(*jsonrpc2.Notification); ok { m.notifications = append(m.notifications, n) @@ -73,22 +78,25 @@ func (m *MockReplier) ReplyMessage(msg jsonrpc2.Message) error { return nil } -// TestServer 实现测试用的服务器 +// TestServer implements a server for testing type TestServer struct { proj *MockProject replier *MockReplier convertError error } +// getProj returns the mock project func (s *TestServer) getProj() *MockProject { return s.proj } +// fromDocumentURI converts a URI to a filesystem path or returns an error +// if convertError is set func (s *TestServer) fromDocumentURI(uri protocol.DocumentURI) (string, error) { if s.convertError != nil { return "", s.convertError } - // 简单地将URI转换为路径，移除file://前缀 + // Simply convert URI to path by removing file:// prefix path := string(uri) if len(path) > 7 && path[:7] == "file://" { path = path[7:] @@ -96,10 +104,12 @@ func (s *TestServer) fromDocumentURI(uri protocol.DocumentURI) (string, error) { return path, nil } +// toDocumentURI converts a filesystem path to a DocumentURI func (s *TestServer) toDocumentURI(path string) protocol.DocumentURI { return protocol.DocumentURI("file://" + path) } +// publishDiagnostics creates and sends a publishDiagnostics notification func (s *TestServer) publishDiagnostics(uri protocol.DocumentURI, diagnostics []protocol.Diagnostic) error { params := &protocol.PublishDiagnosticsParams{ URI: uri, @@ -109,6 +119,7 @@ func (s *TestServer) publishDiagnostics(uri protocol.DocumentURI, diagnostics [] return s.replier.ReplyMessage(notification) } +// didOpen implements the textDocument/didOpen handler for testing func (s *TestServer) didOpen(params *protocol.DidOpenTextDocumentParams) error { path, err := s.fromDocumentURI(params.TextDocument.URI) if err != nil { @@ -122,6 +133,7 @@ func (s *TestServer) didOpen(params *protocol.DidOpenTextDocumentParams) error { }}) } +// didChange implements the textDocument/didChange handler for testing func (s *TestServer) didChange(params *protocol.DidChangeTextDocumentParams) error { path, err := s.fromDocumentURI(params.TextDocument.URI) if err != nil { @@ -137,6 +149,7 @@ func (s *TestServer) didChange(params *protocol.DidChangeTextDocumentParams) err return s.didModifyFile(changes) } +// didSave implements the textDocument/didSave handler for testing func (s *TestServer) didSave(params *protocol.DidSaveTextDocumentParams) error { if params.Text == nil { return nil @@ -154,14 +167,16 @@ func (s *TestServer) didSave(params *protocol.DidSaveTextDocumentParams) error { }}) } +// didClose implements the textDocument/didClose handler for testing func (s *TestServer) didClose(params *protocol.DidCloseTextDocumentParams) error { return s.publishDiagnostics(params.TextDocument.URI, nil) } +// didModifyFile performs file modifications and synchronously handles diagnostics for testing func (s *TestServer) didModifyFile(changes []gop.FileChange) error { s.proj.ModifyFiles(changes) - // 同步处理诊断，简化测试 + // Process diagnostics synchronously to simplify testing for _, change := range changes { uri := s.toDocumentURI(change.Path) diagnostics, _ := s.getDiagnostics(change.Path) @@ -171,10 +186,11 @@ func (s *TestServer) didModifyFile(changes []gop.FileChange) error { return nil } +// getDiagnostics generates diagnostics for testing func (s *TestServer) getDiagnostics(path string) ([]protocol.Diagnostic, error) { var diagnostics []protocol.Diagnostic - // 检查 AST 错误 + // Check for AST errors if s.proj.astError != nil { return []protocol.Diagnostic{{ Range: protocol.Range{ @@ -187,7 +203,7 @@ func (s *TestServer) getDiagnostics(path string) ([]protocol.Diagnostic, error) }}, nil } - // 检查类型错误 + // Check for type errors if s.proj.typeError != nil { diagnostics = append(diagnostics, protocol.Diagnostic{ Range: protocol.Range{ @@ -203,8 +219,9 @@ func (s *TestServer) getDiagnostics(path string) ([]protocol.Diagnostic, error) return diagnostics, nil } -// 以下是测试函数 +// Test functions below +// TestDidOpen tests the didOpen handler functionality func TestDidOpen(t *testing.T) { tests := []struct { name string @@ -243,7 +260,7 @@ func TestDidOpen(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // 准备测试环境 + // Setup test environment mockProj := &MockProject{ files: make(map[string]gop.File), } @@ -255,17 +272,17 @@ func TestDidOpen(t *testing.T) { convertError: tt.convertError, } - // 执行测试 + // Execute test err := server.didOpen(tt.params) - // 验证结果 + // Verify results if (err != nil) != tt.wantErr { t.Errorf("didOpen() error = %v, wantErr %v", err, tt.wantErr) return } if !tt.wantErr { - // 验证文件是否被正确更新 + // Verify file was correctly updated if len(mockProj.updatedPaths) == 0 { t.Errorf("No files were updated") return @@ -285,7 +302,7 @@ func TestDidOpen(t *testing.T) { t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) } - // 验证是否发送了诊断通知 + // Verify diagnostic notification was sent if len(mockReplier.notifications) == 0 { t.Errorf("No diagnostics notifications were sent") } else if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { @@ -296,6 +313,7 @@ func TestDidOpen(t *testing.T) { } } +// TestDidChange tests the didChange handler functionality func TestDidChange(t *testing.T) { tests := []struct { name string @@ -342,7 +360,7 @@ func TestDidChange(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // 准备测试环境 + // Setup test environment mockProj := &MockProject{ files: make(map[string]gop.File), } @@ -354,17 +372,17 @@ func TestDidChange(t *testing.T) { convertError: tt.convertError, } - // 执行测试 + // Execute test err := server.didChange(tt.params) - // 验证结果 + // Verify results if (err != nil) != tt.wantErr { t.Errorf("didChange() error = %v, wantErr %v", err, tt.wantErr) return } if !tt.wantErr { - // 验证文件是否被正确更新 + // Verify file was correctly updated if len(mockProj.updatedPaths) == 0 { t.Errorf("No files were updated") return @@ -384,7 +402,7 @@ func TestDidChange(t *testing.T) { t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) } - // 验证是否发送了诊断通知 + // Verify diagnostic notification was sent if len(mockReplier.notifications) == 0 { t.Errorf("No diagnostics notifications were sent") } else if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { @@ -395,6 +413,7 @@ func TestDidChange(t *testing.T) { } } +// TestDidSave tests the didSave handler functionality func TestDidSave(t *testing.T) { content := "package main\n\nfunc main() {}" tests := []struct { @@ -445,7 +464,7 @@ func TestDidSave(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // 准备测试环境 + // Setup test environment mockProj := &MockProject{ files: make(map[string]gop.File), } @@ -457,17 +476,17 @@ func TestDidSave(t *testing.T) { convertError: tt.convertError, } - // 执行测试 + // Execute test err := server.didSave(tt.params) - // 验证结果 + // Verify results if (err != nil) != tt.wantErr { t.Errorf("didSave() error = %v, wantErr %v", err, tt.wantErr) return } if !tt.wantErr && tt.wantUpdate { - // 验证文件是否被正确更新 + // Verify file was correctly updated if len(mockProj.updatedPaths) == 0 { t.Errorf("No files were updated") return @@ -487,7 +506,7 @@ func TestDidSave(t *testing.T) { t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) } - // 验证是否发送了诊断通知 + // Verify diagnostic notification was sent if len(mockReplier.notifications) == 0 { t.Errorf("No diagnostics notifications were sent") } @@ -502,6 +521,7 @@ func TestDidSave(t *testing.T) { } } +// TestDidClose tests the didClose handler functionality func TestDidClose(t *testing.T) { tests := []struct { name string @@ -521,23 +541,23 @@ func TestDidClose(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // 准备测试环境 + // Setup test environment mockReplier := &MockReplier{} server := &TestServer{ replier: mockReplier, } - // 执行测试 + // Execute test err := server.didClose(tt.params) - // 验证结果 + // Verify results if (err != nil) != tt.wantErr { t.Errorf("didClose() error = %v, wantErr %v", err, tt.wantErr) return } - // 验证是否发送了清空诊断的通知 + // Verify empty diagnostics notification was sent if len(mockReplier.notifications) == 0 { t.Errorf("No diagnostics notifications were sent") return @@ -546,7 +566,8 @@ func TestDidClose(t *testing.T) { if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { t.Errorf("Wrong notification method: %s", mockReplier.notifications[0].Method()) } - // 检查诊断是否为空 + + // Verify diagnostics are empty var params protocol.PublishDiagnosticsParams if err := json.Unmarshal(mockReplier.notifications[0].Params(), ¶ms); err != nil { t.Errorf("Failed to unmarshal notification params: %v", err) @@ -560,6 +581,7 @@ func TestDidClose(t *testing.T) { } } +// TestGetDiagnostics tests the getDiagnostics functionality func TestGetDiagnostics(t *testing.T) { tests := []struct { name string @@ -596,7 +618,7 @@ func TestGetDiagnostics(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // 准备测试环境 + // Setup test environment mockProj := &MockProject{ files: make(map[string]gop.File), astError: tt.astError, @@ -607,10 +629,10 @@ func TestGetDiagnostics(t *testing.T) { proj: mockProj, } - // 执行测试 + // Execute test diagnostics, err := server.getDiagnostics(tt.path) - // 验证结果 + // Verify results if err != nil { t.Errorf("getDiagnostics() error = %v", err) return @@ -633,6 +655,7 @@ func TestGetDiagnostics(t *testing.T) { } } +// TestDidModifyFile tests the didModifyFile functionality func TestDidModifyFile(t *testing.T) { tests := []struct { name string @@ -684,7 +707,7 @@ func TestDidModifyFile(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - // 准备测试环境 + // Setup test environment mockProj := &MockProject{ files: make(map[string]gop.File), astError: tt.astError, @@ -697,16 +720,16 @@ func TestDidModifyFile(t *testing.T) { replier: mockReplier, } - // 执行测试 + // Execute test err := server.didModifyFile(tt.changes) - // 验证结果 + // Verify results if err != nil { t.Errorf("didModifyFile() error = %v", err) return } - // 验证文件是否被更新 + // Verify files were updated if len(mockProj.updatedPaths) != len(tt.changes) { t.Errorf("Updated %d files, want %d", len(mockProj.updatedPaths), len(tt.changes)) } @@ -728,7 +751,7 @@ func TestDidModifyFile(t *testing.T) { } } - // 验证是否发送了诊断通知 + // Verify diagnostic notifications were sent if len(mockReplier.notifications) != len(tt.changes) { t.Errorf("Sent %d notifications, want %d", len(mockReplier.notifications), len(tt.changes)) } @@ -738,7 +761,7 @@ func TestDidModifyFile(t *testing.T) { t.Errorf("Wrong notification method: %s", notification.Method()) } - // 检查诊断内容 + // Check diagnostic content var params protocol.PublishDiagnosticsParams if err := json.Unmarshal(notification.Params(), ¶ms); err != nil { t.Errorf("Failed to unmarshal notification params: %v", err) From 620e0f7c0fd0693cddfc32085c1a4f9112702325 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Wed, 12 Mar 2025 22:28:17 +0800 Subject: [PATCH 04/15] chore: remove unused imports and clean up code structure --- internal/server/text_syncronization_test.go | 778 -------------------- 1 file changed, 778 deletions(-) delete mode 100644 internal/server/text_syncronization_test.go diff --git a/internal/server/text_syncronization_test.go b/internal/server/text_syncronization_test.go deleted file mode 100644 index 6fde4f9..0000000 --- a/internal/server/text_syncronization_test.go +++ /dev/null @@ -1,778 +0,0 @@ -package server - -import ( - "encoding/json" - "errors" - "go/types" - "testing" - - "github.com/goplus/gop/ast" - "github.com/goplus/gop/x/typesutil" - "github.com/goplus/goxlsw/gop" - "github.com/goplus/goxlsw/jsonrpc2" - "github.com/goplus/goxlsw/protocol" -) - -// MockProject implements a mock Project interface for testing -type MockProject struct { - files map[string]gop.File - astError error - typeError error - updatedPaths []string -} - -// AST returns a mock AST file or an error if astError is set -func (m *MockProject) AST(path string) (*ast.File, error) { - if m.astError != nil { - return nil, m.astError - } - // Create a minimal ast.File instance - return &ast.File{ - Name: &ast.Ident{Name: "main"}, - }, nil -} - -// TypeInfo returns mock type information or an error if typeError is set -func (m *MockProject) TypeInfo() (*types.Package, *typesutil.Info, error, error) { - if m.typeError != nil { - return nil, nil, m.typeError, nil - } - - // Create minimal type information instances - pkg := types.NewPackage("main", "main") - info := &typesutil.Info{ - Types: make(map[ast.Expr]types.TypeAndValue), - Defs: make(map[*ast.Ident]types.Object), - Uses: make(map[*ast.Ident]types.Object), - } - - return pkg, info, nil, nil -} - -// ModifyFiles tracks file modifications for testing -func (m *MockProject) ModifyFiles(changes []gop.FileChange) { - for _, change := range changes { - m.files[change.Path] = &gop.FileImpl{ - Content: change.Content, - } - m.updatedPaths = append(m.updatedPaths, change.Path) - } -} - -// File returns a file by path if it exists in the mock project -func (m *MockProject) File(path string) (gop.File, bool) { - file, ok := m.files[path] - return file, ok -} - -// MockReplier implements a message replier for testing -type MockReplier struct { - notifications []*jsonrpc2.Notification -} - -// ReplyMessage records notifications for later verification -func (m *MockReplier) ReplyMessage(msg jsonrpc2.Message) error { - if n, ok := msg.(*jsonrpc2.Notification); ok { - m.notifications = append(m.notifications, n) - } - return nil -} - -// TestServer implements a server for testing -type TestServer struct { - proj *MockProject - replier *MockReplier - convertError error -} - -// getProj returns the mock project -func (s *TestServer) getProj() *MockProject { - return s.proj -} - -// fromDocumentURI converts a URI to a filesystem path or returns an error -// if convertError is set -func (s *TestServer) fromDocumentURI(uri protocol.DocumentURI) (string, error) { - if s.convertError != nil { - return "", s.convertError - } - // Simply convert URI to path by removing file:// prefix - path := string(uri) - if len(path) > 7 && path[:7] == "file://" { - path = path[7:] - } - return path, nil -} - -// toDocumentURI converts a filesystem path to a DocumentURI -func (s *TestServer) toDocumentURI(path string) protocol.DocumentURI { - return protocol.DocumentURI("file://" + path) -} - -// publishDiagnostics creates and sends a publishDiagnostics notification -func (s *TestServer) publishDiagnostics(uri protocol.DocumentURI, diagnostics []protocol.Diagnostic) error { - params := &protocol.PublishDiagnosticsParams{ - URI: uri, - Diagnostics: diagnostics, - } - notification, _ := jsonrpc2.NewNotification("textDocument/publishDiagnostics", params) - return s.replier.ReplyMessage(notification) -} - -// didOpen implements the textDocument/didOpen handler for testing -func (s *TestServer) didOpen(params *protocol.DidOpenTextDocumentParams) error { - path, err := s.fromDocumentURI(params.TextDocument.URI) - if err != nil { - return err - } - - return s.didModifyFile([]gop.FileChange{{ - Path: path, - Content: []byte(params.TextDocument.Text), - Version: int(params.TextDocument.Version), - }}) -} - -// didChange implements the textDocument/didChange handler for testing -func (s *TestServer) didChange(params *protocol.DidChangeTextDocumentParams) error { - path, err := s.fromDocumentURI(params.TextDocument.URI) - if err != nil { - return err - } - - changes := []gop.FileChange{{ - Path: path, - Content: []byte(params.ContentChanges[len(params.ContentChanges)-1].Text), - Version: int(params.TextDocument.Version), - }} - - return s.didModifyFile(changes) -} - -// didSave implements the textDocument/didSave handler for testing -func (s *TestServer) didSave(params *protocol.DidSaveTextDocumentParams) error { - if params.Text == nil { - return nil - } - - path, err := s.fromDocumentURI(params.TextDocument.URI) - if err != nil { - return err - } - - return s.didModifyFile([]gop.FileChange{{ - Path: path, - Content: []byte(*params.Text), - Version: 0, - }}) -} - -// didClose implements the textDocument/didClose handler for testing -func (s *TestServer) didClose(params *protocol.DidCloseTextDocumentParams) error { - return s.publishDiagnostics(params.TextDocument.URI, nil) -} - -// didModifyFile performs file modifications and synchronously handles diagnostics for testing -func (s *TestServer) didModifyFile(changes []gop.FileChange) error { - s.proj.ModifyFiles(changes) - - // Process diagnostics synchronously to simplify testing - for _, change := range changes { - uri := s.toDocumentURI(change.Path) - diagnostics, _ := s.getDiagnostics(change.Path) - s.publishDiagnostics(uri, diagnostics) - } - - return nil -} - -// getDiagnostics generates diagnostics for testing -func (s *TestServer) getDiagnostics(path string) ([]protocol.Diagnostic, error) { - var diagnostics []protocol.Diagnostic - - // Check for AST errors - if s.proj.astError != nil { - return []protocol.Diagnostic{{ - Range: protocol.Range{ - Start: protocol.Position{Line: 0, Character: 0}, - End: protocol.Position{Line: 0, Character: 0}, - }, - Severity: protocol.SeverityError, - Source: "goxlsw", - Message: s.proj.astError.Error(), - }}, nil - } - - // Check for type errors - if s.proj.typeError != nil { - diagnostics = append(diagnostics, protocol.Diagnostic{ - Range: protocol.Range{ - Start: protocol.Position{Line: 0, Character: 0}, - End: protocol.Position{Line: 0, Character: 0}, - }, - Severity: protocol.SeverityError, - Source: "goxlsw", - Message: s.proj.typeError.Error(), - }) - } - - return diagnostics, nil -} - -// Test functions below - -// TestDidOpen tests the didOpen handler functionality -func TestDidOpen(t *testing.T) { - tests := []struct { - name string - params *protocol.DidOpenTextDocumentParams - convertError error - expectedPath string - expectedContent string - wantErr bool - }{ - { - name: "basic open", - params: &protocol.DidOpenTextDocumentParams{ - TextDocument: protocol.TextDocumentItem{ - URI: "file:///test.gop", - Version: 1, - Text: "package main", - }, - }, - expectedPath: "/test.gop", - expectedContent: "package main", - wantErr: false, - }, - { - name: "URI conversion error", - params: &protocol.DidOpenTextDocumentParams{ - TextDocument: protocol.TextDocumentItem{ - URI: "file:///invalid.gop", - Version: 1, - Text: "package main", - }, - }, - convertError: errors.New("URI conversion failed"), - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup test environment - mockProj := &MockProject{ - files: make(map[string]gop.File), - } - mockReplier := &MockReplier{} - - server := &TestServer{ - proj: mockProj, - replier: mockReplier, - convertError: tt.convertError, - } - - // Execute test - err := server.didOpen(tt.params) - - // Verify results - if (err != nil) != tt.wantErr { - t.Errorf("didOpen() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if !tt.wantErr { - // Verify file was correctly updated - if len(mockProj.updatedPaths) == 0 { - t.Errorf("No files were updated") - return - } - - if mockProj.updatedPaths[0] != tt.expectedPath { - t.Errorf("Updated wrong path: got %s, want %s", mockProj.updatedPaths[0], tt.expectedPath) - } - - file, ok := mockProj.files[tt.expectedPath] - if !ok { - t.Errorf("File not found in project: %s", tt.expectedPath) - return - } - - if string(file.Content) != tt.expectedContent { - t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) - } - - // Verify diagnostic notification was sent - if len(mockReplier.notifications) == 0 { - t.Errorf("No diagnostics notifications were sent") - } else if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { - t.Errorf("Wrong notification method: %s", mockReplier.notifications[0].Method()) - } - } - }) - } -} - -// TestDidChange tests the didChange handler functionality -func TestDidChange(t *testing.T) { - tests := []struct { - name string - params *protocol.DidChangeTextDocumentParams - convertError error - expectedPath string - expectedContent string - wantErr bool - }{ - { - name: "basic change", - params: &protocol.DidChangeTextDocumentParams{ - TextDocument: protocol.VersionedTextDocumentIdentifier{ - TextDocumentIdentifier: protocol.TextDocumentIdentifier{ - URI: "file:///test.gop", - }, - Version: 2, - }, - ContentChanges: []protocol.TextDocumentContentChangeEvent{ - {Text: "package main\n\nfunc main() {}"}, - }, - }, - expectedPath: "/test.gop", - expectedContent: "package main\n\nfunc main() {}", - wantErr: false, - }, - { - name: "URI conversion error", - params: &protocol.DidChangeTextDocumentParams{ - TextDocument: protocol.VersionedTextDocumentIdentifier{ - TextDocumentIdentifier: protocol.TextDocumentIdentifier{ - URI: "file:///invalid.gop", - }, - Version: 2, - }, - ContentChanges: []protocol.TextDocumentContentChangeEvent{ - {Text: "package main"}, - }, - }, - convertError: errors.New("URI conversion failed"), - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup test environment - mockProj := &MockProject{ - files: make(map[string]gop.File), - } - mockReplier := &MockReplier{} - - server := &TestServer{ - proj: mockProj, - replier: mockReplier, - convertError: tt.convertError, - } - - // Execute test - err := server.didChange(tt.params) - - // Verify results - if (err != nil) != tt.wantErr { - t.Errorf("didChange() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if !tt.wantErr { - // Verify file was correctly updated - if len(mockProj.updatedPaths) == 0 { - t.Errorf("No files were updated") - return - } - - if mockProj.updatedPaths[0] != tt.expectedPath { - t.Errorf("Updated wrong path: got %s, want %s", mockProj.updatedPaths[0], tt.expectedPath) - } - - file, ok := mockProj.files[tt.expectedPath] - if !ok { - t.Errorf("File not found in project: %s", tt.expectedPath) - return - } - - if string(file.Content) != tt.expectedContent { - t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) - } - - // Verify diagnostic notification was sent - if len(mockReplier.notifications) == 0 { - t.Errorf("No diagnostics notifications were sent") - } else if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { - t.Errorf("Wrong notification method: %s", mockReplier.notifications[0].Method()) - } - } - }) - } -} - -// TestDidSave tests the didSave handler functionality -func TestDidSave(t *testing.T) { - content := "package main\n\nfunc main() {}" - tests := []struct { - name string - params *protocol.DidSaveTextDocumentParams - convertError error - expectedPath string - expectedContent string - wantUpdate bool - wantErr bool - }{ - { - name: "save with content", - params: &protocol.DidSaveTextDocumentParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: "file:///test.gop", - }, - Text: &content, - }, - expectedPath: "/test.gop", - expectedContent: content, - wantUpdate: true, - wantErr: false, - }, - { - name: "save without content", - params: &protocol.DidSaveTextDocumentParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: "file:///test.gop", - }, - }, - wantUpdate: false, - wantErr: false, - }, - { - name: "URI conversion error", - params: &protocol.DidSaveTextDocumentParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: "file:///invalid.gop", - }, - Text: &content, - }, - convertError: errors.New("URI conversion failed"), - wantUpdate: false, - wantErr: true, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup test environment - mockProj := &MockProject{ - files: make(map[string]gop.File), - } - mockReplier := &MockReplier{} - - server := &TestServer{ - proj: mockProj, - replier: mockReplier, - convertError: tt.convertError, - } - - // Execute test - err := server.didSave(tt.params) - - // Verify results - if (err != nil) != tt.wantErr { - t.Errorf("didSave() error = %v, wantErr %v", err, tt.wantErr) - return - } - - if !tt.wantErr && tt.wantUpdate { - // Verify file was correctly updated - if len(mockProj.updatedPaths) == 0 { - t.Errorf("No files were updated") - return - } - - if mockProj.updatedPaths[0] != tt.expectedPath { - t.Errorf("Updated wrong path: got %s, want %s", mockProj.updatedPaths[0], tt.expectedPath) - } - - file, ok := mockProj.files[tt.expectedPath] - if !ok { - t.Errorf("File not found in project: %s", tt.expectedPath) - return - } - - if string(file.Content) != tt.expectedContent { - t.Errorf("File content = %q, want %q", string(file.Content), tt.expectedContent) - } - - // Verify diagnostic notification was sent - if len(mockReplier.notifications) == 0 { - t.Errorf("No diagnostics notifications were sent") - } - } - - if !tt.wantErr && !tt.wantUpdate { - if len(mockProj.updatedPaths) > 0 { - t.Errorf("File was updated but shouldn't be") - } - } - }) - } -} - -// TestDidClose tests the didClose handler functionality -func TestDidClose(t *testing.T) { - tests := []struct { - name string - params *protocol.DidCloseTextDocumentParams - wantErr bool - }{ - { - name: "basic close", - params: &protocol.DidCloseTextDocumentParams{ - TextDocument: protocol.TextDocumentIdentifier{ - URI: "file:///test.gop", - }, - }, - wantErr: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup test environment - mockReplier := &MockReplier{} - - server := &TestServer{ - replier: mockReplier, - } - - // Execute test - err := server.didClose(tt.params) - - // Verify results - if (err != nil) != tt.wantErr { - t.Errorf("didClose() error = %v, wantErr %v", err, tt.wantErr) - return - } - - // Verify empty diagnostics notification was sent - if len(mockReplier.notifications) == 0 { - t.Errorf("No diagnostics notifications were sent") - return - } - - if mockReplier.notifications[0].Method() != "textDocument/publishDiagnostics" { - t.Errorf("Wrong notification method: %s", mockReplier.notifications[0].Method()) - } - - // Verify diagnostics are empty - var params protocol.PublishDiagnosticsParams - if err := json.Unmarshal(mockReplier.notifications[0].Params(), ¶ms); err != nil { - t.Errorf("Failed to unmarshal notification params: %v", err) - return - } - - if len(params.Diagnostics) != 0 { - t.Errorf("Diagnostics not cleared, got %d items", len(params.Diagnostics)) - } - }) - } -} - -// TestGetDiagnostics tests the getDiagnostics functionality -func TestGetDiagnostics(t *testing.T) { - tests := []struct { - name string - path string - astError error - typeError error - wantCount int - }{ - { - name: "no errors", - path: "/test.gop", - wantCount: 0, - }, - { - name: "AST error", - path: "/test.gop", - astError: errors.New("syntax error"), - wantCount: 1, - }, - { - name: "Type error", - path: "/test.gop", - typeError: errors.New("type error"), - wantCount: 1, - }, - { - name: "Both errors", - path: "/test.gop", - astError: errors.New("syntax error"), - typeError: errors.New("type error"), - wantCount: 1, // AST errors take precedence - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup test environment - mockProj := &MockProject{ - files: make(map[string]gop.File), - astError: tt.astError, - typeError: tt.typeError, - } - - server := &TestServer{ - proj: mockProj, - } - - // Execute test - diagnostics, err := server.getDiagnostics(tt.path) - - // Verify results - if err != nil { - t.Errorf("getDiagnostics() error = %v", err) - return - } - - if len(diagnostics) != tt.wantCount { - t.Errorf("getDiagnostics() returned %d diagnostics, want %d", len(diagnostics), tt.wantCount) - } - - if tt.astError != nil && len(diagnostics) > 0 { - if diagnostics[0].Message != tt.astError.Error() { - t.Errorf("Diagnostic message = %q, want %q", diagnostics[0].Message, tt.astError.Error()) - } - } else if tt.typeError != nil && len(diagnostics) > 0 { - if diagnostics[0].Message != tt.typeError.Error() { - t.Errorf("Diagnostic message = %q, want %q", diagnostics[0].Message, tt.typeError.Error()) - } - } - }) - } -} - -// TestDidModifyFile tests the didModifyFile functionality -func TestDidModifyFile(t *testing.T) { - tests := []struct { - name string - changes []gop.FileChange - astError error - typeError error - wantDiags bool - }{ - { - name: "single file no errors", - changes: []gop.FileChange{ - { - Path: "/test.gop", - Content: []byte("package main"), - Version: 1, - }, - }, - wantDiags: false, - }, - { - name: "single file with AST error", - changes: []gop.FileChange{ - { - Path: "/test.gop", - Content: []byte("package main"), - Version: 1, - }, - }, - astError: errors.New("syntax error"), - wantDiags: true, - }, - { - name: "multiple files", - changes: []gop.FileChange{ - { - Path: "/test1.gop", - Content: []byte("package main"), - Version: 1, - }, - { - Path: "/test2.gop", - Content: []byte("package main"), - Version: 1, - }, - }, - wantDiags: false, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Setup test environment - mockProj := &MockProject{ - files: make(map[string]gop.File), - astError: tt.astError, - typeError: tt.typeError, - } - mockReplier := &MockReplier{} - - server := &TestServer{ - proj: mockProj, - replier: mockReplier, - } - - // Execute test - err := server.didModifyFile(tt.changes) - - // Verify results - if err != nil { - t.Errorf("didModifyFile() error = %v", err) - return - } - - // Verify files were updated - if len(mockProj.updatedPaths) != len(tt.changes) { - t.Errorf("Updated %d files, want %d", len(mockProj.updatedPaths), len(tt.changes)) - } - - for i, change := range tt.changes { - if i < len(mockProj.updatedPaths) && mockProj.updatedPaths[i] != change.Path { - t.Errorf("Updated wrong path at index %d: got %s, want %s", - i, mockProj.updatedPaths[i], change.Path) - } - - file, ok := mockProj.files[change.Path] - if !ok { - t.Errorf("File not found in project: %s", change.Path) - continue - } - - if string(file.Content) != string(change.Content) { - t.Errorf("File content = %q, want %q", string(file.Content), string(change.Content)) - } - } - - // Verify diagnostic notifications were sent - if len(mockReplier.notifications) != len(tt.changes) { - t.Errorf("Sent %d notifications, want %d", len(mockReplier.notifications), len(tt.changes)) - } - - for _, notification := range mockReplier.notifications { - if notification.Method() != "textDocument/publishDiagnostics" { - t.Errorf("Wrong notification method: %s", notification.Method()) - } - - // Check diagnostic content - var params protocol.PublishDiagnosticsParams - if err := json.Unmarshal(notification.Params(), ¶ms); err != nil { - t.Errorf("Failed to unmarshal notification params: %v", err) - continue - } - - hasDiags := len(params.Diagnostics) > 0 - if hasDiags != tt.wantDiags { - t.Errorf("Diagnostics present = %v, want %v", hasDiags, tt.wantDiags) - } - } - }) - } -} From 16521dd688504d7108b1f272864543cbfd3b5fe5 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Thu, 13 Mar 2025 21:27:03 +0800 Subject: [PATCH 05/15] feat(server): implement incremental text document updates and enhance diagnostics handling --- internal/server/text_syncronization.go | 251 +++++- internal/server/text_syncronization_test.go | 853 ++++++++++++++++++++ 2 files changed, 1079 insertions(+), 25 deletions(-) create mode 100644 internal/server/text_syncronization_test.go diff --git a/internal/server/text_syncronization.go b/internal/server/text_syncronization.go index 49871e1..43f9a7d 100644 --- a/internal/server/text_syncronization.go +++ b/internal/server/text_syncronization.go @@ -1,7 +1,19 @@ package server import ( + "bytes" + "fmt" + "go/types" + "time" + + "github.com/goplus/gogen" + gopast "github.com/goplus/gop/ast" + gopscanner "github.com/goplus/gop/scanner" + goptoken "github.com/goplus/gop/token" "github.com/goplus/goxlsw/gop" + "github.com/goplus/goxlsw/jsonrpc2" + "github.com/goplus/goxlsw/protocol" + "github.com/qiniu/x/errors" ) // didOpen handles the textDocument/didOpen notification from the LSP client. @@ -31,12 +43,15 @@ func (s *Server) didChange(params *DidChangeTextDocumentParams) error { return err } - // Convert all changes to FileChange - // Note: We currently take only the final state of the document - // rather than applying incremental changes + content, err := s.changedText(path, params.ContentChanges) + if err != nil { + return err + } + + // Create a file change record changes := []gop.FileChange{{ Path: path, - Content: []byte(params.ContentChanges[len(params.ContentChanges)-1].Text), + Content: content, Version: int(params.TextDocument.Version), }} @@ -58,7 +73,7 @@ func (s *Server) didSave(params *DidSaveTextDocumentParams) error { return s.didModifyFile([]gop.FileChange{{ Path: path, Content: []byte(*params.Text), - Version: 0, // Save notifications don't include versions + Version: int(time.Now().UnixMilli()), }}) } return nil @@ -107,6 +122,125 @@ func (s *Server) didModifyFile(changes []gop.FileChange) error { return nil } +// changedText processes document content changes from the client. +// It supports two modes of operation: +// 1. Full replacement: Replace the entire document content (when only one change with no range is provided) +// 2. Incremental updates: Apply specific changes to portions of the document +// +// Returns the updated document content or an error if the changes couldn't be applied. +func (s *Server) changedText(uri string, changes []protocol.TextDocumentContentChangeEvent) ([]byte, error) { + if len(changes) == 0 { + return nil, fmt.Errorf("%w: no content changes provided", jsonrpc2.ErrInternal) + } + + // Check if the client sent the full content of the file. + // We accept a full content change even if the server expected incremental changes. + if len(changes) == 1 && changes[0].Range == nil && changes[0].RangeLength == 0 { + // Full replacement mode + return []byte(changes[0].Text), nil + } + + // Incremental update mode + return s.applyIncrementalChanges(uri, changes) +} + +// applyIncrementalChanges applies a sequence of changes to the document content. +// For each change, it: +// 1. Computes the byte offsets for the specified range +// 2. Verifies the range is valid +// 3. Replaces the specified range with the new text +// +// Returns the updated document content or an error if the changes couldn't be applied. +func (s *Server) applyIncrementalChanges(path string, changes []protocol.TextDocumentContentChangeEvent) ([]byte, error) { + // Get current file content + file, ok := s.getProj().File(path) + if !ok { + return nil, fmt.Errorf("%w: file not found", jsonrpc2.ErrInternal) + } + + content := file.Content + + // Apply each change sequentially + for _, change := range changes { + // Ensure the change includes range information + if change.Range == nil { + return nil, fmt.Errorf("%w: unexpected nil range for change", jsonrpc2.ErrInternal) + } + + // Convert LSP positions to byte offsets + start := s.PositionOffset(content, change.Range.Start) + end := s.PositionOffset(content, change.Range.End) + + // Validate range + if end < start { + return nil, fmt.Errorf("%w: invalid range for content change", jsonrpc2.ErrInternal) + } + + // Apply the change + var buf bytes.Buffer + buf.Write(content[:start]) + buf.WriteString(change.Text) + buf.Write(content[end:]) + content = buf.Bytes() + } + + return content, nil +} + +// PositionOffset converts an LSP position (line, character) to a byte offset in the document. +// It calculates the offset by: +// 1. Finding the starting byte offset of the requested line +// 2. Adding the character offset within that line, converting from UTF-16 to UTF-8 if needed +// +// Parameters: +// - content: The file content as a byte array +// - position: The LSP position with line and character numbers (0-based) +// +// Returns the byte offset from the beginning of the document +func (s *Server) PositionOffset(content []byte, position Position) int { + // If content is empty or position is beyond the content, return 0 + if len(content) == 0 { + return 0 + } + + // Find all line start positions in the document + lineStarts := []int{0} // First line always starts at position 0 + for i := 0; i < len(content); i++ { + if content[i] == '\n' { + lineStarts = append(lineStarts, i+1) // Next line starts after the newline + } + } + + // Ensure the requested line is within range + lineIndex := int(position.Line) + if lineIndex >= len(lineStarts) { + // If line is beyond available lines, return the end of content + return len(content) + } + + // Get the starting offset of the requested line + lineOffset := lineStarts[lineIndex] + + // Extract the content of the requested line + lineEndOffset := len(content) + if lineIndex+1 < len(lineStarts) { + lineEndOffset = lineStarts[lineIndex+1] - 1 // -1 to exclude the newline character + } + + // Ensure we don't go beyond the end of content + if lineOffset >= len(content) { + return len(content) + } + + lineContent := content[lineOffset:min(lineEndOffset, len(content))] + + // Convert UTF-16 character offset to UTF-8 byte offset + utf8Offset := utf16OffsetToUTF8(string(lineContent), int(position.Character)) + + // Ensure the final offset doesn't exceed the content length + return lineOffset + utf8Offset +} + // getDiagnostics generates diagnostic information for a specific file. // It performs two checks: // 1. AST parsing - reports syntax errors @@ -122,18 +256,49 @@ func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { // 1. Get AST diagnostics // Parse the file and check for syntax errors - _, err := proj.AST(path) + astFile, err := proj.AST(path) if err != nil { - // Convert syntax errors to diagnostics with position at the start of file - return []Diagnostic{{ - Range: Range{ - Start: Position{Line: 0, Character: 0}, - End: Position{Line: 0, Character: 0}, - }, - Severity: SeverityError, - Source: "goxlsw", - Message: err.Error(), - }}, nil + var ( + errorList gopscanner.ErrorList + codeError *gogen.CodeError + ) + if errors.As(err, &errorList) { + // Handle parse errors. + for _, e := range errorList { + diagnostics = append(diagnostics, Diagnostic{ + Severity: SeverityError, + Range: s.rangeForASTFilePosition(astFile, e.Pos), + Message: e.Msg, + }) + } + } else if errors.As(err, &codeError) { + // Handle code generation errors. + diagnostics = append(diagnostics, Diagnostic{ + Severity: SeverityError, + Range: s.rangeForPos(codeError.Pos), + Message: codeError.Error(), + }) + } else { + // Handle unknown errors (including recovered panics). + diagnostics = append(diagnostics, Diagnostic{ + Severity: SeverityError, + Message: fmt.Sprintf("failed to parse spx file: %v", err), + }) + } + } + + if astFile == nil { + return diagnostics, nil + } + + handleErr := func(err error) { + if typeErr, ok := err.(types.Error); ok { + diagnostics = append(diagnostics, Diagnostic{ + Severity: SeverityError, + Range: s.rangeForPos(typeErr.Pos), + Message: typeErr.Msg, + }) + } } // 2. Get type checking diagnostics @@ -141,16 +306,52 @@ func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { _, _, err, _ = proj.TypeInfo() if err != nil { // Add type checking errors to diagnostics - diagnostics = append(diagnostics, Diagnostic{ - Range: Range{ - Start: Position{Line: 0, Character: 0}, - End: Position{Line: 0, Character: 0}, - }, - Severity: SeverityError, - Source: "goxlsw", - Message: err.Error(), - }) + switch err := err.(type) { + case errors.List: + for _, e := range err { + handleErr(e) + } + default: + handleErr(err) + } } return diagnostics, nil } + +// fromPosition converts a token.Position to an LSP Position. +func (s *Server) fromPosition(astFile *gopast.File, position goptoken.Position) Position { + tokenFile := s.getProj().Fset.File(astFile.Pos()) + + line := position.Line + lineStart := int(tokenFile.LineStart(line)) + relLineStart := lineStart - tokenFile.Base() + lineContent := astFile.Code[relLineStart : relLineStart+position.Column-1] + utf16Offset := utf8OffsetToUTF16(string(lineContent), position.Column-1) + + return Position{ + Line: uint32(position.Line - 1), + Character: uint32(utf16Offset), + } +} + +// rangeForASTFilePosition returns a [Range] for the given position in an AST file. +func (s *Server) rangeForASTFilePosition(astFile *gopast.File, position goptoken.Position) Range { + p := s.fromPosition(astFile, position) + return Range{Start: p, End: p} +} + +// rangeForPos returns the [Range] for the given position. +func (s *Server) rangeForPos(pos goptoken.Pos) Range { + return s.rangeForASTFilePosition(s.posASTFile(pos), s.getProj().Fset.Position(pos)) +} + +// posASTFile returns the AST file for the given position. +func (s *Server) posASTFile(pos goptoken.Pos) *gopast.File { + return getASTPkg(s.getProj()).Files[s.posFilename(pos)] +} + +// posFilename returns the filename for the given position. +func (s *Server) posFilename(pos goptoken.Pos) string { + return s.getProj().Fset.Position(pos).Filename +} diff --git a/internal/server/text_syncronization_test.go b/internal/server/text_syncronization_test.go new file mode 100644 index 0000000..0d2269c --- /dev/null +++ b/internal/server/text_syncronization_test.go @@ -0,0 +1,853 @@ +package server + +import ( + "errors" + "go/token" + "testing" + "time" + + goptoken "github.com/goplus/gop/token" + "github.com/goplus/goxlsw/gop" + "github.com/goplus/goxlsw/jsonrpc2" + "github.com/goplus/goxlsw/protocol" +) + +// MockReplier implements a message replier for testing +type MockReplier struct { + notifications []*jsonrpc2.Notification +} + +// ReplyMessage records notifications for later verification +func (m *MockReplier) ReplyMessage(msg jsonrpc2.Message) error { + if n, ok := msg.(*jsonrpc2.Notification); ok { + m.notifications = append(m.notifications, n) + } + return nil +} + +func file(text string) gop.File { + return &gop.FileImpl{Content: []byte(text)} +} + +// strPtr returns a pointer to the given string +func strPtr(s string) *string { + return &s +} + +// TestDidOpen tests the didOpen handler functionality +func TestDidOpen(t *testing.T) { + tests := []struct { + name string + params *protocol.DidOpenTextDocumentParams + expectedPath string + expectedContent string + wantErr bool + }{ + { + name: "basic open", + params: &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file://workspace/echo.spx", + Version: 1, + Text: "echo \"100\""}, + }, + expectedPath: "echo.spx", + expectedContent: "echo \"100\"", + wantErr: false, + }, + { + name: "open file with function", + params: &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file://workspace/test_func.spx", + Version: 2, + Text: "onStart {\n say \"Hello, World!\"\n}", + }, + }, + expectedPath: "test_func.spx", + expectedContent: "onStart {\n say \"Hello, World!\"\n}", + wantErr: false, + }, + { + name: "open file with unicode content", + params: &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file://workspace/i18n.spx", + Version: 3, + Text: "onStart {\n say \"你好，世界!\"\n}", + }, + }, + expectedPath: "i18n.spx", + expectedContent: "onStart {\n say \"你好，世界!\"\n}", + wantErr: false, + }, + { + name: "URI conversion error", + params: &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file://error_workspace/error.spx", + Version: 1, + Text: "onStart {}", + }, + }, + wantErr: true, + }, + { + name: "empty file content", + params: &protocol.DidOpenTextDocumentParams{ + TextDocument: protocol.TextDocumentItem{ + URI: "file://workspace/empty.spx", + Version: 1, + Text: "", + }, + }, + expectedPath: "empty.spx", + expectedContent: "", + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup test environment with real Project instead of MockProject + proj := gop.NewProject(token.NewFileSet(), make(map[string]gop.File), 0) + proj.PutFile(tt.expectedPath, file("mock content")) + mockReplier := &MockReplier{} + + // Create a TestServer that extends the real Server + server := &Server{ + workspaceRootFS: proj, + replier: mockReplier, + workspaceRootURI: "file://workspace/", + } + + // Execute test + err := server.didOpen(tt.params) + + time.Sleep(1 * time.Second) + // Verify results + if (err != nil) != tt.wantErr { + t.Errorf("didOpen() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !tt.wantErr { + // Verify file was correctly added to the project + file, ok := proj.File(tt.expectedPath) + if !ok { + t.Errorf("File not found in project: %s", tt.expectedPath) + return + } + + if string(file.Content) != tt.expectedContent { + t.Errorf("File %s content = %q, want %q", tt.expectedPath, string(file.Content), tt.expectedContent) + } + + // If available, check file version + if _, ok := proj.File(tt.expectedPath); ok { + expectedVersion := int(tt.params.TextDocument.Version) + // Note: In a real test, you might need to extract the version from the FileImpl + // This depends on how version is stored in your implementation + t.Logf("File opened with version: %d", expectedVersion) + } + } + }) + } +} + +// TestDidChange tests the didChange handler functionality +func TestDidChange(t *testing.T) { + tests := []struct { + name string + initialContent string + params *protocol.DidChangeTextDocumentParams + convertError error + expectedContent string + wantErr bool + }{ + { + name: "full document replacement", + initialContent: "package main", + params: &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + Version: 2, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + {Text: "package main\n\nfunc main() {}"}, + }, + }, + expectedContent: "package main\n\nfunc main() {}", + wantErr: false, + }, + { + name: "incremental change", + initialContent: "package main\n\nfunc main() {\n\t\n}", + params: &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + Version: 2, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 1}, + End: protocol.Position{Line: 3, Character: 1}, + }, + Text: "fmt.Println(\"Hello, World!\")", + }, + }, + }, + expectedContent: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello, World!\")\n}", + wantErr: false, + }, + { + name: "multiple incremental changes", + initialContent: "package main\n\nfunc main() {\n\t\n}", + params: &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + Version: 3, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 1}, + End: protocol.Position{Line: 3, Character: 1}, + }, + Text: "fmt.Print(\"Hello", + }, + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 17}, + End: protocol.Position{Line: 3, Character: 17}, + }, + Text: ", World!\")", + }, + }, + }, + expectedContent: "package main\n\nfunc main() {\n\tfmt.Print(\"Hello, World!\")\n}", + wantErr: false, + }, + { + name: "URI conversion error", + initialContent: "package main", + params: &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + Version: 2, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{ + {Text: "package main\n\nfunc main() {}"}, + }, + }, + convertError: errors.New("URI conversion failed"), + wantErr: true, + }, + { + name: "empty changes array", + initialContent: "package main", + params: &protocol.DidChangeTextDocumentParams{ + TextDocument: protocol.VersionedTextDocumentIdentifier{ + TextDocumentIdentifier: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + Version: 2, + }, + ContentChanges: []protocol.TextDocumentContentChangeEvent{}, + }, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup test environment with initial file content + files := make(map[string]gop.File) + path := "test.gop" + + files[path] = &gop.FileImpl{ + Content: []byte(tt.initialContent), + ModTime: time.Time{}, + } + + proj := gop.NewProject(token.NewFileSet(), files, gop.FeatAll) + mockReplier := &MockReplier{} + + // Create a TestServer that extends the real Server + server := &Server{ + workspaceRootFS: proj, + replier: mockReplier, + workspaceRootURI: "file://workspace/", + } + + // Execute test + err := server.didChange(tt.params) + + // Verify results + if (err != nil) != tt.wantErr { + t.Errorf("%s didChange() error = %v, wantErr %v", tt.name, err, tt.wantErr) + return + } + + if !tt.wantErr { + // Verify file content was updated + file, ok := proj.File(path) + if !ok { + t.Errorf("%s File not found in project: %s", tt.name, path) + return + } + + if string(file.Content) != tt.expectedContent { + t.Errorf("%s File content = %q, want %q", tt.name, string(file.Content), tt.expectedContent) + } + + // If available, check file version + expectedVersion := int(tt.params.TextDocument.Version) + // Note: For a real implementation, verify the version is stored correctly + t.Logf("%s File changed with version: %d", tt.name, expectedVersion) + } + }) + } +} + +// TestDidSave tests the didSave handler functionality +func TestDidSave(t *testing.T) { + tests := []struct { + name string + initialContent string + params *protocol.DidSaveTextDocumentParams + expectedContent string + contentChanged bool + wantErr bool + }{ + { + name: "save with text", + initialContent: "package main", + params: &protocol.DidSaveTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + Text: strPtr("package main\n\nfunc main() {}"), + }, + expectedContent: "package main\n\nfunc main() {}", + contentChanged: true, + wantErr: false, + }, + { + name: "save without text", + initialContent: "package main", + params: &protocol.DidSaveTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + Text: nil, + }, + expectedContent: "package main", // Content should not change + contentChanged: false, + wantErr: false, + }, + { + name: "URI conversion error", + initialContent: "package main", + params: &protocol.DidSaveTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file://error/test.gop", + }, + Text: strPtr("package main\n\nfunc main() {}"), + }, + contentChanged: false, + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup test environment + fset := goptoken.NewFileSet() + files := make(map[string]gop.File) + path := "test.gop" + + files[path] = &gop.FileImpl{ + Content: []byte(tt.initialContent), + ModTime: time.Time{}, + } + + proj := gop.NewProject(fset, files, gop.FeatAST) + mockReplier := &MockReplier{} + + // Create a TestServer + server := &Server{ + workspaceRootFS: proj, + replier: mockReplier, + workspaceRootURI: "file://workspace/", + } + + // Execute test + err := server.didSave(tt.params) + + // Verify results + if (err != nil) != tt.wantErr { + t.Errorf("%s didSave() error = %v, wantErr %v", tt.name, err, tt.wantErr) + return + } + + if !tt.wantErr { + // Verify file content + file, ok := proj.File(path) + if !ok { + t.Errorf("%s File not found in project: %s", tt.name, path) + return + } + + if string(file.Content) != tt.expectedContent { + t.Errorf("%s File content = %q, want %q", tt.name, string(file.Content), tt.expectedContent) + } + } + }) + } +} + +// TestDidClose tests the didClose handler functionality +func TestDidClose(t *testing.T) { + tests := []struct { + name string + params *protocol.DidCloseTextDocumentParams + wantErr bool + }{ + { + name: "close document", + params: &protocol.DidCloseTextDocumentParams{ + TextDocument: protocol.TextDocumentIdentifier{ + URI: "file://workspace/test.gop", + }, + }, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup test environment + fset := goptoken.NewFileSet() + files := make(map[string]gop.File) + path := "/test.gop" + + files[path] = &gop.FileImpl{ + Content: []byte("package main"), + ModTime: time.Time{}, + } + + proj := gop.NewProject(fset, files, gop.FeatAST) + mockReplier := &MockReplier{} + + // Create a TestServer + server := &Server{ + workspaceRootFS: proj, + replier: mockReplier, + workspaceRootURI: "file://workspace/", + } + + // Execute test + err := server.didClose(tt.params) + + // Verify results + if (err != nil) != tt.wantErr { + t.Errorf("didClose() error = %v, wantErr %v", err, tt.wantErr) + return + } + }) + } +} + +// TestChangedText tests the changedText function for processing document content changes +func TestChangedText(t *testing.T) { + tests := []struct { + name string + initialContent string + changes []protocol.TextDocumentContentChangeEvent + want string + wantErr bool + }{ + { + name: "full document replacement", + initialContent: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello\")\n}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Text: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello, World!\")\n}", + }, + }, + want: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello, World!\")\n}", + wantErr: false, + }, + { + name: "incremental change - add comma", + initialContent: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello\")\n}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 19}, + End: protocol.Position{Line: 3, Character: 19}, + }, + Text: ",", + }, + }, + want: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello,\")\n}", + wantErr: false, + }, + { + name: "incremental change - replace word", + initialContent: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello\")\n}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 14}, + End: protocol.Position{Line: 3, Character: 19}, + }, + Text: "World", + }, + }, + want: "package main\n\nfunc main() {\n\tfmt.Println(\"World\")\n}", + wantErr: false, + }, + { + name: "multiple incremental changes", + initialContent: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello\")\n}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 14}, + End: protocol.Position{Line: 3, Character: 19}, + }, + Text: "World", + }, + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 19}, + End: protocol.Position{Line: 3, Character: 19}, + }, + Text: "!", + }, + }, + want: "package main\n\nfunc main() {\n\tfmt.Println(\"World!\")\n}", + wantErr: false, + }, + { + name: "empty changes array", + initialContent: "package main", + changes: []protocol.TextDocumentContentChangeEvent{}, + want: "", + wantErr: true, + }, + { + name: "invalid range - end before start", + initialContent: "package main", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 0, Character: 5}, + End: protocol.Position{Line: 0, Character: 3}, + }, + Text: "invalid", + }, + }, + want: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup test environment + fset := goptoken.NewFileSet() + files := make(map[string]gop.File) + path := "/test.gop" + + // Create initial file + files[path] = &gop.FileImpl{ + Content: []byte(tt.initialContent), + ModTime: time.Now(), + } + + proj := gop.NewProject(fset, files, gop.FeatAST) + + // For AST parsing to work, we need a real file with content + // parsed into the AST before we can apply changes + _, err := proj.AST(path) + if err != nil { + t.Fatalf("Failed to setup test: %v", err) + } + + server := &Server{ + workspaceRootFS: proj, + } + + // Execute test + got, err := server.changedText(path, tt.changes) + + // Verify results + if (err != nil) != tt.wantErr { + t.Errorf("changedText() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !tt.wantErr { + if string(got) != tt.want { + t.Errorf("%s changedText() = %q, want %q", tt.name, string(got), tt.want) + } + } + }) + } +} + +// TestApplyIncrementalChanges tests the applyIncrementalChanges function +func TestApplyIncrementalChanges(t *testing.T) { + tests := []struct { + name string + initialContent string + changes []protocol.TextDocumentContentChangeEvent + want string + wantErr bool + }{ + { + name: "add text at beginning", + initialContent: "func main() {}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 0, Character: 0}, + End: protocol.Position{Line: 0, Character: 0}, + }, + Text: "package main\n\n", + }, + }, + want: "package main\n\nfunc main() {}", + wantErr: false, + }, + { + name: "add text in middle", + initialContent: "func main() {}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 0, Character: 13}, + End: protocol.Position{Line: 0, Character: 13}, + }, + Text: "\n\tfmt.Println(\"Hello\")\n", + }, + }, + want: "func main() {\n\tfmt.Println(\"Hello\")\n}", + wantErr: false, + }, + { + name: "delete text", + initialContent: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello\")\n}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 2, Character: 0}, + End: protocol.Position{Line: 4, Character: 1}, + }, + Text: "", + }, + }, + want: "package main\n\n", + wantErr: false, + }, + { + name: "replace entire line", + initialContent: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello\")\n}", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 3, Character: 0}, + End: protocol.Position{Line: 3, Character: 21}, + }, + Text: "\tfmt.Println(\"Hello, World!\")", + }, + }, + want: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello, World!\")\n}", + wantErr: false, + }, + { + name: "nil range", + initialContent: "package main", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: nil, + Text: "new content", + }, + }, + want: "", + wantErr: true, + }, + { + name: "non-existent file", + initialContent: "", + changes: []protocol.TextDocumentContentChangeEvent{ + { + Range: &protocol.Range{ + Start: protocol.Position{Line: 0, Character: 0}, + End: protocol.Position{Line: 0, Character: 0}, + }, + Text: "package main", + }, + }, + want: "", + wantErr: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup test environment + fset := goptoken.NewFileSet() + files := make(map[string]gop.File) + path := "/test.gop" + + if tt.initialContent != "" { + files[path] = &gop.FileImpl{ + Content: []byte(tt.initialContent), + ModTime: time.Now(), + } + } + + proj := gop.NewProject(fset, files, gop.FeatAST) + + // For tests with content, ensure we have AST + if tt.initialContent != "" { + _, err := proj.AST(path) + if err != nil { + t.Fatalf("Failed to setup test: %v", err) + } + } + + server := &Server{ + workspaceRootFS: proj, + } + + // Execute test + got, err := server.applyIncrementalChanges(path, tt.changes) + + // Verify results + if (err != nil) != tt.wantErr { + t.Errorf("applyIncrementalChanges() error = %v, wantErr %v", err, tt.wantErr) + return + } + + if !tt.wantErr { + if string(got) != tt.want { + t.Errorf("%s applyIncrementalChanges() = %q, want %q", tt.name, string(got), tt.want) + } + } + }) + } +} + +// TestGetDiagnostics tests the getDiagnostics function for generating diagnostic information +func TestGetDiagnostics(t *testing.T) { + tests := []struct { + name string + content string + path string + wantDiagCount int + wantSeverities []protocol.DiagnosticSeverity + wantErr bool + }{ + { + name: "import errors", + content: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello, World!\")\n}", + path: "/test.gop", + wantDiagCount: 1, + wantSeverities: []protocol.DiagnosticSeverity{SeverityError}, + wantErr: false, + }, + { + name: "syntax error", + content: "package main\n\nfunc main() {\n\tfmt.Println(\"Hello, World!\"\n}", // Missing closing parenthesis + path: "/syntax_error.gop", + wantDiagCount: 8, + wantSeverities: []protocol.DiagnosticSeverity{SeverityError}, + wantErr: false, + }, + { + name: "type error", + content: "package main\n\nfunc main() {\n\tvar x int = \"string\"\n}", // Type mismatch + path: "/type_error.gop", + wantDiagCount: 1, + wantSeverities: []protocol.DiagnosticSeverity{SeverityError}, + wantErr: false, + }, + { + name: "no error", + content: "package main\n\nfunc main() {\n\t}", + path: "/code_error.gop", + wantDiagCount: 0, + wantSeverities: []protocol.DiagnosticSeverity{}, + wantErr: false, + }, + { + name: "multiple type errors", + content: "package main\n\nfunc main() {\n\tvar x int = \"string\"\n\tvar y bool = 42\n}", + path: "/multiple_errors.gop", + wantDiagCount: 2, + wantSeverities: []protocol.DiagnosticSeverity{SeverityError}, + wantErr: false, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Setup test environment + fset := goptoken.NewFileSet() + files := make(map[string]gop.File) + + // Create the test file + files[tt.path] = &gop.FileImpl{ + Content: []byte(tt.content), + ModTime: time.Now(), + } + + // Create a mock Project that returns our predefined errors + server := &Server{ + workspaceRootFS: gop.NewProject(fset, files, gop.FeatAll), + } + + // Execute test + diagnostics, err := server.getDiagnostics(tt.path) + + // Verify results + if (err != nil) != tt.wantErr { + t.Errorf("getDiagnostics() error = %v, wantErr %v", err, tt.wantErr) + return + } + + for _, d := range diagnostics { + t.Logf("%s Diagnostic: %v; Range: %d/%d %d/%d", tt.name, d.Message, + d.Range.Start.Line, d.Range.Start.Character, d.Range.End.Line, d.Range.End.Character) + } + + if len(diagnostics) != tt.wantDiagCount { + t.Errorf("%s getDiagnostics() returned %v diagnostics, want %d", tt.name, len(diagnostics), tt.wantDiagCount) + } + + // Check diagnostic severities + for i, diag := range diagnostics { + if i >= len(tt.wantSeverities) { + break + } + if diag.Severity != tt.wantSeverities[i] { + t.Errorf("diagnostic[%d] severity = %d, want %d", i, diag.Severity, tt.wantSeverities[i]) + } + } + }) + } +} From 49b673e16b5b12e708f0bd96898f5037c7a1aa25 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Thu, 13 Mar 2025 21:33:11 +0800 Subject: [PATCH 06/15] test(server): update test case URI for DidChangeTextDocumentParams --- internal/server/text_syncronization_test.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/server/text_syncronization_test.go b/internal/server/text_syncronization_test.go index 0d2269c..45b9658 100644 --- a/internal/server/text_syncronization_test.go +++ b/internal/server/text_syncronization_test.go @@ -241,7 +241,7 @@ func TestDidChange(t *testing.T) { params: &protocol.DidChangeTextDocumentParams{ TextDocument: protocol.VersionedTextDocumentIdentifier{ TextDocumentIdentifier: protocol.TextDocumentIdentifier{ - URI: "file://workspace/test.gop", + URI: "file://error/test.gop", }, Version: 2, }, From 2e2fbf22b5c5d802d1428801b313bcd0d161a309 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Thu, 13 Mar 2025 22:05:48 +0800 Subject: [PATCH 07/15] fix(server): add mutex for thread-safe diagnostic generation --- internal/server/text_syncronization.go | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/internal/server/text_syncronization.go b/internal/server/text_syncronization.go index 43f9a7d..21a8136 100644 --- a/internal/server/text_syncronization.go +++ b/internal/server/text_syncronization.go @@ -4,6 +4,7 @@ import ( "bytes" "fmt" "go/types" + "sync" "time" "github.com/goplus/gogen" @@ -241,6 +242,8 @@ func (s *Server) PositionOffset(content []byte, position Position) int { return lineOffset + utf8Offset } +var mu sync.Mutex + // getDiagnostics generates diagnostic information for a specific file. // It performs two checks: // 1. AST parsing - reports syntax errors @@ -251,6 +254,8 @@ func (s *Server) PositionOffset(content []byte, position Position) int { // Returns a slice of diagnostics and an error (if diagnostic generation failed). func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { var diagnostics []Diagnostic + mu.Lock() + defer mu.Unlock() proj := s.getProj() From b24ab2995a5fc8b449be2062016407e3e5c0342b Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Thu, 13 Mar 2025 23:29:42 +0800 Subject: [PATCH 08/15] feat(project): add ModifyFiles method to handle batch file changes --- gop/proj.go | 1 + 1 file changed, 1 insertion(+) diff --git a/gop/proj.go b/gop/proj.go index 84c9749..f5f967a 100644 --- a/gop/proj.go +++ b/gop/proj.go @@ -202,6 +202,7 @@ func (p *Project) PutFile(path string, file File) { p.deleteCache(path) } +// ModifyFiles modifies files in the project. func (p *Project) ModifyFiles(changes []FileChange) { // Process all changes in a batch for _, change := range changes { From c26f3d14c4e2e12b4504635b0b435aeef41509a0 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Mon, 17 Mar 2025 13:16:54 +0800 Subject: [PATCH 09/15] refactor: remove deprecated ModTime field and associated ModifyFiles method --- gop/proj.go | 32 +---- gop/proj_test.go | 127 ------------------ internal/server/server.go | 39 ++++++ internal/server/text_syncronization.go | 91 ++++++------- internal/server/text_syncronization_test.go | 140 +++++++++++++++++++- 5 files changed, 219 insertions(+), 210 deletions(-) diff --git a/gop/proj.go b/gop/proj.go index f5f967a..b70f709 100644 --- a/gop/proj.go +++ b/gop/proj.go @@ -65,14 +65,9 @@ type fileKey struct { type File = *FileImpl type FileImpl struct { Content []byte + // Deprecated: ModTime is no longer supported due to lsp text sync specification. Use Version instead. ModTime time.Time -} - -// FileChange represents a file change. -type FileChange struct { - Path string - Content []byte - Version int // Version is timestamp in milliseconds + Version int } // Project represents a project. @@ -202,29 +197,6 @@ func (p *Project) PutFile(path string, file File) { p.deleteCache(path) } -// ModifyFiles modifies files in the project. -func (p *Project) ModifyFiles(changes []FileChange) { - // Process all changes in a batch - for _, change := range changes { - // Create new file with updated content - file := &FileImpl{ - Content: change.Content, - ModTime: time.UnixMilli(int64(change.Version)), - } - - // Check if file exists - if oldFile, ok := p.File(change.Path); ok { - // Only update if version is newer - if change.Version > int(oldFile.ModTime.UnixMilli()) { - p.PutFile(change.Path, file) - } - } else { - // New file, always add - p.PutFile(change.Path, file) - } - } -} - // UpdateFiles updates all files in the project with the provided map of files. // This will remove existing files not present in the new map and add/update files from the new map. func (p *Project) UpdateFiles(newFiles map[string]File) { diff --git a/gop/proj_test.go b/gop/proj_test.go index 90e48d3..043c7cc 100644 --- a/gop/proj_test.go +++ b/gop/proj_test.go @@ -258,130 +258,3 @@ func TestUpdateFiles(t *testing.T) { t.Fatal("Cache should be invalidated when ModTime changes") } } - -func TestModifyFiles(t *testing.T) { - tests := []struct { - name string - initial map[string]File - changes []FileChange - want map[string]string // path -> expected content - }{ - { - name: "add new files", - initial: map[string]File{}, - changes: []FileChange{ - { - Path: "new.go", - Content: []byte("package main"), - Version: 100, - }, - }, - want: map[string]string{ - "new.go": "package main", - }, - }, - { - name: "update existing file with newer version", - initial: map[string]File{ - "main.go": &FileImpl{ - Content: []byte("old content"), - ModTime: time.UnixMilli(100), - }, - }, - changes: []FileChange{ - { - Path: "main.go", - Content: []byte("new content"), - Version: 200, - }, - }, - want: map[string]string{ - "main.go": "new content", - }, - }, - { - name: "ignore older version update", - initial: map[string]File{ - "main.go": &FileImpl{ - Content: []byte("current content"), - ModTime: time.UnixMilli(200), - }, - }, - changes: []FileChange{ - { - Path: "main.go", - Content: []byte("old content"), - Version: 100, - }, - }, - want: map[string]string{ - "main.go": "current content", - }, - }, - { - name: "multiple file changes", - initial: map[string]File{ - "file1.go": &FileImpl{ - Content: []byte("content1"), - ModTime: time.UnixMilli(100), - }, - "file2.go": &FileImpl{ - Content: []byte("content2"), - ModTime: time.UnixMilli(100), - }, - }, - changes: []FileChange{ - { - Path: "file1.go", - Content: []byte("new content1"), - Version: 200, - }, - { - Path: "file3.go", - Content: []byte("content3"), - Version: 200, - }, - }, - want: map[string]string{ - "file1.go": "new content1", - "file2.go": "content2", - "file3.go": "content3", - }, - }, - } - - for _, tt := range tests { - t.Run(tt.name, func(t *testing.T) { - // Create new project with initial files - proj := NewProject(nil, tt.initial, FeatAll) - - // Apply changes - proj.ModifyFiles(tt.changes) - - // Verify results - for path, wantContent := range tt.want { - file, ok := proj.File(path) - if !ok { - t.Errorf("file %s not found", path) - continue - } - if got := string(file.Content); got != wantContent { - t.Errorf("file %s content = %q, want %q", path, got, wantContent) - } - } - - // Verify no extra files exist - count := 0 - proj.RangeFiles(func(path string) bool { - count++ - if _, ok := tt.want[path]; !ok { - t.Errorf("unexpected file: %s", path) - } - return true - }) - if count != len(tt.want) { - t.Errorf("got %d files, want %d", count, len(tt.want)) - } - }) - } -} diff --git a/internal/server/server.go b/internal/server/server.go index e6dda35..8e3dcf3 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -7,6 +7,8 @@ import ( "slices" "strings" + gopast "github.com/goplus/gop/ast" + goptoken "github.com/goplus/gop/token" "github.com/goplus/goxlsw/gop" "github.com/goplus/goxlsw/internal/analysis" "github.com/goplus/goxlsw/internal/vfs" @@ -334,3 +336,40 @@ func (s *Server) fromDocumentURI(documentURI DocumentURI) (string, error) { func (s *Server) toDocumentURI(path string) DocumentURI { return DocumentURI(string(s.workspaceRootURI) + path) } + +// fromPosition converts a token.Position to an LSP Position. +func (s *Server) fromPosition(astFile *gopast.File, position goptoken.Position) Position { + tokenFile := s.getProj().Fset.File(astFile.Pos()) + + line := position.Line + lineStart := int(tokenFile.LineStart(line)) + relLineStart := lineStart - tokenFile.Base() + lineContent := astFile.Code[relLineStart : relLineStart+position.Column-1] + utf16Offset := utf8OffsetToUTF16(string(lineContent), position.Column-1) + + return Position{ + Line: uint32(position.Line - 1), + Character: uint32(utf16Offset), + } +} + +// rangeForASTFilePosition returns a [Range] for the given position in an AST file. +func (s *Server) rangeForASTFilePosition(astFile *gopast.File, position goptoken.Position) Range { + p := s.fromPosition(astFile, position) + return Range{Start: p, End: p} +} + +// rangeForPos returns the [Range] for the given position. +func (s *Server) rangeForPos(pos goptoken.Pos) Range { + return s.rangeForASTFilePosition(s.posASTFile(pos), s.getProj().Fset.Position(pos)) +} + +// posASTFile returns the AST file for the given position. +func (s *Server) posASTFile(pos goptoken.Pos) *gopast.File { + return getASTPkg(s.getProj()).Files[s.posFilename(pos)] +} + +// posFilename returns the filename for the given position. +func (s *Server) posFilename(pos goptoken.Pos) string { + return s.getProj().Fset.Position(pos).Filename +} diff --git a/internal/server/text_syncronization.go b/internal/server/text_syncronization.go index 21a8136..2f1806a 100644 --- a/internal/server/text_syncronization.go +++ b/internal/server/text_syncronization.go @@ -4,13 +4,10 @@ import ( "bytes" "fmt" "go/types" - "sync" "time" "github.com/goplus/gogen" - gopast "github.com/goplus/gop/ast" gopscanner "github.com/goplus/gop/scanner" - goptoken "github.com/goplus/gop/token" "github.com/goplus/goxlsw/gop" "github.com/goplus/goxlsw/jsonrpc2" "github.com/goplus/goxlsw/protocol" @@ -27,7 +24,7 @@ func (s *Server) didOpen(params *DidOpenTextDocumentParams) error { return err } - return s.didModifyFile([]gop.FileChange{{ + return s.didModifyFile([]FileChange{{ Path: path, Content: []byte(params.TextDocument.Text), Version: int(params.TextDocument.Version), @@ -50,7 +47,7 @@ func (s *Server) didChange(params *DidChangeTextDocumentParams) error { } // Create a file change record - changes := []gop.FileChange{{ + changes := []FileChange{{ Path: path, Content: content, Version: int(params.TextDocument.Version), @@ -71,7 +68,7 @@ func (s *Server) didSave(params *DidSaveTextDocumentParams) error { return err } - return s.didModifyFile([]gop.FileChange{{ + return s.didModifyFile([]FileChange{{ Path: path, Content: []byte(*params.Text), Version: int(time.Now().UnixMilli()), @@ -94,9 +91,9 @@ func (s *Server) didClose(params *DidCloseTextDocumentParams) error { // 1. Updates the project's files with the provided changes // 2. Starts a goroutine to generate and publish diagnostics for each changed file // 3. Returns immediately after updating files for better responsiveness -func (s *Server) didModifyFile(changes []gop.FileChange) error { +func (s *Server) didModifyFile(changes []FileChange) error { // 1. Update files synchronously - s.getProj().ModifyFiles(changes) + s.ModifyFiles(changes) // 2. Asynchronously generate and publish diagnostics // This allows for quick response while diagnostics computation happens in background @@ -242,8 +239,6 @@ func (s *Server) PositionOffset(content []byte, position Position) int { return lineOffset + utf8Offset } -var mu sync.Mutex - // getDiagnostics generates diagnostic information for a specific file. // It performs two checks: // 1. AST parsing - reports syntax errors @@ -254,8 +249,6 @@ var mu sync.Mutex // Returns a slice of diagnostics and an error (if diagnostic generation failed). func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { var diagnostics []Diagnostic - mu.Lock() - defer mu.Unlock() proj := s.getProj() @@ -296,13 +289,18 @@ func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { return diagnostics, nil } + astFilePos := proj.Fset.Position(astFile.Pos()) + handleErr := func(err error) { if typeErr, ok := err.(types.Error); ok { - diagnostics = append(diagnostics, Diagnostic{ - Severity: SeverityError, - Range: s.rangeForPos(typeErr.Pos), - Message: typeErr.Msg, - }) + position := typeErr.Fset.Position(typeErr.Pos) + if position.Filename == astFilePos.Filename { + diagnostics = append(diagnostics, Diagnostic{ + Severity: SeverityError, + Range: s.rangeForPos(typeErr.Pos), + Message: typeErr.Msg, + }) + } } } @@ -324,39 +322,34 @@ func (s *Server) getDiagnostics(path string) ([]Diagnostic, error) { return diagnostics, nil } -// fromPosition converts a token.Position to an LSP Position. -func (s *Server) fromPosition(astFile *gopast.File, position goptoken.Position) Position { - tokenFile := s.getProj().Fset.File(astFile.Pos()) - - line := position.Line - lineStart := int(tokenFile.LineStart(line)) - relLineStart := lineStart - tokenFile.Base() - lineContent := astFile.Code[relLineStart : relLineStart+position.Column-1] - utf16Offset := utf8OffsetToUTF16(string(lineContent), position.Column-1) - - return Position{ - Line: uint32(position.Line - 1), - Character: uint32(utf16Offset), - } +// FileChange represents a file change. +type FileChange struct { + Path string + Content []byte + Version int // Version is timestamp in milliseconds } -// rangeForASTFilePosition returns a [Range] for the given position in an AST file. -func (s *Server) rangeForASTFilePosition(astFile *gopast.File, position goptoken.Position) Range { - p := s.fromPosition(astFile, position) - return Range{Start: p, End: p} -} - -// rangeForPos returns the [Range] for the given position. -func (s *Server) rangeForPos(pos goptoken.Pos) Range { - return s.rangeForASTFilePosition(s.posASTFile(pos), s.getProj().Fset.Position(pos)) -} - -// posASTFile returns the AST file for the given position. -func (s *Server) posASTFile(pos goptoken.Pos) *gopast.File { - return getASTPkg(s.getProj()).Files[s.posFilename(pos)] -} +// ModifyFiles modifies files in the project. +func (s *Server) ModifyFiles(changes []FileChange) { + // Get project + p := s.getProj() + // Process all changes in a batch + for _, change := range changes { + // Create new file with updated content + file := &gop.FileImpl{ + Content: change.Content, + Version: change.Version, + } -// posFilename returns the filename for the given position. -func (s *Server) posFilename(pos goptoken.Pos) string { - return s.getProj().Fset.Position(pos).Filename + // Check if file exists + if oldFile, ok := p.File(change.Path); ok { + // Only update if version is newer + if change.Version > oldFile.Version { + p.PutFile(change.Path, file) + } + } else { + // New file, always add + p.PutFile(change.Path, file) + } + } } diff --git a/internal/server/text_syncronization_test.go b/internal/server/text_syncronization_test.go index 45b9658..a7f355b 100644 --- a/internal/server/text_syncronization_test.go +++ b/internal/server/text_syncronization_test.go @@ -34,6 +34,138 @@ func strPtr(s string) *string { return &s } +func TestModifyFiles(t *testing.T) { + tests := []struct { + name string + initial map[string]gop.File + changes []FileChange + want map[string]string // path -> expected content + }{ + { + name: "add new files", + initial: map[string]gop.File{}, + changes: []FileChange{ + { + Path: "new.go", + Content: []byte("package main"), + Version: 100, + }, + }, + want: map[string]string{ + "new.go": "package main", + }, + }, + { + name: "update existing file with newer version", + initial: map[string]gop.File{ + "main.go": &gop.FileImpl{ + Content: []byte("old content"), + ModTime: time.UnixMilli(100), + }, + }, + changes: []FileChange{ + { + Path: "main.go", + Content: []byte("new content"), + Version: 200, + }, + }, + want: map[string]string{ + "main.go": "new content", + }, + }, + { + name: "ignore older version update", + initial: map[string]gop.File{ + "main.go": &gop.FileImpl{ + Content: []byte("current content"), + Version: 200, + }, + }, + changes: []FileChange{ + { + Path: "main.go", + Content: []byte("old content"), + Version: 100, + }, + }, + want: map[string]string{ + "main.go": "current content", + }, + }, + { + name: "multiple file changes", + initial: map[string]gop.File{ + "file1.go": &gop.FileImpl{ + Content: []byte("content1"), + ModTime: time.UnixMilli(100), + }, + "file2.go": &gop.FileImpl{ + Content: []byte("content2"), + ModTime: time.UnixMilli(100), + }, + }, + changes: []FileChange{ + { + Path: "file1.go", + Content: []byte("new content1"), + Version: 200, + }, + { + Path: "file3.go", + Content: []byte("content3"), + Version: 200, + }, + }, + want: map[string]string{ + "file1.go": "new content1", + "file2.go": "content2", + "file3.go": "content3", + }, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + // Create new project with initial files + proj := gop.NewProject(token.NewFileSet(), tt.initial, gop.FeatAll) + + // Create a TestServer that extends the real Server + server := &Server{ + workspaceRootFS: proj, + } + + // Apply changes + server.ModifyFiles(tt.changes) + + // Verify results + for path, wantContent := range tt.want { + file, ok := proj.File(path) + if !ok { + t.Errorf("file %s not found", path) + continue + } + if got := string(file.Content); got != wantContent { + t.Errorf("%s file %s content = %q, want %q", tt.name, path, got, wantContent) + } + } + + // Verify no extra files exist + count := 0 + proj.RangeFiles(func(path string) bool { + count++ + if _, ok := tt.want[path]; !ok { + t.Errorf("unexpected file: %s", path) + } + return true + }) + if count != len(tt.want) { + t.Errorf("got %d files, want %d", count, len(tt.want)) + } + }) + } +} + // TestDidOpen tests the didOpen handler functionality func TestDidOpen(t *testing.T) { tests := []struct { @@ -830,10 +962,10 @@ func TestGetDiagnostics(t *testing.T) { return } - for _, d := range diagnostics { - t.Logf("%s Diagnostic: %v; Range: %d/%d %d/%d", tt.name, d.Message, - d.Range.Start.Line, d.Range.Start.Character, d.Range.End.Line, d.Range.End.Character) - } + // for _, d := range diagnostics { + // t.Logf("%s Diagnostic: %v; Range: %d/%d %d/%d", tt.name, d.Message, + // d.Range.Start.Line, d.Range.Start.Character, d.Range.End.Line, d.Range.End.Character) + // } if len(diagnostics) != tt.wantDiagCount { t.Errorf("%s getDiagnostics() returned %v diagnostics, want %d", tt.name, len(diagnostics), tt.wantDiagCount) From 4738c4154483d08ee56e8f002da02f3b121a7aa2 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Mon, 17 Mar 2025 18:05:22 +0800 Subject: [PATCH 10/15] feat(project): implement singleflight to prevent duplicate builder calls in cache methods --- gop/proj.go | 31 +++++++++++++++---- ...cronization.go => text_synchronization.go} | 0 ...n_test.go => text_synchronization_test.go} | 0 3 files changed, 25 insertions(+), 6 deletions(-) rename internal/server/{text_syncronization.go => text_synchronization.go} (100%) rename internal/server/{text_syncronization_test.go => text_synchronization_test.go} (100%) diff --git a/gop/proj.go b/gop/proj.go index b70f709..3480537 100644 --- a/gop/proj.go +++ b/gop/proj.go @@ -26,6 +26,7 @@ import ( "github.com/goplus/gop/x/typesutil" "github.com/goplus/mod/gopmod" + "golang.org/x/sync/singleflight" ) var ( @@ -95,6 +96,9 @@ type Project struct { // The caller is responsible for initialization (optional). NewTypeInfo func() *typesutil.Info + + // singleflight group to avoid duplicate builder calls + group singleflight.Group } // NewProject creates a new project. @@ -265,6 +269,8 @@ func (p *Project) InitCache(kind string, builder func(root *Project) (any, error } // FileCache gets a file level cache. +// It uses singleflight to prevent duplicate builder calls when multiple +// goroutines request the same cache simultaneously. func (p *Project) FileCache(kind, path string) (any, error) { key := fileKey{kind, path} if v, ok := p.fileCaches.Load(key); ok { @@ -278,12 +284,20 @@ func (p *Project) FileCache(kind, path string) (any, error) { if !ok { return nil, fs.ErrNotExist } - data, err := builder(p, path, file) - p.fileCaches.Store(key, encodeDataOrErr(data, err)) - return data, err + + // Use singleflight to ensure only one builder is running for the same key + v, err, _ := p.group.Do(path, func() (interface{}, error) { + data, err := builder(p, path, file) + p.fileCaches.Store(key, encodeDataOrErr(data, err)) + return data, nil + }) + + return v, err } // Cache gets a project level cache. +// It uses singleflight to prevent duplicate builder calls when multiple +// goroutines request the same cache simultaneously. func (p *Project) Cache(kind string) (any, error) { if v, ok := p.caches.Load(kind); ok { return decodeDataOrErr(v) @@ -292,9 +306,14 @@ func (p *Project) Cache(kind string) (any, error) { if !ok { return nil, ErrUnknownKind } - data, err := builder(p) - p.caches.Store(kind, encodeDataOrErr(data, err)) - return data, err + + // Use singleflight to ensure only one builder is running for the same key + v, err, _ := p.group.Do(p.Path, func() (interface{}, error) { + data, err := builder(p) + p.caches.Store(kind, encodeDataOrErr(data, err)) + return data, err + }) + return v, err } func decodeDataOrErr(v any) (any, error) { diff --git a/internal/server/text_syncronization.go b/internal/server/text_synchronization.go similarity index 100% rename from internal/server/text_syncronization.go rename to internal/server/text_synchronization.go diff --git a/internal/server/text_syncronization_test.go b/internal/server/text_synchronization_test.go similarity index 100% rename from internal/server/text_syncronization_test.go rename to internal/server/text_synchronization_test.go From 22f0c80d3144abc55600e4e9dd0d52a67acf55f0 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Mon, 17 Mar 2025 19:35:38 +0800 Subject: [PATCH 11/15] refactor(project): remove singleflight usage from cache methods --- gop/proj.go | 29 ++++++----------------------- 1 file changed, 6 insertions(+), 23 deletions(-) diff --git a/gop/proj.go b/gop/proj.go index 3480537..1259838 100644 --- a/gop/proj.go +++ b/gop/proj.go @@ -26,7 +26,6 @@ import ( "github.com/goplus/gop/x/typesutil" "github.com/goplus/mod/gopmod" - "golang.org/x/sync/singleflight" ) var ( @@ -96,9 +95,6 @@ type Project struct { // The caller is responsible for initialization (optional). NewTypeInfo func() *typesutil.Info - - // singleflight group to avoid duplicate builder calls - group singleflight.Group } // NewProject creates a new project. @@ -269,8 +265,6 @@ func (p *Project) InitCache(kind string, builder func(root *Project) (any, error } // FileCache gets a file level cache. -// It uses singleflight to prevent duplicate builder calls when multiple -// goroutines request the same cache simultaneously. func (p *Project) FileCache(kind, path string) (any, error) { key := fileKey{kind, path} if v, ok := p.fileCaches.Load(key); ok { @@ -285,19 +279,12 @@ func (p *Project) FileCache(kind, path string) (any, error) { return nil, fs.ErrNotExist } - // Use singleflight to ensure only one builder is running for the same key - v, err, _ := p.group.Do(path, func() (interface{}, error) { - data, err := builder(p, path, file) - p.fileCaches.Store(key, encodeDataOrErr(data, err)) - return data, nil - }) - - return v, err + data, err := builder(p, path, file) + p.fileCaches.Store(key, encodeDataOrErr(data, err)) + return data, err } // Cache gets a project level cache. -// It uses singleflight to prevent duplicate builder calls when multiple -// goroutines request the same cache simultaneously. func (p *Project) Cache(kind string) (any, error) { if v, ok := p.caches.Load(kind); ok { return decodeDataOrErr(v) @@ -307,13 +294,9 @@ func (p *Project) Cache(kind string) (any, error) { return nil, ErrUnknownKind } - // Use singleflight to ensure only one builder is running for the same key - v, err, _ := p.group.Do(p.Path, func() (interface{}, error) { - data, err := builder(p) - p.caches.Store(kind, encodeDataOrErr(data, err)) - return data, err - }) - return v, err + data, err := builder(p) + p.caches.Store(kind, encodeDataOrErr(data, err)) + return data, err } func decodeDataOrErr(v any) (any, error) { From 23133754327a83d98bf7a57fe850e6f48520efde Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Mon, 17 Mar 2025 21:30:15 +0800 Subject: [PATCH 12/15] chore(deps): update gogen to v1.16.8 in go.mod and go.sum --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index e274abd..db1e32e 100644 --- a/go.mod +++ b/go.mod @@ -3,7 +3,7 @@ module github.com/goplus/goxlsw go 1.23.4 require ( - github.com/goplus/gogen v1.16.6 + github.com/goplus/gogen v1.16.8 github.com/goplus/gop v1.3.5 github.com/goplus/mod v0.13.17 github.com/goplus/spx v1.1.1-0.20250214074125-e9e1f6362499 diff --git a/go.sum b/go.sum index 2cdadd0..37d812d 100644 --- a/go.sum +++ b/go.sum @@ -20,8 +20,8 @@ github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0 h1:DACJavvAHhabrF0 github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k= github.com/goplus/canvas v0.1.0 h1:Vx3f2+U8UANvWf5/01YsQYKNbZDm1GZCjhlEBFrQkeU= github.com/goplus/canvas v0.1.0/go.mod h1:Rhcvo5qkpD9WuXFnvnXtrBSY97l6h7sXQuofrmiLNdM= -github.com/goplus/gogen v1.16.6 h1:Zwv18HoTbPDk8s2ajxgVeqZE5i4/GMV722KHl6GS8Yk= -github.com/goplus/gogen v1.16.6/go.mod h1:6TQYbabXDF9LCdDkOOzHmfg1R4ENfXQ3XpHa9RhTSD8= +github.com/goplus/gogen v1.16.8 h1:idakC+4OZIAvDSi3wkPGHlhpNEd7xkmvodXDMDvfn4s= +github.com/goplus/gogen v1.16.8/go.mod h1:6TQYbabXDF9LCdDkOOzHmfg1R4ENfXQ3XpHa9RhTSD8= github.com/goplus/gop v1.3.5 h1:WmkGpOnyANI/b0Oj/k3qkOPdlhEiBbpds/X/cgUVzDs= github.com/goplus/gop v1.3.5/go.mod h1:BnMzJG+PYoOWjKT4Q7CwxLUTBRnTFuuxW8W4O3vq/TU= github.com/goplus/mod v0.13.17 h1:aWp14xosENrh7t0/0qcIejDmQEiTgI3ou2+KoLDlSlE= From 77c611f58be25f15b4798a4db138d3755ff84f72 Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Mon, 17 Mar 2025 21:37:43 +0800 Subject: [PATCH 13/15] fix(deps): resolve merge conflict in go.sum and update gogen to v1.16.8 --- go.sum | 7 ------- 1 file changed, 7 deletions(-) diff --git a/go.sum b/go.sum index d62fd0c..37d812d 100644 --- a/go.sum +++ b/go.sum @@ -20,17 +20,10 @@ github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0 h1:DACJavvAHhabrF0 github.com/golang/freetype v0.0.0-20170609003504-e2365dfdc4a0/go.mod h1:E/TSTwGwJL78qG/PmXZO1EjYhfJinVAhrmmHX6Z8B9k= github.com/goplus/canvas v0.1.0 h1:Vx3f2+U8UANvWf5/01YsQYKNbZDm1GZCjhlEBFrQkeU= github.com/goplus/canvas v0.1.0/go.mod h1:Rhcvo5qkpD9WuXFnvnXtrBSY97l6h7sXQuofrmiLNdM= -<<<<<<< HEAD github.com/goplus/gogen v1.16.8 h1:idakC+4OZIAvDSi3wkPGHlhpNEd7xkmvodXDMDvfn4s= github.com/goplus/gogen v1.16.8/go.mod h1:6TQYbabXDF9LCdDkOOzHmfg1R4ENfXQ3XpHa9RhTSD8= github.com/goplus/gop v1.3.5 h1:WmkGpOnyANI/b0Oj/k3qkOPdlhEiBbpds/X/cgUVzDs= github.com/goplus/gop v1.3.5/go.mod h1:BnMzJG+PYoOWjKT4Q7CwxLUTBRnTFuuxW8W4O3vq/TU= -======= -github.com/goplus/gogen v1.16.7 h1:OVN6byjWVtzs4plS7q80h747Jh8G7J+VUPN0L9YWQ68= -github.com/goplus/gogen v1.16.7/go.mod h1:6TQYbabXDF9LCdDkOOzHmfg1R4ENfXQ3XpHa9RhTSD8= -github.com/goplus/gop v1.3.6 h1:I9GUJXt+elFXjO3tNi/BXWiR7MujHoxrdawLQfMPrTM= -github.com/goplus/gop v1.3.6/go.mod h1:grsLKBbSesOXhwXH9tktcXHi+SBOn/yk4LKsL3dRmMg= ->>>>>>> main github.com/goplus/mod v0.13.17 h1:aWp14xosENrh7t0/0qcIejDmQEiTgI3ou2+KoLDlSlE= github.com/goplus/mod v0.13.17/go.mod h1:XlHf8mnQ4QkRDX14Of2tpywuHDd+JVpPStvh3egog+0= github.com/goplus/spx v1.1.1-0.20250214074125-e9e1f6362499 h1:wt9bXZWSZ6MKgLw0c87GSbKrXjj69aCt7JVM1RwLp3c= From 4905ee17423469f82148a5f963de783f7b65538c Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Mon, 17 Mar 2025 21:38:37 +0800 Subject: [PATCH 14/15] chore(deps): update gop package from v1.3.5 to v1.3.6 --- go.mod | 2 +- go.sum | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/go.mod b/go.mod index db1e32e..672ef02 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.23.4 require ( github.com/goplus/gogen v1.16.8 - github.com/goplus/gop v1.3.5 + github.com/goplus/gop v1.3.6 github.com/goplus/mod v0.13.17 github.com/goplus/spx v1.1.1-0.20250214074125-e9e1f6362499 github.com/qiniu/x v1.13.12 diff --git a/go.sum b/go.sum index 37d812d..6e909e7 100644 --- a/go.sum +++ b/go.sum @@ -22,8 +22,8 @@ github.com/goplus/canvas v0.1.0 h1:Vx3f2+U8UANvWf5/01YsQYKNbZDm1GZCjhlEBFrQkeU= github.com/goplus/canvas v0.1.0/go.mod h1:Rhcvo5qkpD9WuXFnvnXtrBSY97l6h7sXQuofrmiLNdM= github.com/goplus/gogen v1.16.8 h1:idakC+4OZIAvDSi3wkPGHlhpNEd7xkmvodXDMDvfn4s= github.com/goplus/gogen v1.16.8/go.mod h1:6TQYbabXDF9LCdDkOOzHmfg1R4ENfXQ3XpHa9RhTSD8= -github.com/goplus/gop v1.3.5 h1:WmkGpOnyANI/b0Oj/k3qkOPdlhEiBbpds/X/cgUVzDs= -github.com/goplus/gop v1.3.5/go.mod h1:BnMzJG+PYoOWjKT4Q7CwxLUTBRnTFuuxW8W4O3vq/TU= +github.com/goplus/gop v1.3.6 h1:I9GUJXt+elFXjO3tNi/BXWiR7MujHoxrdawLQfMPrTM= +github.com/goplus/gop v1.3.6/go.mod h1:grsLKBbSesOXhwXH9tktcXHi+SBOn/yk4LKsL3dRmMg= github.com/goplus/mod v0.13.17 h1:aWp14xosENrh7t0/0qcIejDmQEiTgI3ou2+KoLDlSlE= github.com/goplus/mod v0.13.17/go.mod h1:XlHf8mnQ4QkRDX14Of2tpywuHDd+JVpPStvh3egog+0= github.com/goplus/spx v1.1.1-0.20250214074125-e9e1f6362499 h1:wt9bXZWSZ6MKgLw0c87GSbKrXjj69aCt7JVM1RwLp3c= From b51fa4d3b86f87a52f31e09ecb09a3768aa0962e Mon Sep 17 00:00:00 2001 From: wuxinyi Date: Tue, 18 Mar 2025 09:19:22 +0800 Subject: [PATCH 15/15] refactor(server): replace PositionOffset method with positionOffset function --- internal/server/text_synchronization.go | 58 +------------------------ internal/server/util.go | 54 +++++++++++++++++++++++ 2 files changed, 56 insertions(+), 56 deletions(-) diff --git a/internal/server/text_synchronization.go b/internal/server/text_synchronization.go index 2f1806a..488d72c 100644 --- a/internal/server/text_synchronization.go +++ b/internal/server/text_synchronization.go @@ -166,8 +166,8 @@ func (s *Server) applyIncrementalChanges(path string, changes []protocol.TextDoc } // Convert LSP positions to byte offsets - start := s.PositionOffset(content, change.Range.Start) - end := s.PositionOffset(content, change.Range.End) + start := positionOffset(content, change.Range.Start) + end := positionOffset(content, change.Range.End) // Validate range if end < start { @@ -185,60 +185,6 @@ func (s *Server) applyIncrementalChanges(path string, changes []protocol.TextDoc return content, nil } -// PositionOffset converts an LSP position (line, character) to a byte offset in the document. -// It calculates the offset by: -// 1. Finding the starting byte offset of the requested line -// 2. Adding the character offset within that line, converting from UTF-16 to UTF-8 if needed -// -// Parameters: -// - content: The file content as a byte array -// - position: The LSP position with line and character numbers (0-based) -// -// Returns the byte offset from the beginning of the document -func (s *Server) PositionOffset(content []byte, position Position) int { - // If content is empty or position is beyond the content, return 0 - if len(content) == 0 { - return 0 - } - - // Find all line start positions in the document - lineStarts := []int{0} // First line always starts at position 0 - for i := 0; i < len(content); i++ { - if content[i] == '\n' { - lineStarts = append(lineStarts, i+1) // Next line starts after the newline - } - } - - // Ensure the requested line is within range - lineIndex := int(position.Line) - if lineIndex >= len(lineStarts) { - // If line is beyond available lines, return the end of content - return len(content) - } - - // Get the starting offset of the requested line - lineOffset := lineStarts[lineIndex] - - // Extract the content of the requested line - lineEndOffset := len(content) - if lineIndex+1 < len(lineStarts) { - lineEndOffset = lineStarts[lineIndex+1] - 1 // -1 to exclude the newline character - } - - // Ensure we don't go beyond the end of content - if lineOffset >= len(content) { - return len(content) - } - - lineContent := content[lineOffset:min(lineEndOffset, len(content))] - - // Convert UTF-16 character offset to UTF-8 byte offset - utf8Offset := utf16OffsetToUTF8(string(lineContent), int(position.Character)) - - // Ensure the final offset doesn't exceed the content length - return lineOffset + utf8Offset -} - // getDiagnostics generates diagnostic information for a specific file. // It performs two checks: // 1. AST parsing - reports syntax errors diff --git a/internal/server/util.go b/internal/server/util.go index 2eb4090..4d439b7 100644 --- a/internal/server/util.go +++ b/internal/server/util.go @@ -339,3 +339,57 @@ func utf8OffsetToUTF16(s string, utf8Offset int) int { } return utf16Units } + +// positionOffset converts an LSP position (line, character) to a byte offset in the document. +// It calculates the offset by: +// 1. Finding the starting byte offset of the requested line +// 2. Adding the character offset within that line, converting from UTF-16 to UTF-8 if needed +// +// Parameters: +// - content: The file content as a byte array +// - position: The LSP position with line and character numbers (0-based) +// +// Returns the byte offset from the beginning of the document +func positionOffset(content []byte, position Position) int { + // If content is empty or position is beyond the content, return 0 + if len(content) == 0 { + return 0 + } + + // Find all line start positions in the document + lineStarts := []int{0} // First line always starts at position 0 + for i := 0; i < len(content); i++ { + if content[i] == '\n' { + lineStarts = append(lineStarts, i+1) // Next line starts after the newline + } + } + + // Ensure the requested line is within range + lineIndex := int(position.Line) + if lineIndex >= len(lineStarts) { + // If line is beyond available lines, return the end of content + return len(content) + } + + // Get the starting offset of the requested line + lineOffset := lineStarts[lineIndex] + + // Extract the content of the requested line + lineEndOffset := len(content) + if lineIndex+1 < len(lineStarts) { + lineEndOffset = lineStarts[lineIndex+1] - 1 // -1 to exclude the newline character + } + + // Ensure we don't go beyond the end of content + if lineOffset >= len(content) { + return len(content) + } + + lineContent := content[lineOffset:min(lineEndOffset, len(content))] + + // Convert UTF-16 character offset to UTF-8 byte offset + utf8Offset := utf16OffsetToUTF8(string(lineContent), int(position.Character)) + + // Ensure the final offset doesn't exceed the content length + return lineOffset + utf8Offset +}