Update arcanist to use the PhutilSymbolLoader.

Summary: This should also fix the bug with double help for certain commands

Test Plan:

Reviewers:

CC:

scripts/arcanist.php

@@ -23,7 +23,7 @@ phutil_require_module('phutil', 'conduit/client');
 phutil_require_module('phutil', 'console');
 phutil_require_module('phutil', 'future/exec');
 phutil_require_module('phutil', 'filesystem');
-phutil_require_module('phutil', 'autoload');
+phutil_require_module('phutil', 'symbols');
 
 phutil_require_module('arcanist', 'exception/usage');
 phutil_require_module('arcanist', 'configuration');
@@ -69,7 +69,7 @@ try {
 
   $config = $working_copy->getConfig('arcanist_configuration');
   if ($config) {
-    phutil_autoload_class($config);
+    PhutilSymbolLoader::loadClass($config);
     $config = new $config();
   } else {
     $config = new ArcanistConfiguration();

scripts/phutil_analyzer.php

@@ -23,7 +23,9 @@ $builtin_functions  = get_defined_functions();
 $builtin_functions  = $builtin_functions['internal'];
 
 $builtin = array(
-  'class'     => array_fill_keys($builtin_classes, true),
+  'class'     => array_fill_keys($builtin_classes, true) + array(
+    'PhutilBootloader' => true,
+  ),
   'function'  => array_fill_keys($builtin_functions, true) + array(
     'empty' => true,
     'isset' => true,
@@ -31,8 +33,6 @@ $builtin = array(
     'print' => true,
     'exit'  => true,
     'die'   => true,
-
-    'phutil_module_exists' => true,
   ),
   'interface' => array_fill_keys($builtin_interfaces, true),
 );

src/configuration/ArcanistConfiguration.php

@@ -29,12 +29,7 @@ class ArcanistConfiguration {
       return null;
     }
 
-    if (!phutil_module_exists('arcanist', 'workflow/'.$command)) {
-      return null;
-    }
-
     $workflow_class = 'Arcanist'.ucfirst($command).'Workflow';
-
     $workflow_class = preg_replace_callback(
       '/-([a-z])/',
       array(
@@ -43,18 +38,31 @@ class ArcanistConfiguration {
       ),
       $workflow_class);
 
-    phutil_autoload_class($workflow_class);
+    $symbols = id(new PhutilSymbolLoader())
+      ->setType('class')
+      ->setName($workflow_class)
+      ->setLibrary('arcanist')
+      ->selectAndLoadSymbols();
+
+    if (!$symbols) {
+      return null;
+    }
 
     return newv($workflow_class, array());
   }
 
   public function buildAllWorkflows() {
-    $classes = phutil_find_class_descendants('ArcanistBaseWorkflow');
+    $symbols = id(new PhutilSymbolLoader())
+      ->setType('class')
+      ->setAncestorClass('ArcanistBaseWorkflow')
+      ->setLibrary('arcanist')
+      ->selectAndLoadSymbols();
+
     $workflows = array();
-    foreach ($classes as $class) {
+    foreach ($symbols as $symbol) {
+      $class = $class['name'];
       $name = preg_replace('/^Arcanist(\w+)Workflow$/', '\1', $class);
       $name = strtolower($name);
-      phutil_autoload_class($class);
       $workflows[$name] = newv($class, array());
     }
 

src/configuration/__init__.php

@@ -6,7 +6,7 @@
 
 
 
-phutil_require_module('phutil', 'autoload');
+phutil_require_module('phutil', 'symbols');
 phutil_require_module('phutil', 'utils');
 
 

src/lint/linter/phutilmodule/ArcanistPhutilModuleLinter.php

@@ -241,13 +241,16 @@ class ArcanistPhutilModuleLinter extends ArcanistLinter {
             $places);
 
           if ($type == 'class' || $type == 'interface') {
-            $class_spec = PhutilLibraryMapRegistry::findClass(
+            $loader = new PhutilSymbolLoader();
-              $library = null,
+            $loader->setType($type);
-              $name);
+            $loader->setName($name);
-            if ($class_spec) {
+            $symbols = $loader->selectSymbolsWithoutLoading();
+            if ($symbols) {
+              $class_spec = reset($symbols);
               try {
-                $loaded = phutil_autoload_class($name);
+                $loader->selectAndLoadSymbols();
-              } catch (PhutilLibraryLoadException $ex) {
+                $loaded = true;
+              } catch (PhutilMissingSymbolException $ex) {
                 $loaded = false;
               }
               if ($loaded) {
@@ -279,13 +282,16 @@ class ArcanistPhutilModuleLinter extends ArcanistLinter {
               }
             }
           } else {
-            $func_spec = PhutilLibraryMapRegistry::findFunction(
+            $loader = new PhutilSymbolLoader();
-              $library = null,
+            $loader->setType($type);
-              $name);
+            $loader->setName($name);
-            if ($func_spec) {
+            $symbols = $loader->selectSymbolsWithoutLoading();
+            if ($symbols) {
+              $func_spec = reset($symbols);
               try {
-                $loaded = phutil_autoload_function($name);
+                $loader->selectAndLoadSymbols();
-              } catch (PhutilLibraryLoadException $ex) {
+                $loaded = true;
+              } catch (PhutilMissingSymbolException $ex) {
                 $loaded = false;
               }
               if ($loaded) {

src/lint/linter/phutilmodule/__init__.php

@@ -9,12 +9,12 @@
 phutil_require_module('arcanist', 'lint/linter/base');
 phutil_require_module('arcanist', 'lint/severity');
 
-phutil_require_module('phutil', 'autoload');
 phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'future');
 phutil_require_module('phutil', 'future/exec');
 phutil_require_module('phutil', 'moduleutils');
 phutil_require_module('phutil', 'parser/xhpast/bin');
+phutil_require_module('phutil', 'symbols');
 
 
 phutil_require_source('ArcanistPhutilModuleLinter.php');

src/repository/api/subversion/ArcanistSubversionAPI.php

@@ -171,7 +171,7 @@ class ArcanistSubversionAPI extends ArcanistRepositoryAPI {
       // To reproduce, do:
       //
       //  $ ln -s working_copy working_link
-      //  $ svn info working_copy # ok  
+      //  $ svn info working_copy # ok
       //  $ svn info working_link # fails
       //
       // Work around this by cd-ing into the directory before executing
@@ -411,7 +411,7 @@ EODIFF;
 
     return $blame;
   }
-  
+
   public function getOriginalFileData($path) {
     // SVN issues warnings for nonexistent paths, directories, etc., but still
     // returns no error code. However, for new paths in the working copy it
@@ -425,7 +425,7 @@ EODIFF;
     }
     return $stdout;
   }
-  
+
   public function getCurrentFileData($path) {
     $full_path = $this->getPath($path);
     if (Filesystem::pathExists($full_path)) {

src/unit/engine/phutil/PhutilUnitTestEngine.php

@@ -20,6 +20,8 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine {
 
   public function run() {
 
+    $bootloader = PhutilBootloader::getInstance();
+
     $tests = array();
     foreach ($this->getPaths() as $path) {
       $library_root = phutil_get_library_root_for_path($path);
@@ -41,7 +43,10 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine {
       if (basename($library_path) == '__tests__') {
         // Okay, this is a __tests__ module.
       } else {
-        if (phutil_module_exists($library_name, $library_path.'/__tests__')) {
+        $exists = $bootloader->moduleExists(
+          $library_name,
+          $library_path.'/__tests__');
+        if ($exists) {
           // This is a module which has a __tests__ module in it.
           $path .= '/__tests__';
         } else {
@@ -72,14 +77,16 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine {
     }
 
     $run_tests = array();
-    $all_test_classes = phutil_find_class_descendants('ArcanistPhutilTestCase');
-    $all_test_classes = array_fill_keys($all_test_classes, true);
     foreach ($tests as $test) {
-      $local_classes = phutil_find_classes_declared_in_module(
+      $symbols = id(new PhutilSymbolLoader())
-        $test['library'],
+        ->setType('class')
-        $test['module']);
+        ->setLibrary($test['library'])
-      $local_classes = array_fill_keys($local_classes, true);
+        ->setModule($test['module'])
-      $run_tests += array_intersect($local_classes, $all_test_classes);
+        ->setAncestorClass('ArcanistPhutilTestCase')
+        ->selectAndLoadSymbols();
+      foreach ($symbols as $symbol) {
+        $run_tests[$symbol['name']] = true;
+      }
     }
     $run_tests = array_keys($run_tests);
 
@@ -90,7 +97,7 @@ class PhutilUnitTestEngine extends ArcanistBaseUnitTestEngine {
 
     $results = array();
     foreach ($run_tests as $test_class) {
-      phutil_autoload_class($test_class);
+      PhutilSymbolLoader::loadClass($test_class);
       $test_case = newv($test_class, array());
       $results[] = $test_case->run();
     }

src/unit/engine/phutil/__init__.php

@@ -9,9 +9,9 @@
 phutil_require_module('arcanist', 'exception/usage/noeffect');
 phutil_require_module('arcanist', 'unit/engine/base');
 
-phutil_require_module('phutil', 'autoload');
 phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'moduleutils');
+phutil_require_module('phutil', 'symbols');
 phutil_require_module('phutil', 'utils');
 
 

src/workflow/lint/ArcanistLintWorkflow.php

@@ -136,11 +136,7 @@ EOTEXT
         "specify a lint engine.");
     }
 
-    $ok = phutil_autoload_class($engine);
+    PhutilSymbolLoader::loadClass($engine);
-    if (!$ok) {
-      throw new ArcanistUsageException(
-        "Configured lint engine '{$engine}' could not be loaded.");
-    }
 
     $engine = newv($engine, array());
     $engine->setWorkingCopy($working_copy);

src/workflow/lint/__init__.php

@@ -14,12 +14,12 @@ phutil_require_module('arcanist', 'lint/severity');
 phutil_require_module('arcanist', 'repository/api/base');
 phutil_require_module('arcanist', 'workflow/base');
 
-phutil_require_module('phutil', 'autoload');
 phutil_require_module('phutil', 'console');
 phutil_require_module('phutil', 'filesystem');
 phutil_require_module('phutil', 'filesystem/filelist');
 phutil_require_module('phutil', 'filesystem/tempfile');
 phutil_require_module('phutil', 'future/exec');
+phutil_require_module('phutil', 'symbols');
 phutil_require_module('phutil', 'utils');
 phutil_require_module('phutil', 'xsprintf/csprintf');
 

src/workflow/svn-hook-pre-commit/ArcanistSvnHookPreCommitWorkflow.php

@@ -46,12 +46,12 @@ EOTEXT
 
     // TODO: Do stuff with commit message.
     var_dump($commit_message);
-    
+
     list($changed) = execx(
       'svnlook changed --transaction %s %s',
       $transaction,
       $repository);
-      
+
     $paths = array();
     $changed = explode("\n", trim($changed));
     foreach ($changed as $line) {
@@ -59,13 +59,13 @@ EOTEXT
       preg_match('/^..\s*(.*)$/', $line, $matches);
       $paths[$matches[1]] = strlen($matches[1]);
     }
-    
+
     $resolved = array();
     $failed = array();
     $missing = array();
     $found = array();
     asort($paths);
-    
+
     foreach ($paths as $path => $length) {
       foreach ($resolved as $rpath => $root) {
         if (!strncmp($path, $rpath, strlen($rpath))) {
@@ -74,12 +74,12 @@ EOTEXT
         }
       }
       $config = $path;
-      
+
       if (basename($config) == '.arcconfig') {
         $resolved[$config] = $config;
         continue;
       }
-      
+
       $config = rtrim($config, '/');
       $last_config = $config;
       do {
@@ -109,12 +109,12 @@ EOTEXT
         }
         $last_config = $config;
       } while (true);
-      
+
       if (empty($resolved[$path])) {
         $failed[] = $path;
       }
     }
-    
+
     if ($failed && $resolved) {
       $failed_paths = '        '.implode("\n        ", $failed);
       $resolved_paths = '        '.implode("\n        ", array_keys($resolved));
@@ -127,12 +127,12 @@ EOTEXT
         "Files not in projects:\n\n".
         $failed_paths);
     }
-    
+
     if (!$resolved) {
       // None of the affected paths are beneath a .arcconfig file.
       return 3;
     }
-    
+
     $groups = array();
     foreach ($resolved as $path => $project) {
       $groups[$project][] = $path;
@@ -150,10 +150,10 @@ EOTEXT
         "only that project.\n\n".
         $message);
     }
-    
+
     $project_root = key($groups);
     $paths = reset($groups);
-    
+
     $data = array();
     foreach ($paths as $path) {
       list($err, $filedata) = exec_manual(
@@ -163,7 +163,7 @@ EOTEXT
         $path);
       $data[$path] = $err ? null : $filedata;
     }
-    
+
     // TODO: Do stuff with data.
     var_dump($data);
 

src/workflow/svn-hook-pre-commit/__init__.php

@@ -6,10 +6,11 @@
 
 
 
+phutil_require_module('arcanist', 'exception/usage');
 phutil_require_module('arcanist', 'workflow/base');
 
 phutil_require_module('phutil', 'console');
 phutil_require_module('phutil', 'future/exec');
 
 
-phutil_require_source('ArcanistSvnHoookPreCommitWorkflow.php');
+phutil_require_source('ArcanistSvnHookPreCommitWorkflow.php');

src/workflow/unit/ArcanistUnitWorkflow.php

@@ -62,12 +62,6 @@ EOTEXT
         "to specify a unit test engine.");
     }
 
-    $ok = phutil_autoload_class($engine_class);
-    if (!$ok) {
-      throw new ArcanistUsageException(
-        "Configured unit test engine '{$engine_class}' could not be loaded.");
-    }
-
     $repository_api = $this->getRepositoryAPI();
 
     if ($this->getArgument('paths')) {
@@ -79,6 +73,7 @@ EOTEXT
       $paths = array_keys($paths);
     }
 
+    PhutilSymbolLoader::loadClass($engine_class);
     $engine = newv($engine_class, array());
     $engine->setWorkingCopy($working_copy);
     $engine->setPaths($paths);

src/workflow/unit/__init__.php

@@ -6,13 +6,12 @@
 
 
 
-phutil_require_module('arcanist', 'exception/usage');
 phutil_require_module('arcanist', 'exception/usage/noengine');
 phutil_require_module('arcanist', 'unit/result');
 phutil_require_module('arcanist', 'workflow/base');
 
-phutil_require_module('phutil', 'autoload');
 phutil_require_module('phutil', 'console');
+phutil_require_module('phutil', 'symbols');
 phutil_require_module('phutil', 'utils');