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

@@ -1202,11 +1202,11 @@ final class ArcanistXHPASTLinter extends ArcanistBaseXHPASTLinter {
         }
       }
 
-      $catches = $body
+      // Include "catch (Exception $ex)", but not variables in the body of the
-        ->selectDescendantsOfType('n_CATCH')
+      // catch block.
-        ->selectDescendantsOfType('n_VARIABLE');
+      $catches = $body->selectDescendantsOfType('n_CATCH');
-      foreach ($catches as $var) {
+      foreach ($catches as $catch) {
-        $vars[] = $var;
+        $vars[] = $catch->getChildOfType(1, 'n_VARIABLE');
       }
 
       $binary = $body->selectDescendantsOfType('n_BINARY_EXPRESSION');

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