Add warnings to ValidatingArrayLoader that are simply stripped by default, add reporting of warnings when loading branches

src/Composer/Package/Loader/InvalidPackageException.php

@@ -18,13 +18,15 @@ namespace Composer\Package\Loader;
 class InvalidPackageException extends \Exception
 {
     private $errors;
+    private $warnings;
     private $data;
 
-    public function __construct(array $errors, array $data)
+    public function __construct(array $errors, array $warnings, array $data)
     {
         $this->errors = $errors;
+        $this->warnings = $warnings;
         $this->data = $data;
-        parent::__construct("Invalid package information: \n".implode("\n", $errors));
+        parent::__construct("Invalid package information: \n".implode("\n", array_merge($errors, $warnings)));
     }
 
     public function getData()
@@ -36,4 +38,9 @@ class InvalidPackageException extends \Exception
     {
         return $this->errors;
     }
+
+    public function getWarnings()
+    {
+        return $this->warnings;
+    }
 }

src/Composer/Package/Loader/ValidatingArrayLoader.php

@@ -23,20 +23,20 @@ class ValidatingArrayLoader implements LoaderInterface
 {
     private $loader;
     private $versionParser;
-    private $ignoreErrors;
     private $errors;
+    private $warnings;
     private $config;
 
-    public function __construct(LoaderInterface $loader, $ignoreErrors = true, VersionParser $parser = null)
+    public function __construct(LoaderInterface $loader, VersionParser $parser = null)
     {
         $this->loader = $loader;
-        $this->ignoreErrors = $ignoreErrors;
         $this->versionParser = $parser ?: new VersionParser();
     }
 
     public function load(array $config, $class = 'Composer\Package\CompletePackage')
     {
         $this->errors = array();
+        $this->warnings = array();
         $this->config = $config;
 
         $this->validateRegex('name', '[A-Za-z0-9][A-Za-z0-9_.-]*/[A-Za-z0-9][A-Za-z0-9_.-]*', true);
@@ -77,30 +77,27 @@ class ValidatingArrayLoader implements LoaderInterface
             }
         }
 
