Skip to content

Commit f3cf443

Browse files
committed
fix: handle list_files for paths inside ignored directories
- Add checkPathForIgnoredSegments helper to detect if a path contains any ignored directory segments - Modify buildRecursiveArgs to treat paths inside ignored directories (e.g., node_modules/@types/) the same as explicitly targeting those directories - Apply same logic to buildNonRecursiveArgs for consistency - Add comprehensive tests for node_modules/@types scenarios This fixes the issue where list_files would return only directories when targeting paths inside ignored ancestors like node_modules/@types/stream-json. Fixes #4263
1 parent 2263d86 commit f3cf443

File tree

2 files changed

+311
-40
lines changed

2 files changed

+311
-40
lines changed

src/services/glob/__tests__/list-files.spec.ts

Lines changed: 240 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -558,3 +558,243 @@ describe("buildRecursiveArgs edge cases", () => {
558558
expect(args).not.toContain("--no-ignore")
559559
})
560560
})
561+
562+
describe("node_modules/@types scenarios", () => {
563+
beforeEach(() => {
564+
vi.clearAllMocks()
565+
})
566+
567+
it("should include files when targeting paths inside node_modules", async () => {
568+
const mockSpawn = vi.mocked(childProcess.spawn)
569+
const mockProcess = {
570+
stdout: {
571+
on: vi.fn((event, callback) => {
572+
if (event === "data") {
573+
// Simulate files that should be found in node_modules/@types/stream-json
574+
setTimeout(() => {
575+
callback("index.d.ts\n")
576+
callback("Parser.d.ts\n")
577+
callback("StreamBase.d.ts\n")
578+
}, 10)
579+
}
580+
}),
581+
},
582+
stderr: {
583+
on: vi.fn(),
584+
},
585+
on: vi.fn((event, callback) => {
586+
if (event === "close") {
587+
setTimeout(() => callback(0), 20)
588+
}
589+
}),
590+
kill: vi.fn(),
591+
}
592+
mockSpawn.mockReturnValue(mockProcess as any)
593+
594+
// Mock directory listing for node_modules/@types/stream-json
595+
const mockReaddir = vi.fn()
596+
vi.mocked(fs.promises).readdir = mockReaddir
597+
mockReaddir.mockResolvedValueOnce([])
598+
599+
// Call listFiles targeting node_modules/@types/stream-json
600+
const [files] = await listFiles("/test/node_modules/@types/stream-json", true, 100)
601+
602+
// Verify ripgrep was called with correct arguments
603+
const [rgPath, args] = mockSpawn.mock.calls[0]
604+
605+
// When targeting a path inside node_modules, these flags should be present
606+
expect(args).toContain("--no-ignore-vcs")
607+
expect(args).toContain("--no-ignore")
608+
609+
// Check for the inclusion patterns
610+
expect(args).toContain("-g")
611+
const gIndex = args.indexOf("-g")
612+
expect(args[gIndex + 1]).toBe("*")
613+
614+
// Verify that files are included in the results
615+
const fileNames = files.map((f) => path.basename(f))
616+
expect(fileNames).toContain("index.d.ts")
617+
expect(fileNames).toContain("Parser.d.ts")
618+
expect(fileNames).toContain("StreamBase.d.ts")
619+
})
620+
621+
it("should include files when targeting nested paths inside ignored directories", async () => {
622+
const mockSpawn = vi.mocked(childProcess.spawn)
623+
const mockProcess = {
624+
stdout: {
625+
on: vi.fn((event, callback) => {
626+
if (event === "data") {
627+
// Simulate files in a deeply nested ignored path
628+
setTimeout(() => {
629+
callback("lib/index.js\n")
630+
callback("lib/utils.js\n")
631+
callback("package.json\n")
632+
}, 10)
633+
}
634+
}),
635+
},
636+
stderr: {
637+
on: vi.fn(),
638+
},
639+
on: vi.fn((event, callback) => {
640+
if (event === "close") {
641+
setTimeout(() => callback(0), 20)
642+
}
643+
}),
644+
kill: vi.fn(),
645+
}
646+
mockSpawn.mockReturnValue(mockProcess as any)
647+
648+
// Mock directory listing
649+
const mockReaddir = vi.fn()
650+
vi.mocked(fs.promises).readdir = mockReaddir
651+
mockReaddir.mockResolvedValueOnce([{ name: "lib", isDirectory: () => true, isSymbolicLink: () => false }])
652+
653+
// Call listFiles targeting a path deep inside node_modules
654+
const [files] = await listFiles("/test/node_modules/some-package/dist", true, 100)
655+
656+
// Verify ripgrep was called with correct arguments
657+
const [rgPath, args] = mockSpawn.mock.calls[0]
658+
659+
// Should have the special flags for ignored paths
660+
expect(args).toContain("--no-ignore-vcs")
661+
expect(args).toContain("--no-ignore")
662+
663+
// Verify files are included
664+
const fileNames = files.map((f) => path.basename(f))
665+
expect(fileNames).toContain("index.js")
666+
expect(fileNames).toContain("utils.js")
667+
expect(fileNames).toContain("package.json")
668+
})
669+
670+
it("should handle non-recursive listing inside ignored directories", async () => {
671+
const mockSpawn = vi.mocked(childProcess.spawn)
672+
const mockProcess = {
673+
stdout: {
674+
on: vi.fn((event, callback) => {
675+
if (event === "data") {
676+
// Simulate files at the root of node_modules/@types
677+
setTimeout(() => {
678+
callback("stream-json/\n")
679+
callback("node/\n")
680+
}, 10)
681+
}
682+
}),
683+
},
684+
stderr: {
685+
on: vi.fn(),
686+
},
687+
on: vi.fn((event, callback) => {
688+
if (event === "close") {
689+
setTimeout(() => callback(0), 20)
690+
}
691+
}),
692+
kill: vi.fn(),
693+
}
694+
mockSpawn.mockReturnValue(mockProcess as any)
695+
696+
// Mock directory listing
697+
const mockReaddir = vi.fn()
698+
vi.mocked(fs.promises).readdir = mockReaddir
699+
mockReaddir.mockResolvedValueOnce([
700+
{ name: "stream-json", isDirectory: () => true, isSymbolicLink: () => false },
701+
{ name: "node", isDirectory: () => true, isSymbolicLink: () => false },
702+
])
703+
704+
// Call listFiles non-recursively inside node_modules/@types
705+
const [files] = await listFiles("/test/node_modules/@types", false, 100)
706+
707+
// Verify ripgrep was called with correct arguments
708+
const [rgPath, args] = mockSpawn.mock.calls[0]
709+
710+
// Should have the special flags for ignored paths
711+
expect(args).toContain("--no-ignore-vcs")
712+
expect(args).toContain("--no-ignore")
713+
expect(args).toContain("--maxdepth")
714+
expect(args).toContain("1")
715+
716+
// Verify directories are included
717+
const dirNames = files
718+
.filter((f) => f.endsWith("/"))
719+
.map((f) => {
720+
const parts = f.split("/")
721+
return parts[parts.length - 2] + "/"
722+
})
723+
expect(dirNames).toContain("stream-json/")
724+
expect(dirNames).toContain("node/")
725+
})
726+
727+
it("should not apply node_modules exclusion to paths containing node_modules", async () => {
728+
const mockSpawn = vi.mocked(childProcess.spawn)
729+
const mockProcess = {
730+
stdout: {
731+
on: vi.fn((event, callback) => {
732+
if (event === "data") {
733+
setTimeout(() => callback("file.txt\n"), 10)
734+
}
735+
}),
736+
},
737+
stderr: {
738+
on: vi.fn(),
739+
},
740+
on: vi.fn((event, callback) => {
741+
if (event === "close") {
742+
setTimeout(() => callback(0), 20)
743+
}
744+
}),
745+
kill: vi.fn(),
746+
}
747+
mockSpawn.mockReturnValue(mockProcess as any)
748+
749+
// Test with a path containing node_modules
750+
await listFiles("/test/node_modules/package/src", true, 100)
751+
752+
const [rgPath, args] = mockSpawn.mock.calls[0]
753+
754+
// Should NOT have the node_modules exclusion pattern since we're inside node_modules
755+
expect(args).not.toContain("!**/node_modules/**")
756+
757+
// Should have the flags to ignore gitignore rules
758+
expect(args).toContain("--no-ignore-vcs")
759+
expect(args).toContain("--no-ignore")
760+
})
761+
762+
it("should handle paths with multiple ignored segments correctly", async () => {
763+
const mockSpawn = vi.mocked(childProcess.spawn)
764+
const mockProcess = {
765+
stdout: {
766+
on: vi.fn((event, callback) => {
767+
if (event === "data") {
768+
setTimeout(() => callback("output.js\n"), 10)
769+
}
770+
}),
771+
},
772+
stderr: {
773+
on: vi.fn(),
774+
},
775+
on: vi.fn((event, callback) => {
776+
if (event === "close") {
777+
setTimeout(() => callback(0), 20)
778+
}
779+
}),
780+
kill: vi.fn(),
781+
}
782+
mockSpawn.mockReturnValue(mockProcess as any)
783+
784+
// Test with a path containing multiple ignored segments
785+
await listFiles("/test/node_modules/package/dist/bundle", true, 100)
786+
787+
const [rgPath, args] = mockSpawn.mock.calls[0]
788+
789+
// Should have the special handling flags
790+
expect(args).toContain("--no-ignore-vcs")
791+
expect(args).toContain("--no-ignore")
792+
793+
// Should not exclude the first ignored segment found (node_modules)
794+
expect(args).not.toContain("!**/node_modules/**")
795+
796+
// But should still exclude other ignored directories not in the path
797+
expect(args).toContain("!**/__pycache__/**")
798+
expect(args).toContain("!**/venv/**")
799+
})
800+
})

0 commit comments

Comments
 (0)