minor #17985 [Filesystem] Reduce complexity of ->remove() (nicolas-grekas)

This PR was merged into the 2.3 branch.

Discussion
----------

[Filesystem] Reduce complexity of ->remove()

| Q             | A
| ------------- | ---
| Branch        | 2.3
| Bug fix?      | no
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | no
| Fixed tickets | -
| License       | MIT
| Doc PR        | -

Commits
-------

065acb7 [Filesystem] Reduce complexity of ->remove()
This commit is contained in:
Fabien Potencier 2016-03-02 15:50:41 +01:00
commit 9a4bf1e099
2 changed files with 30 additions and 37 deletions

View File

@ -143,34 +143,23 @@ class Filesystem
$files = iterator_to_array($this->toIterator($files)); $files = iterator_to_array($this->toIterator($files));
$files = array_reverse($files); $files = array_reverse($files);
foreach ($files as $file) { foreach ($files as $file) {
if (!$this->exists($file) && !is_link($file)) { if (is_link($file)) {
continue; // Workaround https://bugs.php.net/52176
} if (!@unlink($file) && !@rmdir($file)) {
$error = error_get_last();
if (is_dir($file) && !is_link($file)) { throw new IOException(sprintf('Failed to remove symlink "%s": %s.', $file, $error['message']));
}
} elseif (is_dir($file)) {
$this->remove(new \FilesystemIterator($file)); $this->remove(new \FilesystemIterator($file));
if (true !== @rmdir($file)) { if (!@rmdir($file)) {
throw new IOException(sprintf('Failed to remove directory %s', $file)); $error = error_get_last();
throw new IOException(sprintf('Failed to remove directory "%s": %s.', $file, $error['message']));
} }
} else { } elseif ($this->exists($file)) {
// https://bugs.php.net/bug.php?id=52176 if (!@unlink($file)) {
if ('\\' === DIRECTORY_SEPARATOR && is_dir($file)) { $error = error_get_last();
if (true !== @rmdir($file)) { throw new IOException(sprintf('Failed to remove file "%s": %s.', $file, $error['message']));
throw new IOException(sprintf('Failed to remove file %s', $file));
}
} else {
if (true !== @unlink($file)) {
// handle broken symlinks on Windows systems
if (is_link($file) && false === @readlink($file)) {
if (true !== @rmdir($file)) {
throw new IOException(sprintf('Failed to remove broken symlink "%s".', $file), 0, null, $file);
}
} else {
$error = error_get_last();
throw new IOException(sprintf('Failed to remove file "%s": %s.', $file, $error['message']));
}
}
} }
} }
} }

View File

@ -281,7 +281,7 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
$this->filesystem->remove($basePath); $this->filesystem->remove($basePath);
$this->assertTrue(!is_dir($basePath)); $this->assertFileNotExists($basePath);
} }
public function testRemoveCleansArrayOfFilesAndDirectories() public function testRemoveCleansArrayOfFilesAndDirectories()
@ -297,8 +297,8 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
$this->filesystem->remove($files); $this->filesystem->remove($files);
$this->assertTrue(!is_dir($basePath.'dir')); $this->assertFileNotExists($basePath.'dir');
$this->assertTrue(!is_file($basePath.'file')); $this->assertFileNotExists($basePath.'file');
} }
public function testRemoveCleansTraversableObjectOfFilesAndDirectories() public function testRemoveCleansTraversableObjectOfFilesAndDirectories()
@ -314,8 +314,8 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
$this->filesystem->remove($files); $this->filesystem->remove($files);
$this->assertTrue(!is_dir($basePath.'dir')); $this->assertFileNotExists($basePath.'dir');
$this->assertTrue(!is_file($basePath.'file')); $this->assertFileNotExists($basePath.'file');
} }
public function testRemoveIgnoresNonExistingFiles() public function testRemoveIgnoresNonExistingFiles()
@ -330,7 +330,7 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
$this->filesystem->remove($files); $this->filesystem->remove($files);
$this->assertTrue(!is_dir($basePath.'dir')); $this->assertFileNotExists($basePath.'dir');
} }
public function testRemoveCleansInvalidLinks() public function testRemoveCleansInvalidLinks()
@ -342,11 +342,19 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
mkdir($basePath); mkdir($basePath);
mkdir($basePath.'dir'); mkdir($basePath.'dir');
// create symlink to nonexistent file // create symlink to nonexistent file
@symlink($basePath.'file', $basePath.'link'); @symlink($basePath.'file', $basePath.'file-link');
// create symlink to dir using trailing forward slash
$this->filesystem->symlink($basePath.'dir/', $basePath.'dir-link');
$this->assertTrue(is_dir($basePath.'dir-link'));
// create symlink to nonexistent dir
rmdir($basePath.'dir');
$this->assertFalse(is_dir($basePath.'dir-link'));
$this->filesystem->remove($basePath); $this->filesystem->remove($basePath);
$this->assertTrue(!is_dir($basePath)); $this->assertFileNotExists($basePath);
} }
public function testFilesExists() public function testFilesExists()
@ -1062,10 +1070,6 @@ class FilesystemTest extends \PHPUnit_Framework_TestCase
*/ */
private function markAsSkippedIfSymlinkIsMissing($relative = false) private function markAsSkippedIfSymlinkIsMissing($relative = false)
{ {
if (!function_exists('symlink')) {
$this->markTestSkipped('symlink is not supported');
}
if (false === self::$symlinkOnWindows) { if (false === self::$symlinkOnWindows) {
$this->markTestSkipped('symlink requires "Create symbolic links" privilege on Windows'); $this->markTestSkipped('symlink requires "Create symbolic links" privilege on Windows');
} }