refactor: extract editor utilities into EditorUtils module and add tests

Author: samhvw8

src/core/__tests__/CodeActionProvider.test.ts

@@ -1,5 +1,6 @@
 import * as vscode from "vscode"
 import { CodeActionProvider, ACTION_NAMES } from "../CodeActionProvider"
+import { EditorUtils } from "../EditorUtils"
 
 // Mock VSCode API
 jest.mock("vscode", () => ({
@@ -16,13 +17,6 @@ jest.mock("vscode", () => ({
 		start: { line: startLine, character: startChar },
 		end: { line: endLine, character: endChar },
 	})),
-	Position: jest.fn().mockImplementation((line, character) => ({
-		line,
-		character,
-	})),
-	workspace: {
-		getWorkspaceFolder: jest.fn(),
-	},
 	DiagnosticSeverity: {
 		Error: 0,
 		Warning: 1,
@@ -31,6 +25,16 @@ jest.mock("vscode", () => ({
 	},
 }))
 
+// Mock EditorUtils
+jest.mock("../EditorUtils", () => ({
+	EditorUtils: {
+		getEffectiveRange: jest.fn(),
+		getFilePath: jest.fn(),
+		hasIntersectingRange: jest.fn(),
+		createDiagnosticData: jest.fn(),
+	},
+}))
+
 describe("CodeActionProvider", () => {
 	let provider: CodeActionProvider
 	let mockDocument: any
@@ -55,68 +59,32 @@ describe("CodeActionProvider", () => {
 		mockContext = {
 			diagnostics: [],
 		}
-	})
-
-	describe("getEffectiveRange", () => {
-		it("should return selected text when available", () => {
-			mockDocument.getText.mockReturnValue("selected text")
-
-			const result = (provider as any).getEffectiveRange(mockDocument, mockRange)
-
-			expect(result).toEqual({
-				range: mockRange,
-				text: "selected text",
-			})
-		})
-
-		it("should return null for empty line", () => {
-			mockDocument.getText.mockReturnValue("")
-			mockDocument.lineAt.mockReturnValue({ text: "", lineNumber: 0 })
-
-			const result = (provider as any).getEffectiveRange(mockDocument, mockRange)
-
-			expect(result).toBeNull()
-		})
-	})
-
-	describe("getFilePath", () => {
-		it("should return relative path when in workspace", () => {
-			const mockWorkspaceFolder = {
-				uri: { fsPath: "/test" },
-			}
-			;(vscode.workspace.getWorkspaceFolder as jest.Mock).mockReturnValue(mockWorkspaceFolder)
-
-			const result = (provider as any).getFilePath(mockDocument)
-
-			expect(result).toBe("file.ts")
-		})
-
-		it("should return absolute path when not in workspace", () => {
-			;(vscode.workspace.getWorkspaceFolder as jest.Mock).mockReturnValue(null)
 
-			const result = (provider as any).getFilePath(mockDocument)
-
-			expect(result).toBe("/test/file.ts")
+		// Setup default EditorUtils mocks
+		;(EditorUtils.getEffectiveRange as jest.Mock).mockReturnValue({
+			range: mockRange,
+			text: "test code",
 		})
+		;(EditorUtils.getFilePath as jest.Mock).mockReturnValue("/test/file.ts")
+		;(EditorUtils.hasIntersectingRange as jest.Mock).mockReturnValue(true)
+		;(EditorUtils.createDiagnosticData as jest.Mock).mockImplementation((d) => d)
 	})
 
 	describe("provideCodeActions", () => {
-		beforeEach(() => {
-			mockDocument.getText.mockReturnValue("test code")
-			mockDocument.lineAt.mockReturnValue({ text: "test code", lineNumber: 0 })
-		})
-
-		it("should provide explain and improve actions by default", () => {
+		it("should provide explain, improve, fix logic, and add to context actions by default", () => {
 			const actions = provider.provideCodeActions(mockDocument, mockRange, mockContext)
 
-			expect(actions).toHaveLength(4)
+			expect(actions).toHaveLength(7) // 2 explain + 2 fix logic + 2 improve + 1 add to context
 			expect((actions as any)[0].title).toBe(`${ACTION_NAMES.EXPLAIN} in New Task`)
 			expect((actions as any)[1].title).toBe(`${ACTION_NAMES.EXPLAIN} in Current Task`)
-			expect((actions as any)[2].title).toBe(`${ACTION_NAMES.IMPROVE} in New Task`)
-			expect((actions as any)[3].title).toBe(`${ACTION_NAMES.IMPROVE} in Current Task`)
+			expect((actions as any)[2].title).toBe(`${ACTION_NAMES.FIX_LOGIC} in New Task`)
+			expect((actions as any)[3].title).toBe(`${ACTION_NAMES.FIX_LOGIC} in Current Task`)
+			expect((actions as any)[4].title).toBe(`${ACTION_NAMES.IMPROVE} in New Task`)
+			expect((actions as any)[5].title).toBe(`${ACTION_NAMES.IMPROVE} in Current Task`)
+			expect((actions as any)[6].title).toBe(ACTION_NAMES.ADD_TO_CONTEXT)
 		})
 
-		it("should provide fix action when diagnostics exist", () => {
+		it("should provide fix action instead of fix logic when diagnostics exist", () => {
 			mockContext.diagnostics = [
 				{
 					message: "test error",
@@ -127,22 +95,33 @@ describe("CodeActionProvider", () => {
 
 			const actions = provider.provideCodeActions(mockDocument, mockRange, mockContext)
 
-			expect(actions).toHaveLength(6)
+			expect(actions).toHaveLength(7) // 2 explain + 2 fix + 2 improve + 1 add to context
 			expect((actions as any).some((a: any) => a.title === `${ACTION_NAMES.FIX} in New Task`)).toBe(true)
 			expect((actions as any).some((a: any) => a.title === `${ACTION_NAMES.FIX} in Current Task`)).toBe(true)
+			expect((actions as any).some((a: any) => a.title === `${ACTION_NAMES.FIX_LOGIC} in New Task`)).toBe(false)
+			expect((actions as any).some((a: any) => a.title === `${ACTION_NAMES.FIX_LOGIC} in Current Task`)).toBe(
+				false,
+			)
+		})
+
+		it("should return empty array when no effective range", () => {
+			;(EditorUtils.getEffectiveRange as jest.Mock).mockReturnValue(null)
+
+			const actions = provider.provideCodeActions(mockDocument, mockRange, mockContext)
+
+			expect(actions).toEqual([])
 		})
 
 		it("should handle errors gracefully", () => {
 			const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation(() => {})
-			mockDocument.getText.mockImplementation(() => {
+			;(EditorUtils.getEffectiveRange as jest.Mock).mockImplementation(() => {
 				throw new Error("Test error")
 			})
-			mockDocument.lineAt.mockReturnValue({ text: "test", lineNumber: 0 })
 
 			const actions = provider.provideCodeActions(mockDocument, mockRange, mockContext)
 
 			expect(actions).toEqual([])
-			expect(consoleErrorSpy).toHaveBeenCalledWith("Error getting effective range:", expect.any(Error))
+			expect(consoleErrorSpy).toHaveBeenCalledWith("Error providing code actions:", expect.any(Error))
 
 			consoleErrorSpy.mockRestore()
 		})

src/core/__tests__/EditorUtils.test.ts

@@ -0,0 +1,75 @@
+import * as vscode from "vscode"
+import { EditorUtils } from "../EditorUtils"
+
+// Mock VSCode API
+jest.mock("vscode", () => ({
+	Range: jest.fn().mockImplementation((startLine, startChar, endLine, endChar) => ({
+		start: { line: startLine, character: startChar },
+		end: { line: endLine, character: endChar },
+	})),
+	Position: jest.fn().mockImplementation((line, character) => ({
+		line,
+		character,
+	})),
+	workspace: {
+		getWorkspaceFolder: jest.fn(),
+	},
+}))
+
+describe("EditorUtils", () => {
+	let mockDocument: any
+
+	beforeEach(() => {
+		mockDocument = {
+			getText: jest.fn(),
+			lineAt: jest.fn(),
+			lineCount: 10,
+			uri: { fsPath: "/test/file.ts" },
+		}
+	})
+
+	describe("getEffectiveRange", () => {
+		it("should return selected text when available", () => {
+			const mockRange = new vscode.Range(0, 0, 0, 10)
+			mockDocument.getText.mockReturnValue("selected text")
+
+			const result = EditorUtils.getEffectiveRange(mockDocument, mockRange)
+
+			expect(result).toEqual({
+				range: mockRange,
+				text: "selected text",
+			})
+		})
+
+		it("should return null for empty line", () => {
+			const mockRange = new vscode.Range(0, 0, 0, 10)
+			mockDocument.getText.mockReturnValue("")
+			mockDocument.lineAt.mockReturnValue({ text: "", lineNumber: 0 })
+
+			const result = EditorUtils.getEffectiveRange(mockDocument, mockRange)
+
+			expect(result).toBeNull()
+		})
+	})
+
+	describe("getFilePath", () => {
+		it("should return relative path when in workspace", () => {
+			const mockWorkspaceFolder = {
+				uri: { fsPath: "/test" },
+			}
+			;(vscode.workspace.getWorkspaceFolder as jest.Mock).mockReturnValue(mockWorkspaceFolder)
+
+			const result = EditorUtils.getFilePath(mockDocument)
+
+			expect(result).toBe("file.ts")
+		})
+
+		it("should return absolute path when not in workspace", () => {
+			;(vscode.workspace.getWorkspaceFolder as jest.Mock).mockReturnValue(null)
+
+			const result = EditorUtils.getFilePath(mockDocument)
+
+			expect(result).toBe("/test/file.ts")
+		})
+	})
+})