Merge pull request #3481 from naderman/optimize-solver

Solver optimizations

src/Composer/DependencyResolver/DefaultPolicy.php

@@ -151,18 +151,18 @@ class DefaultPolicy implements PolicyInterface
             }
 
             // priority equal, sort by package id to make reproducible
-            if ($a->getId() === $b->getId()) {
+            if ($a->id === $b->id) {
                 return 0;
             }
 
-            return ($a->getId() < $b->getId()) ? -1 : 1;
+            return ($a->id < $b->id) ? -1 : 1;
         }
 
-        if (isset($installedMap[$a->getId()])) {
+        if (isset($installedMap[$a->id])) {
             return -1;
         }
 
-        if (isset($installedMap[$b->getId()])) {
+        if (isset($installedMap[$b->id])) {
             return 1;
         }
 
@@ -227,7 +227,7 @@ class DefaultPolicy implements PolicyInterface
         foreach ($literals as $literal) {
             $package = $pool->literalToPackage($literal);
 
-            if (isset($installedMap[$package->getId()])) {
+            if (isset($installedMap[$package->id])) {
                 $selected[] = $literal;
                 continue;
             }

src/Composer/DependencyResolver/Pool.php

@@ -103,7 +103,7 @@ class Pool
                     if ($exempt || $this->isPackageAcceptable($names, $stability)) {
                         $package->setId($this->id++);
                         $this->packages[] = $package;
-                        $this->packageByExactName[$package->getName()][$package->getId()] = $package;
+                        $this->packageByExactName[$package->getName()][$package->id] = $package;
 
                         foreach ($names as $provided) {
                             $this->packageByName[$provided][] = $package;
@@ -122,7 +122,7 @@ class Pool
 
                             $package->getRepository()->addPackage($aliasPackage);
                             $this->packages[] = $aliasPackage;
-                            $this->packageByExactName[$aliasPackage->getName()][$aliasPackage->getId()] = $aliasPackage;
+                            $this->packageByExactName[$aliasPackage->getName()][$aliasPackage->id] = $aliasPackage;
 
                             foreach ($aliasPackage->getNames() as $name) {
                                 $this->packageByName[$name][] = $aliasPackage;
@@ -186,7 +186,7 @@ class Pool
         foreach ($this->providerRepos as $repo) {
             foreach ($repo->whatProvides($this, $name) as $candidate) {
                 $candidates[] = $candidate;
-                if ($candidate->getId() < 1) {
+                if ($candidate->id < 1) {
                     $candidate->setId($this->id++);
                     $this->packages[$this->id - 2] = $candidate;
                 }
@@ -217,8 +217,8 @@ class Pool
             }
 
             if ($this->whitelist !== null && (
-                (!($candidate instanceof AliasPackage) && !isset($this->whitelist[$candidate->getId()])) ||
-                ($candidate instanceof AliasPackage && !isset($this->whitelist[$aliasOfCandidate->getId()]))
+                (!($candidate instanceof AliasPackage) && !isset($this->whitelist[$candidate->id])) ||
+                ($candidate instanceof AliasPackage && !isset($this->whitelist[$aliasOfCandidate->id]))
             )) {
                 continue;
             }
@@ -275,7 +275,7 @@ class Pool
     {
         $package = $this->literalToPackage($literal);
 
-        if (isset($installedMap[$package->getId()])) {
+        if (isset($installedMap[$package->id])) {
             $prefix = ($literal > 0 ? 'keep' : 'remove');
         } else {
             $prefix = ($literal > 0 ? 'install' : 'don\'t install');

src/Composer/DependencyResolver/Problem.php

@@ -130,7 +130,7 @@ class Problem
                 $messages[] = $this->jobToText($job);
             } elseif ($rule) {
                 if ($rule instanceof Rule) {
-                    $messages[] = $rule->getPrettyString($installedMap);
+                    $messages[] = $rule->getPrettyString($this->pool, $installedMap);
                 }
             }
         }

src/Composer/DependencyResolver/Rule.php

@@ -29,10 +29,13 @@ class Rule
     const RULE_LEARNED = 12;
     const RULE_PACKAGE_ALIAS = 13;
 
-    protected $pool;
+    /**
+     * READ-ONLY: The literals this rule consists of.
+     * @var array
+     */
+    public $literals;
 
     protected $disabled;
-    protected $literals;
     protected $type;
     protected $id;
     protected $reason;
@@ -42,10 +45,8 @@ class Rule
 
     protected $ruleHash;
 
-    public function __construct(Pool $pool, array $literals, $reason, $reasonData, $job = null)
+    public function __construct(array $literals, $reason, $reasonData, $job = null)
     {
-        $this->pool = $pool;
-
         // sort all packages ascending by id
         sort($literals);
 
@@ -160,6 +161,9 @@ class Rule
         return !$this->disabled;
     }
 
+    /**
+     * @deprecated Use public literals member
+     */
     public function getLiterals()
     {
         return $this->literals;
@@ -170,14 +174,14 @@ class Rule
         return 1 === count($this->literals);
     }
 
-    public function getPrettyString(array $installedMap = array())
+    public function getPrettyString(Pool $pool, array $installedMap = array())
     {
         $ruleText = '';
         foreach ($this->literals as $i => $literal) {
             if ($i != 0) {
                 $ruleText .= '|';
             }
-            $ruleText .= $this->pool->literalToPrettyString($literal, $installedMap);
+            $ruleText .= $pool->literalToPrettyString($literal, $installedMap);
         }
 
         switch ($this->reason) {
@@ -191,24 +195,24 @@ class Rule
                 return "Remove command rule ($ruleText)";
 
             case self::RULE_PACKAGE_CONFLICT:
-                $package1 = $this->pool->literalToPackage($this->literals[0]);
-                $package2 = $this->pool->literalToPackage($this->literals[1]);
+                $package1 = $pool->literalToPackage($this->literals[0]);
+                $package2 = $pool->literalToPackage($this->literals[1]);
 
-                return $package1->getPrettyString().' conflicts with '.$this->formatPackagesUnique(array($package2)).'.';
+                return $package1->getPrettyString().' conflicts with '.$this->formatPackagesUnique($pool, array($package2)).'.';
 
             case self::RULE_PACKAGE_REQUIRES:
                 $literals = $this->literals;
                 $sourceLiteral = array_shift($literals);
-                $sourcePackage = $this->pool->literalToPackage($sourceLiteral);
+                $sourcePackage = $pool->literalToPackage($sourceLiteral);
 
                 $requires = array();
                 foreach ($literals as $literal) {
-                    $requires[] = $this->pool->literalToPackage($literal);
+                    $requires[] = $pool->literalToPackage($literal);
                 }
 
                 $text = $this->reasonData->getPrettyString($sourcePackage);
                 if ($requires) {
-                    $text .= ' -> satisfiable by ' . $this->formatPackagesUnique($requires) . '.';
+                    $text .= ' -> satisfiable by ' . $this->formatPackagesUnique($pool, $requires) . '.';
                 } else {
                     $targetName = $this->reasonData->getTarget();
 
@@ -235,22 +239,24 @@ class Rule
             case self::RULE_INSTALLED_PACKAGE_OBSOLETES:
                 return $ruleText;
             case self::RULE_PACKAGE_SAME_NAME:
-                return 'Can only install one of: ' . $this->formatPackagesUnique($this->literals) . '.';
+                return 'Can only install one of: ' . $this->formatPackagesUnique($pool, $this->literals) . '.';
             case self::RULE_PACKAGE_IMPLICIT_OBSOLETES:
                 return $ruleText;
             case self::RULE_LEARNED:
                 return 'Conclusion: '.$ruleText;
             case self::RULE_PACKAGE_ALIAS:
                 return $ruleText;
+            default:
+                return '('.$ruleText.')';
         }
     }
 
-    protected function formatPackagesUnique(array $packages)
+    protected function formatPackagesUnique($pool, array $packages)
     {
         $prepared = array();
         foreach ($packages as $package) {
             if (!is_object($package)) {
-                $package = $this->pool->literalToPackage($package);
+                $package = $pool->literalToPackage($package);
             }
             $prepared[$package->getName()]['name'] = $package->getPrettyName();
             $prepared[$package->getName()]['versions'][$package->getVersion()] = $package->getPrettyVersion();
@@ -275,7 +281,7 @@ class Rule
             if ($i != 0) {
                 $result .= '|';
             }
-            $result .= $this->pool->literalToString($literal);
+            $result .= $literal;
         }
 
         $result .= ')';

src/Composer/DependencyResolver/RuleSet.php

@@ -22,6 +22,13 @@ class RuleSet implements \IteratorAggregate, \Countable
     const TYPE_JOB = 1;
     const TYPE_LEARNED = 4;
 
+    /**
+     * READ-ONLY: Lookup table for rule id to rule object
+     *
+     * @var Rule[]
+     */
+    public $ruleById;
+
     protected static $types = array(
         -1 => 'UNKNOWN',
         self::TYPE_PACKAGE => 'PACKAGE',
@@ -30,7 +37,6 @@ class RuleSet implements \IteratorAggregate, \Countable
     );
 
     protected $rules;
-    protected $ruleById;
     protected $nextRuleId;
 
     protected $rulesByHash;
@@ -144,17 +150,22 @@ class RuleSet implements \IteratorAggregate, \Countable
         return false;
     }
 
-    public function __toString()
+    public function getPrettyString(Pool $pool = null)
     {
         $string = "\n";
         foreach ($this->rules as $type => $rules) {
             $string .= str_pad(self::$types[$type], 8, ' ') . ": ";
             foreach ($rules as $rule) {
-                $string .= $rule."\n";
+                $string .= ($pool ? $rule->getPrettyString($pool) : $rule)."\n";
             }
             $string .= "\n\n";
         }
 
         return $string;
     }
+
+    public function __toString()
+    {
+        return $this->getPrettyString(null);
+    }
 }

src/Composer/DependencyResolver/RuleSetGenerator.php

@@ -51,17 +51,17 @@ class RuleSetGenerator
      */
     protected function createRequireRule(PackageInterface $package, array $providers, $reason, $reasonData = null)
     {
-        $literals = array(-$package->getId());
+        $literals = array(-$package->id);
 
         foreach ($providers as $provider) {
             // self fulfilling rule?
             if ($provider === $package) {
                 return null;
             }
-            $literals[] = $provider->getId();
+            $literals[] = $provider->id;
         }
 
-        return new Rule($this->pool, $literals, $reason, $reasonData);
+        return new Rule($literals, $reason, $reasonData);
     }
 
     /**
@@ -80,10 +80,10 @@ class RuleSetGenerator
     {
         $literals = array();
         foreach ($packages as $package) {
-            $literals[] = $package->getId();
+            $literals[] = $package->id;
         }
 
-        return new Rule($this->pool, $literals, $reason, $job['packageName'], $job);
+        return new Rule($literals, $reason, $job['packageName'], $job);
     }
 
     /**
@@ -99,7 +99,7 @@ class RuleSetGenerator
      */
     protected function createRemoveRule(PackageInterface $package, $reason, $job)
     {
-        return new Rule($this->pool, array(-$package->getId()), $reason, $job['packageName'], $job);
+        return new Rule(array(-$package->id), $reason, $job['packageName'], $job);
     }
 
     /**
@@ -123,7 +123,7 @@ class RuleSetGenerator
             return null;
         }
 
-        return new Rule($this->pool, array(-$issuer->getId(), -$provider->getId()), $reason, $reasonData);
+        return new Rule(array(-$issuer->id, -$provider->id), $reason, $reasonData);
     }
 
     /**
@@ -151,11 +151,11 @@ class RuleSetGenerator
 
         while (!$workQueue->isEmpty()) {
             $package = $workQueue->dequeue();
-            if (isset($this->whitelistedMap[$package->getId()])) {
+            if (isset($this->whitelistedMap[$package->id])) {
                 continue;
             }
 
-            $this->whitelistedMap[$package->getId()] = true;
+            $this->whitelistedMap[$package->id] = true;
 
             foreach ($package->getRequires() as $link) {
                 $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint(), true);
@@ -186,11 +186,11 @@ class RuleSetGenerator
 
         while (!$workQueue->isEmpty()) {
             $package = $workQueue->dequeue();
-            if (isset($this->addedMap[$package->getId()])) {
+            if (isset($this->addedMap[$package->id])) {
                 continue;
             }
 
-            $this->addedMap[$package->getId()] = true;
+            $this->addedMap[$package->id] = true;
 
             foreach ($package->getRequires() as $link) {
                 if ($ignorePlatformReqs && preg_match(PlatformRepository::PLATFORM_PACKAGE_REGEX, $link->getTarget())) {
@@ -215,7 +215,7 @@ class RuleSetGenerator
             }
 
             // check obsoletes and implicit obsoletes of a package
-            $isInstalled = (isset($this->installedMap[$package->getId()]));
+            $isInstalled = (isset($this->installedMap[$package->id]));
 
             foreach ($package->getReplaces() as $link) {
                 $obsoleteProviders = $this->pool->whatProvides($link->getTarget(), $link->getConstraint());
@@ -289,7 +289,7 @@ class RuleSetGenerator
                     $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
                     if ($packages) {
                         foreach ($packages as $package) {
-                            if (!isset($this->installedMap[$package->getId()])) {
+                            if (!isset($this->installedMap[$package->id])) {
                                 $this->addRulesForPackage($package, $ignorePlatformReqs);
                             }
                         }

src/Composer/DependencyResolver/RuleWatchGraph.php

@@ -95,7 +95,7 @@ class RuleWatchGraph
             $otherWatch = $node->getOtherWatch($literal);
 
             if (!$node->getRule()->isDisabled() && !$decisions->satisfy($otherWatch)) {
-                $ruleLiterals = $node->getRule()->getLiterals();
+                $ruleLiterals = $node->getRule()->literals;
 
                 $alternativeLiterals = array_filter($ruleLiterals, function ($ruleLiteral) use ($literal, $otherWatch, $decisions) {
                     return $literal !== $ruleLiteral &&

src/Composer/DependencyResolver/RuleWatchNode.php

@@ -35,7 +35,7 @@ class RuleWatchNode
     {
         $this->rule = $rule;
 
-        $literals = $rule->getLiterals();
+        $literals = $rule->literals;
 
         $this->watch1 = count($literals) > 0 ? $literals[0] : 0;
         $this->watch2 = count($literals) > 1 ? $literals[1] : 0;
@@ -51,7 +51,7 @@ class RuleWatchNode
      */
     public function watch2OnHighest(Decisions $decisions)
     {
-        $literals = $this->rule->getLiterals();
+        $literals = $this->rule->literals;
 
         // if there are only 2 elements, both are being watched anyway
         if (count($literals) < 3) {

src/Composer/DependencyResolver/Solver.php

@@ -56,13 +56,13 @@ class Solver
 
         $rulesCount = count($this->rules);
         for ($ruleIndex = 0; $ruleIndex < $rulesCount; $ruleIndex++) {
-            $rule = $this->rules->ruleById($ruleIndex);
+            $rule = $this->rules->ruleById[$ruleIndex];
 
             if (!$rule->isAssertion() || $rule->isDisabled()) {
                 continue;
             }
 
-            $literals = $rule->getLiterals();
+            $literals = $rule->literals;
             $literal = $literals[0];
 
             if (!$this->decisions->decided(abs($literal))) {
@@ -104,7 +104,7 @@ class Solver
                     continue;
                 }
 
-                $assertRuleLiterals = $assertRule->getLiterals();
+                $assertRuleLiterals = $assertRule->literals;
                 $assertRuleLiteral = $assertRuleLiterals[0];
 
                 if (abs($literal) !== abs($assertRuleLiteral)) {
@@ -125,7 +125,7 @@ class Solver
     {
         $this->installedMap = array();
         foreach ($this->installed->getPackages() as $package) {
-            $this->installedMap[$package->getId()] = $package;
+            $this->installedMap[$package->id] = $package;
         }
     }
 
@@ -136,15 +136,15 @@ class Solver
                 case 'update':
                     $packages = $this->pool->whatProvides($job['packageName'], $job['constraint']);
                     foreach ($packages as $package) {
-                        if (isset($this->installedMap[$package->getId()])) {
-                            $this->updateMap[$package->getId()] = true;
+                        if (isset($this->installedMap[$package->id])) {
+                            $this->updateMap[$package->id] = true;
                         }
                     }
                     break;
 
                 case 'update-all':
                     foreach ($this->installedMap as $package) {
-                        $this->updateMap[$package->getId()] = true;
+                        $this->updateMap[$package->id] = true;
                     }
                     break;
 
@@ -155,7 +155,7 @@ class Solver
 
                     if (!$this->pool->whatProvides($job['packageName'], $job['constraint'])) {
                         $problem = new Problem($this->pool);
-                        $problem->addRule(new Rule($this->pool, array(), null, null, $job));
+                        $problem->addRule(new Rule(array(), null, null, $job));
                         $this->problems[] = $problem;
                     }
                     break;
@@ -356,7 +356,7 @@ class Solver
         while (true) {
             $this->learnedPool[count($this->learnedPool) - 1][] = $rule;
 
-            foreach ($rule->getLiterals() as $literal) {
+            foreach ($rule->literals as $literal) {
                 // skip the one true literal
                 if ($this->decisions->satisfy($literal)) {
                     continue;
@@ -441,7 +441,7 @@ class Solver
             );
         }
 
-        $newRule = new Rule($this->pool, $learnedLiterals, Rule::RULE_LEARNED, $why);
+        $newRule = new Rule($learnedLiterals, Rule::RULE_LEARNED, $why);
 
         return array($learnedLiterals[0], $ruleLevel, $newRule, $why);
     }
@@ -480,7 +480,7 @@ class Solver
         $this->problems[] = $problem;
 
         $seen = array();
-        $literals = $conflictRule->getLiterals();
+        $literals = $conflictRule->literals;
 
         foreach ($literals as $literal) {
             // skip the one true literal
@@ -503,7 +503,7 @@ class Solver
             $problem->addRule($why);
             $this->analyzeUnsolvableRule($problem, $why);
 
-            $literals = $why->getLiterals();
+            $literals = $why->literals;
 
             foreach ($literals as $literal) {
                 // skip the one true literal
@@ -627,7 +627,7 @@ class Solver
                         $decisionQueue = array();
                         $noneSatisfied = true;
 
-                        foreach ($rule->getLiterals() as $literal) {
+                        foreach ($rule->literals as $literal) {
                             if ($this->decisions->satisfy($literal)) {
                                 $noneSatisfied = false;
                                 break;
@@ -687,8 +687,8 @@ class Solver
                     $i = 0;
                 }
 
-                $rule = $this->rules->ruleById($i);
-                $literals = $rule->getLiterals();
+                $rule = $this->rules->ruleById[$i];
+                $literals = $rule->literals;
 
                 if ($rule->isDisabled()) {
                     continue;

src/Composer/DependencyResolver/Transaction.php

@@ -49,7 +49,7 @@ class Transaction
             $package = $this->pool->literalToPackage($literal);
 
             // wanted & installed || !wanted & !installed
-            if (($literal > 0) == (isset($this->installedMap[$package->getId()]))) {
+            if (($literal > 0) == (isset($this->installedMap[$package->id]))) {
                 continue;
             }
 
@@ -57,7 +57,7 @@ class Transaction
                 if (isset($installMeansUpdateMap[abs($literal)]) && !$package instanceof AliasPackage) {
                     $source = $installMeansUpdateMap[abs($literal)];
 
-                    $updateMap[$package->getId()] = array(
+                    $updateMap[$package->id] = array(
                         'package' => $package,
                         'source' => $source,
                         'reason' => $reason,
@@ -65,9 +65,9 @@ class Transaction
 
                     // avoid updates to one package from multiple origins
                     unset($installMeansUpdateMap[abs($literal)]);
-                    $ignoreRemove[$source->getId()] = true;
+                    $ignoreRemove[$source->id] = true;
                 } else {
-                    $installMap[$package->getId()] = array(
+                    $installMap[$package->id] = array(
                         'package' => $package,
                         'reason' => $reason,
                     );
@@ -81,9 +81,9 @@ class Transaction
             $package = $this->pool->literalToPackage($literal);
 
             if ($literal <= 0 &&
-                isset($this->installedMap[$package->getId()]) &&
-                !isset($ignoreRemove[$package->getId()])) {
-                $uninstallMap[$package->getId()] = array(
+                isset($this->installedMap[$package->id]) &&
+                !isset($ignoreRemove[$package->id])) {
+                $uninstallMap[$package->id] = array(
                     'package' => $package,
                     'reason' => $reason,
                 );
@@ -107,7 +107,7 @@ class Transaction
 
         while (!empty($queue)) {
             $package = array_pop($queue);
-            $packageId = $package->getId();
+            $packageId = $package->id;
 
             if (!isset($visited[$packageId])) {
                 array_push($queue, $package);
@@ -124,7 +124,7 @@ class Transaction
                     }
                 }
 
-                $visited[$package->getId()] = true;
+                $visited[$package->id] = true;
             } else {
                 if (isset($installMap[$packageId])) {
                     $this->install(
@@ -165,7 +165,7 @@ class Transaction
                 $possibleRequires = $this->pool->whatProvides($link->getTarget(), $link->getConstraint());
 
                 foreach ($possibleRequires as $require) {
-                    unset($roots[$require->getId()]);
+                    unset($roots[$require->id]);
                 }
             }
         }
@@ -186,13 +186,13 @@ class Transaction
             }
 
             // !wanted & installed
-            if ($literal <= 0 && isset($this->installedMap[$package->getId()])) {
+            if ($literal <= 0 && isset($this->installedMap[$package->id])) {
                 $updates = $this->policy->findUpdatePackages($this->pool, $this->installedMap, $package);
 
-                $literals = array($package->getId());
+                $literals = array($package->id);
 
                 foreach ($updates as $update) {
-                    $literals[] = $update->getId();
+                    $literals[] = $update->id;
                 }
 
                 foreach ($literals as $updateLiteral) {

src/Composer/Package/BasePackage.php

@@ -44,13 +44,19 @@ abstract class BasePackage implements PackageInterface
         'dev'    => self::STABILITY_DEV,
     );
 
+    /**
+     * READ-ONLY: The package id, public for fast access in dependency solver
+     * @var int
+     */
+    public $id;
+
     protected $name;
     protected $prettyName;
 
     protected $repository;
-    protected $id;
     protected $transportOptions;
 
+
     /**
      * All descendants' constructors should call this parent constructor
      *

tests/Composer/Test/DependencyResolver/RuleSetIteratorTest.php

@@ -27,11 +27,11 @@ class RuleSetIteratorTest extends \PHPUnit_Framework_TestCase
 
         $this->rules = array(
             RuleSet::TYPE_JOB => array(
-                new Rule($this->pool, array(), 'job1', null),
-                new Rule($this->pool, array(), 'job2', null),
+                new Rule(array(), 'job1', null),
+                new Rule(array(), 'job2', null),
             ),
             RuleSet::TYPE_LEARNED => array(
-                new Rule($this->pool, array(), 'update1', null),
+                new Rule(array(), 'update1', null),
             ),
             RuleSet::TYPE_PACKAGE => array(),
         );

tests/Composer/Test/DependencyResolver/RuleSetTest.php

@@ -32,11 +32,11 @@ class RuleSetTest extends TestCase
         $rules = array(
             RuleSet::TYPE_PACKAGE => array(),
             RuleSet::TYPE_JOB => array(
-                new Rule($this->pool, array(), 'job1', null),
-                new Rule($this->pool, array(), 'job2', null),
+                new Rule(array(), 'job1', null),
+                new Rule(array(), 'job2', null),
             ),
             RuleSet::TYPE_LEARNED => array(
-                new Rule($this->pool, array(), 'update1', null),
+                new Rule(array(), 'update1', null),
             ),
         );
 
@@ -56,15 +56,15 @@ class RuleSetTest extends TestCase
     {
         $ruleSet = new RuleSet;
 
-        $ruleSet->add(new Rule($this->pool, array(), 'job1', null), 7);
+        $ruleSet->add(new Rule(array(), 'job1', null), 7);
     }
 
     public function testCount()
     {
         $ruleSet = new RuleSet;
 
-        $ruleSet->add(new Rule($this->pool, array(), 'job1', null), RuleSet::TYPE_JOB);
-        $ruleSet->add(new Rule($this->pool, array(), 'job2', null), RuleSet::TYPE_JOB);
+        $ruleSet->add(new Rule(array(), 'job1', null), RuleSet::TYPE_JOB);
+        $ruleSet->add(new Rule(array(), 'job2', null), RuleSet::TYPE_JOB);
 
         $this->assertEquals(2, $ruleSet->count());
     }
@@ -73,18 +73,18 @@ class RuleSetTest extends TestCase
     {
         $ruleSet = new RuleSet;
 
-        $rule = new Rule($this->pool, array(), 'job1', null);
+        $rule = new Rule(array(), 'job1', null);
         $ruleSet->add($rule, RuleSet::TYPE_JOB);
 
-        $this->assertSame($rule, $ruleSet->ruleById(0));
+        $this->assertSame($rule, $ruleSet->ruleById[0]);
     }
 
     public function testGetIterator()
     {
         $ruleSet = new RuleSet;
 
-        $rule1 = new Rule($this->pool, array(), 'job1', null);
-        $rule2 = new Rule($this->pool, array(), 'job1', null);
+        $rule1 = new Rule(array(), 'job1', null);
+        $rule2 = new Rule(array(), 'job1', null);
         $ruleSet->add($rule1, RuleSet::TYPE_JOB);
         $ruleSet->add($rule2, RuleSet::TYPE_LEARNED);
 
@@ -98,8 +98,8 @@ class RuleSetTest extends TestCase
     public function testGetIteratorFor()
     {
         $ruleSet = new RuleSet;
-        $rule1 = new Rule($this->pool, array(), 'job1', null);
-        $rule2 = new Rule($this->pool, array(), 'job1', null);
+        $rule1 = new Rule(array(), 'job1', null);
+        $rule2 = new Rule(array(), 'job1', null);
 
         $ruleSet->add($rule1, RuleSet::TYPE_JOB);
         $ruleSet->add($rule2, RuleSet::TYPE_LEARNED);
@@ -112,8 +112,8 @@ class RuleSetTest extends TestCase
     public function testGetIteratorWithout()
     {
         $ruleSet = new RuleSet;
-        $rule1 = new Rule($this->pool, array(), 'job1', null);
-        $rule2 = new Rule($this->pool, array(), 'job1', null);
+        $rule1 = new Rule(array(), 'job1', null);
+        $rule2 = new Rule(array(), 'job1', null);
 
         $ruleSet->add($rule1, RuleSet::TYPE_JOB);
         $ruleSet->add($rule2, RuleSet::TYPE_LEARNED);
@@ -155,7 +155,7 @@ class RuleSetTest extends TestCase
         $this->assertFalse($ruleSet->containsEqual($rule3));
     }
 
-    public function testToString()
+    public function testPrettyString()
     {
         $repo = new ArrayRepository;
         $repo->addPackage($p = $this->getPackage('foo', '2.1'));
@@ -163,11 +163,11 @@ class RuleSetTest extends TestCase
 
         $ruleSet = new RuleSet;
         $literal = $p->getId();
-        $rule = new Rule($this->pool, array($literal), 'job1', null);
+        $rule = new Rule(array($literal), 'job1', null);
 
         $ruleSet->add($rule, RuleSet::TYPE_JOB);
 
-        $this->assertContains('JOB     : (+foo-2.1.0.0)', $ruleSet->__toString());
+        $this->assertContains('JOB     : (install foo 2.1)', $ruleSet->getPrettyString($this->pool));
     }
 
     private function getRuleMock()

tests/Composer/Test/DependencyResolver/RuleTest.php

@@ -28,14 +28,14 @@ class RuleTest extends TestCase
 
     public function testGetHash()
     {
-        $rule = new Rule($this->pool, array(123), 'job1', null);
+        $rule = new Rule(array(123), 'job1', null);
 
         $this->assertEquals(substr(md5('123'), 0, 5), $rule->getHash());
     }
 
     public function testSetAndGetId()
     {
-        $rule = new Rule($this->pool, array(), 'job1', null);
+        $rule = new Rule(array(), 'job1', null);
         $rule->setId(666);
 
         $this->assertEquals(666, $rule->getId());
@@ -43,31 +43,31 @@ class RuleTest extends TestCase
 
     public function testEqualsForRulesWithDifferentHashes()
     {
-        $rule = new Rule($this->pool, array(1, 2), 'job1', null);
-        $rule2 = new Rule($this->pool, array(1, 3), 'job1', null);
+        $rule = new Rule(array(1, 2), 'job1', null);
+        $rule2 = new Rule(array(1, 3), 'job1', null);
 
         $this->assertFalse($rule->equals($rule2));
     }
 
     public function testEqualsForRulesWithDifferLiteralsQuantity()
     {
-        $rule = new Rule($this->pool, array(1, 12), 'job1', null);
-        $rule2 = new Rule($this->pool, array(1), 'job1', null);
+        $rule = new Rule(array(1, 12), 'job1', null);
+        $rule2 = new Rule(array(1), 'job1', null);
 
         $this->assertFalse($rule->equals($rule2));
     }
 
     public function testEqualsForRulesWithSameLiterals()
     {
-        $rule = new Rule($this->pool, array(1, 12), 'job1', null);
-        $rule2 = new Rule($this->pool, array(1, 12), 'job1', null);
+        $rule = new Rule(array(1, 12), 'job1', null);
+        $rule2 = new Rule(array(1, 12), 'job1', null);
 
         $this->assertTrue($rule->equals($rule2));
     }
 
     public function testSetAndGetType()
     {
-        $rule = new Rule($this->pool, array(), 'job1', null);
+        $rule = new Rule(array(), 'job1', null);
         $rule->setType('someType');
 
         $this->assertEquals('someType', $rule->getType());
@@ -75,7 +75,7 @@ class RuleTest extends TestCase
 
     public function testEnable()
     {
-        $rule = new Rule($this->pool, array(), 'job1', null);
+        $rule = new Rule(array(), 'job1', null);
         $rule->disable();
         $rule->enable();
 
@@ -85,7 +85,7 @@ class RuleTest extends TestCase
 
     public function testDisable()
     {
-        $rule = new Rule($this->pool, array(), 'job1', null);
+        $rule = new Rule(array(), 'job1', null);
         $rule->enable();
         $rule->disable();
 
@@ -95,22 +95,22 @@ class RuleTest extends TestCase
 
     public function testIsAssertions()
     {
-        $rule = new Rule($this->pool, array(1, 12), 'job1', null);
-        $rule2 = new Rule($this->pool, array(1), 'job1', null);
+        $rule = new Rule(array(1, 12), 'job1', null);
+        $rule2 = new Rule(array(1), 'job1', null);
 
         $this->assertFalse($rule->isAssertion());
         $this->assertTrue($rule2->isAssertion());
     }
 
-    public function testToString()
+    public function testPrettyString()
     {
         $repo = new ArrayRepository;
         $repo->addPackage($p1 = $this->getPackage('foo', '2.1'));
         $repo->addPackage($p2 = $this->getPackage('baz', '1.1'));
         $this->pool->addRepository($repo);
 
-        $rule = new Rule($this->pool, array($p1->getId(), -$p2->getId()), 'job1', null);
+        $rule = new Rule(array($p1->getId(), -$p2->getId()), 'job1', null);
 
-        $this->assertEquals('(-baz-1.1.0.0|+foo-2.1.0.0)', $rule->__toString());
+        $this->assertEquals('(don\'t install baz 1.1|install foo 2.1)', $rule->getPrettyString($this->pool));
     }
 }