From c999f3e6b5c7edef82761ed1db00d79683e2e37a Mon Sep 17 00:00:00 2001
From: epriestley <git@epriestley.com>
Date: Wed, 21 May 2014 12:23:51 -0700
Subject: [PATCH] Fix XHPAST to detect use of undeclared variables in `catch`

Summary:
Currently, when code has a block like:

  } catch (Exception $ex) {

...we attempt to mark "$ex" as declared. However, we incorrectly mark every variable used anywhere in the block as declared. This means we'll never raise this warning in a `catch` block.

Instead //only// mark the caught exception as declared.

Test Plan: Added a failing unit test and made it pass.

Reviewers: joshuaspence, btrahan

Reviewed By: btrahan

Subscribers: lpriestley, epriestley

Differential Revision: https://secure.phabricator.com/D9239
---
 src/lint/linter/ArcanistXHPASTLinter.php               | 10 +++++-----
 .../__tests__/xhpast/undeclared-variables.lint-test    |  9 +++++++++
 2 files changed, 14 insertions(+), 5 deletions(-)

diff --git a/src/lint/linter/ArcanistXHPASTLinter.php b/src/lint/linter/ArcanistXHPASTLinter.php
index 06b983b1..39be6e50 100644
--- a/src/lint/linter/ArcanistXHPASTLinter.php
+++ b/src/lint/linter/ArcanistXHPASTLinter.php
@@ -1202,11 +1202,11 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
         }
       }
 
-      $catches = $body
-        ->selectDescendantsOfType('n_CATCH')
-        ->selectDescendantsOfType('n_VARIABLE');
-      foreach ($catches as $var) {
-        $vars[] = $var;
+      // Include "catch (Exception $ex)", but not variables in the body of the
+      // catch block.
+      $catches = $body->selectDescendantsOfType('n_CATCH');
+      foreach ($catches as $catch) {
+        $vars[] = $catch->getChildOfType(1, 'n_VARIABLE');
       }
 
       $binary = $body->selectDescendantsOfType('n_BINARY_EXPRESSION');
diff --git a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
index 47516e5b..3fe66f2a 100644
--- a/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
+++ b/src/lint/linter/__tests__/xhpast/undeclared-variables.lint-test
@@ -166,6 +166,14 @@ function strings() {
   echo "$b";
 }
 
+function catchy() {
+  try {
+    dangerous();
+  } catch (Exception $ex) {
+    $y->z();
+  }
+}
+
 ~~~~~~~~~~
 disabled:3:1
 error:30:3
@@ -185,3 +193,4 @@ error:125:3 Should only warn once in this function.
 error:146:8
 error:152:9
 error:166:9
+error:173:5