Skip to content

Commit 1179dc5

Browse files
committed
Refactored ServiceMap, improved tests, added conflicts
1 parent ff93b74 commit 1179dc5

15 files changed

+132
-71
lines changed

composer.json

+4-2
Original file line numberDiff line numberDiff line change
@@ -32,8 +32,10 @@
3232
"satooshi/php-coveralls": "^1.0",
3333
"slevomat/coding-standard": "^4.5.2",
3434
"phpstan/phpstan-phpunit": "^0.10",
35-
"symfony/framework-bundle": "^4.0",
36-
"symfony/http-foundation": "^4.0"
35+
"symfony/framework-bundle": "^3.0 || ^4.0"
36+
},
37+
"conflict": {
38+
"symfony/framework-bundle": "<3.0"
3739
},
3840
"autoload": {
3941
"psr-4": {

phpstan.neon

-3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,3 @@ includes:
66
parameters:
77
excludes_analyse:
88
- */tests/*/data/*
9-
ignoreErrors:
10-
- '#Call to an undefined method object::noMethod\(\)\.#'
11-
- '#Offset string does not exist on array\(\)\.#'

src/Rules/Symfony/ContainerInterfacePrivateServiceRule.php

+3-3
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,7 @@ public function getNodeType(): string
2828
/**
2929
* @param MethodCall $node
3030
* @param Scope $scope
31-
* @return mixed[]
31+
* @return string[]
3232
*/
3333
public function processNode(Node $node, Scope $scope): array
3434
{
@@ -50,8 +50,8 @@ public function processNode(Node $node, Scope $scope): array
5050
$serviceId = ServiceMap::getServiceIdFromNode($node->args[0]->value, $scope);
5151
if ($serviceId !== null) {
5252
$service = $this->serviceMap->getService($serviceId);
53-
if ($service !== null && $service['public'] === false) {
54-
return [sprintf('Service "%s" is private.', $service['id'])];
53+
if ($service !== null && !$service->isPublic()) {
54+
return [sprintf('Service "%s" is private.', $serviceId)];
5555
}
5656
}
5757

src/Symfony/Service.php

+63
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Symfony;
4+
5+
final class Service
6+
{
7+
8+
/** @var string */
9+
private $id;
10+
11+
/** @var string|null */
12+
private $class;
13+
14+
/** @var bool */
15+
private $public;
16+
17+
/** @var bool */
18+
private $synthetic;
19+
20+
/** @var string|null */
21+
private $alias;
22+
23+
public function __construct(
24+
string $id,
25+
?string $class,
26+
bool $public,
27+
bool $synthetic,
28+
?string $alias
29+
)
30+
{
31+
$this->id = $id;
32+
$this->class = $class;
33+
$this->public = $public;
34+
$this->synthetic = $synthetic;
35+
$this->alias = $alias;
36+
}
37+
38+
public function getId(): string
39+
{
40+
return $this->id;
41+
}
42+
43+
public function getClass(): ?string
44+
{
45+
return $this->class;
46+
}
47+
48+
public function isPublic(): bool
49+
{
50+
return $this->public;
51+
}
52+
53+
public function isSynthetic(): bool
54+
{
55+
return $this->synthetic;
56+
}
57+
58+
public function getAlias(): ?string
59+
{
60+
return $this->alias;
61+
}
62+
63+
}

src/Symfony/ServiceMap.php

+27-28
Original file line numberDiff line numberDiff line change
@@ -4,17 +4,18 @@
44

55
use PhpParser\Node\Expr;
66
use PHPStan\Analyser\Scope;
7-
use PHPStan\Type\Constant\ConstantStringType;
7+
use PHPStan\Type\TypeUtils;
88

99
final class ServiceMap
1010
{
1111

12-
/** @var array<string, array> */
13-
private $services;
12+
/** @var Service[] */
13+
private $services = [];
1414

1515
public function __construct(string $containerXml)
1616
{
17-
$this->services = $aliases = [];
17+
/** @var Service[] $aliases */
18+
$aliases = [];
1819
/** @var \SimpleXMLElement|false $xml */
1920
$xml = @simplexml_load_file($containerXml);
2021
if ($xml === false) {
@@ -25,44 +26,42 @@ public function __construct(string $containerXml)
2526
if (!isset($attrs->id)) {
2627
continue;
2728
}
28-
$service = [
29-
'id' => (string) $attrs->id,
30-
'class' => isset($attrs->class) ? (string) $attrs->class : null,
31-
'public' => !isset($attrs->public) || (string) $attrs->public !== 'false',
32-
'synthetic' => isset($attrs->synthetic) && (string) $attrs->synthetic === 'true',
33-
];
34-
if (isset($attrs->alias)) {
35-
$aliases[(string) $attrs->id] = array_merge($service, ['alias' => (string) $attrs->alias]);
29+
$service = new Service(
30+
(string) $attrs->id,
31+
isset($attrs->class) ? (string) $attrs->class : null,
32+
!isset($attrs->public) || (string) $attrs->public !== 'false',
33+
isset($attrs->synthetic) && (string) $attrs->synthetic === 'true',
34+
isset($attrs->alias) ? (string) $attrs->alias : null
35+
);
36+
if ($service->getAlias() !== null) {
37+
$aliases[] = $service;
3638
} else {
37-
$this->services[(string) $attrs->id] = $service;
39+
$this->services[$service->getId()] = $service;
3840
}
3941
}
40-
foreach ($aliases as $id => $alias) {
41-
if (!array_key_exists($alias['alias'], $this->services)) {
42+
foreach ($aliases as $service) {
43+
if ($service->getAlias() !== null && !array_key_exists($service->getAlias(), $this->services)) {
4244
continue;
4345
}
44-
$this->services[$id] = [
45-
'id' => $id,
46-
'class' => $this->services[$alias['alias']]['class'],
47-
'public' => $alias['public'],
48-
'synthetic' => $alias['synthetic'],
49-
];
46+
$this->services[$service->getId()] = new Service(
47+
$service->getId(),
48+
$this->services[$service->getAlias()]->getClass(),
49+
$service->isPublic(),
50+
$service->isSynthetic(),
51+
null
52+
);
5053
}
5154
}
5255

53-
/**
54-
* @param string $id
55-
* @return mixed[]|null
56-
*/
57-
public function getService(string $id): ?array
56+
public function getService(string $id): ?Service
5857
{
5958
return $this->services[$id] ?? null;
6059
}
6160

6261
public static function getServiceIdFromNode(Expr $node, Scope $scope): ?string
6362
{
64-
$nodeType = $scope->getType($node);
65-
return $nodeType instanceof ConstantStringType ? $nodeType->getValue() : null;
63+
$strings = TypeUtils::getConstantStrings($scope->getType($node));
64+
return count($strings) === 1 ? $strings[0]->getValue() : null;
6665
}
6766

6867
}

src/Type/Symfony/ContainerInterfaceDynamicReturnTypeExtension.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ public function getTypeFromMethodCall(
4646
$serviceId = ServiceMap::getServiceIdFromNode($methodCall->args[0]->value, $scope);
4747
if ($serviceId !== null) {
4848
$service = $this->serviceMap->getService($serviceId);
49-
if ($service !== null && $service['synthetic'] === false) {
50-
return new ObjectType($service['class'] ?? $service['id']);
49+
if ($service !== null && !$service->isSynthetic()) {
50+
return new ObjectType($service->getClass() ?? $serviceId);
5151
}
5252
}
5353

src/Type/Symfony/ControllerDynamicReturnTypeExtension.php

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,8 +46,8 @@ public function getTypeFromMethodCall(
4646
$serviceId = ServiceMap::getServiceIdFromNode($methodCall->args[0]->value, $scope);
4747
if ($serviceId !== null) {
4848
$service = $this->serviceMap->getService($serviceId);
49-
if ($service !== null && $service['synthetic'] === false) {
50-
return new ObjectType($service['class'] ?? $service['id']);
49+
if ($service !== null && !$service->isSynthetic()) {
50+
return new ObjectType($service->getClass() ?? $serviceId);
5151
}
5252
}
5353

tests/Rules/Symfony/ContainerInterfacePrivateServiceRuleTest.php

+2-5
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@
55
use PHPStan\Rules\Rule;
66
use PHPStan\Symfony\ServiceMap;
77

8-
/**
9-
* @covers \PHPStan\Rules\Symfony\ContainerInterfacePrivateServiceRule
10-
*/
118
final class ContainerInterfacePrivateServiceRuleTest extends \PHPStan\Testing\RuleTestCase
129
{
1310

@@ -21,11 +18,11 @@ protected function getRule(): Rule
2118
public function testGetPrivateService(): void
2219
{
2320
$this->analyse(
24-
[__DIR__ . '/ExampleController.php'],
21+
[__DIR__ . '/../../Symfony/data/ExampleController.php'],
2522
[
2623
[
2724
'Service "private" is private.',
28-
15,
25+
14,
2926
],
3027
]
3128
);

tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php

+6-9
Original file line numberDiff line numberDiff line change
@@ -5,9 +5,6 @@
55
use PHPStan\Rules\Rule;
66
use PHPStan\Symfony\ServiceMap;
77

8-
/**
9-
* @covers \PHPStan\Rules\Symfony\ContainerInterfaceUnknownServiceRule
10-
*/
118
final class ContainerInterfaceUnknownServiceRuleTest extends \PHPStan\Testing\RuleTestCase
129
{
1310

@@ -21,23 +18,23 @@ protected function getRule(): Rule
2118
public function testGetUnknownService(): void
2219
{
2320
$this->analyse(
24-
[__DIR__ . '/ExampleController.php'],
21+
[__DIR__ . '/../../Symfony/data/ExampleController.php'],
2522
[
2623
[
2724
'Service "service.not.found" is not registered in the container.',
28-
21,
25+
20,
2926
],
3027
[
3128
'Service "PHPStan\Symfony\ServiceMap" is not registered in the container.',
32-
27,
29+
26,
3330
],
3431
[
3532
'Service "service.PHPStan\Symfony\ServiceMap" is not registered in the container.',
36-
39,
33+
38,
3734
],
3835
[
39-
'Service "PHPStan\Rules\Symfony\ExampleController" is not registered in the container.',
40-
45,
36+
'Service "PHPStan\Symfony\ExampleController" is not registered in the container.',
37+
44,
4138
],
4239
]
4340
);

tests/Symfony/ServiceMapTest.php

+2-6
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,8 @@
99
use PhpParser\PrettyPrinter\Standard;
1010
use PHPStan\Analyser\Scope;
1111
use PHPStan\Analyser\ScopeContext;
12-
use PHPStan\Rules\Symfony\ExampleController;
1312
use PHPStan\Testing\TestCase;
1413

15-
/**
16-
* @covers \PHPStan\Symfony\ServiceMap
17-
*/
1814
final class ServiceMapTest extends TestCase
1915
{
2016

@@ -48,8 +44,8 @@ public function testGetServiceIdFromNode(): void
4844
$scope = new Scope($broker, $printer, $typeSpecifier, ScopeContext::create(''));
4945

5046
self::assertSame('foo', ServiceMap::getServiceIdFromNode(new String_('foo'), $scope));
51-
self::assertSame('bar', ServiceMap::getServiceIdFromNode(new ClassConstFetch(new Name('PHPStan\Rules\Symfony\ExampleController'), 'BAR'), $scope));
52-
self::assertSame('foobar', ServiceMap::getServiceIdFromNode(new Concat(new String_('foo'), new ClassConstFetch(new Name('PHPStan\Rules\Symfony\ExampleController'), 'BAR')), $scope));
47+
self::assertSame('bar', ServiceMap::getServiceIdFromNode(new ClassConstFetch(new Name(ExampleController::class), 'BAR'), $scope));
48+
self::assertSame('foobar', ServiceMap::getServiceIdFromNode(new Concat(new String_('foo'), new ClassConstFetch(new Name(ExampleController::class), 'BAR')), $scope));
5349

5450
$scope = $scope->enterClass($broker->getClass(ExampleController::class));
5551
self::assertSame('bar', ServiceMap::getServiceIdFromNode(new ClassConstFetch(new Name('static'), 'BAR'), $scope));

tests/Symfony/ServiceTest.php

+20
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
<?php declare(strict_types = 1);
2+
3+
namespace PHPStan\Symfony;
4+
5+
use PHPUnit\Framework\TestCase;
6+
7+
final class ServiceTest extends TestCase
8+
{
9+
10+
public function testGetSet(): void
11+
{
12+
$service = new Service('foo', 'Bar', true, true, 'alias');
13+
self::assertSame('foo', $service->getId());
14+
self::assertSame('Bar', $service->getClass());
15+
self::assertTrue($service->isPublic());
16+
self::assertTrue($service->isSynthetic());
17+
self::assertSame('alias', $service->getAlias());
18+
}
19+
20+
}

tests/Rules/Symfony/ExampleController.php renamed to tests/Symfony/data/ExampleController.php

+1-2
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
<?php declare(strict_types = 1);
22

3-
namespace PHPStan\Rules\Symfony;
3+
namespace PHPStan\Symfony;
44

5-
use PHPStan\Symfony\ServiceMap;
65
use Symfony\Bundle\FrameworkBundle\Controller\Controller;
76

87
final class ExampleController extends Controller

tests/Type/Symfony/ContainerInterfaceDynamicReturnTypeExtensionTest.php

-3
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717
use PHPStan\Type\Type;
1818
use PHPUnit\Framework\TestCase;
1919

20-
/**
21-
* @covers \PHPStan\Type\Symfony\ContainerInterfaceDynamicReturnTypeExtension
22-
*/
2320
final class ContainerInterfaceDynamicReturnTypeExtensionTest extends TestCase
2421
{
2522

tests/Type/Symfony/ControllerDynamicReturnTypeExtensionTest.php

-3
Original file line numberDiff line numberDiff line change
@@ -17,9 +17,6 @@
1717
use PHPStan\Type\Type;
1818
use PHPUnit\Framework\TestCase;
1919

20-
/**
21-
* @covers \PHPStan\Type\Symfony\ControllerDynamicReturnTypeExtension
22-
*/
2320
final class ControllerDynamicReturnTypeExtensionTest extends TestCase
2421
{
2522

tests/Type/Symfony/RequestDynamicReturnTypeExtensionTest.php

-3
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,6 @@
1616
use PHPStan\Type\Type;
1717
use PHPUnit\Framework\TestCase;
1818

19-
/**
20-
* @covers \PHPStan\Type\Symfony\RequestDynamicReturnTypeExtension
21-
*/
2219
final class RequestDynamicReturnTypeExtensionTest extends TestCase
2320
{
2421

0 commit comments

Comments
 (0)