!!! Allow plugins to modify Config::$fileExtensions early

ProjectAnalyzer consumed Config::$fileExtensions early in its constructor - without having processed plugins' modifications, registering their custom scanners or analyzer implementations. This change * adds new specific interface \Psalm\Plugin\FileExtensionsInterface to be used by plugin implementations * extracts file extension handling from \Psalm\PluginRegistrationSocket and interface \Psalm\Plugin\RegistrationInterface to a new dedicated \Psalm\PluginFileExtensionsSocket and new interface \Psalm\Plugin\FileExtensionsInterface !!! this is a breaking change in PluginRegistrationSocket !!! * adds runtime in-memory cache for Config::$plugins * calls new method Config::processPluginFileExtensions(), providing modifications to file extension only early in ProjectAnalyzer * adjusts documentation
vimeo · Nov 1, 2021 · bddf808 · bddf808
1 parent 81ca05f
commit bddf808
Show file tree

Hide file tree

Showing 13 changed files with 299 additions and 194 deletions.
diff --git a/docs/running_psalm/checking_non_php_files.md b/docs/running_psalm/checking_non_php_files.md
@@ -23,14 +23,21 @@ Plugins can register their own custom scanner  and analyzer implementations for
 namespace Psalm\Example;
 
 use Psalm\Plugin\PluginEntryPointInterface;
+use Psalm\Plugin\PluginFileExtensionsInterface;
+use Psalm\Plugin\FileExtensionsInterface;
 use Psalm\Plugin\RegistrationInterface;
 
-class CustomPlugin implements PluginEntryPointInterface
+class CustomPlugin implements PluginEntryPointInterface, PluginFileExtensionsInterface
 {
     public function __invoke(RegistrationInterface $registration, ?\SimpleXMLElement $config = null): void
     {
-        $registration->addFileTypeScanner('phpt', TemplateScanner::class);
-        $registration->addFileTypeAnalyzer('phpt', TemplateAnalyzer::class);
+        // ... regular plugin processes, stub registration, hook registration
     }
+
+    public function processFileExtensions(FileExtensionsInterface $fileExtensions, ?SimpleXMLElement $config = null): void
+    {
+        $fileExtensions->addFileTypeScanner('phpt', TemplateScanner::class);
+        $fileExtensions->addFileTypeAnalyzer('phpt', TemplateAnalyzer::class);
+    }    
 }
 ```
diff --git a/docs/running_psalm/plugins/authoring_plugins.md b/docs/running_psalm/plugins/authoring_plugins.md
@@ -39,8 +39,8 @@ To register a stub file manually use `Psalm\Plugin\RegistrationInterface::addStu
 In addition to XML configuration node `<fileExtensions>` plugins can register their own custom scanner
 and analyzer implementations for particular file extensions, e.g.
 
-* `Psalm\Plugin\RegistrationInterface::addFileTypeScanner('html', CustomFileScanner::class)`
-* `Psalm\Plugin\RegistrationInterface::addFileTypeAnalyzer('html', CustomFileAnalyzer::class)`
+* `Psalm\Plugin\FileExtensionsInterface::addFileTypeScanner('html', CustomFileScanner::class)`
+* `Psalm\Plugin\FileExtensionsInterface::addFileTypeAnalyzer('html', CustomFileAnalyzer::class)`
 
 ## Publishing your plugin on Packagist
 

diff --git a/src/Psalm/Config.php b/src/Psalm/Config.php
@@ -27,10 +27,14 @@
 use Psalm\Issue\MethodIssue;
 use Psalm\Issue\PropertyIssue;
 use Psalm\Issue\VariableIssue;
+use Psalm\Plugin\PluginEntryPointInterface;
+use Psalm\Plugin\PluginFileExtensionsInterface;
+use Psalm\Plugin\PluginInterface;
 use Psalm\Progress\Progress;
 use Psalm\Progress\VoidProgress;
 use SimpleXMLElement;
 use SimpleXMLIterator;
+use Throwable;
 use Webmozart\PathUtil\Path;
 use XdgBaseDir\Xdg;
 
@@ -552,6 +556,11 @@ class Config
      */
     public $trigger_error_exits = 'default';
 
+    /**
+     * @var array<class-string, PluginInterface>
+     */
+    private $plugins = [];
+
     protected function __construct()
     {
         self::$instance = $this;
@@ -1236,6 +1245,40 @@ public function getPluginClasses(): array
         return $this->plugin_classes;
     }
 