-        $this->validateArray('authors');
-        if (!empty($this->config['authors'])) {
+        if ($this->validateArray('authors') && !empty($this->config['authors'])) {
             foreach ($this->config['authors'] as $key => $author) {
                 if (!is_array($author)) {
                     $this->errors[] = 'authors.'.$key.' : should be an array, '.gettype($author).' given';
                     unset($this->config['authors'][$key]);
                     continue;
                 }
+                foreach (array('homepage', 'email', 'name', 'role') as $authorData) {
+                    if (isset($author[$authorData]) && !is_string($author[$authorData])) {
+                        $this->errors[] = 'authors.'.$key.'.'.$authorData.' : invalid value, must be a string';
+                        unset($this->config['authors'][$key][$authorData]);
+                    }
+                }
                 if (isset($author['homepage']) && !$this->filterUrl($author['homepage'])) {
-                    $this->errors[] = 'authors.'.$key.'.homepage : invalid value, must be a valid http/https URL';
+                    $this->warnings[] = 'authors.'.$key.'.homepage : invalid value, must be a valid http/https URL';
                     unset($this->config['authors'][$key]['homepage']);
                 }
                 if (isset($author['email']) && !filter_var($author['email'], FILTER_VALIDATE_EMAIL)) {
-                    $this->errors[] = 'authors.'.$key.'.email : invalid value, must be a valid email address';
+                    $this->warnings[] = 'authors.'.$key.'.email : invalid value, must be a valid email address';
                     unset($this->config['authors'][$key]['email']);
                 }
-                if (isset($author['name']) && !is_string($author['name'])) {
-                    $this->errors[] = 'authors.'.$key.'.name : invalid value, must be a string';
-                    unset($this->config['authors'][$key]['name']);
-                }
-                if (isset($author['role']) && !is_string($author['role'])) {
-                    $this->errors[] = 'authors.'.$key.'.role : invalid value, must be a string';
-                    unset($this->config['authors'][$key]['role']);
-                }
                 if (empty($this->config['authors'][$key])) {
                     unset($this->config['authors'][$key]);
                 }
@@ -110,23 +107,29 @@ class ValidatingArrayLoader implements LoaderInterface
             }
         }
 
-        $this->validateArray('support');
-        if (!empty($this->config['support'])) {
+        if ($this->validateArray('support') && !empty($this->config['support'])) {
+            foreach (array('issues', 'forum', 'wiki', 'source', 'email', 'irc') as $key) {
+                if (isset($this->config['support'][$key]) && !is_string($this->config['support'][$key])) {
+                    $this->errors[] = 'support.'.$key.' : invalid value, must be a string';
+                    unset($this->config['support'][$key]);
+                }
+            }
+
             if (isset($this->config['support']['email']) && !filter_var($this->config['support']['email'], FILTER_VALIDATE_EMAIL)) {
-                $this->errors[] = 'support.email : invalid value, must be a valid email address';
+                $this->warnings[] = 'support.email : invalid value, must be a valid email address';
                 unset($this->config['support']['email']);
             }
 
             if (isset($this->config['support']['irc'])
                 && (!filter_var($this->config['support']['irc'], FILTER_VALIDATE_URL) || !preg_match('{^irc://}iu', $this->config['support']['irc']))
             ) {
-                $this->errors[] = 'support.irc : invalid value, must be ';
+                $this->warnings[] = 'support.irc : invalid value, must be ';
                 unset($this->config['support']['irc']);
             }
 
             foreach (array('issues', 'forum', 'wiki', 'source') as $key) {
                 if (isset($this->config['support'][$key]) && !$this->filterUrl($this->config['support'][$key])) {
-                    $this->errors[] = 'support.'.$key.' : invalid value, must be a valid http/https URL';
+                    $this->warnings[] = 'support.'.$key.' : invalid value, must be a valid http/https URL';
                     unset($this->config['support'][$key]);
                 }
             }
@@ -136,7 +139,7 @@ class ValidatingArrayLoader implements LoaderInterface
         }
 
         foreach (array_keys(BasePackage::$supportedLinkTypes) as $linkType) {
-            if (isset($this->config[$linkType])) {
+            if ($this->validateArray($linkType) && isset($this->config[$linkType])) {
                 foreach ($this->config[$linkType] as $package => $constraint) {
                     if (!is_string($constraint)) {
                         $this->errors[] = $linkType.'.'.$package.' : invalid value, must be a string containing a version constraint';
@@ -153,8 +156,7 @@ class ValidatingArrayLoader implements LoaderInterface
             }
         }
 
-        $this->validateArray('suggest');
-        if (!empty($this->config['suggest'])) {
+        if ($this->validateArray('suggest') && !empty($this->config['suggest'])) {
             foreach ($this->config['suggest'] as $package => $description) {
                 if (!is_string($description)) {
                     $this->errors[] = 'suggest.'.$package.' : invalid value, must be a string describing why the package is suggested';
@@ -205,17 +207,26 @@ class ValidatingArrayLoader implements LoaderInterface
             }
         }
 
-        if ($this->errors && !$this->ignoreErrors) {
-            throw new InvalidPackageException($this->errors, $config);
+        if ($this->errors) {
+            throw new InvalidPackageException($this->errors, $this->warnings, $config);
         }
 
         $package = $this->loader->load($this->config, $class);
-        $this->errors = array();
         $this->config = null;
 
         return $package;
     }
 
+    public function getWarnings()
+    {
+        return $this->warnings;
+    }
+
+    public function getErrors()
+    {
+        return $this->errors;
+    }
+
     private function validateRegex($property, $regex, $mandatory = false)
     {
         if (!$this->validateString($property, $mandatory)) {
@@ -307,11 +318,13 @@ class ValidatingArrayLoader implements LoaderInterface
         }
 
         if (!$this->filterUrl($this->config[$property])) {
-            $this->errors[] = $property.' : invalid value, must be a valid http/https URL';
+            $this->warnings[] = $property.' : invalid value, must be a valid http/https URL';
             unset($this->config[$property]);
 
             return false;
         }
+
+        return true;
     }
 
     private function filterUrl($value)

src/Composer/Repository/VcsRepository.php

@@ -16,6 +16,8 @@ use Composer\Downloader\TransportException;
 use Composer\Repository\Vcs\VcsDriverInterface;
 use Composer\Package\Version\VersionParser;
 use Composer\Package\Loader\ArrayLoader;
+use Composer\Package\Loader\ValidatingArrayLoader;
+use Composer\Package\Loader\InvalidPackageException;
 use Composer\Package\Loader\LoaderInterface;
 use Composer\IO\IOInterface;
 use Composer\Config;
@@ -217,7 +219,12 @@ class VcsRepository extends ArrayRepository
                     $this->io->write('Importing branch '.$branch.' ('.$data['version'].')');
                 }
 
-                $this->addPackage($this->loader->load($this->preProcess($driver, $data, $identifier)));
+                $packageData = $this->preProcess($driver, $data, $identifier);
+                $package = $this->loader->load($packageData);
+                if ($this->loader instanceof ValidatingArrayLoader && $this->loader->getWarnings()) {
+                    throw new InvalidPackageException($this->loader->getErrors(), $this->loader->getWarnings(), $packageData);
+                }
+                $this->addPackage($package);
             } catch (TransportException $e) {
                 if ($verbose) {
                     $this->io->write('Skipped branch '.$branch.', no composer file was found');

src/Composer/Util/ConfigValidator.php

@@ -97,7 +97,7 @@ class ConfigValidator
         }
 
         try {
-            $loader = new ValidatingArrayLoader(new ArrayLoader(), false);
+            $loader = new ValidatingArrayLoader(new ArrayLoader());
             if (!isset($manifest['version'])) {
                 $manifest['version'] = '1.0.0';
             }
@@ -107,6 +107,7 @@ class ConfigValidator
             $loader->load($manifest);
         } catch (InvalidPackageException $e) {
             $errors = array_merge($errors, $e->getErrors());
+            $warnings = array_merge($warnings, $e->getWarnings());
         }
 
         return array($errors, $publishErrors, $warnings);

tests/Composer/Test/Package/Loader/ValidatingArrayLoaderTest.php

@@ -29,7 +29,7 @@ class ValidatingArrayLoaderTest extends \PHPUnit_Framework_TestCase
             ->method('load')
             ->with($config);
 
-        $loader = new ValidatingArrayLoader($internalLoader, false);
+        $loader = new ValidatingArrayLoader($internalLoader);
         $loader->load($config);
     }
 
@@ -148,12 +148,12 @@ class ValidatingArrayLoaderTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @dataProvider failureProvider
+     * @dataProvider errorProvider
      */
     public function testLoadFailureThrowsException($config, $expectedErrors)
     {
         $internalLoader = $this->getMock('Composer\Package\Loader\LoaderInterface');
-        $loader = new ValidatingArrayLoader($internalLoader, false);
+        $loader = new ValidatingArrayLoader($internalLoader);
         try {
             $loader->load($config);
             $this->fail('Expected exception to be thrown');
@@ -166,9 +166,24 @@ class ValidatingArrayLoaderTest extends \PHPUnit_Framework_TestCase
     }
 
     /**
-     * @dataProvider failureProvider
+     * @dataProvider warningProvider
      */
-    public function testLoadSkipsInvalidDataWhenIgnoringErrors($config)
+    public function testLoadWarnings($config, $expectedWarnings)
+    {
+        $internalLoader = $this->getMock('Composer\Package\Loader\LoaderInterface');
+        $loader = new ValidatingArrayLoader($internalLoader);
+
+        $loader->load($config);
+        $warnings = $loader->getWarnings();
+        sort($expectedWarnings);
+        sort($warnings);
+        $this->assertEquals($expectedWarnings, $warnings);
+    }
+
+    /**
+     * @dataProvider warningProvider
+     */
+    public function testLoadSkipsWarningDataWhenIgnoringErrors($config)
     {
         $internalLoader = $this->getMock('Composer\Package\Loader\LoaderInterface');
         $internalLoader
@@ -176,12 +191,12 @@ class ValidatingArrayLoaderTest extends \PHPUnit_Framework_TestCase
             ->method('load')
             ->with(array('name' => 'a/b'));
 
-        $loader = new ValidatingArrayLoader($internalLoader, true);
+        $loader = new ValidatingArrayLoader($internalLoader);
         $config['name'] = 'a/b';
         $loader->load($config);
     }
 
-    public function failureProvider()
+    public function errorProvider()
     {
         return array(
             array(
@@ -192,6 +207,32 @@ class ValidatingArrayLoaderTest extends \PHPUnit_Framework_TestCase
                     'name : invalid value, must match [A-Za-z0-9][A-Za-z0-9_.-]*/[A-Za-z0-9][A-Za-z0-9_.-]*'
                 )
             ),
+            array(
+                array(
+                    'name' => 'foo/bar',
+                    'homepage' => 43,
+                ),
+                array(
+                    'homepage : should be a string, integer given',
+                )
+            ),
+            array(
+                array(
+                    'name' => 'foo/bar',
+                    'support' => array(
+                        'source' => array(),
+                    ),
+                ),
+                array(
+                    'support.source : invalid value, must be a string',
+                )
+            ),
+        );
+    }
+
+    public function warningProvider()
+    {
+        return array(
             array(
                 array(
                     'name' => 'foo/bar',