Generate a warning when a commit reference is used

Closes #4485

src/Composer/Util/ConfigValidator.php

@@ -30,6 +30,8 @@ class ConfigValidator
 {
     private $io;
 
+    private $warnings = array();
+
     public function __construct(IOInterface $io)
     {
         $this->io = $io;
@@ -47,7 +49,6 @@ class ConfigValidator
     {
         $errors = array();
         $publishErrors = array();
-        $warnings = array();
 
         // validate json schema
         $laxValid = false;
@@ -85,18 +86,18 @@ class ConfigValidator
 
             $licenseValidator = new SpdxLicenses();
             if ('proprietary' !== $manifest['license'] && array() !== $manifest['license'] && !$licenseValidator->validate($manifest['license'])) {
-                $warnings[] = sprintf(
+                $this->warnings[] = sprintf(
                     'License %s is not a valid SPDX license identifier, see http://www.spdx.org/licenses/ if you use an open license.'
                     ."\nIf the software is closed-source, you may use \"proprietary\" as license.",
                     json_encode($manifest['license'])
                 );
             }
         } else {
-            $warnings[] = 'No license specified, it is recommended to do so. For closed-source software you may use "proprietary" as license.';
+            $this->warnings[] = 'No license specified, it is recommended to do so. For closed-source software you may use "proprietary" as license.';
         }
 
         if (isset($manifest['version'])) {
-            $warnings[] = 'The version field is present, it is recommended to leave it out if the package is published on Packagist.';
+            $this->warnings[] = 'The version field is present, it is recommended to leave it out if the package is published on Packagist.';
         }
 
         if (!empty($manifest['name']) && preg_match('{[A-Z]}', $manifest['name'])) {
@@ -111,25 +112,28 @@ class ConfigValidator
         }
 
         if (!empty($manifest['type']) && $manifest['type'] == 'composer-installer') {
-            $warnings[] = "The package type 'composer-installer' is deprecated. Please distribute your custom installers as plugins from now on. See http://getcomposer.org/doc/articles/plugins.md for plugin documentation.";
+            $this->warnings[] = "The package type 'composer-installer' is deprecated. Please distribute your custom installers as plugins from now on. See http://getcomposer.org/doc/articles/plugins.md for plugin documentation.";
         }
 
         // check for require-dev overrides
         if (isset($manifest['require']) && isset($manifest['require-dev'])) {
             $requireOverrides = array_intersect_key($manifest['require'], $manifest['require-dev']);
-
             if (!empty($requireOverrides)) {
                 $plural = (count($requireOverrides) > 1) ? 'are' : 'is';
-                $warnings[] = implode(', ', array_keys($requireOverrides)). " {$plural} required both in require and require-dev, this can lead to unexpected behavior";
+                $this->warnings[] = implode(', ', array_keys($requireOverrides)). " {$plural} required both in require and require-dev, this can lead to unexpected behavior";
             }
         }
 
+        // check for commit references
+        $this->checkForCommitReferences($manifest['require']);
+        $this->checkForCommitReferences($manifest['require-dev']);
+
         // check for empty psr-0/psr-4 namespace prefixes
         if (isset($manifest['autoload']['psr-0'][''])) {
-            $warnings[] = "Defining autoload.psr-0 with an empty namespace prefix is a bad idea for performance";
+            $this->warnings[] = "Defining autoload.psr-0 with an empty namespace prefix is a bad idea for performance";
         }
         if (isset($manifest['autoload']['psr-4'][''])) {
-            $warnings[] = "Defining autoload.psr-4 with an empty namespace prefix is a bad idea for performance";
+            $this->warnings[] = "Defining autoload.psr-4 with an empty namespace prefix is a bad idea for performance";
         }
 
         try {
@@ -145,8 +149,27 @@ class ConfigValidator
             $errors = array_merge($errors, $e->getErrors());
         }
 
-        $warnings = array_merge($warnings, $loader->getWarnings());
+        $this->warnings = array_merge($this->warnings, $loader->getWarnings());
 
-        return array($errors, $publishErrors, $warnings);
+        return array($errors, $publishErrors, $this->warnings);
+    }
+
+    /**
+     * Check for explicit commit references.
+     *
+     * @param array $packages An array of packages and their versions
+     *
+     * @return void
+     */
+    private function checkForCommitReferences(array $packages)
+    {
+        foreach ($packages as $package => $version) {
+            if (preg_match('/#/', $version) === 1) {
+                $this->warnings[] = sprintf(
+                    'The package "%s" is pointing to a commit-ref, this is bad practice and can cause unforeseen issues.',
+                    $package
+                );
+            }
+        }
     }
 }

tests/Composer/Test/Util/ConfigValidatorTest.php

@@ -0,0 +1,48 @@
+<?php
+
+/*
+ * This file is part of Composer.
+ *
+ * (c) Nils Adermann <naderman@naderman.de>
+ *     Jordi Boggiano <j.boggiano@seld.be>
+ *
+ * For the full copyright and license information, please view the LICENSE
+ * file that was distributed with this source code.
+ */
+
+namespace Composer\Test\Util;
+
+use Composer\IO\NullIO;
+use Composer\Util\ConfigValidator;
+use Composer\TestCase;
+
+/**
+ * ConfigValidator test case
+ */
+class ConfigValidatorTest extends TestCase
+{
+    /**
+     * Test ConfigValidator warns on commit reference
+     */
+    public function testConfigValidatorCommitRefWarning()
+    {
+        $configValidator = new ConfigValidator(new NullIO());
+        $reflection      = new \ReflectionClass(get_class($configValidator));
+        $method          = $reflection->getMethod('checkForCommitReferences');
+        $warnings        = $reflection->getProperty('warnings');
+
+        $method->setAccessible(true);
+        $warnings->setAccessible(true);
+
+        $this->assertEquals(0, count($warnings->getValue($configValidator)));
+
+        $method->invokeArgs($configValidator, array(
+            array(
+                'some-package'    => 'dev-master#62c4da6',
+                'another-package' => '^1.0.0'
+            )
+        ));
+
+        $this->assertEquals(1, count($warnings->getValue($configValidator)));
+    }
+}