Skip to content

Commit 0ae2474

Browse files
weaverryanweaverryan
committed
bug #121 Fixing bug where makers couldn't advertise needed *dev* dependencies (weaverryan)
This PR was merged into the 1.0-dev branch. Discussion ---------- Fixing bug where makers couldn't advertise needed *dev* dependencies Diff is a bit big so that I could get the message just right, but it's covered by tests: you can now say that your maker has a `require` dependency or a `require-dev` dependency and it will report it correctly to the user (e.g. `composer require symfony/foo --dev`). Commits ------- d198168 Fixing bug where makers couldn't advertise needed *dev* dependencies
2 parents 589f673 + d198168 commit 0ae2474

File tree

8 files changed

+246
-34
lines changed

8 files changed

+246
-34
lines changed

‎src/Command/MakerCommand.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -61,8 +61,9 @@ protected function initialize(InputInterface $input, OutputInterface $output)
6161
if ($this->checkDependencies) {
6262
$dependencies = new DependencyBuilder();
6363
$this->maker->configureDependencies($dependencies);
64-
if ($missingPackages = $dependencies->getMissingDependencies()) {
65-
throw new RuntimeCommandException(sprintf("Missing package%s: to use the %s command, run: \n\ncomposer require %s", 1 === count($missingPackages) ? '' : 's', $this->getName(), implode(' ', $missingPackages)));
64+
65+
if ($missingPackagesMessage = $dependencies->getMissingPackagesMessage($this->getName())) {
66+
throw new RuntimeCommandException($missingPackagesMessage);
6667
}
6768
}
6869
}

‎src/DependencyBuilder.php‎

Lines changed: 94 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
final class DependencyBuilder
1515
{
1616
private $dependencies = [];
17+
private $devDependencies = [];
1718

1819
/**
1920
* Add a dependency that will be reported if the given class is missing.
@@ -22,52 +23,118 @@ final class DependencyBuilder
2223
* the user if other required dependencies are missing. An example
2324
* is the "validator" when trying to work with forms.
2425
*/
25-
public function addClassDependency(string $class, string $package, bool $required = true)
26+
public function addClassDependency(string $class, string $package, bool $required = true, bool $devDependency = false)
2627
{
27-
$this->dependencies[$class] = [
28-
'name' => $package,
29-
'required' => $required,
30-
];
28+
if ($devDependency) {
29+
$this->devDependencies[] = [
30+
'class' => $class,
31+
'name' => $package,
32+
'required' => $required,
33+
];
34+
} else {
35+
$this->dependencies[] = [
36+
'class' => $class,
37+
'name' => $package,
38+
'required' => $required,
39+
];
40+
}
3141
}
3242

43+
/**
44+
* @internal
45+
*/
3346
public function getMissingDependencies(): array
3447
{
35-
$missingPackages = [];
36-
$missingOptionalPackages = [];
37-
38-
foreach ($this->dependencies as $class => $package) {
39-
if (class_exists($class)) {
40-
continue;
41-
}
48+
return $this->calculateMissingDependencies($this->dependencies);
49+
}
4250

43-
if (true === $package['required']) {
44-
$missingPackages[] = $package['name'];
45-
} else {
46-
$missingOptionalPackages[] = $package['name'];
47-
}
48-
}
51+
/**
52+
* @internal
53+
*/
54+
public function getMissingDevDependencies(): array
55+
{
56+
return $this->calculateMissingDependencies($this->devDependencies);
57+
}
4958

50-
if (empty($missingPackages)) {
51-
return [];
52-
}
59+
/**
60+
* @internal
61+
*/
62+
public function getAllRequiredDependencies(): array
63+
{
64+
return $this->getRequiredDependencyNames($this->dependencies);
65+
}
5366

54-
return array_merge($missingPackages, $missingOptionalPackages);
67+
/**
68+
* @internal
69+
*/
70+
public function getAllRequiredDevDependencies(): array
71+
{
72+
return $this->getRequiredDependencyNames($this->devDependencies);
5573
}
5674

5775
/**
5876
* @internal
5977
*/
60-
public function getAllRequiredDependencies(): array
78+
public function getMissingPackagesMessage(string $commandName): string
79+
{
80+
$packages = $this->getMissingDependencies();
81+
$packagesDev = $this->getMissingDevDependencies();
82+
83+
if (empty($packages) && empty($packagesDev)) {
84+
return '';
85+
}
86+
87+
$packagesCount = count($packages) + count($packagesDev);
88+
89+
$message = sprintf(
90+
"Missing package%s: to use the %s command, run:\n",
91+
$packagesCount > 1 ? 's' : '',
92+
$commandName
93+
//
94+
);
95+
96+
if (!empty($packages)) {
97+
$message .= sprintf("\ncomposer require %s", implode(' ', $packages));
98+
}
99+
100+
if (!empty($packagesDev)) {
101+
$message .= sprintf("\ncomposer require %s --dev", implode(' ', $packagesDev));
102+
}
103+
104+
return $message;
105+
}
106+
107+
private function getRequiredDependencyNames(array $dependencies)
61108
{
62-
$dependencies = [];
63-
foreach ($this->dependencies as $class => $package) {
109+
$packages = [];
110+
foreach ($dependencies as $package) {
64111
if (!$package['required']) {
65112
continue;
66113
}
114+
$packages[] = $package['name'];
115+
}
67116

68-
$dependencies[] = $package['name'];
117+
return $packages;
118+
}
119+
120+
private function calculateMissingDependencies(array $dependencies)
121+
{
122+
$missingPackages = [];
123+
$missingOptionalPackages = [];
124+
foreach ($dependencies as $package) {
125+
if (class_exists($package['class'])) {
126+
continue;
127+
}
128+
if (true === $package['required']) {
129+
$missingPackages[] = $package['name'];
130+
} else {
131+
$missingOptionalPackages[] = $package['name'];
132+
}
133+
}
134+
if (empty($missingPackages)) {
135+
return [];
69136
}
70137

71-
return $dependencies;
138+
return array_merge($missingPackages, $missingOptionalPackages);
72139
}
73140
}

‎src/Generator.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@
1818
* @author Javier Eguiluz <[email protected]>
1919
* @author Ryan Weaver <[email protected]>
2020
*/
21-
final class Generator
21+
class Generator
2222
{
2323
private $fileManager;
2424
private $pendingOperations = [];

‎src/Maker/MakeFixtures.php‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,9 @@ public function configureDependencies(DependencyBuilder $dependencies)
7373
);
7474
$dependencies->addClassDependency(
7575
Fixture::class,
76-
'orm-fixtures'
76+
'orm-fixtures',
77+
true,
78+
true
7779
);
7880
}
7981
}

