use try-finally when possible

symfony · Sep 28, 2015 · 49edef2 · 49edef2
1 parent 8d7b498
commit 49edef2
Show file tree

Hide file tree

Showing 10 changed files with 18 additions and 85 deletions.
diff --git a/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php b/src/Symfony/Bundle/FrameworkBundle/Routing/DelegatingLoader.php
@@ -56,7 +56,7 @@ public function load($resource, $type = null)
             // Here is the scenario:
             // - while routes are being loaded by parent::load() below, a fatal error
             //   occurs (e.g. parse error in a controller while loading annotations);
-            // - PHP abruptly empties the stack trace, bypassing all catch blocks;
+            // - PHP abruptly empties the stack trace, bypassing all catch/finally blocks;
             //   it then calls the registered shutdown functions;
             // - the ErrorHandler catches the fatal error and re-injects it for rendering
             //   thanks to HttpKernel->terminateWithException() (that calls handleException());
@@ -74,13 +74,10 @@ public function load($resource, $type = null)
 
         try {
             $collection = parent::load($resource, $type);
-        } catch (\Exception $e) {
+        } finally {
             $this->loading = false;
-            throw $e;
         }
 
-        $this->loading = false;
-
         foreach ($collection->all() as $route) {
             if ($controller = $route->getDefault('_controller')) {
                 try {

diff --git a/src/Symfony/Component/Config/Loader/FileLoader.php b/src/Symfony/Component/Config/Loader/FileLoader.php
@@ -101,13 +101,10 @@ public function import($resource, $type = null, $ignoreErrors = false, $sourceRe
 
             try {
                 $ret = $loader->load($resource, $type);
-            } catch (\Exception $e) {
+            } finally {
                 unset(self::$loading[$resource]);
-                throw $e;
             }
 
-            unset(self::$loading[$resource]);
-
             return $ret;
         } catch (FileLoaderImportCircularReferenceException $e) {
             throw $e;

diff --git a/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php b/src/Symfony/Component/Config/Tests/Util/XmlUtilsTest.php
@@ -169,16 +169,11 @@ public function testLoadWrongEmptyXMLWithErrorHandler()
             } catch (\InvalidArgumentException $e) {
                 $this->assertEquals(sprintf('File %s does not contain valid XML, it is empty.', $file), $e->getMessage());
             }
-        } catch (\Exception $e) {
+        } finally {
             restore_error_handler();
             error_reporting($errorReporting);
-
-            throw $e;
         }
 
-        restore_error_handler();
-        error_reporting($errorReporting);
-
         $disableEntities = libxml_disable_entity_loader(true);
         libxml_disable_entity_loader($disableEntities);
 

diff --git a/src/Symfony/Component/Debug/DebugClassLoader.php b/src/Symfony/Component/Debug/DebugClassLoader.php
@@ -130,14 +130,10 @@ public function loadClass($class)
                 call_user_func($this->classLoader, $class);
                 $file = false;
             }
-        } catch (\Exception $e) {
+        } finally {
             ErrorHandler::unstackErrors();
-
-            throw $e;
         }
 
-        ErrorHandler::unstackErrors();
-
         $exists = class_exists($class, false) || interface_exists($class, false) || trait_exists($class, false);
 
         if ('\\' === $class[0]) {

diff --git a/src/Symfony/Component/Debug/ErrorHandler.php b/src/Symfony/Component/Debug/ErrorHandler.php
@@ -468,11 +468,8 @@ public function handleError($type, $message, $file, $line, array $context, array
             try {
                 $this->isRecursive = true;
                 $this->loggers[$type][0]->log(($type & $level) ? $this->loggers[$type][1] : LogLevel::DEBUG, $message, $e);
+            } finally {
                 $this->isRecursive = false;
-            } catch (\Exception $e) {
-                $this->isRecursive = false;
-
-                throw $e;
             }
         }
 

diff --git a/src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php b/src/Symfony/Component/Debug/Tests/DebugClassLoaderTest.php
@@ -108,8 +108,6 @@ class ChildTestingStacking extends TestingStacking { function foo($bar) {} }
             $this->fail('ContextErrorException expected');
         } catch (\ErrorException $exception) {
             // if an exception is thrown, the test passed
-            restore_error_handler();
-            restore_exception_handler();
             $this->assertStringStartsWith(__FILE__, $exception->getFile());
             if (PHP_VERSION_ID < 70000) {
                 $this->assertRegExp('/^Runtime Notice: Declaration/', $exception->getMessage());
@@ -118,11 +116,9 @@ class ChildTestingStacking extends TestingStacking { function foo($bar) {} }
                 $this->assertRegExp('/^Warning: Declaration/', $exception->getMessage());
                 $this->assertEquals(E_WARNING, $exception->getSeverity());
             }
-        } catch (\Exception $exception) {
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-
-            throw $exception;
         }
     }
 

