Merge pull request #7454 from stof/conflict_optimization

Refactor the handling of conflict rules in the solver

src/Composer/DependencyResolver/Pool.php

@@ -317,12 +317,12 @@ class Pool implements \Countable
      * Checks if the package matches the given constraint directly or through
      * provided or replaced packages
      *
-     * @param  array|PackageInterface $candidate
+     * @param  PackageInterface       $candidate
      * @param  string                 $name       Name of the package to be matched
      * @param  ConstraintInterface    $constraint The constraint to verify
      * @return int                    One of the MATCH* constants of this class or 0 if there is no match
      */
-    private function match($candidate, $name, ConstraintInterface $constraint = null, $bypassFilters)
+    public function match($candidate, $name, ConstraintInterface $constraint = null, $bypassFilters)
     {
         $candidateName = $candidate->getName();
         $candidateVersion = $candidate->getVersion();

src/Composer/DependencyResolver/RuleSetGenerator.php

@@ -28,6 +28,9 @@ class RuleSetGenerator
     protected $installedMap;
     protected $whitelistedMap;
     protected $addedMap;
+    protected $conflictAddedMap;
+    protected $addedPackages;
+    protected $addedPackagesByNames;
 
     public function __construct(PolicyInterface $policy, Pool $pool)
     {
@@ -185,6 +188,7 @@ class RuleSetGenerator
         $workQueue->enqueue($package);
 
         while (!$workQueue->isEmpty()) {
+            /** @var PackageInterface $package */
             $package = $workQueue->dequeue();
             if (isset($this->addedMap[$package->id])) {
                 continue;
@@ -192,6 +196,11 @@ class RuleSetGenerator
 
             $this->addedMap[$package->id] = true;
 
+            $this->addedPackages[] = $package;
+            foreach ($package->getNames() as $name) {
+                $this->addedPackagesByNames[$name][] = $package;
+            }
+
             foreach ($package->getRequires() as $link) {
                 if ($ignorePlatformReqs && preg_match(PlatformRepository::PLATFORM_PACKAGE_REGEX, $link->getTarget())) {
                     continue;
@@ -206,32 +215,6 @@ class RuleSetGenerator
                 }
             }
 
-            foreach ($package->getConflicts() as $link) {
-                $possibleConflicts = $this->pool->whatProvides($link->getTarget(), $link->getConstraint());
-
-                foreach ($possibleConflicts as $conflict) {
-                    $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $conflict, Rule::RULE_PACKAGE_CONFLICT, $link));
-                }
-            }
-
-            // check obsoletes and implicit obsoletes of a package
-            $isInstalled = isset($this->installedMap[$package->id]);
-
-            foreach ($package->getReplaces() as $link) {
-                $obsoleteProviders = $this->pool->whatProvides($link->getTarget(), $link->getConstraint());
-
-                foreach ($obsoleteProviders as $provider) {
-                    if ($provider === $package) {
-                        continue;
-                    }
-
-                    if (!$this->obsoleteImpossibleForAlias($package, $provider)) {
-                        $reason = $isInstalled ? Rule::RULE_INSTALLED_PACKAGE_OBSOLETES : Rule::RULE_PACKAGE_OBSOLETES;
-                        $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $provider, $reason, $link));
-                    }
-                }
-            }
-
             $packageName = $package->getName();
             $obsoleteProviders = $this->pool->whatProvides($packageName, null);
 
@@ -250,6 +233,49 @@ class RuleSetGenerator
         }
     }
 
