Skip to content
This repository has been archived by the owner on Jun 26, 2018. It is now read-only.

Support class constants for services' names #1

Closed
wants to merge 5 commits into from
Closed

Support class constants for services' names #1

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
3c9e676
Support class constants for services' names
hectorj Jul 18, 2017
2785e0f
Replace PHP7.1 nullable return type by good old PHPDoc
hectorj Jul 18, 2017
e917165
Merge remote-tracking branch 'upstream/master' into class-const
hectorj Jul 18, 2017
ecffa0f
Fix codestyle
hectorj Jul 18, 2017
ba8dc8d
Fix PHPStan issue
hectorj Jul 18, 2017
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 9 additions & 13 deletions src/Rules/ContainerInterfacePrivateServiceRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Scalar\String_;
use Symfony\Component\DependencyInjection\ContainerInterface;

final class ContainerInterfacePrivateServiceRule implements Rule
Expand All @@ -33,17 +31,15 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
$services = $this->serviceMap->getServices();
return $node instanceof MethodCall
&& $node->name === 'get'
&& $scope->getType($node->var)->getClass() === ContainerInterface::class
&& isset($node->args[0])
&& $node->args[0] instanceof Arg
&& $node->args[0]->value instanceof String_
&& \array_key_exists($node->args[0]->value->value, $services)
&& !$services[$node->args[0]->value->value]['public']
? [\sprintf('Service "%s" is private.', $node->args[0]->value->value)]
: [];
if ($node instanceof MethodCall && $node->name === 'get' && $scope->getType($node->var)->getClass() === ContainerInterface::class) {
$service = $this->serviceMap->getServiceFromNode($node->args[0] ?? null);

if ($service !== null && !$service['public']) {
return [\sprintf('Service "%s" is private.', $service['id'])];
}
}

return [];
}

}
21 changes: 9 additions & 12 deletions src/Rules/ContainerInterfaceUnknownServiceRule.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,7 @@
use PHPStan\Analyser\Scope;
use PHPStan\Rules\Rule;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Scalar\String_;
use Symfony\Component\DependencyInjection\ContainerInterface;

final class ContainerInterfaceUnknownServiceRule implements Rule
Expand All @@ -33,16 +31,15 @@ public function getNodeType(): string

public function processNode(Node $node, Scope $scope): array
{
$services = $this->serviceMap->getServices();
return $node instanceof MethodCall
&& $node->name === 'get'
&& $scope->getType($node->var)->getClass() === ContainerInterface::class
&& isset($node->args[0])
&& $node->args[0] instanceof Arg
&& $node->args[0]->value instanceof String_
&& !\array_key_exists($node->args[0]->value->value, $services)
? [\sprintf('Service "%s" is not registered in the container.', $node->args[0]->value->value)]
: [];
if ($node instanceof MethodCall && $node->name === 'get' && $scope->getType($node->var)->getClass() === ContainerInterface::class) {
$service = $this->serviceMap->getServiceFromNode($node->args[0] ?? null);

if ($service === null) {
return [\sprintf('Service "%s" is not registered in the container.', ServiceMap::getServiceIdFromNode($node))];
}
}

return [];
}

}
37 changes: 35 additions & 2 deletions src/ServiceMap.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@

namespace Lookyman\PHPStan\Symfony;

use PhpParser\Node\Arg;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Scalar\String_;

