fix(lsp): improve error handling and input validation for rename

- Add debug logging to prepareRename() and rename() catch handlers
- Filter out expected "method not found" errors from logging
- Add input validation for empty/whitespace newName parameter
- Update JSDoc to clarify null return semantics
- Add unit tests for empty newName validation
- Add documentation test for documentChanges normalization

Addresses PR #19 review feedback:
- Silent error handling now logs unexpected failures
- Ambiguous null returns are now documented

Author: amondnet

packages/lsp/src/index.ts

@@ -763,6 +763,11 @@ export class LSPManager {
    * Prepare rename at the given position
    * Validates if the symbol at the position can be renamed
    *
+   * @returns PrepareRenameResult if symbol can be renamed, null if:
+   *          - Symbol cannot be renamed (LSP server returned null)
+   *          - Position is not on a renameable symbol
+   *          - All LSP servers failed (errors are logged)
+   *
    * @see https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_prepareRename
    */
   async prepareRename(input: {
@@ -785,7 +790,14 @@ export class LSPManager {
             },
           })
           .then((result: unknown) => this.normalizePrepareRename(result))
-          .catch(() => null),
+          .catch((err: unknown) => {
+            // Log unexpected errors (not "method not found" which is expected for some servers)
+            const message = err instanceof Error ? err.message : String(err)
+            if (!message.includes('-32601') && !message.includes('Method not found')) {
+              console.error(`[lsp:${client.serverID}] prepareRename failed:`, message)
+            }
+            return null
+          }),
       ),
     )
 
@@ -797,6 +809,12 @@ export class LSPManager {
    * Rename the symbol at the given position
    * Returns a WorkspaceEdit with all changes needed
    *
+   * @param input.newName - The new name for the symbol (must be non-empty)
+   * @returns WorkspaceEdit if rename succeeded, null if:
+   *          - Symbol cannot be renamed
+   *          - newName is empty or whitespace-only
+   *          - All LSP servers failed (errors are logged)
+   *
    * @see https://microsoft.github.io/language-server-protocol/specifications/lsp/3.17/specification/#textDocument_rename
    */
   async rename(input: {
@@ -805,6 +823,12 @@ export class LSPManager {
     character: number
     newName: string
   }): Promise<WorkspaceEdit | null> {
+    // Validate newName
+    if (!input.newName || input.newName.trim() === '') {
+      console.warn('[lsp] rename called with empty newName')
+      return null
+    }
+
     const clients = await this.getClients(input.file)
 
     const results = await Promise.all(
@@ -821,7 +845,12 @@ export class LSPManager {
             newName: input.newName,
           })
           .then((result: unknown) => this.normalizeWorkspaceEdit(result))
-          .catch(() => null),
+          .catch((err: unknown) => {
+            // Log rename errors - these are more serious since rename is a mutating operation
+            const message = err instanceof Error ? err.message : String(err)
+            console.error(`[lsp:${client.serverID}] rename failed:`, message)
+            return null
+          }),
       ),
     )
 

packages/lsp/test/unit/index.test.ts

@@ -39,6 +39,28 @@ describe('LSPManager', () => {
     await manager.shutdown()
     // Should not throw
   })
+
+  test('rename returns null for empty newName', async () => {
+    const manager = new LSPManager('/test/project')
+    const result = await manager.rename({
+      file: '/test/project/test.ts',
+      line: 0,
+      character: 0,
+      newName: '',
+    })
+    expect(result).toBeNull()
+  })
+
+  test('rename returns null for whitespace-only newName', async () => {
+    const manager = new LSPManager('/test/project')
+    const result = await manager.rename({
+      file: '/test/project/test.ts',
+      line: 0,
+      character: 0,
+      newName: '   ',
+    })
+    expect(result).toBeNull()
+  })
 })
 
 describe('formatDiagnostic', () => {
@@ -254,6 +276,36 @@ describe('WorkspaceEditSchema', () => {
     const result = WorkspaceEditSchema.safeParse(multiFileEdit)
     expect(result.success).toBe(true)
   })
+
+  /**
+   * Note: LSP servers can return WorkspaceEdit in two formats:
+   * 1. 'changes' format: { changes: { [uri]: TextEdit[] } }
+   * 2. 'documentChanges' format: { documentChanges: TextDocumentEdit[] }
+   *
+   * The schema only validates the normalized 'changes' format output.
+   * LSPManager.normalizeWorkspaceEdit() converts 'documentChanges' to 'changes' at runtime.
+   */
+  test('schema validates normalized changes format (documentChanges is normalized at runtime)', () => {
+    // This test documents that the schema validates the normalized output format
+    // documentChanges format from LSP servers:
+    // { documentChanges: [{ textDocument: { uri: 'file:///test.ts' }, edits: [...] }] }
+    // gets normalized to:
+    // { changes: { 'file:///test.ts': [...] } }
+
+    const normalizedFromDocumentChanges = {
+      changes: {
+        'file:///test.ts': [
+          { range: { start: { line: 0, character: 0 }, end: { line: 0, character: 5 } }, newText: 'renamed' },
+        ],
+        'file:///other.ts': [
+          { range: { start: { line: 10, character: 0 }, end: { line: 10, character: 5 } }, newText: 'renamed' },
+        ],
+      },
+    }
+
+    const result = WorkspaceEditSchema.safeParse(normalizedFromDocumentChanges)
+    expect(result.success).toBe(true)
+  })
 })
 
 describe('PrepareRenameResultSchema', () => {