+    protected function addConflictRules()
+    {
+        /** @var PackageInterface $package */
+        foreach ($this->addedPackages as $package) {
+            foreach ($package->getConflicts() as $link) {
+                if (!isset($this->addedPackagesByNames[$link->getTarget()])) {
+                    continue;
+                }
+
+                /** @var PackageInterface $possibleConflict */
+                foreach ($this->addedPackagesByNames[$link->getTarget()] as $possibleConflict) {
+                    $conflictMatch = $this->pool->match($possibleConflict, $link->getTarget(), $link->getConstraint(), true);
+
+                    if ($conflictMatch === Pool::MATCH || $conflictMatch === Pool::MATCH_REPLACE) {
+                        $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $possibleConflict, Rule::RULE_PACKAGE_CONFLICT, $link));
+                    }
+
+                }
+            }
+
+            // check obsoletes and implicit obsoletes of a package
+            $isInstalled = isset($this->installedMap[$package->id]);
+
+            foreach ($package->getReplaces() as $link) {
+                if (!isset($this->addedPackagesByNames[$link->getTarget()])) {
+                    continue;
+                }
+
+                /** @var PackageInterface $possibleConflict */
+                foreach ($this->addedPackagesByNames[$link->getTarget()] as $provider) {
+                    if ($provider === $package) {
+                        continue;
+                    }
+
+                    if (!$this->obsoleteImpossibleForAlias($package, $provider)) {
+                        $reason = $isInstalled ? Rule::RULE_INSTALLED_PACKAGE_OBSOLETES : Rule::RULE_PACKAGE_OBSOLETES;
+                        $this->addRule(RuleSet::TYPE_PACKAGE, $this->createRule2Literals($package, $provider, $reason, $link));
+                    }
+                }
+            }
+        }
+    }
+
     protected function obsoleteImpossibleForAlias($package, $provider)
     {
         $packageIsAlias = $package instanceof AliasPackage;
@@ -327,12 +353,20 @@ class RuleSetGenerator
         $this->pool->setWhitelist($this->whitelistedMap);
 
         $this->addedMap = array();
+        $this->conflictAddedMap = array();
+        $this->addedPackages = array();
+        $this->addedPackagesByNames = array();
         foreach ($this->installedMap as $package) {
             $this->addRulesForPackage($package, $ignorePlatformReqs);
         }
 
         $this->addRulesForJobs($ignorePlatformReqs);
 
+        $this->addConflictRules();
+
+        // Remove references to packages
+        $this->addedPackages = $this->addedPackagesByNames = null;
+
         return $this->rules;
     }
 }

tests/Composer/Test/Fixtures/installer/replaced-packages-should-not-be-installed-when-installing-from-lock.test

@@ -1,5 +1,5 @@
 --TEST--
-Requiring a replaced package in a version, that is not provided by the replacing package, should install correctly (although that is not a very smart idea)
+Requiring a replaced package in a version, that is not provided by the replacing package, should result in a conflict, when installing from lock
 --COMPOSER--
 {
     "repositories": [
@@ -17,9 +17,7 @@ Requiring a replaced package in a version, that is not provided by the replacing
         "foo/replaced": "2.0.0"
     }
 }
---RUN--
+--LOCK--
-install
---EXPECT-LOCK--
 {
     "packages": [
         { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"}, "type": "library" },
@@ -34,8 +32,8 @@ install
     "platform": [],
     "platform-dev": []
 }
+--RUN--
+install
 --EXPECT-EXIT-CODE--
-0
+2
 --EXPECT--
-Installing foo/original (1.0.0)
-Installing foo/replaced (2.0.0)

tests/Composer/Test/Fixtures/installer/replaced-packages-should-not-be-installed.test

@@ -0,0 +1,24 @@
+--TEST--
+Requiring a replaced package in a version, that is not provided by the replacing package, should result in a conflict
+--COMPOSER--
+{
+    "repositories": [
+        {
+            "type": "package",
+            "package": [
+                { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"} },
+                { "name": "foo/replaced", "version": "1.0.0" },
+                { "name": "foo/replaced", "version": "2.0.0" }
+            ]
+        }
+    ],
+    "require": {
+        "foo/original": "1.0.0",
+        "foo/replaced": "2.0.0"
+    }
+}
+--RUN--
+install
+--EXPECT-EXIT-CODE--
+2
+--EXPECT--

tests/Composer/Test/Fixtures/installer/replaced-packages-wrong-version-install-from-lock.test

@@ -1,41 +0,0 @@
---TEST--
-Requiring a replaced package in a version, that is not provided by the replacing package, should install correctly (although that is not a very smart idea) also when installing from lock
---COMPOSER--
-{
-    "repositories": [
-        {
-            "type": "package",
-            "package": [
-                { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"} },
-                { "name": "foo/replaced", "version": "1.0.0" },
-                { "name": "foo/replaced", "version": "2.0.0" }
-            ]
-        }
-    ],
-    "require": {
-        "foo/original": "1.0.0",
-        "foo/replaced": "2.0.0"
-    }
-}
---LOCK--
-{
-    "packages": [
-        { "name": "foo/original", "version": "1.0.0", "replace": {"foo/replaced": "1.0.0"}, "type": "library" },
-        { "name": "foo/replaced", "version": "2.0.0", "type": "library" }
-    ],
-    "packages-dev": [],
-    "aliases": [],
-    "minimum-stability": "stable",
-    "stability-flags": {},
-    "prefer-stable": false,
-    "prefer-lowest": false,
-    "platform": [],
-    "platform-dev": []
-}
---RUN--
-install
---EXPECT-EXIT-CODE--
-0
---EXPECT--
-Installing foo/original (1.0.0)
-Installing foo/replaced (2.0.0)