mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-10-08 03:07:53 +00:00
fix: restore Swift parser support instead of using fallback chunking
Based on user feedback, the Swift parser is stable and works well in production. Reverting the fallback chunking approach and restoring proper Swift parser support. - Removed Swift from fallbackExtensions list - Restored Swift case in languageParser.ts - Removed fallback chunking checks for Swift in index.ts - Added tests to verify Swift parser functionality - Removed old fallback test file
This commit is contained in:
parent
4c7a0a91da
commit
8c1c99dc36
5 changed files with 239 additions and 152 deletions
|
|
@ -21,7 +21,6 @@ export const scannerExtensions = allExtensions
|
|||
export const fallbackExtensions = [
|
||||
".vb", // Visual Basic .NET - no dedicated WASM parser
|
||||
".scala", // Scala - uses fallback chunking instead of Lua query workaround
|
||||
".swift", // Swift - uses fallback chunking due to parser instability
|
||||
]
|
||||
|
||||
/**
|
||||
|
|
|
|||
|
|
@ -1,121 +0,0 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest"
|
||||
import { parseSourceCodeDefinitionsForFile } from "../index"
|
||||
import { loadRequiredLanguageParsers } from "../languageParser"
|
||||
import * as fs from "fs/promises"
|
||||
import { fileExistsAtPath } from "../../../utils/fs"
|
||||
|
||||
// Mock the modules
|
||||
vi.mock("fs/promises")
|
||||
vi.mock("../../../utils/fs")
|
||||
vi.mock("../languageParser", () => ({
|
||||
loadRequiredLanguageParsers: vi.fn(),
|
||||
}))
|
||||
|
||||
describe("Swift Fallback Handling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it("should use fallback chunking for Swift files and not load the parser", async () => {
|
||||
// Mock file existence check
|
||||
vi.mocked(fileExistsAtPath).mockResolvedValue(true)
|
||||
|
||||
// Mock file content
|
||||
const swiftContent = `
|
||||
import Foundation
|
||||
|
||||
class ViewController: UIViewController {
|
||||
override func viewDidLoad() {
|
||||
super.viewDidLoad()
|
||||
print("Hello, Swift!")
|
||||
}
|
||||
}
|
||||
`.trim()
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(swiftContent)
|
||||
|
||||
// Call the function with a Swift file
|
||||
const result = await parseSourceCodeDefinitionsForFile("test.swift")
|
||||
|
||||
// Verify that the parser was NOT loaded for Swift files
|
||||
expect(loadRequiredLanguageParsers).not.toHaveBeenCalled()
|
||||
|
||||
// Verify that the result indicates fallback chunking is used
|
||||
expect(result).toBeDefined()
|
||||
expect(result).toContain("test.swift")
|
||||
expect(result).toContain("This file type uses fallback chunking for stability")
|
||||
})
|
||||
|
||||
it("should still load parsers for non-fallback file types", async () => {
|
||||
// Mock file existence check
|
||||
vi.mocked(fileExistsAtPath).mockResolvedValue(true)
|
||||
|
||||
// Mock file content for a TypeScript file
|
||||
const tsContent = `
|
||||
export function hello() {
|
||||
console.log("Hello, TypeScript!");
|
||||
}
|
||||
`.trim()
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(tsContent)
|
||||
|
||||
// Mock the parser loading with proper types
|
||||
const mockParser = {
|
||||
parse: vi.fn().mockReturnValue({
|
||||
rootNode: {
|
||||
startPosition: { row: 0, column: 0 },
|
||||
endPosition: { row: 3, column: 1 },
|
||||
},
|
||||
}),
|
||||
language: null,
|
||||
delete: vi.fn(),
|
||||
setLanguage: vi.fn(),
|
||||
reset: vi.fn(),
|
||||
getLanguage: vi.fn(),
|
||||
setTimeoutMicros: vi.fn(),
|
||||
getTimeoutMicros: vi.fn(),
|
||||
setLogger: vi.fn(),
|
||||
getLogger: vi.fn(),
|
||||
printDotGraphs: vi.fn(),
|
||||
} as any
|
||||
|
||||
const mockQuery = {
|
||||
captures: vi.fn().mockReturnValue([]),
|
||||
captureNames: [],
|
||||
captureQuantifiers: [],
|
||||
predicates: {},
|
||||
setProperties: vi.fn(),
|
||||
assertedCaptureCount: vi.fn(),
|
||||
matchLimit: 0,
|
||||
setMatchLimit: vi.fn(),
|
||||
didExceedMatchLimit: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
matches: vi.fn(),
|
||||
disableCapture: vi.fn(),
|
||||
disablePattern: vi.fn(),
|
||||
isPatternGuaranteedAtStep: vi.fn(),
|
||||
isPatternRooted: vi.fn(),
|
||||
isPatternNonLocal: vi.fn(),
|
||||
startByteForPattern: vi.fn(),
|
||||
endByteForPattern: vi.fn(),
|
||||
startIndexForPattern: vi.fn(),
|
||||
endIndexForPattern: vi.fn(),
|
||||
} as any
|
||||
|
||||
vi.mocked(loadRequiredLanguageParsers).mockResolvedValue({
|
||||
ts: { parser: mockParser, query: mockQuery },
|
||||
})
|
||||
|
||||
// Call the function with a TypeScript file
|
||||
await parseSourceCodeDefinitionsForFile("test.ts")
|
||||
|
||||
// Verify that the parser WAS loaded for TypeScript files
|
||||
expect(loadRequiredLanguageParsers).toHaveBeenCalledWith(["test.ts"])
|
||||
})
|
||||
|
||||
it("should handle multiple Swift files in parseSourceCodeForDefinitionsTopLevel", async () => {
|
||||
// This test would require more complex mocking of the directory listing
|
||||
// and is included here as a placeholder for comprehensive testing
|
||||
expect(true).toBe(true)
|
||||
})
|
||||
})
|
||||
233
src/services/tree-sitter/__tests__/swift-parser.spec.ts
Normal file
233
src/services/tree-sitter/__tests__/swift-parser.spec.ts
Normal file
|
|
@ -0,0 +1,233 @@
|
|||
import { describe, it, expect, vi, beforeEach } from "vitest"
|
||||
import { parseSourceCodeDefinitionsForFile } from "../index"
|
||||
import { loadRequiredLanguageParsers } from "../languageParser"
|
||||
import * as fs from "fs/promises"
|
||||
import { fileExistsAtPath } from "../../../utils/fs"
|
||||
|
||||
// Mock the modules
|
||||
vi.mock("fs/promises")
|
||||
vi.mock("../../../utils/fs")
|
||||
vi.mock("../languageParser", () => ({
|
||||
loadRequiredLanguageParsers: vi.fn(),
|
||||
}))
|
||||
|
||||
describe("Swift Parser Handling", () => {
|
||||
beforeEach(() => {
|
||||
vi.clearAllMocks()
|
||||
})
|
||||
|
||||
it("should load and use the Swift parser for Swift files", async () => {
|
||||
// Mock file existence check
|
||||
vi.mocked(fileExistsAtPath).mockResolvedValue(true)
|
||||
|
||||
// Mock file content
|
||||
const swiftContent = `
|
||||
import Foundation
|
||||
|
||||
class ViewController: UIViewController {
|
||||
override func viewDidLoad() {
|
||||
super.viewDidLoad()
|
||||
print("Hello, Swift!")
|
||||
}
|
||||
}
|
||||
`.trim()
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(swiftContent)
|
||||
|
||||
// Mock the parser loading with proper types
|
||||
const mockParser = {
|
||||
parse: vi.fn().mockReturnValue({
|
||||
rootNode: {
|
||||
startPosition: { row: 0, column: 0 },
|
||||
endPosition: { row: 7, column: 1 },
|
||||
},
|
||||
}),
|
||||
language: null,
|
||||
delete: vi.fn(),
|
||||
setLanguage: vi.fn(),
|
||||
reset: vi.fn(),
|
||||
getLanguage: vi.fn(),
|
||||
setTimeoutMicros: vi.fn(),
|
||||
getTimeoutMicros: vi.fn(),
|
||||
setLogger: vi.fn(),
|
||||
getLogger: vi.fn(),
|
||||
printDotGraphs: vi.fn(),
|
||||
} as any
|
||||
|
||||
const mockQuery = {
|
||||
captures: vi.fn().mockReturnValue([
|
||||
{
|
||||
name: "definition.class",
|
||||
node: {
|
||||
text: "ViewController",
|
||||
startPosition: { row: 2, column: 0 },
|
||||
endPosition: { row: 7, column: 1 },
|
||||
parent: null,
|
||||
},
|
||||
},
|
||||
]),
|
||||
captureNames: [],
|
||||
captureQuantifiers: [],
|
||||
predicates: {},
|
||||
setProperties: vi.fn(),
|
||||
assertedCaptureCount: vi.fn(),
|
||||
matchLimit: 0,
|
||||
setMatchLimit: vi.fn(),
|
||||
didExceedMatchLimit: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
matches: vi.fn(),
|
||||
disableCapture: vi.fn(),
|
||||
disablePattern: vi.fn(),
|
||||
isPatternGuaranteedAtStep: vi.fn(),
|
||||
isPatternRooted: vi.fn(),
|
||||
isPatternNonLocal: vi.fn(),
|
||||
startByteForPattern: vi.fn(),
|
||||
endByteForPattern: vi.fn(),
|
||||
startIndexForPattern: vi.fn(),
|
||||
endIndexForPattern: vi.fn(),
|
||||
} as any
|
||||
|
||||
vi.mocked(loadRequiredLanguageParsers).mockResolvedValue({
|
||||
swift: { parser: mockParser, query: mockQuery },
|
||||
})
|
||||
|
||||
// Call the function with a Swift file
|
||||
const result = await parseSourceCodeDefinitionsForFile("test.swift")
|
||||
|
||||
// Verify that the parser WAS loaded for Swift files
|
||||
expect(loadRequiredLanguageParsers).toHaveBeenCalledWith(["test.swift"])
|
||||
|
||||
// Verify that the result contains parsed content, not fallback message
|
||||
expect(result).toBeDefined()
|
||||
expect(result).toContain("test.swift")
|
||||
expect(result).not.toContain("fallback chunking")
|
||||
expect(result).toContain("class ViewController")
|
||||
})
|
||||
|
||||
it("should handle Swift files with multiple definitions", async () => {
|
||||
// Mock file existence check
|
||||
vi.mocked(fileExistsAtPath).mockResolvedValue(true)
|
||||
|
||||
// Mock file content with multiple Swift constructs
|
||||
const swiftContent = `
|
||||
import Foundation
|
||||
|
||||
protocol DataSource {
|
||||
func numberOfItems() -> Int
|
||||
}
|
||||
|
||||
struct Item {
|
||||
let id: String
|
||||
let name: String
|
||||
}
|
||||
|
||||
class ViewController: UIViewController {
|
||||
private var items: [Item] = []
|
||||
|
||||
override func viewDidLoad() {
|
||||
super.viewDidLoad()
|
||||
}
|
||||
|
||||
func loadData() {
|
||||
// Load data implementation
|
||||
}
|
||||
}
|
||||
|
||||
extension ViewController: DataSource {
|
||||
func numberOfItems() -> Int {
|
||||
return items.count
|
||||
}
|
||||
}
|
||||
`.trim()
|
||||
|
||||
vi.mocked(fs.readFile).mockResolvedValue(swiftContent)
|
||||
|
||||
// Mock the parser with multiple captures
|
||||
const mockParser = {
|
||||
parse: vi.fn().mockReturnValue({
|
||||
rootNode: {
|
||||
startPosition: { row: 0, column: 0 },
|
||||
endPosition: { row: 28, column: 1 },
|
||||
},
|
||||
}),
|
||||
language: null,
|
||||
delete: vi.fn(),
|
||||
setLanguage: vi.fn(),
|
||||
reset: vi.fn(),
|
||||
getLanguage: vi.fn(),
|
||||
setTimeoutMicros: vi.fn(),
|
||||
getTimeoutMicros: vi.fn(),
|
||||
setLogger: vi.fn(),
|
||||
getLogger: vi.fn(),
|
||||
printDotGraphs: vi.fn(),
|
||||
} as any
|
||||
|
||||
const mockQuery = {
|
||||
captures: vi.fn().mockReturnValue([
|
||||
{
|
||||
name: "definition.interface",
|
||||
node: {
|
||||
text: "DataSource",
|
||||
startPosition: { row: 2, column: 0 },
|
||||
endPosition: { row: 4, column: 1 },
|
||||
parent: null,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "definition.class",
|
||||
node: {
|
||||
text: "Item",
|
||||
startPosition: { row: 6, column: 0 },
|
||||
endPosition: { row: 9, column: 1 },
|
||||
parent: null,
|
||||
},
|
||||
},
|
||||
{
|
||||
name: "definition.class",
|
||||
node: {
|
||||
text: "ViewController",
|
||||
startPosition: { row: 11, column: 0 },
|
||||
endPosition: { row: 21, column: 1 },
|
||||
parent: null,
|
||||
},
|
||||
},
|
||||
]),
|
||||
captureNames: [],
|
||||
captureQuantifiers: [],
|
||||
predicates: {},
|
||||
setProperties: vi.fn(),
|
||||
assertedCaptureCount: vi.fn(),
|
||||
matchLimit: 0,
|
||||
setMatchLimit: vi.fn(),
|
||||
didExceedMatchLimit: vi.fn(),
|
||||
delete: vi.fn(),
|
||||
matches: vi.fn(),
|
||||
disableCapture: vi.fn(),
|
||||
disablePattern: vi.fn(),
|
||||
isPatternGuaranteedAtStep: vi.fn(),
|
||||
isPatternRooted: vi.fn(),
|
||||
isPatternNonLocal: vi.fn(),
|
||||
startByteForPattern: vi.fn(),
|
||||
endByteForPattern: vi.fn(),
|
||||
startIndexForPattern: vi.fn(),
|
||||
endIndexForPattern: vi.fn(),
|
||||
} as any
|
||||
|
||||
vi.mocked(loadRequiredLanguageParsers).mockResolvedValue({
|
||||
swift: { parser: mockParser, query: mockQuery },
|
||||
})
|
||||
|
||||
// Call the function with a Swift file
|
||||
const result = await parseSourceCodeDefinitionsForFile("test.swift")
|
||||
|
||||
// Verify that the parser was loaded
|
||||
expect(loadRequiredLanguageParsers).toHaveBeenCalledWith(["test.swift"])
|
||||
|
||||
// Verify that the result contains parsed content
|
||||
expect(result).toBeDefined()
|
||||
expect(result).toContain("test.swift")
|
||||
// The actual output shows line numbers and first lines of definitions
|
||||
expect(result).toMatch(/struct Item/)
|
||||
expect(result).toMatch(/class ViewController/)
|
||||
})
|
||||
})
|
||||
|
|
@ -6,7 +6,6 @@ import { fileExistsAtPath } from "../../utils/fs"
|
|||
import { parseMarkdown } from "./markdownParser"
|
||||
import { RooIgnoreController } from "../../core/ignore/RooIgnoreController"
|
||||
import { QueryCapture } from "web-tree-sitter"
|
||||
import { shouldUseFallbackChunking } from "../code-index/shared/supported-extensions"
|
||||
|
||||
// Private constant
|
||||
const DEFAULT_MIN_COMPONENT_LINES_VALUE = 4
|
||||
|
|
@ -138,13 +137,6 @@ export async function parseSourceCodeDefinitionsForFile(
|
|||
return undefined
|
||||
}
|
||||
|
||||
// Check if this extension should use fallback chunking (e.g., Swift)
|
||||
if (shouldUseFallbackChunking(ext)) {
|
||||
// Return a message indicating this file type uses fallback chunking
|
||||
// This prevents attempting to load the potentially unstable parser
|
||||
return `# ${path.basename(filePath)}\n// This file type uses fallback chunking for stability`
|
||||
}
|
||||
|
||||
// For other file types, load parser and use tree-sitter
|
||||
const languageParsers = await loadRequiredLanguageParsers([filePath])
|
||||
|
||||
|
|
@ -179,23 +171,20 @@ export async function parseSourceCodeForDefinitionsTopLevel(
|
|||
// Filter filepaths for access if controller is provided
|
||||
const allowedFilesToParse = rooIgnoreController ? rooIgnoreController.filterPaths(filesToParse) : filesToParse
|
||||
|
||||
// Separate markdown files, fallback files, and other files
|
||||
// Separate markdown files and other files
|
||||
const markdownFiles: string[] = []
|
||||
const fallbackFiles: string[] = []
|
||||
const otherFiles: string[] = []
|
||||
|
||||
for (const file of allowedFilesToParse) {
|
||||
const ext = path.extname(file).toLowerCase()
|
||||
if (ext === ".md" || ext === ".markdown") {
|
||||
markdownFiles.push(file)
|
||||
} else if (shouldUseFallbackChunking(ext)) {
|
||||
fallbackFiles.push(file)
|
||||
} else {
|
||||
otherFiles.push(file)
|
||||
}
|
||||
}
|
||||
|
||||
// Load language parsers only for non-markdown and non-fallback files
|
||||
// Load language parsers only for non-markdown files
|
||||
const languageParsers = await loadRequiredLanguageParsers(otherFiles)
|
||||
|
||||
// Process markdown files
|
||||
|
|
@ -226,15 +215,6 @@ export async function parseSourceCodeForDefinitionsTopLevel(
|
|||
}
|
||||
}
|
||||
|
||||
// Process fallback files (e.g., Swift) without loading parsers
|
||||
for (const file of fallbackFiles) {
|
||||
// Check if we have permission to access this file
|
||||
if (rooIgnoreController && !rooIgnoreController.validateAccess(file)) {
|
||||
continue
|
||||
}
|
||||
result += `# ${path.relative(dirPath, file).toPosix()}\n// This file type uses fallback chunking for stability\n`
|
||||
}
|
||||
|
||||
// Process other files using tree-sitter
|
||||
for (const file of otherFiles) {
|
||||
const definitions = await parseFile(file, languageParsers, rooIgnoreController)
|
||||
|
|
|
|||
|
|
@ -1,6 +1,5 @@
|
|||
import * as path from "path"
|
||||
import { Parser as ParserT, Language as LanguageT, Query as QueryT } from "web-tree-sitter"
|
||||
import { shouldUseFallbackChunking } from "../code-index/shared/supported-extensions"
|
||||
import {
|
||||
javascriptQuery,
|
||||
typescriptQuery,
|
||||
|
|
@ -93,12 +92,6 @@ export async function loadRequiredLanguageParsers(filesToParse: string[], source
|
|||
const parsers: LanguageParser = {}
|
||||
|
||||
for (const ext of extensionsToLoad) {
|
||||
// Skip extensions that should use fallback chunking (e.g., Swift)
|
||||
if (shouldUseFallbackChunking(`.${ext}`)) {
|
||||
console.log(`Skipping parser load for .${ext} - using fallback chunking for stability`)
|
||||
continue
|
||||
}
|
||||
|
||||
let language: LanguageT
|
||||
let query: QueryT
|
||||
let parserKey = ext // Default to using extension as key
|
||||
|
|
@ -156,7 +149,10 @@ export async function loadRequiredLanguageParsers(filesToParse: string[], source
|
|||
language = await loadLanguage("php", sourceDirectory)
|
||||
query = new Query(language, phpQuery)
|
||||
break
|
||||
// Swift case removed - it uses fallback chunking for stability
|
||||
case "swift":
|
||||
language = await loadLanguage("swift", sourceDirectory)
|
||||
query = new Query(language, swiftQuery)
|
||||
break
|
||||
case "kt":
|
||||
case "kts":
|
||||
language = await loadLanguage("kotlin", sourceDirectory)
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue