[Filesystem] removed getPath() on Exceptions and cleaned up CS and error messages

src/Symfony/Component/Filesystem/Exception/ExceptionInterface.php

@@ -20,5 +20,4 @@ namespace Symfony\Component\Filesystem\Exception;
  */
 interface ExceptionInterface
 {
-
 }

src/Symfony/Component/Filesystem/Exception/FileNotFoundException.php

@@ -14,18 +14,21 @@ namespace Symfony\Component\Filesystem\Exception;
 /**
  * Exception class thrown when a file couldn't be found
  *
+ * @author Fabien Potencier <fabien@symfony.com>
  * @author Christian Gärtner <christiangaertner.film@googlemail.com>
  */
 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) {
+        if (null === $message) {
-            $message = sprintf('File "%s" could not be found', $path);
+            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, $path);
-
-        parent::__construct($message, $code, $previous);
     }
 }

src/Symfony/Component/Filesystem/Exception/IOException.php

@@ -16,21 +16,19 @@ namespace Symfony\Component\Filesystem\Exception;
  *
  * @author Romain Neutron <imprec@gmail.com>
  * @author Christian Gärtner <christiangaertner.film@googlemail.com>
+ * @author Fabien Potencier <fabien@symfony.com>
  *
  * @api
  */
 class IOException extends \RuntimeException implements IOExceptionInterface
 {
-
     private $path;
 
-    /**
+    public function __construct($message, $code = 0, \Exception $previous = null, $path = null)
-     * Set the path associated with this IOException
-     * @param string $path The path
-     */
-    public function setPath($path)
     {
         $this->path = $path;
+
+        parent::__construct($message, $code, $previous);
     }
 
     /**

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();

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 @@ class Filesystem
             unset($source, $target);
 
             if (!is_file($targetFile)) {
-                $e = new IOException(sprintf('Failed to copy %s to %s', $originFile, $targetFile));
+                throw new IOException(sprintf('Failed to copy "%s" to "%s".', $originFile, $targetFile), 0, null, $originFile);
-                $e->setPath($originFile);
-                throw $e;
             }
         }
     }
@@ -82,9 +80,7 @@ class Filesystem
             }
 
             if (true !== @mkdir($dir, $mode, true)) {
-                $e = new IOException(sprintf('Failed to create %s', $dir));
+                throw new IOException(sprintf('Failed to create "%s".', $dir), 0, null, $dir);
-                $e->setPath($dir);
-                throw $e;
             }
         }
     }
@@ -121,9 +117,7 @@ class Filesystem
         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));
+                throw new IOException(sprintf('Failed to touch "%s".', $file), 0, null, $file);
-                $e->setPath($file);
-                throw $e;
             }
         }
     }
@@ -148,23 +142,17 @@ class Filesystem
                 $this->remove(new \FilesystemIterator($file));
 
                 if (true !== @rmdir($file)) {
-                    $e = new IOException(sprintf('Failed to remove directory %s', $file));
+                    throw new IOException(sprintf('Failed to remove directory "%s".', $file), 0, null, $file);
-                    $e->setPath($file);
-                    throw $e;
                 }
             } 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));
+                        throw new IOException(sprintf('Failed to remove file "%s".', $file), 0, null, $file);
-                        $e->setPath($file);
-                        throw $e;
                     }
                 } else {
                     if (true !== @unlink($file)) {
-                        $e = new IOException(sprintf('Failed to remove file %s', $file));
+                        throw new IOException(sprintf('Failed to remove file "%s".', $file), 0, null, $file);
-                        $e->setPath($file);
-                        throw $e;
                     }
                 }
             }
@@ -188,9 +176,7 @@ class Filesystem
                 $this->chmod(new \FilesystemIterator($file), $mode, $umask, true);
             }
             if (true !== @chmod($file, $mode & ~$umask)) {
-                $e = new IOException(sprintf('Failed to chmod file %s', $file));
+                throw new IOException(sprintf('Failed to chmod file "%s".', $file), 0, null, $file);
-                $e->setPath($file);
-                throw $e;
             }
         }
     }
@@ -212,15 +198,11 @@ class Filesystem
             }
             if (is_link($file) && function_exists('lchown')) {
                 if (true !== @lchown($file, $user)) {
-                    $e = new IOException(sprintf('Failed to chown file %s', $file));
+                    throw new IOException(sprintf('Failed to chown file "%s".', $file), 0, null, $file);
-                    $e->setPath($file);
-                    throw $e;
                 }
             } else {
                 if (true !== @chown($file, $user)) {
-                    $e = new IOException(sprintf('Failed to chown file %s', $file));
+                    throw new IOException(sprintf('Failed to chown file "%s".', $file), 0, null, $file);
-                    $e->setPath($file);
-                    throw $e;
                 }
             }
         }
@@ -243,15 +225,11 @@ class Filesystem
             }
             if (is_link($file) && function_exists('lchgrp')) {
                 if (true !== @lchgrp($file, $group)) {
-                    $e = new IOException(sprintf('Failed to chgrp file %s', $file));
+                    throw new IOException(sprintf('Failed to chgrp file "%s".', $file), 0, null, $file);
-                    $e->setPath($file);
-                    throw $e;
                 }
             } else {
                 if (true !== @chgrp($file, $group)) {
-                    $e = new IOException(sprintf('Failed to chgrp file %s', $file));
+                    throw new IOException(sprintf('Failed to chgrp file "%s".', $file), 0, null, $file);
-                    $e->setPath($file);
-                    throw $e;
                 }
             }
         }
@@ -271,15 +249,11 @@ class Filesystem
     {
         // 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));
+            throw new IOException(sprintf('Cannot rename because the target "%s" already exists.', $target), 0, null, $target);
-            $e->setPath($target);
-            throw $e;
         }
 
         if (true !== @rename($origin, $target)) {
-            $e = new IOException(sprintf('Cannot rename "%s" to "%s".', $origin, $target));
+            throw new IOException(sprintf('Cannot rename "%s" to "%s".', $origin, $target), 0, null, $target);
-            $e->setPath($target);
-            throw $e;
         }
     }
 
@@ -316,14 +290,11 @@ class Filesystem
                 $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?');
+                        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->setPath(null);
-                        throw $e;
                     }
                 }
-                $e = new IOException(sprintf('Failed to create symbolic link from %s to %s', $originDir, $targetDir));
+
-                $e->setPath($targetDir);
+                throw new IOException(sprintf('Failed to create symbolic link from "%s" to "%s".', $originDir, $targetDir), 0, null, $targetDir);
-                throw $e;
             }
         }
     }
@@ -421,9 +392,7 @@ class Filesystem
                 } elseif (is_dir($file)) {
                     $this->mkdir($target);
                 } else {
-                    $e = new IOException(sprintf('Unable to guess "%s" file type.', $file));
+                    throw new IOException(sprintf('Unable to guess "%s" file type.', $file), 0, null, $file);
-                    $e->setPath($file);
-                    throw $e;
                 }
             } else {
                 if (is_link($file)) {
@@ -433,9 +402,7 @@ class Filesystem
                 } 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));
+                    throw new IOException(sprintf('Unable to guess "%s" file type.', $file), 0, null, $file);
-                    $e->setPath($file);
-                    throw $e;
                 }
             }
         }
@@ -478,17 +445,13 @@ class Filesystem
         if (!is_dir($dir)) {
             $this->mkdir($dir);
         } elseif (!is_writable($dir)) {
-            $e = new IOException(sprintf('Unable to write to the "%s" directory.', $dir));
+            throw new IOException(sprintf('Unable to write to the "%s" directory.', $dir), 0, null, $dir);
-            $e->setPath($dir);
-            throw $e;
         }
 
         $tmpFile = tempnam($dir, basename($filename));
 
         if (false === @file_put_contents($tmpFile, $content)) {
-            $e = new IOException(sprintf('Failed to write file "%s".', $filename));
+            throw new IOException(sprintf('Failed to write file "%s".', $filename), 0, null, $filename);
-            $e->setPath($filename);
-            throw $e;
         }
 
         $this->rename($tmpFile, $filename, true);

src/Symfony/Component/Filesystem/Tests/ExceptionTest.php

@@ -19,33 +19,28 @@ use Symfony\Component\Filesystem\Exception\FileNotFoundException;
  */
 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 = new IOException('', 0, null, '/foo');
-        $e->setPath('/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.');
     }
-}
+}