final class ServiceMap
{

Expand All @@ -21,7 +25,9 @@ public function __construct(string $containerXml)
if (!isset($attrs->id)) {
continue;
}

$service = [
'id' => (string) $attrs->id,
'class' => isset($attrs->class) ? (string) $attrs->class : \null,
'public' => !isset($attrs->public) || (string) $attrs->public !== 'false',
'synthetic' => isset($attrs->synthetic) && (string) $attrs->synthetic === 'true',
Expand All @@ -35,6 +41,7 @@ public function __construct(string $containerXml)
foreach ($aliases as $id => $alias) {
if (\array_key_exists($alias['alias'], $this->services)) {
$this->services[$id] = [
'id' => $id,
'class' => $this->services[$alias['alias']]['class'],
'public' => $alias['public'],
'synthetic' => $alias['synthetic'],
Expand All @@ -43,9 +50,35 @@ public function __construct(string $containerXml)
}
}

public function getServices(): array
/**
* @param mixed $node
* @return array|null
*/
public function getServiceFromNode($node)
{
$serviceId = self::getServiceIdFromNode($node);

if ($serviceId !== null && \array_key_exists($serviceId, $this->services) && !$this->services[$serviceId]['synthetic']) {
return $this->services[$serviceId];
}

return null;
}

/**
* @param mixed $node
* @return null|string
*/
public static function getServiceIdFromNode($node)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not a fan of having this here... but I needed a way to get the ID there : https://github.com/hectorj/phpstan-symfony/blob/ba8dc8d950a5da6117172f1e231fe55b75f34715/src/Rules/ContainerInterfaceUnknownServiceRule.php#L38

I'm open to suggestions.
Don't hesitate to edit the PR if you want to refactor.

{
return $this->services;
if ($node instanceof Arg) {
if ($node->value instanceof String_) {
return $node->value->value;
} elseif ($node->value instanceof ClassConstFetch) {
return $node->value->class->toString();
}
}
return null;
}

}
16 changes: 6 additions & 10 deletions src/Type/ContainerInterfaceDynamicReturnTypeExtension.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,9 +10,7 @@
use PHPStan\Type\DynamicMethodReturnTypeExtension;
use PHPStan\Type\ObjectType;
use PHPStan\Type\Type;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Scalar\String_;
use Symfony\Component\DependencyInjection\ContainerInterface;

final class ContainerInterfaceDynamicReturnTypeExtension implements DynamicMethodReturnTypeExtension
Expand Down Expand Up @@ -43,14 +41,12 @@ public function getTypeFromMethodCall(
MethodCall $methodCall,
Scope $scope
): Type {
$services = $this->serviceMap->getServices();
return isset($methodCall->args[0])
&& $methodCall->args[0] instanceof Arg
&& $methodCall->args[0]->value instanceof String_
&& \array_key_exists($methodCall->args[0]->value->value, $services)
&& !$services[$methodCall->args[0]->value->value]['synthetic']
? new ObjectType($services[$methodCall->args[0]->value->value]['class'])
: $methodReflection->getReturnType();
$service = $this->serviceMap->getServiceFromNode($methodCall->args[0] ?? null);
if ($service === null || $service['synthetic']) {
return $methodReflection->getReturnType();
}

return new ObjectType($service['class'] ?? $service['id']);
}

}
19 changes: 18 additions & 1 deletion tests/ServiceMapTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,45 +15,62 @@ final class ServiceMapTest extends TestCase
public function testGetServices()
{
$serviceMap = new ServiceMap(__DIR__ . '/container.xml');

$servicesProperty = (new \ReflectionClass($serviceMap))->getProperty('services');
$servicesProperty->setAccessible(true);

self::assertEquals(
[
'withoutClass' => [
'id' => 'withoutClass',
'class' => \null,
'public' => \true,
'synthetic' => \false,
],
'withClass' => [
'id' => 'withClass',
'class' => 'Foo',
'public' => \true,
'synthetic' => \false,
],
'FullyQualified\Foo' => [
'id' => 'FullyQualified\Foo',
'class' => null,
'public' => \true,
'synthetic' => \false,
],
'withoutPublic' => [
'id' => 'withoutPublic',
'class' => 'Foo',
'public' => \true,
'synthetic' => \false,
],
'publicNotFalse' => [
'id' => 'publicNotFalse',
'class' => 'Foo',
'public' => \true,
'synthetic' => \false,
],
'private' => [
'id' => 'private',
'class' => 'Foo',
'public' => \false,
'synthetic' => \false,
],
'synthetic' => [
'id' => 'synthetic',
'class' => 'Foo',
'public' => \true,
'synthetic' => \true,
],
'alias' => [
'id' => 'alias',
'class' => 'Foo',
'public' => \true,
'synthetic' => \false,
],
],
$serviceMap->getServices()
$servicesProperty->getValue($serviceMap)
);
}

Expand Down
7 changes: 7 additions & 0 deletions tests/Type/ContainerInterfaceDynamicReturnTypeExtensionTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,9 @@
use PHPUnit\Framework\TestCase;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
use PhpParser\Node\Expr\ClassConstFetch;
use PhpParser\Node\Expr\MethodCall;
use PhpParser\Node\Name\FullyQualified;
use PhpParser\Node\Scalar\String_;
use Symfony\Component\DependencyInjection\ContainerInterface;

Expand Down Expand Up @@ -77,6 +79,11 @@ public function getTypeFromMethodCallProvider(): array
new MethodCall($this->createMock(Expr::class), '', [new Arg(new String_('withClass'))]),
new ObjectType('Foo'),
],
'foundFromClassConst' => [
$this->createMock(MethodReflection::class),
new MethodCall($this->createMock(Expr::class), '', [new Arg(new ClassConstFetch(new FullyQualified('FullyQualified\Foo'), 'class'))]),
new ObjectType('FullyQualified\Foo'),
],
'notFound' => [
$methodReflectionNotFound,
new MethodCall($this->createMock(Expr::class), ''),
Expand Down
1 change: 1 addition & 0 deletions tests/container.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
<service></service><!-- without id -->
<service id="withoutClass"></service>
<service id="withClass" class="Foo"></service>
<service id="FullyQualified\Foo"></service>
<service id="withoutPublic" class="Foo"></service>
<service id="publicNotFalse" class="Foo" public="true"></service>
<service id="private" class="Foo" public="false"></service>
Expand Down