diff --git a/src/services/glob/__tests__/gitignore-integration.spec.ts b/src/services/glob/__tests__/gitignore-integration.spec.ts new file mode 100644 index 0000000000..1361816b17 --- /dev/null +++ b/src/services/glob/__tests__/gitignore-integration.spec.ts @@ -0,0 +1,209 @@ +import { vi, describe, it, expect, beforeEach, afterEach } from "vitest" +import * as path from "path" +import * as fs from "fs" +import * as os from "os" + +// Mock ripgrep to avoid filesystem dependencies +vi.mock("../../ripgrep", () => ({ + getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"), +})) + +// Mock vscode +vi.mock("vscode", () => ({ + env: { + appRoot: "/mock/app/root", + }, +})) + +// Mock child_process to simulate ripgrep behavior +vi.mock("child_process", () => ({ + spawn: vi.fn(), +})) + +vi.mock("../../path", () => ({ + arePathsEqual: vi.fn().mockReturnValue(false), +})) + +import { listFiles } from "../list-files" +import * as childProcess from "child_process" + +describe("list-files gitignore integration", () => { + let tempDir: string + let originalCwd: string + + beforeEach(async () => { + vi.clearAllMocks() + + // Create a temporary directory for testing + tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "roo-gitignore-test-")) + originalCwd = process.cwd() + }) + + afterEach(async () => { + process.chdir(originalCwd) + // Clean up temp directory + await fs.promises.rm(tempDir, { recursive: true, force: true }) + }) + + it("should properly filter directories based on .gitignore patterns", async () => { + // Setup test directory structure + await fs.promises.mkdir(path.join(tempDir, "src")) + await fs.promises.mkdir(path.join(tempDir, "node_modules")) + await fs.promises.mkdir(path.join(tempDir, "build")) + await fs.promises.mkdir(path.join(tempDir, "dist")) + await fs.promises.mkdir(path.join(tempDir, "allowed-dir")) + + // Create .gitignore file + await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\nbuild/\ndist/\n*.log\n") + + // Create some files + await fs.promises.writeFile(path.join(tempDir, "src", "index.ts"), "console.log('hello')") + await fs.promises.writeFile(path.join(tempDir, "allowed-dir", "file.txt"), "content") + + // Mock ripgrep to return files that would not be gitignored + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + // Simulate ripgrep output (files that are not gitignored) + const files = + [path.join(tempDir, "src", "index.ts"), path.join(tempDir, "allowed-dir", "file.txt")].join( + "\n", + ) + "\n" + setTimeout(() => callback(files), 10) + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 20) + } + }), + kill: vi.fn(), + } + + mockSpawn.mockReturnValue(mockProcess as any) + + // Call listFiles in recursive mode + const [files, didHitLimit] = await listFiles(tempDir, true, 100) + + // Filter out only directories from the results + const directoriesInResult = files.filter((f) => f.endsWith("/")) + + // Verify that gitignored directories are NOT included + expect(directoriesInResult).not.toContain(path.join(tempDir, "node_modules") + "/") + expect(directoriesInResult).not.toContain(path.join(tempDir, "build") + "/") + expect(directoriesInResult).not.toContain(path.join(tempDir, "dist") + "/") + + // Verify that allowed directories ARE included + expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/") + expect(directoriesInResult).toContain(path.join(tempDir, "allowed-dir") + "/") + }) + + it("should handle nested .gitignore files correctly", async () => { + // Setup nested directory structure + await fs.promises.mkdir(path.join(tempDir, "src"), { recursive: true }) + await fs.promises.mkdir(path.join(tempDir, "src", "components")) + await fs.promises.mkdir(path.join(tempDir, "src", "temp")) + await fs.promises.mkdir(path.join(tempDir, "src", "utils")) + + // Create root .gitignore + await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\n") + + // Create nested .gitignore in src/ + await fs.promises.writeFile(path.join(tempDir, "src", ".gitignore"), "temp/\n") + + // Mock ripgrep + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + setTimeout(() => callback(""), 10) + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 20) + } + }), + kill: vi.fn(), + } + + mockSpawn.mockReturnValue(mockProcess as any) + + // Call listFiles in recursive mode + const [files, didHitLimit] = await listFiles(tempDir, true, 100) + + // Filter out only directories from the results + const directoriesInResult = files.filter((f) => f.endsWith("/")) + + // Verify that nested gitignored directories are NOT included + expect(directoriesInResult).not.toContain(path.join(tempDir, "src", "temp") + "/") + + // Verify that allowed directories ARE included + expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/") + expect(directoriesInResult).toContain(path.join(tempDir, "src", "components") + "/") + expect(directoriesInResult).toContain(path.join(tempDir, "src", "utils") + "/") + }) + + it("should respect .gitignore in non-recursive mode too", async () => { + // Setup test directory structure + await fs.promises.mkdir(path.join(tempDir, "src")) + await fs.promises.mkdir(path.join(tempDir, "node_modules")) + await fs.promises.mkdir(path.join(tempDir, "allowed-dir")) + + // Create .gitignore file + await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\n") + + // Mock ripgrep for non-recursive mode + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + // In non-recursive mode, ripgrep should now respect .gitignore + const files = [path.join(tempDir, "src"), path.join(tempDir, "allowed-dir")].join("\n") + "\n" + setTimeout(() => callback(files), 10) + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 20) + } + }), + kill: vi.fn(), + } + + mockSpawn.mockReturnValue(mockProcess as any) + + // Call listFiles in NON-recursive mode + const [files, didHitLimit] = await listFiles(tempDir, false, 100) + + // Verify ripgrep was called without --no-ignore-vcs (should respect .gitignore) + const [rgPath, args] = mockSpawn.mock.calls[0] + expect(args).not.toContain("--no-ignore-vcs") + + // Filter out only directories from the results + const directoriesInResult = files.filter((f) => f.endsWith("/")) + + // Verify that gitignored directories are NOT included even in non-recursive mode + expect(directoriesInResult).not.toContain(path.join(tempDir, "node_modules") + "/") + + // Verify that allowed directories ARE included + expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/") + expect(directoriesInResult).toContain(path.join(tempDir, "allowed-dir") + "/") + }) +}) diff --git a/src/services/glob/__tests__/gitignore-test.spec.ts b/src/services/glob/__tests__/gitignore-test.spec.ts new file mode 100644 index 0000000000..cef884b584 --- /dev/null +++ b/src/services/glob/__tests__/gitignore-test.spec.ts @@ -0,0 +1,147 @@ +import { vi, describe, it, expect, beforeEach, afterEach } from "vitest" +import * as path from "path" +import * as fs from "fs" +import * as os from "os" + +// Mock ripgrep to avoid filesystem dependencies +vi.mock("../../ripgrep", () => ({ + getBinPath: vi.fn().mockResolvedValue("/mock/path/to/rg"), +})) + +// Mock vscode +vi.mock("vscode", () => ({ + env: { + appRoot: "/mock/app/root", + }, +})) + +vi.mock("child_process", () => ({ + spawn: vi.fn(), +})) + +vi.mock("../../path", () => ({ + arePathsEqual: vi.fn().mockReturnValue(false), +})) + +import { listFiles } from "../list-files" +import * as childProcess from "child_process" + +describe("list-files gitignore support", () => { + let tempDir: string + let originalCwd: string + + beforeEach(async () => { + vi.clearAllMocks() + + // Create a temporary directory for testing + tempDir = await fs.promises.mkdtemp(path.join(os.tmpdir(), "roo-test-")) + originalCwd = process.cwd() + process.chdir(tempDir) + }) + + afterEach(async () => { + process.chdir(originalCwd) + // Clean up temp directory + await fs.promises.rm(tempDir, { recursive: true, force: true }) + }) + + it("should respect .gitignore patterns for directories in recursive mode", async () => { + // Setup test directory structure + await fs.promises.mkdir(path.join(tempDir, "src")) + await fs.promises.mkdir(path.join(tempDir, "node_modules")) + await fs.promises.mkdir(path.join(tempDir, "build")) + await fs.promises.mkdir(path.join(tempDir, "ignored-dir")) + + // Create .gitignore file + await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\nbuild/\nignored-dir/\n") + + // Create some files + await fs.promises.writeFile(path.join(tempDir, "src", "index.ts"), "") + await fs.promises.writeFile(path.join(tempDir, "node_modules", "package.json"), "") + await fs.promises.writeFile(path.join(tempDir, "build", "output.js"), "") + await fs.promises.writeFile(path.join(tempDir, "ignored-dir", "file.txt"), "") + + // Mock ripgrep to return only non-ignored files + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + // Ripgrep should respect .gitignore and only return src/index.ts + setTimeout(() => callback(`${path.join(tempDir, "src", "index.ts")}\n`), 10) + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 20) + } + }), + kill: vi.fn(), + } + + mockSpawn.mockReturnValue(mockProcess as any) + + // Call listFiles in recursive mode + const [files, didHitLimit] = await listFiles(tempDir, true, 100) + + // Verify that gitignored directories are not included + const directoriesInResult = files.filter((f) => f.endsWith("/")) + + expect(directoriesInResult).not.toContain(path.join(tempDir, "node_modules") + "/") + expect(directoriesInResult).not.toContain(path.join(tempDir, "build") + "/") + expect(directoriesInResult).not.toContain(path.join(tempDir, "ignored-dir") + "/") + + // But src/ should be included + expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/") + }) + + it("should handle nested .gitignore files", async () => { + // Setup nested directory structure + await fs.promises.mkdir(path.join(tempDir, "src"), { recursive: true }) + await fs.promises.mkdir(path.join(tempDir, "src", "components")) + await fs.promises.mkdir(path.join(tempDir, "src", "temp")) + + // Create root .gitignore + await fs.promises.writeFile(path.join(tempDir, ".gitignore"), "node_modules/\n") + + // Create nested .gitignore in src/ + await fs.promises.writeFile(path.join(tempDir, "src", ".gitignore"), "temp/\n") + + // Mock ripgrep + const mockSpawn = vi.mocked(childProcess.spawn) + const mockProcess = { + stdout: { + on: vi.fn((event, callback) => { + if (event === "data") { + setTimeout(() => callback(""), 10) + } + }), + }, + stderr: { + on: vi.fn(), + }, + on: vi.fn((event, callback) => { + if (event === "close") { + setTimeout(() => callback(0), 20) + } + }), + kill: vi.fn(), + } + + mockSpawn.mockReturnValue(mockProcess as any) + + // Call listFiles in recursive mode + const [files, didHitLimit] = await listFiles(tempDir, true, 100) + + // Verify that nested gitignored directories are not included + const directoriesInResult = files.filter((f) => f.endsWith("/")) + + expect(directoriesInResult).not.toContain(path.join(tempDir, "src", "temp") + "/") + expect(directoriesInResult).toContain(path.join(tempDir, "src") + "/") + expect(directoriesInResult).toContain(path.join(tempDir, "src", "components") + "/") + }) +}) diff --git a/src/services/glob/list-files.ts b/src/services/glob/list-files.ts index 0dc2a06f1e..9245915ce1 100644 --- a/src/services/glob/list-files.ts +++ b/src/services/glob/list-files.ts @@ -3,6 +3,7 @@ import * as path from "path" import * as fs from "fs" import * as childProcess from "child_process" import * as vscode from "vscode" +import ignore from "ignore" import { arePathsEqual } from "../../utils/path" import { getBinPath } from "../../services/ripgrep" import { DIRS_TO_IGNORE } from "./constants" @@ -29,9 +30,9 @@ export async function listFiles(dirPath: string, recursive: boolean, limit: numb // Get files using ripgrep const files = await listFilesWithRipgrep(rgPath, dirPath, recursive, limit) - // Get directories with proper filtering - const gitignorePatterns = await parseGitignoreFile(dirPath, recursive) - const directories = await listFilteredDirectories(dirPath, recursive, gitignorePatterns) + // Get directories with proper filtering using ignore library + const ignoreInstance = await createIgnoreInstance(dirPath) + const directories = await listFilteredDirectories(dirPath, recursive, ignoreInstance) // Combine and format the results return formatAndCombineResults(files, directories, limit) @@ -129,8 +130,8 @@ function buildNonRecursiveArgs(): string[] { args.push("-g", "*") args.push("--maxdepth", "1") // ripgrep uses maxdepth, not max-depth - // Don't respect .gitignore in non-recursive mode (consistent with original behavior) - args.push("--no-ignore-vcs") + // Respect .gitignore in non-recursive mode too + // (ripgrep respects .gitignore by default) // Apply directory exclusions for non-recursive searches for (const dir of DIRS_TO_IGNORE) { @@ -148,37 +149,61 @@ function buildNonRecursiveArgs(): string[] { } /** - * Parse the .gitignore file if it exists and is relevant + * Create an ignore instance that handles .gitignore files properly + * This replaces the custom gitignore parsing with the proper ignore library */ -async function parseGitignoreFile(dirPath: string, recursive: boolean): Promise { - if (!recursive) { - return [] // Only needed for recursive mode +async function createIgnoreInstance(dirPath: string): Promise> { + const ignoreInstance = ignore() + const absolutePath = path.resolve(dirPath) + + // Find all .gitignore files from the target directory up to the root + const gitignoreFiles = await findGitignoreFiles(absolutePath) + + // Add patterns from all .gitignore files + for (const gitignoreFile of gitignoreFiles) { + try { + const content = await fs.promises.readFile(gitignoreFile, "utf8") + ignoreInstance.add(content) + } catch (err) { + // Continue if we can't read a .gitignore file + console.warn(`Error reading .gitignore at ${gitignoreFile}: ${err}`) + } } - const absolutePath = path.resolve(dirPath) - const gitignorePath = path.join(absolutePath, ".gitignore") + // Always ignore .gitignore files themselves + ignoreInstance.add(".gitignore") + + return ignoreInstance +} + +/** + * Find all .gitignore files from the given directory up to the workspace root + */ +async function findGitignoreFiles(startPath: string): Promise { + const gitignoreFiles: string[] = [] + let currentPath = startPath - try { - // Check if .gitignore exists - const exists = await fs.promises - .access(gitignorePath) - .then(() => true) - .catch(() => false) + // Walk up the directory tree looking for .gitignore files + while (currentPath && currentPath !== path.dirname(currentPath)) { + const gitignorePath = path.join(currentPath, ".gitignore") - if (!exists) { - return [] + try { + await fs.promises.access(gitignorePath) + gitignoreFiles.push(gitignorePath) + } catch { + // .gitignore doesn't exist at this level, continue } - // Read and parse .gitignore file - const content = await fs.promises.readFile(gitignorePath, "utf8") - return content - .split("\n") - .map((line) => line.trim()) - .filter((line) => line && !line.startsWith("#")) - } catch (err) { - console.warn(`Error reading .gitignore: ${err}`) - return [] // Continue without gitignore patterns on error + // Move up one directory + const parentPath = path.dirname(currentPath) + if (parentPath === currentPath) { + break // Reached root + } + currentPath = parentPath } + + // Return in reverse order (root .gitignore first, then more specific ones) + return gitignoreFiles.reverse() } /** @@ -187,7 +212,7 @@ async function parseGitignoreFile(dirPath: string, recursive: boolean): Promise< async function listFilteredDirectories( dirPath: string, recursive: boolean, - gitignorePatterns: string[], + ignoreInstance: ReturnType, ): Promise { const absolutePath = path.resolve(dirPath) const directories: string[] = [] @@ -204,7 +229,7 @@ async function listFilteredDirectories( const fullDirPath = path.join(currentPath, dirName) // Check if this directory should be included - if (shouldIncludeDirectory(dirName, recursive, gitignorePatterns)) { + if (shouldIncludeDirectory(dirName, fullDirPath, dirPath, ignoreInstance)) { // Add the directory to our results (with trailing slash) const formattedPath = fullDirPath.endsWith("/") ? fullDirPath : `${fullDirPath}/` directories.push(formattedPath) @@ -231,7 +256,12 @@ async function listFilteredDirectories( /** * Determine if a directory should be included in results based on filters */ -function shouldIncludeDirectory(dirName: string, recursive: boolean, gitignorePatterns: string[]): boolean { +function shouldIncludeDirectory( + dirName: string, + fullDirPath: string, + basePath: string, + ignoreInstance: ReturnType, +): boolean { // Skip hidden directories if configured to ignore them if (dirName.startsWith(".") && DIRS_TO_IGNORE.includes(".*")) { return false @@ -242,8 +272,13 @@ function shouldIncludeDirectory(dirName: string, recursive: boolean, gitignorePa return false } - // Check against gitignore patterns in recursive mode - if (recursive && gitignorePatterns.length > 0 && isIgnoredByGitignore(dirName, gitignorePatterns)) { + // Check against gitignore patterns using the ignore library + // Calculate relative path from the base directory + const relativePath = path.relative(basePath, fullDirPath) + const normalizedPath = relativePath.replace(/\\/g, "/") + + // Check if the directory is ignored by .gitignore + if (ignoreInstance.ignores(normalizedPath) || ignoreInstance.ignores(normalizedPath + "/")) { return false } @@ -272,37 +307,6 @@ function isDirectoryExplicitlyIgnored(dirName: string): boolean { return false } -/** - * Check if a directory matches any gitignore patterns - */ -function isIgnoredByGitignore(dirName: string, gitignorePatterns: string[]): boolean { - for (const pattern of gitignorePatterns) { - // Directory patterns (ending with /) - if (pattern.endsWith("/")) { - const dirPattern = pattern.slice(0, -1) - if (dirName === dirPattern) { - return true - } - if (pattern.startsWith("**/") && dirName === dirPattern.slice(3)) { - return true - } - } - // Simple name patterns - else if (dirName === pattern) { - return true - } - // Wildcard patterns - else if (pattern.includes("*")) { - const regexPattern = pattern.replace(/\\/g, "\\\\").replace(/\./g, "\\.").replace(/\*/g, ".*") - const regex = new RegExp(`^${regexPattern}$`) - if (regex.test(dirName)) { - return true - } - } - } - - return false -} /** * Combine file and directory results and format them properly