+    public function processPluginFileExtensions(ProjectAnalyzer $projectAnalyzer): void
+    {
+        $projectAnalyzer->progress->debug('Process plugin adjustments...' . PHP_EOL);
+        $socket = new PluginFileExtensionsSocket($this);
+        foreach ($this->plugin_classes as $pluginClassEntry) {
+            $pluginClassName = $pluginClassEntry['class'];
+            $pluginConfig = $pluginClassEntry['config'];
+            $plugin = $this->loadPlugin($projectAnalyzer, $pluginClassName);
+            if (!$plugin instanceof PluginFileExtensionsInterface) {
+                continue;
+            }
+            try {
+                $plugin->processFileExtensions($socket, $pluginConfig);
+            } catch (Throwable $t) {
+                throw new ConfigException(
+                    'Failed to process plugin file extensions ' . $pluginClassName,
+                    1635800581,
+                    $t
+                );
+            }
+            $projectAnalyzer->progress->debug('Initialized plugin ' . $pluginClassName . ' successfully' . PHP_EOL);
+        }
+        // populate additional aspects after plugins have been initialized
+        foreach ($socket->getAdditionalFileExtensions() as $fileExtension) {
+            $this->file_extensions[] = $fileExtension;
+        }
+        foreach ($socket->getAdditionalFileTypeScanners() as $extension => $className) {
+            $this->filetype_scanners[$extension] = $className;
+        }
+        foreach ($socket->getAdditionalFileTypeAnalyzers() as $extension => $className) {
+            $this->filetype_analyzers[$extension] = $className;
+        }
+    }
+
     /**
      * Initialises all the plugins (done once the config is fully loaded)
      *
@@ -1253,38 +1296,20 @@ public function initializePlugins(ProjectAnalyzer $project_analyzer): void
             $plugin_class_name = $plugin_class_entry['class'];
             $plugin_config = $plugin_class_entry['config'];
 
+            $plugin = $this->loadPlugin($project_analyzer, $plugin_class_name);
+            if (!$plugin instanceof PluginEntryPointInterface) {
+                continue;
+            }
             try {
-                // Below will attempt to load plugins from the project directory first.
-                // Failing that, it will use registered autoload chain, which will load
-                // plugins from Psalm directory or phar file. If that fails as well, it
-                // will fall back to project autoloader. It may seem that the last step
-                // will always fail, but it's only true if project uses Composer autoloader
-                if ($this->composer_class_loader
-                    && ($plugin_class_path = $this->composer_class_loader->findFile($plugin_class_name))
-                ) {
-                    $project_analyzer->progress->debug(
-                        'Loading plugin ' . $plugin_class_name . ' via require' . PHP_EOL
-                    );
-
-                    self::requirePath($plugin_class_path);
-                } else {
-                    if (!class_exists($plugin_class_name)) {
-                        throw new \UnexpectedValueException($plugin_class_name . ' is not a known class');
-                    }
-                }
-
-                /**
-                 * @psalm-suppress InvalidStringClass
-                 *
-                 * @var Plugin\PluginEntryPointInterface
-                 */
-                $plugin_object = new $plugin_class_name;
-                $plugin_object($socket, $plugin_config);
-            } catch (\Throwable $e) {
-                throw new ConfigException('Failed to load plugin ' . $plugin_class_name, 0, $e);
+                $plugin($socket, $plugin_config);
+            } catch (Throwable $t) {
+                throw new ConfigException(
+                    'Failed to invoke plugin ' . $plugin_class_name,
+                    1635800582,
+                    $t
+                );
             }
-
-            $project_analyzer->progress->debug('Loaded plugin ' . $plugin_class_name . ' successfully' . PHP_EOL);
+            $project_analyzer->progress->debug('Initialized plugin ' . $plugin_class_name . ' successfully' . PHP_EOL);
         }
 
         foreach ($this->filetype_scanner_paths as $extension => $path) {
@@ -1313,28 +1338,53 @@ public function initializePlugins(ProjectAnalyzer $project_analyzer): void
 
         foreach ($this->plugin_paths as $path) {
             try {
-                $plugin_object = new FileBasedPluginAdapter($path, $this, $codebase);
-                $plugin_object($socket);
-            } catch (\Throwable $e) {
+                $plugin = new FileBasedPluginAdapter($path, $this, $codebase);
+                $plugin($socket);
+            } catch (Throwable $e) {
                 throw new ConfigException('Failed to load plugin ' . $path, 0, $e);
             }
         }
-        // populate additional aspects after plugins have been initialized
-        foreach ($socket->getAdditionalFileExtensions() as $fileExtension) {
-            $this->file_extensions[] = $fileExtension;
-        }
-        foreach ($socket->getAdditionalFileTypeScanners() as $extension => $className) {
-            $this->filetype_scanners[$extension] = $className;
-        }
-        foreach ($socket->getAdditionalFileTypeAnalyzers() as $extension => $className) {
-            $this->filetype_analyzers[$extension] = $className;
-        }
 
         new \Psalm\Internal\Provider\AddRemoveTaints\HtmlFunctionTainter();
 
         $socket->registerHooksFromClass(\Psalm\Internal\Provider\AddRemoveTaints\HtmlFunctionTainter::class);
     }
 
