From c2e43d0aa4f361f034912ef72849c26f289a960e Mon Sep 17 00:00:00 2001 From: Fabien Potencier Date: Fri, 27 Sep 2013 16:53:24 +0200 Subject: [PATCH] [Filesystem] removed getPath() on Exceptions and cleaned up CS and error messages --- .../Exception/ExceptionInterface.php | 1 - .../Exception/FileNotFoundException.php | 15 ++-- .../Filesystem/Exception/IOException.php | 10 +-- .../Exception/IOExceptionInterface.php | 2 +- .../Component/Filesystem/Filesystem.php | 79 +++++-------------- .../Filesystem/Tests/ExceptionTest.php | 27 +++---- 6 files changed, 46 insertions(+), 88 deletions(-) diff --git a/src/Symfony/Component/Filesystem/Exception/ExceptionInterface.php b/src/Symfony/Component/Filesystem/Exception/ExceptionInterface.php index bc9748d752e7..c212e664d487 100644 --- a/src/Symfony/Component/Filesystem/Exception/ExceptionInterface.php +++ b/src/Symfony/Component/Filesystem/Exception/ExceptionInterface.php @@ -20,5 +20,4 @@ */ interface ExceptionInterface { - } diff --git a/src/Symfony/Component/Filesystem/Exception/FileNotFoundException.php b/src/Symfony/Component/Filesystem/Exception/FileNotFoundException.php index 242ff070e508..15533db40896 100644 --- a/src/Symfony/Component/Filesystem/Exception/FileNotFoundException.php +++ b/src/Symfony/Component/Filesystem/Exception/FileNotFoundException.php @@ -14,18 +14,21 @@ /** * Exception class thrown when a file couldn't be found * + * @author Fabien Potencier * @author Christian Gärtner */ class FileNotFoundException extends IOException { - public function __construct($path, $message = null, $code = 0, \Exception $previous = null) + public function __construct($message = null, $code = 0, \Exception $previous = null, $path = null) { - if ($message === null) { - $message = sprintf('File "%s" could not be found', $path); + if (null === $message) { + if (null === $path) { + $message = 'File could not be found.'; + } else { + $message = sprintf('File "%s" could not be found.', $path); + } } - $this->setPath($path); - - parent::__construct($message, $code, $previous); + parent::__construct($message, $code, $previous, $path); } } diff --git a/src/Symfony/Component/Filesystem/Exception/IOException.php b/src/Symfony/Component/Filesystem/Exception/IOException.php index e9f12371da61..4b551af71b76 100644 --- a/src/Symfony/Component/Filesystem/Exception/IOException.php +++ b/src/Symfony/Component/Filesystem/Exception/IOException.php @@ -16,21 +16,19 @@ * * @author Romain Neutron * @author Christian Gärtner + * @author Fabien Potencier * * @api */ class IOException extends \RuntimeException implements IOExceptionInterface { - private $path; - /** - * Set the path associated with this IOException - * @param string $path The path - */ - public function setPath($path) + public function __construct($message, $code = 0, \Exception $previous = null, $path = null) { $this->path = $path; + + parent::__construct($message, $code, $previous); } /** diff --git a/src/Symfony/Component/Filesystem/Exception/IOExceptionInterface.php b/src/Symfony/Component/Filesystem/Exception/IOExceptionInterface.php index 41d8d3bdee26..de9f3e714a3b 100644 --- a/src/Symfony/Component/Filesystem/Exception/IOExceptionInterface.php +++ b/src/Symfony/Component/Filesystem/Exception/IOExceptionInterface.php @@ -20,7 +20,7 @@ interface IOExceptionInterface extends ExceptionInterface { /** * Returns the associated path for the exception - * + * * @return string The path. */ public function getPath(); diff --git a/src/Symfony/Component/Filesystem/Filesystem.php b/src/Symfony/Component/Filesystem/Filesystem.php index f591780ff75c..9e45d2ba3b18 100644 --- a/src/Symfony/Component/Filesystem/Filesystem.php +++ b/src/Symfony/Component/Filesystem/Filesystem.php @@ -38,7 +38,7 @@ class Filesystem public function copy($originFile, $targetFile, $override = false) { if (stream_is_local($originFile) && !is_file($originFile)) { - throw new FileNotFoundException($originFile, sprintf('Failed to copy %s because file does not exist', $originFile)); + throw new FileNotFoundException(sprintf('Failed to copy "%s" because file does not exist.', $originFile), 0, null, $originFile); } $this->mkdir(dirname($targetFile)); @@ -59,9 +59,7 @@ public function copy($originFile, $targetFile, $override = false) unset($source, $target); if (!is_file($targetFile)) { - $e = new IOException(sprintf('Failed to copy %s to %s', $originFile, $targetFile)); - $e->setPath($originFile); - throw $e; + throw new IOException(sprintf('Failed to copy "%s" to "%s".', $originFile, $targetFile), 0, null, $originFile); } } } @@ -82,9 +80,7 @@ public function mkdir($dirs, $mode = 0777) } if (true !== @mkdir($dir, $mode, true)) { - $e = new IOException(sprintf('Failed to create %s', $dir)); - $e->setPath($dir); - throw $e; + throw new IOException(sprintf('Failed to create "%s".', $dir), 0, null, $dir); } } } @@ -121,9 +117,7 @@ public function touch($files, $time = null, $atime = null) foreach ($this->toIterator($files) as $file) { $touch = $time ? @touch($file, $time, $atime) : @touch($file); if (true !== $touch) { - $e = new IOException(sprintf('Failed to touch %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to touch "%s".', $file), 0, null, $file); } } } @@ -148,23 +142,17 @@ public function remove($files) $this->remove(new \FilesystemIterator($file)); if (true !== @rmdir($file)) { - $e = new IOException(sprintf('Failed to remove directory %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to remove directory "%s".', $file), 0, null, $file); } } else { // https://bugs.php.net/bug.php?id=52176 if (defined('PHP_WINDOWS_VERSION_MAJOR') && is_dir($file)) { if (true !== @rmdir($file)) { - $e = new IOException(sprintf('Failed to remove file %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to remove file "%s".', $file), 0, null, $file); } } else { if (true !== @unlink($file)) { - $e = new IOException(sprintf('Failed to remove file %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to remove file "%s".', $file), 0, null, $file); } } } @@ -188,9 +176,7 @@ public function chmod($files, $mode, $umask = 0000, $recursive = false) $this->chmod(new \FilesystemIterator($file), $mode, $umask, true); } if (true !== @chmod($file, $mode & ~$umask)) { - $e = new IOException(sprintf('Failed to chmod file %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to chmod file "%s".', $file), 0, null, $file); } } } @@ -212,15 +198,11 @@ public function chown($files, $user, $recursive = false) } if (is_link($file) && function_exists('lchown')) { if (true !== @lchown($file, $user)) { - $e = new IOException(sprintf('Failed to chown file %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to chown file "%s".', $file), 0, null, $file); } } else { if (true !== @chown($file, $user)) { - $e = new IOException(sprintf('Failed to chown file %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to chown file "%s".', $file), 0, null, $file); } } } @@ -243,15 +225,11 @@ public function chgrp($files, $group, $recursive = false) } if (is_link($file) && function_exists('lchgrp')) { if (true !== @lchgrp($file, $group)) { - $e = new IOException(sprintf('Failed to chgrp file %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to chgrp file "%s".', $file), 0, null, $file); } } else { if (true !== @chgrp($file, $group)) { - $e = new IOException(sprintf('Failed to chgrp file %s', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Failed to chgrp file "%s".', $file), 0, null, $file); } } } @@ -271,15 +249,11 @@ public function rename($origin, $target, $overwrite = false) { // we check that target does not exist if (!$overwrite && is_readable($target)) { - $e = new IOException(sprintf('Cannot rename because the target "%s" already exists.', $target)); - $e->setPath($target); - throw $e; + throw new IOException(sprintf('Cannot rename because the target "%s" already exists.', $target), 0, null, $target); } if (true !== @rename($origin, $target)) { - $e = new IOException(sprintf('Cannot rename "%s" to "%s".', $origin, $target)); - $e->setPath($target); - throw $e; + throw new IOException(sprintf('Cannot rename "%s" to "%s".', $origin, $target), 0, null, $target); } } @@ -316,14 +290,11 @@ public function symlink($originDir, $targetDir, $copyOnWindows = false) $report = error_get_last(); if (is_array($report)) { if (defined('PHP_WINDOWS_VERSION_MAJOR') && false !== strpos($report['message'], 'error code(1314)')) { - $e = new IOException('Unable to create symlink due to error code 1314: \'A required privilege is not held by the client\'. Do you have the required Administrator-rights?'); - $e->setPath(null); - throw $e; + throw new IOException('Unable to create symlink due to error code 1314: \'A required privilege is not held by the client\'. Do you have the required Administrator-rights?'); } } - $e = new IOException(sprintf('Failed to create symbolic link from %s to %s', $originDir, $targetDir)); - $e->setPath($targetDir); - throw $e; + + throw new IOException(sprintf('Failed to create symbolic link from "%s" to "%s".', $originDir, $targetDir), 0, null, $targetDir); } } } @@ -421,9 +392,7 @@ public function mirror($originDir, $targetDir, \Traversable $iterator = null, $o } elseif (is_dir($file)) { $this->mkdir($target); } else { - $e = new IOException(sprintf('Unable to guess "%s" file type.', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Unable to guess "%s" file type.', $file), 0, null, $file); } } else { if (is_link($file)) { @@ -433,9 +402,7 @@ public function mirror($originDir, $targetDir, \Traversable $iterator = null, $o } elseif (is_file($file)) { $this->copy($file, $target, isset($options['override']) ? $options['override'] : false); } else { - $e = new IOException(sprintf('Unable to guess "%s" file type.', $file)); - $e->setPath($file); - throw $e; + throw new IOException(sprintf('Unable to guess "%s" file type.', $file), 0, null, $file); } } } @@ -478,17 +445,13 @@ public function dumpFile($filename, $content, $mode = 0666) if (!is_dir($dir)) { $this->mkdir($dir); } elseif (!is_writable($dir)) { - $e = new IOException(sprintf('Unable to write to the "%s" directory.', $dir)); - $e->setPath($dir); - throw $e; + throw new IOException(sprintf('Unable to write to the "%s" directory.', $dir), 0, null, $dir); } $tmpFile = tempnam($dir, basename($filename)); if (false === @file_put_contents($tmpFile, $content)) { - $e = new IOException(sprintf('Failed to write file "%s".', $filename)); - $e->setPath($filename); - throw $e; + throw new IOException(sprintf('Failed to write file "%s".', $filename), 0, null, $filename); } $this->rename($tmpFile, $filename, true); diff --git a/src/Symfony/Component/Filesystem/Tests/ExceptionTest.php b/src/Symfony/Component/Filesystem/Tests/ExceptionTest.php index ead75a2eb62e..53bd8db76a00 100644 --- a/src/Symfony/Component/Filesystem/Tests/ExceptionTest.php +++ b/src/Symfony/Component/Filesystem/Tests/ExceptionTest.php @@ -19,33 +19,28 @@ */ class ExceptionTest extends \PHPUnit_Framework_TestCase { - public function testSetPath() - { - $e = new IOException(); - $e->setPath('/foo'); - $reflection = new \ReflectionProperty($e, 'path'); - $reflection->setAccessible(true); - - $this->assertEquals('/foo', $reflection->getValue($e), 'The path should get stored in the "path" property'); - } - public function testGetPath() { - $e = new IOException(); - $e->setPath('/foo'); + $e = new IOException('', 0, null, '/foo'); $this->assertEquals('/foo', $e->getPath(), 'The pass should be returned.'); } public function testGeneratedMessage() { - $e = new FileNotFoundException('/foo'); + $e = new FileNotFoundException(null, 0, null, '/foo'); $this->assertEquals('/foo', $e->getPath()); - $this->assertEquals('File "/foo" could not be found', $e->getMessage(), 'A message should be generated.'); + $this->assertEquals('File "/foo" could not be found.', $e->getMessage(), 'A message should be generated.'); + } + + public function testGeneratedMessageWithoutPath() + { + $e = new FileNotFoundException(); + $this->assertEquals('File could not be found.', $e->getMessage(), 'A message should be generated.'); } public function testCustomMessage() { - $e = new FileNotFoundException('/foo', 'bar'); + $e = new FileNotFoundException('bar', 0, null, '/foo'); $this->assertEquals('bar', $e->getMessage(), 'A custom message should be possible still.'); } -} \ No newline at end of file +}