diff --git a/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php b/src/Symfony/Component/Debug/Tests/ErrorHandlerTest.php
@@ -73,9 +73,6 @@ public function testNotice()
             $this->fail('ContextErrorException expected');
         } catch (ContextErrorException $exception) {
             // if an exception is thrown, the test passed
-            restore_error_handler();
-            restore_exception_handler();
-
             $this->assertEquals(E_NOTICE, $exception->getSeverity());
             $this->assertEquals(__FILE__, $exception->getFile());
             $this->assertRegExp('/^Notice: Undefined variable: (foo|bar)/', $exception->getMessage());
@@ -96,11 +93,9 @@ public function testNotice()
             $this->assertEquals(__CLASS__, $trace[2]['class']);
             $this->assertEquals(__FUNCTION__, $trace[2]['function']);
             $this->assertEquals('->', $trace[2]['type']);
-        } catch (\Exception $e) {
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-
-            throw $e;
         }
     }
 
@@ -118,14 +113,9 @@ public function testConstruct()
             $handler = ErrorHandler::register();
             $handler->throwAt(3, true);
             $this->assertEquals(3 | E_RECOVERABLE_ERROR | E_USER_ERROR, $handler->throwAt(0));
-
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-        } catch (\Exception $e) {
-            restore_error_handler();
-            restore_exception_handler();
-
-            throw $e;
         }
     }
 
@@ -157,14 +147,9 @@ public function testDefaultLogger()
                 E_CORE_ERROR => array(null, LogLevel::CRITICAL),
             );
             $this->assertSame($loggers, $handler->setLoggers(array()));
-
-            restore_error_handler();
-            restore_exception_handler();
-        } catch (\Exception $e) {
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-
-            throw $e;
         }
     }
 
@@ -283,14 +268,9 @@ public function testHandleUserError()
             }
 
             $this->assertSame($x, $e);
-
-            restore_error_handler();
-            restore_exception_handler();
-        } catch (\Exception $e) {
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-
-            throw $e;
         }
     }
 
@@ -350,14 +330,9 @@ public function testHandleException()
             });
 
             $handler->handleException($exception);
-
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-        } catch (\Exception $e) {
-            restore_error_handler();
-            restore_exception_handler();
-
-            throw $e;
         }
     }
 
@@ -384,14 +359,9 @@ public function testErrorStacking()
             @trigger_error('Silenced warning', E_USER_WARNING);
             $logger->log(LogLevel::WARNING, 'Dummy log');
             ErrorHandler::unstackErrors();
-
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-        } catch (\Exception $e) {
-            restore_error_handler();
-            restore_exception_handler();
-
-            throw $e;
         }
     }
 
@@ -513,14 +483,9 @@ public function testHandleFatalErrorOnHHVM()
 
             call_user_func_array(array($handler, 'handleError'), $error);
             $handler->handleFatalError($error);
-
+        } finally {
             restore_error_handler();
             restore_exception_handler();
-        } catch (\Exception $e) {
-            restore_error_handler();
-            restore_exception_handler();
-
-            throw $e;
         }
     }
 }
diff --git a/src/Symfony/Component/DependencyInjection/ContainerBuilder.php b/src/Symfony/Component/DependencyInjection/ContainerBuilder.php
@@ -457,14 +457,10 @@ public function get($id, $invalidBehavior = ContainerInterface::EXCEPTION_ON_INV
 
         try {
             $service = $this->createService($definition, $id);
-        } catch (\Exception $e) {
+        } finally {
             unset($this->loading[$id]);
-
-            throw $e;
         }
 
-        unset($this->loading[$id]);
-
         return $service;
     }
 

diff --git a/src/Symfony/Component/OptionsResolver/OptionsResolver.php b/src/Symfony/Component/OptionsResolver/OptionsResolver.php
@@ -783,11 +783,9 @@ public function offsetGet($option)
                 foreach ($this->lazy[$option] as $closure) {
                     $value = $closure($this, $value);
                 }
-            } catch (\Exception $e) {
+            } finally {
                 unset($this->calling[$option]);
-                throw $e;
             }
-            unset($this->calling[$option]);
             // END
         }
 
@@ -885,11 +883,9 @@ public function offsetGet($option)
             $this->calling[$option] = true;
             try {
                 $value = $normalizer($this, $value);
-            } catch (\Exception $e) {
+            } finally {
                 unset($this->calling[$option]);
-                throw $e;
             }
-            unset($this->calling[$option]);
             // END
         }
 

diff --git a/src/Symfony/Component/Yaml/Tests/InlineTest.php b/src/Symfony/Component/Yaml/Tests/InlineTest.php
@@ -58,10 +58,8 @@ public function testDumpNumericValueWithLocale()
 
             $this->assertEquals('1.2', Inline::dump(1.2));
             $this->assertContains('fr', strtolower(setlocale(LC_NUMERIC, 0)));
+        } finally {
             setlocale(LC_NUMERIC, $locale);
-        } catch (\Exception $e) {
-            setlocale(LC_NUMERIC, $locale);
-            throw $e;
         }
     }