Refactoring

- changed "SPI" into something more familiar, like "implementation"
- throw exceptions on invalid implementation types or invalid class names
- use null instead of false when querying
- refactored the tests accordingly

src/Composer/Plugin/Capable.php

@@ -26,19 +26,17 @@ interface Capable
      * with a special structure.
      *
      * The key must be a string, representing a fully qualified class/interface name
-     * which Composer Plugin API exposes - named "API class".
+     * which Composer Plugin API exposes.
      * The value must be a string as well, representing the fully qualified class name
-     * of the API class - named "SPI class".
+     * of the API class.
      *
-     * Every SPI must implement their API class.
+     * Every implementation will be passed a single array parameter via their constructor.
      *
-     * Every SPI will be passed a single array parameter via their constructor.
+     * @tutorial
      *
-     * Example:
-     * // API as key, SPI as value
      * return array(
-     *      'Composer\Plugin\Capability\CommandProvider' => 'My\CommandProvider',
-     *      'Composer\Plugin\Capability\Validator'       => 'My\Validator',
+     *     'Composer\Plugin\Capability\CommandProvider' => 'My\CommandProvider',
+     *     'Composer\Plugin\Capability\Validator'       => 'My\Validator',
      * );
      *
      * @return string[]

src/Composer/Plugin/PluginManager.php

@@ -186,6 +186,16 @@ class PluginManager
         }
     }
 
+    /**
+     * Returns the version of the internal composer-plugin-api package.
+     *
+     * @return string
+     */
+    protected function getPluginApiVersion()
+    {
+        return PluginInterface::PLUGIN_API_VERSION;
+    }
+
     /**
      * Adds a plugin, activates it and registers it with the event dispatcher
      *
@@ -291,56 +301,58 @@ class PluginManager
         return $this->globalComposer->getInstallationManager()->getInstallPath($package);
     }
 
-    /**
-     * Returns the version of the internal composer-plugin-api package.
-     *
-     * @return string
-     */
-    protected function getPluginApiVersion()
-    {
-        return PluginInterface::PLUGIN_API_VERSION;
-    }
-
     /**
      * @param PluginInterface $plugin
      * @param string $capability
-     * @return bool|string The fully qualified class of the implementation or false if none was provided
+     * @return null|string The fully qualified class of the implementation or null if Plugin is not of Capable type
+     * @throws \RuntimeException On empty or non-string implementation class name value
      */
     protected function getCapabilityImplementationClassName(PluginInterface $plugin, $capability)
     {
         if (!($plugin instanceof Capable)) {
-            return false;
+            return null;
         }
 
         $capabilities = (array) $plugin->getCapabilities();
 
+        if (!empty($capabilities[$capability]) && is_string($capabilities[$capability])) {
+            $capabilities[$capability] = trim($capabilities[$capability]);
+        }
+
         if (empty($capabilities[$capability]) || !is_string($capabilities[$capability])) {
-            return false;
+            throw new \RuntimeException('Plugin provided invalid capability class name(s)');
         }
 
-        return trim($capabilities[$capability]);
+        return $capabilities[$capability];
     }
 
     /**
      * @param PluginInterface $plugin
-     * @param string          $capability The fully qualified name of the API interface which the plugin may provide
+     * @param string $capabilityClassName The fully qualified name of the API interface which the plugin may provide
      *                                    an implementation.
-     * @param array           $ctorArgs   Arguments passed to Capability's constructor.
-     *                                    Keeping it an array will allow future values to be passed w\o changing the signature.
-     * @return Capability|boolean         Bool false if the Plugin has no implementation of the requested Capability.
+     * @param array $ctorArgs Arguments passed to Capability's constructor.
+     *                        Keeping it an array will allow future values to be passed w\o changing the signature.
+     * @return null|Capability
      */
-    public function getPluginCapability(PluginInterface $plugin, $capability, array $ctorArgs = array())
+    public function getPluginCapability(PluginInterface $plugin, $capabilityClassName, array $ctorArgs = array())
     {
-        if ($capabilityClass = $this->getCapabilityImplementationClassName($plugin, $capability)) {
+        if ($capabilityClass = $this->getCapabilityImplementationClassName($plugin, $capabilityClassName)) {
             if (class_exists($capabilityClass)) {
                 $capabilityObj = new $capabilityClass($ctorArgs);
                 if ($capabilityObj instanceof Capability &&
-                    $capabilityObj instanceof $capability
+                    $capabilityObj instanceof $capabilityClassName
                 ) {
                     return $capabilityObj;
+                } else {
+                    throw new \RuntimeException(
+                        'Class ' . $capabilityClass . ' must be of both \Composer\Plugin\Capability\Capability and '.
+                        $capabilityClassName . ' types.'
+                    );
                 }
+            } else {
+                throw new \RuntimeException("Cannot instantiate Capability, as class $capabilityClass does not exist.");
             }
         }
-        return false;
+        return null;
     }
 }