‎src/Maker/MakeFunctionalTest.php‎

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,15 @@ public function configureDependencies(DependencyBuilder $dependencies)
6969
{
7070
$dependencies->addClassDependency(
7171
Client::class,
72-
'browser-kit'
72+
'browser-kit',
73+
true,
74+
true
7375
);
7476
$dependencies->addClassDependency(
7577
CssSelectorConverter::class,
76-
'css-selector'
78+
'css-selector',
79+
true,
80+
true
7781
);
7882
}
7983
}

‎src/Test/MakerTestDetails.php‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -157,6 +157,10 @@ public function getDependencies()
157157
$depBuilder = new DependencyBuilder();
158158
$this->maker->configureDependencies($depBuilder);
159159

160-
return array_merge($depBuilder->getAllRequiredDependencies(), $this->extraDependencies);
160+
return array_merge(
161+
$depBuilder->getAllRequiredDependencies(),
162+
$depBuilder->getAllRequiredDevDependencies(),
163+
$this->extraDependencies
164+
);
161165
}
162166
}

‎tests/Command/MakerCommandTest.php‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
<?php
2+
3+
namespace Symfony\Bundle\MakerBundle\Tests\Command;
4+
5+
use PHPUnit\Framework\TestCase;
6+
use Symfony\Bundle\MakerBundle\Command\MakerCommand;
7+
use Symfony\Bundle\MakerBundle\DependencyBuilder;
8+
use Symfony\Bundle\MakerBundle\Generator;
9+
use Symfony\Bundle\MakerBundle\MakerInterface;
10+
use Symfony\Component\Console\Tester\CommandTester;
11+
12+
class MakerCommandTest extends TestCase
13+
{
14+
/**
15+
* @expectedException \Symfony\Bundle\MakerBundle\Exception\RuntimeCommandException
16+
* @expectedExceptionMessageRegExp /composer require foo-package/
17+
*/
18+
public function testExceptionOnMissingDependencies()
19+
{
20+
$maker = $this->createMock(MakerInterface::class);
21+
$maker->expects($this->once())
22+
->method('configureDependencies')
23+
->willReturnCallback(function(DependencyBuilder $depBuilder) {
24+
$depBuilder->addClassDependency('Foo', 'foo-package');
25+
});
26+
27+
$generator = $this->createMock(Generator::class);
28+
29+
$command = new MakerCommand($maker, $generator);
30+
// needed because it's normally set by the Application
31+
$command->setName('make:foo');
32+
$tester = new CommandTester($command);
33+
$tester->execute(array());
34+
}
35+
}