+    private function loadPlugin(ProjectAnalyzer $projectAnalyzer, string $pluginClassName): PluginInterface
+    {
+        if (isset($this->plugins[$pluginClassName])) {
+            return $this->plugins[$pluginClassName];
+        }
+        try {
+            // Below will attempt to load plugins from the project directory first.
+            // Failing that, it will use registered autoload chain, which will load
+            // plugins from Psalm directory or phar file. If that fails as well, it
+            // will fall back to project autoloader. It may seem that the last step
+            // will always fail, but it's only true if project uses Composer autoloader
+            if ($this->composer_class_loader
+                && ($pluginclas_class_path = $this->composer_class_loader->findFile($pluginClassName))
+            ) {
+                $projectAnalyzer->progress->debug(
+                    'Loading plugin ' . $pluginClassName . ' via require' . PHP_EOL
+                );
+
+                self::requirePath($pluginclas_class_path);
+            } else {
+                if (!class_exists($pluginClassName)) {
+                    throw new \UnexpectedValueException($pluginClassName . ' is not a known class');
+                }
+            }
+            if (!is_a($pluginClassName, PluginInterface::class, true)) {
+                throw new \UnexpectedValueException($pluginClassName . ' is not a PluginInterface implementation');
+            }
+            $this->plugins[$pluginClassName] = new $pluginClassName;
+            $projectAnalyzer->progress->debug('Loaded plugin ' . $pluginClassName . PHP_EOL);
+            return $this->plugins[$pluginClassName];
+        } catch (Throwable $e) {
+            throw new ConfigException('Failed to load plugin ' . $pluginClassName, 0, $e);
+        }
+    }
+
     private static function requirePath(string $path): void
     {
         /** @psalm-suppress UnresolvableInclude */

diff --git a/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php b/src/Psalm/Internal/Analyzer/ProjectAnalyzer.php
@@ -289,6 +289,7 @@ public function __construct(
         $this->stdout_report_options = $stdout_report_options;
         $this->generated_report_options = $generated_report_options;
 
+        $this->config->processPluginFileExtensions($this);
         $file_extensions = $this->config->getFileExtensions();
 
         foreach ($this->config->getProjectDirectories() as $dir_name) {

diff --git a/src/Psalm/Plugin/FileExtensionsInterface.php b/src/Psalm/Plugin/FileExtensionsInterface.php
@@ -0,0 +1,20 @@
+<?php
+namespace Psalm\Plugin;
+
+use Psalm\Internal\Analyzer\FileAnalyzer;
+use Psalm\Internal\Scanner\FileScanner;
+
+interface FileExtensionsInterface
+{
+    /**
+     * @param string $fileExtension e.g. `'html'`
+     * @param class-string<FileScanner> $className
+     */
+    public function addFileTypeScanner(string $fileExtension, string $className): void;
+
+    /**
+     * @param string $fileExtension e.g. `'html'`
+     * @param class-string<FileAnalyzer> $className
+     */
+    public function addFileTypeAnalyzer(string $fileExtension, string $className): void;
+}
diff --git a/src/Psalm/Plugin/PluginEntryPointInterface.php b/src/Psalm/Plugin/PluginEntryPointInterface.php
@@ -3,7 +3,7 @@
 
 use SimpleXMLElement;
 
-interface PluginEntryPointInterface
+interface PluginEntryPointInterface extends PluginInterface
 {
     public function __invoke(RegistrationInterface $registration, ?SimpleXMLElement $config = null): void;
 }
diff --git a/src/Psalm/Plugin/PluginFileExtensionsInterface.php b/src/Psalm/Plugin/PluginFileExtensionsInterface.php
@@ -0,0 +1,12 @@
+<?php
+namespace Psalm\Plugin;
+
+use SimpleXMLElement;
+
+interface PluginFileExtensionsInterface extends PluginInterface
+{
+    public function processFileExtensions(
+        FileExtensionsInterface $fileExtensions,
+        ?SimpleXMLElement $config = null
+    ): void;
+}
diff --git a/src/Psalm/Plugin/PluginInterface.php b/src/Psalm/Plugin/PluginInterface.php
@@ -0,0 +1,6 @@
+<?php
+namespace Psalm\Plugin;
+
+interface PluginInterface
+{
+}
diff --git a/src/Psalm/Plugin/RegistrationInterface.php b/src/Psalm/Plugin/RegistrationInterface.php
@@ -1,9 +1,6 @@
 <?php
 namespace Psalm\Plugin;
 
-use Psalm\Internal\Analyzer\FileAnalyzer;
-use Psalm\Internal\Scanner\FileScanner;
-
 interface RegistrationInterface
 {
     public function addStubFile(string $file_name): void;
@@ -12,16 +9,4 @@ public function addStubFile(string $file_name): void;
      * @param class-string $handler
      */
     public function registerHooksFromClass(string $handler): void;
-
-    /**
-     * @param string $fileExtension e.g. `'html'`
-     * @param class-string<FileScanner> $className
-     */
-    public function addFileTypeScanner(string $fileExtension, string $className): void;
-
-    /**
-     * @param string $fileExtension e.g. `'html'`
-     * @param class-string<FileAnalyzer> $className
-     */
-    public function addFileTypeAnalyzer(string $fileExtension, string $className): void;
 }