tests/Composer/Test/Plugin/PluginInstallerTest.php

@@ -302,31 +302,31 @@ class PluginInstallerTest extends TestCase
         $plugin = $this->getMockBuilder('Composer\Plugin\PluginInterface')
                        ->getMock();
 
-        $this->assertFalse($this->pm->getPluginCapability($plugin, 'Fake\Ability'));
+        $this->assertNull($this->pm->getPluginCapability($plugin, 'Fake\Ability'));
     }
 
     public function testCapabilityImplementsComposerPluginApiClassAndIsConstructedWithArgs()
     {
         $capabilityApi = 'Composer\Plugin\Capability\Capability';
-        $capabilitySpi = 'Composer\Test\Plugin\Mock\Capability';
+        $capabilityImplementation = 'Composer\Test\Plugin\Mock\Capability';
 
         $plugin = $this->getMockBuilder('Composer\Test\Plugin\Mock\CapablePluginInterface')
                        ->getMock();
 
         $plugin->expects($this->once())
                ->method('getCapabilities')
-               ->will($this->returnCallback(function() use ($capabilitySpi, $capabilityApi) {
-                   return array($capabilityApi => $capabilitySpi);
+               ->will($this->returnCallback(function() use ($capabilityImplementation, $capabilityApi) {
+                   return array($capabilityApi => $capabilityImplementation);
                }));
 
         $capability = $this->pm->getPluginCapability($plugin, $capabilityApi, array('a' => 1, 'b' => 2));
 
         $this->assertInstanceOf($capabilityApi, $capability);
-        $this->assertInstanceOf($capabilitySpi, $capability);
+        $this->assertInstanceOf($capabilityImplementation, $capability);
         $this->assertSame(array('a' => 1, 'b' => 2), $capability->args);
     }
 
-    public function invalidSpiValues()
+    public function invalidImplementationClassNames()
     {
         return array(
             array(null),
@@ -337,14 +337,22 @@ class PluginInstallerTest extends TestCase
             array(array(1)),
             array(array()),
             array(new \stdClass()),
-            array("NonExistentClassLikeMiddleClass"),
+        );
+    }
+
+    public function nonExistingOrInvalidImplementationClassTypes()
+    {
+        return array(
+            array('\stdClass'),
+            array('NonExistentClassLikeMiddleClass'),
         );
     }
 
     /**
-     * @dataProvider invalidSpiValues
+     * @dataProvider invalidImplementationClassNames
+     * @expectedException \RuntimeException
      */
-    public function testInvalidCapabilitySpiDeclarationsAreDisregarded($invalidSpi)
+    public function testQueryingWithInvalidCapabilityClassNameThrows($invalidImplementationClassNames)
     {
         $capabilityApi = 'Composer\Plugin\Capability\Capability';
 
@@ -353,10 +361,19 @@ class PluginInstallerTest extends TestCase
 
         $plugin->expects($this->once())
                ->method('getCapabilities')
-               ->will($this->returnCallback(function() use ($invalidSpi, $capabilityApi) {
-                   return array($capabilityApi => $invalidSpi);
+               ->will($this->returnCallback(function() use ($invalidImplementationClassNames, $capabilityApi) {
+                   return array($capabilityApi => $invalidImplementationClassNames);
                }));
 
-        $this->assertFalse($this->pm->getPluginCapability($plugin, $capabilityApi));
+        $this->pm->getPluginCapability($plugin, $capabilityApi);
+    }
+
+    /**
+     * @dataProvider nonExistingOrInvalidImplementationClassTypes
+     * @expectedException \RuntimeException
+     */
+    public function testQueryingWithNonExistingOrWrongCapabilityClassTypesThrows($wrongImplementationClassTypes)
+    {
+        $this->testQueryingWithInvalidCapabilityClassNameThrows($wrongImplementationClassTypes);
     }
 }