‎tests/DependencyBuilderTest.php‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<?php
2+
3+
namespace Symfony\Bundle\MakerBundle\Tests;
4+
5+
use PHPUnit\Framework\TestCase;
6+
use Symfony\Bundle\MakerBundle\DependencyBuilder;
7+
8+
class DependencyBuilderTest extends TestCase
9+
{
10+
public function testGetAllRequiredDependencies()
11+
{
12+
$depBuilder = new DependencyBuilder();
13+
$depBuilder->addClassDependency('Foo', 'foo-package');
14+
$depBuilder->addClassDependency('Bar', 'bar-package');
15+
$depBuilder->addClassDependency('DevStuff', 'dev-stuff-package', true, true);
16+
$depBuilder->addClassDependency('DevStuff2', 'dev-stuff2-package', true, true);
17+
18+
$this->assertSame(['foo-package', 'bar-package'], $depBuilder->getAllRequiredDependencies());
19+
$this->assertSame(['dev-stuff-package', 'dev-stuff2-package'], $depBuilder->getAllRequiredDevDependencies());
20+
}
21+
22+
public function testGetMissingDependencies()
23+
{
24+
$depBuilder = new DependencyBuilder();
25+
$depBuilder->addClassDependency('Foo', 'foo-package');
26+
$depBuilder->addClassDependency('Bar', 'bar-package', false);
27+
$depBuilder->addClassDependency('DevStuff', 'dev-stuff-package', true, true);
28+
$depBuilder->addClassDependency('DevStuff2', 'dev-stuff2-package', false, true);
29+
30+
$this->assertSame(['foo-package', 'bar-package'], $depBuilder->getMissingDependencies());
31+
$this->assertSame(['dev-stuff-package', 'dev-stuff2-package'], $depBuilder->getMissingDevDependencies());
32+
}
33+
34+
public function testGetMissingDependenciesNoMissingRequired()
35+
{
36+
$depBuilder = new DependencyBuilder();
37+
$depBuilder->addClassDependency(__CLASS__, 'foo-package');
38+
$depBuilder->addClassDependency('Bar', 'bar-package', false);
39+
40+
$actualDeps = $depBuilder->getMissingDependencies();
41+
$this->assertSame([], $actualDeps);
42+
}
43+
44+
/**
45+
* @dataProvider getMissingPackagesMessageTests
46+
*/
47+
public function testGetMissingPackagesMessage(array $missingPackages, array $missingDevPackages, string $expectedMessage)
48+
{
49+
$depBuilder = new DependencyBuilder();
50+
foreach ($missingPackages as $missingPackage) {
51+
$depBuilder->addClassDependency('Foo', $missingPackage);
52+
}
53+
54+
foreach ($missingDevPackages as $missingPackage) {
55+
$depBuilder->addClassDependency('Foo', $missingPackage, true, true);
56+
}
57+
58+
$this->assertSame($expectedMessage, $depBuilder->getMissingPackagesMessage('make:something'));
59+
}
60+
61+
public function getMissingPackagesMessageTests()
62+
{
63+
yield 'nothing_missing' => [
64+
[],
65+
[],
66+
''
67+
];
68+
69+
yield 'missing_one_package' => [
70+
['bar-package'],
71+
[],
72+
<<<EOF
73+
Missing package: to use the make:something command, run:
74+
75+
composer require bar-package
76+
EOF
77+
];
78+
79+
yield 'missing_multiple_packages' => [
80+
['bar-package', 'other-package'],
81+
[],
82+
<<<EOF
83+
Missing packages: to use the make:something command, run:
84+
85+
composer require bar-package other-package
86+
EOF
87+
];
88+
89+
yield 'missing_dev_packages' => [
90+
[],
91+
['bar-package', 'other-package'],
92+
<<<EOF
93+
Missing packages: to use the make:something command, run:
94+
95+
composer require bar-package other-package --dev
96+
EOF
97+
];
98+
}
99+
}

0 commit comments

Comments
 (0)