Skip to content

Commit 4660545

Browse files
authored
Disk file system provider cannot delete empty folders (fix microsoft#181898) (microsoft#181899)
* Disk file system provider cannot delete empty folders (fix microsoft#181898) * forgot to throw original error * more tests * skip for symlinks
1 parent 3dd678d commit 4660545

File tree

2 files changed

+54
-9
lines changed

2 files changed

+54
-9
lines changed

src/vs/platform/files/node/diskFileSystemProvider.ts

Lines changed: 27 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -540,11 +540,36 @@ export class DiskFileSystemProvider extends AbstractDiskFileSystemProvider imple
540540
async delete(resource: URI, opts: IFileDeleteOptions): Promise<void> {
541541
try {
542542
const filePath = this.toFilePath(resource);
543-
544543
if (opts.recursive) {
545544
await Promises.rm(filePath, RimRafMode.MOVE);
546545
} else {
547-
await Promises.unlink(filePath);
546+
try {
547+
await Promises.unlink(filePath);
548+
} catch (unlinkError) {
549+
550+
// `fs.unlink` will throw when used on directories
551+
// we try to detect this error and then see if the
552+
// provided resource is actually a directory. in that
553+
// case we use `fs.rmdir` to delete the directory.
554+
555+
if (unlinkError.code === 'EPERM' || unlinkError.code === 'EISDIR') {
556+
let isDirectory = false;
557+
try {
558+
const { stat, symbolicLink } = await SymlinkSupport.stat(filePath);
559+
isDirectory = stat.isDirectory() && !symbolicLink;
560+
} catch (statError) {
561+
// ignore
562+
}
563+
564+
if (isDirectory) {
565+
await Promises.rmdir(filePath);
566+
} else {
567+
throw unlinkError;
568+
}
569+
} else {
570+
throw unlinkError;
571+
}
572+
}
548573
}
549574
} catch (error) {
550575
throw this.toFileSystemProviderError(error);

src/vs/platform/files/test/node/diskFileService.integrationTest.ts

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -489,23 +489,27 @@ flakySuite('Disk File Service', function () {
489489
assert.ok(result.ctime! > 0);
490490
});
491491

492-
test('deleteFile', async () => {
493-
return testDeleteFile(false);
492+
test('deleteFile (non recursive)', async () => {
493+
return testDeleteFile(false, false);
494+
});
495+
496+
test('deleteFile (recursive)', async () => {
497+
return testDeleteFile(false, true);
494498
});
495499

496500
(isLinux /* trash is unreliable on Linux */ ? test.skip : test)('deleteFile (useTrash)', async () => {
497-
return testDeleteFile(true);
501+
return testDeleteFile(true, false);
498502
});
499503

500-
async function testDeleteFile(useTrash: boolean): Promise<void> {
504+
async function testDeleteFile(useTrash: boolean, recursive: boolean): Promise<void> {
501505
let event: FileOperationEvent;
502506
disposables.add(service.onDidRunOperation(e => event = e));
503507

504508
const resource = URI.file(join(testDir, 'deep', 'conway.js'));
505509
const source = await service.resolve(resource);
506510

507-
assert.strictEqual(await service.canDelete(source.resource, { useTrash }), true);
508-
await service.del(source.resource, { useTrash });
511+
assert.strictEqual(await service.canDelete(source.resource, { useTrash, recursive }), true);
512+
await service.del(source.resource, { useTrash, recursive });
509513

510514
assert.strictEqual(existsSync(source.resource.fsPath), false);
511515

@@ -515,7 +519,7 @@ flakySuite('Disk File Service', function () {
515519

516520
let error: Error | undefined = undefined;
517521
try {
518-
await service.del(source.resource, { useTrash });
522+
await service.del(source.resource, { useTrash, recursive });
519523
} catch (e) {
520524
error = e;
521525
}
@@ -604,6 +608,22 @@ flakySuite('Disk File Service', function () {
604608
assert.ok(error);
605609
});
606610

611+
test('deleteFolder empty folder (recursive)', () => {
612+
return testDeleteEmptyFolder(true);
613+
});
614+
615+
test('deleteFolder empty folder (non recursive)', () => {
616+
return testDeleteEmptyFolder(false);
617+
});
618+
619+
async function testDeleteEmptyFolder(recursive: boolean): Promise<void> {
620+
const { resource } = await service.createFolder(URI.file(join(testDir, 'deep', 'empty')));
621+
622+
await service.del(resource, { recursive });
623+
624+
assert.strictEqual(await service.exists(resource), false);
625+
}
626+
607627
test('move', async () => {
608628
let event: FileOperationEvent;
609629
disposables.add(service.onDidRunOperation(e => event = e));

0 commit comments

Comments
 (0)