Fix CS & simplify code

bin/fetch-spdx-identifiers

@@ -16,7 +16,7 @@ class SPDXLicenseIdentifiersOnline
     private $identifiers;
 
     /**
-     * @return string[]
+     * @return array
      */
     public function getStrings()
     {
@@ -59,7 +59,7 @@ class JsonPrinter
 {
     /**
      *
-     * @param string[] $array
+     * @param array $array
      */
     public function printStringArray(array $array)
     {

src/Composer/Command/ValidateCommand.php

@@ -18,7 +18,7 @@ use Symfony\Component\Console\Output\OutputInterface;
 use Composer\Json\JsonFile;
 use Composer\Json\JsonValidationException;
 use Composer\Util\RemoteFilesystem;
-use Composer\Util\SPDXLicenseIdentifier;
+use Composer\Util\SpdxLicenseIdentifier;
 
 /**
  * ValidateCommand
@@ -37,8 +37,8 @@ class ValidateCommand extends Command
             ->setName('validate')
             ->setDescription('Validates a composer.json')
             ->setDefinition(array(
-            new InputArgument('file', InputArgument::OPTIONAL, 'path to composer.json file', './composer.json')
-        ))
+                new InputArgument('file', InputArgument::OPTIONAL, 'path to composer.json file', './composer.json')
+            ))
             ->setHelp(<<<EOT
 The validate command validates a given composer.json
 
@@ -47,8 +47,8 @@ EOT
     }
 
     /**
-     * @param \Symfony\Component\Console\Input\InputInterface $input
-     * @param \Symfony\Component\Console\Output\OutputInterface $output
+     * @param InputInterface $input
+     * @param OutputInterface $output
      *
      * @return int
      */
@@ -95,11 +95,11 @@ EOT
         }
 
         if (isset($manifest['license'])) {
-            try {
-                $identifier = new SPDXLicenseIdentifier($manifest['license']);
-            } catch (\InvalidArgumentException $e) {
+            $licenseValidator = new SpdxLicenseIdentifier();
+            if (!$licenseValidator->validate($manifest['license'])) {
                 $output->writeln(sprintf(
-                    '<warning>License "%s" is not a SPDX license identifier.</warning>',
+                    '<warning>License "%s" is not a valid SPDX license identifier</warning>'."\n".
+                    '<warning>see http://www.spdx.org/licenses/ and http://getcomposer.org/doc/04-schema.md#license</warning>',
                     print_r($manifest['license'], true)
                 ));
             }

src/Composer/Util/SpdxLicenseIdentifier.php

@@ -12,48 +12,24 @@
 
 namespace Composer\Util;
 
+use Composer\Json\JsonFile;
+
 /**
- * SPDX License Identifier
- *
  * Supports composer array and SPDX tag notation for disjunctive/conjunctive
  * licenses.
  *
  * @author Tom Klingenberg <tklingenberg@lastflood.net>
  */
-class SPDXLicenseIdentifier
+class SpdxLicenseIdentifier
 {
     /**
      * @var array
      */
     private $identifiers;
-    /**
-     * @var array|string
-     */
-    private $license;
 
-    /**
-     * @param string|string[] $license
-     */
-    public function __construct($license)
+    public function __construct()
     {
         $this->initIdentifiers();
-        $this->setLicense($license);
-    }
-
-    /**
-     * @return string
-     */
-    public function __toString()
-    {
-        return $this->getLicense();
-    }
-
-    /**
-     * @return string
-     */
-    public function getLicense()
-    {
-        return $this->license;
     }
 
     /**
@@ -61,10 +37,10 @@ class SPDXLicenseIdentifier
      *
      * @throws \InvalidArgumentException
      */
-    public function setLicense($license)
+    public function validate($license)
     {
         if (is_array($license)) {
-            $license = $this->getLicenseFromArray($license);
+            $license = count($license) > 1 ? '('.implode(' or ', $license).')' : reset($license);
         }
         if (!is_string($license)) {
             throw new \InvalidArgumentException(sprintf(
@@ -72,52 +48,19 @@ class SPDXLicenseIdentifier
             ));
         }
         if (!$this->isValidLicenseString($license)) {
-            throw new \InvalidArgumentException(sprintf(
-                'Invalid license: "%s"', $license
-            ));
-        }
-        $this->license = $license;
-    }
-
-    /**
-     * @param array $licenses
-     *
-     * @return string
-     */
-    private function getLicenseFromArray(array $licenses)
-    {
-        $buffer = '';
-        foreach ($licenses as $license) {
-            $buffer .= ($buffer ? ' or ' : '(') . (string)$license;
+            return false;
         }
-        $buffer .= $buffer ? ')' : '';
 
-        return $buffer;
+        return true;
     }
 
     /**
-     * init SPDX identifiers
+     * Loads SPDX identifiers
      */
     private function initIdentifiers()
     {
-        $jsonFile = __DIR__ . '/../../../res/spdx-identifier.json';
-        $this->identifiers = $this->arrayFromJSONFile($jsonFile);
-    }
-
-    /**
-     * @param string $file
-     *
-     * @return array
-     * @throws \RuntimeException
-     */
-    private function arrayFromJSONFile($file)
-    {
-        $data = json_decode(file_get_contents($file));
-        if (!$data || !is_array($data)) {
-            throw new \RuntimeException(sprintf('Not a json array in file "%s"', $file));
-        }
-
-        return $data;
+        $jsonFile = new JsonFile(__DIR__ . '/../../../res/spdx-identifier.json');
+        $this->identifiers = $jsonFile->read();
     }
 
     /**
@@ -148,14 +91,16 @@ class SPDXLicenseIdentifier
             'ws' => '\s+',
             '_' => '.',
         );
-        $next = function () use ($license, $tokens)
-        {
+
+        $next = function () use ($license, $tokens) {
             static $offset = 0;
+
             if ($offset >= strlen($license)) {
                 return null;
             }
+
             foreach ($tokens as $name => $token) {
-                if (false === $r = preg_match("~$token~", $license, $matches, PREG_OFFSET_CAPTURE, $offset)) {
+                if (false === $r = preg_match('{' . $token . '}', $license, $matches, PREG_OFFSET_CAPTURE, $offset)) {
                     throw new \RuntimeException('Pattern for token %s failed (regex error).', $name);
                 }
                 if ($r === 0) {
@@ -168,12 +113,15 @@ class SPDXLicenseIdentifier
 
                 return array($name, $matches[0][0]);
             }
+
             throw new \RuntimeException('At least the last pattern needs to match, but it did not (dot-match-all is missing?).');
         };
+
         $open = 0;
         $require = 1;
         $lastop = null;
-        while (list ($token, $string) = $next()) {
+
+        while (list($token, $string) = $next()) {
             switch ($token) {
                 case 'po':
                     if ($open || !$require) {

tests/Composer/Test/Util/SpdxLicenseIdentifierTest.php

@@ -2,11 +2,10 @@
 namespace Composer\Test\Util;
 
 use Composer\Test\TestCase;
-use Composer\Util\SPDXLicenseIdentifier;
+use Composer\Util\SpdxLicenseIdentifier;
 
-class SPDXLicenseIdentifierTest extends TestCase
+class SpdxLicenseIdentifierTest extends TestCase
 {
-
     public static function provideValidLicenses()
     {
         $valid = array_merge(
@@ -32,7 +31,6 @@ class SPDXLicenseIdentifierTest extends TestCase
     public static function provideInvalidLicenses()
     {
         return array(
-            array(NULL),
             array(""),
             array("The system pwns you"),
             array("()"),
@@ -56,28 +54,28 @@ class SPDXLicenseIdentifierTest extends TestCase
      * @dataProvider provideValidLicenses
      * @param $license
      */
-    public function testConstructor($license)
+    public function testValidate($license)
     {
-        $identifier = new SPDXLicenseIdentifier($license);
-        $this->assertInstanceOf('Composer\Util\SPDXLicenseIdentifier', $identifier);
+        $validator = new SpdxLicenseIdentifier();
+        $this->assertTrue($validator->validate($license));
     }
 
     /**
      * @dataProvider provideInvalidLicenses
-     * @expectedException InvalidArgumentException
      * @param string|array $invalidLicense
      */
     public function testInvalidLicenses($invalidLicense)
     {
-        $identifier = new SPDXLicenseIdentifier($invalidLicense);
+        $validator = new SpdxLicenseIdentifier();
+        $this->assertFalse($validator->validate($invalidLicense));
     }
 
-    public function testGetLicense()
+    /**
+     * @expectedException InvalidArgumentException
+     */
+    public function testInvalidArgument()
     {
-        $license = new SPDXLicenseIdentifier('NONE');
-        $string = $license->getLicense();
-        $this->assertInternalType('string', $string);
-        $string = (string)$license;
-        $this->assertInternalType('string', $string);
+        $validator = new SpdxLicenseIdentifier();
+        $validator->validate(null);
     }
 }