Refactored ServiceMap, improved tests, added conflicts

lookyman · lookyman · commit 1179dc589c6b · 2018-05-29T09:56:43.000+02:00
diff --git a/composer.json b/composer.json
@@ -32,8 +32,10 @@
     "satooshi/php-coveralls": "^1.0",
     "slevomat/coding-standard": "^4.5.2",
     "phpstan/phpstan-phpunit": "^0.10",
-    "symfony/framework-bundle": "^4.0",
-    "symfony/http-foundation": "^4.0"
+    "symfony/framework-bundle": "^3.0 || ^4.0"
+  },
+  "conflict": {
+    "symfony/framework-bundle": "<3.0"
   },
   "autoload": {
     "psr-4": {
diff --git a/phpstan.neon b/phpstan.neon
@@ -6,6 +6,3 @@ includes:
 parameters:
 	excludes_analyse:
 		- */tests/*/data/*
-	ignoreErrors:
-		- '#Call to an undefined method object::noMethod\(\)\.#'
-		- '#Offset string does not exist on array\(\)\.#'
diff --git a/src/Rules/Symfony/ContainerInterfacePrivateServiceRule.php b/src/Rules/Symfony/ContainerInterfacePrivateServiceRule.php
@@ -28,7 +28,7 @@ public function getNodeType(): string
 	/**
 	 * @param MethodCall $node
 	 * @param Scope $scope
-	 * @return mixed[]
+	 * @return string[]
 	 */
 	public function processNode(Node $node, Scope $scope): array
 	{
@@ -50,8 +50,8 @@ public function processNode(Node $node, Scope $scope): array
 		$serviceId = ServiceMap::getServiceIdFromNode($node->args[0]->value, $scope);
 		if ($serviceId !== null) {
 			$service = $this->serviceMap->getService($serviceId);
-			if ($service !== null && $service['public'] === false) {
-				return [sprintf('Service "%s" is private.', $service['id'])];
+			if ($service !== null && !$service->isPublic()) {
+				return [sprintf('Service "%s" is private.', $serviceId)];
 			}
 		}
 
diff --git a/src/Symfony/Service.php b/src/Symfony/Service.php
@@ -0,0 +1,63 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Symfony;
+
+final class Service
+{
+
+	/** @var string */
+	private $id;
+
+	/** @var string|null */
+	private $class;
+
+	/** @var bool */
+	private $public;
+
+	/** @var bool */
+	private $synthetic;
+
+	/** @var string|null */
+	private $alias;
+
+	public function __construct(
+		string $id,
+		?string $class,
+		bool $public,
+		bool $synthetic,
+		?string $alias
+	)
+	{
+		$this->id = $id;
+		$this->class = $class;
+		$this->public = $public;
+		$this->synthetic = $synthetic;
+		$this->alias = $alias;
+	}
+
+	public function getId(): string
+	{
+		return $this->id;
+	}
+
+	public function getClass(): ?string
+	{
+		return $this->class;
+	}
+
+	public function isPublic(): bool
+	{
+		return $this->public;
+	}
+
+	public function isSynthetic(): bool
+	{
+		return $this->synthetic;
+	}
+
+	public function getAlias(): ?string
+	{
+		return $this->alias;
+	}
+
+}
diff --git a/src/Symfony/ServiceMap.php b/src/Symfony/ServiceMap.php
@@ -4,17 +4,18 @@
 
 use PhpParser\Node\Expr;
 use PHPStan\Analyser\Scope;
-use PHPStan\Type\Constant\ConstantStringType;
+use PHPStan\Type\TypeUtils;
 
 final class ServiceMap
 {
 
-	/** @var array<string, array> */
-	private $services;
+	/** @var Service[] */
+	private $services = [];
 
 	public function __construct(string $containerXml)
 	{
-		$this->services = $aliases = [];
+		/** @var Service[] $aliases */
+		$aliases = [];
 		/** @var \SimpleXMLElement|false $xml */
 		$xml = @simplexml_load_file($containerXml);
 		if ($xml === false) {
@@ -25,44 +26,42 @@ 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',
-			];
-			if (isset($attrs->alias)) {
-				$aliases[(string) $attrs->id] = array_merge($service, ['alias' => (string) $attrs->alias]);
+			$service = new Service(
+				(string) $attrs->id,
+				isset($attrs->class) ? (string) $attrs->class : null,
+				!isset($attrs->public) || (string) $attrs->public !== 'false',
+				isset($attrs->synthetic) && (string) $attrs->synthetic === 'true',
+				isset($attrs->alias) ? (string) $attrs->alias : null
+			);
+			if ($service->getAlias() !== null) {
+				$aliases[] = $service;
 			} else {
-				$this->services[(string) $attrs->id] = $service;
+				$this->services[$service->getId()] = $service;
 			}
 		}
-		foreach ($aliases as $id => $alias) {
-			if (!array_key_exists($alias['alias'], $this->services)) {
+		foreach ($aliases as $service) {
+			if ($service->getAlias() !== null && !array_key_exists($service->getAlias(), $this->services)) {
 				continue;
 			}
-			$this->services[$id] = [
-				'id' => $id,
-				'class' => $this->services[$alias['alias']]['class'],
-				'public' => $alias['public'],
-				'synthetic' => $alias['synthetic'],
-			];
+			$this->services[$service->getId()] = new Service(
+				$service->getId(),
+				$this->services[$service->getAlias()]->getClass(),
+				$service->isPublic(),
+				$service->isSynthetic(),
+				null
+			);
 		}
 	}
 
-	/**
-	 * @param string $id
-	 * @return mixed[]|null
-	 */
-	public function getService(string $id): ?array
+	public function getService(string $id): ?Service
 	{
 		return $this->services[$id] ?? null;
 	}
 
 	public static function getServiceIdFromNode(Expr $node, Scope $scope): ?string
 	{
-		$nodeType = $scope->getType($node);
-		return $nodeType instanceof ConstantStringType ? $nodeType->getValue() : null;
+		$strings = TypeUtils::getConstantStrings($scope->getType($node));
+		return count($strings) === 1 ? $strings[0]->getValue() : null;
 	}
 
 }
diff --git a/src/Type/Symfony/ContainerInterfaceDynamicReturnTypeExtension.php b/src/Type/Symfony/ContainerInterfaceDynamicReturnTypeExtension.php
@@ -46,8 +46,8 @@ public function getTypeFromMethodCall(
 		$serviceId = ServiceMap::getServiceIdFromNode($methodCall->args[0]->value, $scope);
 		if ($serviceId !== null) {
 			$service = $this->serviceMap->getService($serviceId);
-			if ($service !== null && $service['synthetic'] === false) {
-				return new ObjectType($service['class'] ?? $service['id']);
+			if ($service !== null && !$service->isSynthetic()) {
+				return new ObjectType($service->getClass() ?? $serviceId);
 			}
 		}
 
diff --git a/src/Type/Symfony/ControllerDynamicReturnTypeExtension.php b/src/Type/Symfony/ControllerDynamicReturnTypeExtension.php
@@ -46,8 +46,8 @@ public function getTypeFromMethodCall(
 		$serviceId = ServiceMap::getServiceIdFromNode($methodCall->args[0]->value, $scope);
 		if ($serviceId !== null) {
 			$service = $this->serviceMap->getService($serviceId);
-			if ($service !== null && $service['synthetic'] === false) {
-				return new ObjectType($service['class'] ?? $service['id']);
+			if ($service !== null && !$service->isSynthetic()) {
+				return new ObjectType($service->getClass() ?? $serviceId);
 			}
 		}
 
diff --git a/tests/Rules/Symfony/ContainerInterfacePrivateServiceRuleTest.php b/tests/Rules/Symfony/ContainerInterfacePrivateServiceRuleTest.php
@@ -5,9 +5,6 @@
 use PHPStan\Rules\Rule;
 use PHPStan\Symfony\ServiceMap;
 
-/**
- * @covers \PHPStan\Rules\Symfony\ContainerInterfacePrivateServiceRule
- */
 final class ContainerInterfacePrivateServiceRuleTest extends \PHPStan\Testing\RuleTestCase
 {
 
@@ -21,11 +18,11 @@ protected function getRule(): Rule
 	public function testGetPrivateService(): void
 	{
 		$this->analyse(
-			[__DIR__ . '/ExampleController.php'],
+			[__DIR__ . '/../../Symfony/data/ExampleController.php'],
 			[
 				[
 					'Service "private" is private.',
-					15,
+					14,
 				],
 			]
 		);
diff --git a/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php b/tests/Rules/Symfony/ContainerInterfaceUnknownServiceRuleTest.php
@@ -5,9 +5,6 @@
 use PHPStan\Rules\Rule;
 use PHPStan\Symfony\ServiceMap;
 
-/**
- * @covers \PHPStan\Rules\Symfony\ContainerInterfaceUnknownServiceRule
- */
 final class ContainerInterfaceUnknownServiceRuleTest extends \PHPStan\Testing\RuleTestCase
 {
 
@@ -21,23 +18,23 @@ protected function getRule(): Rule
 	public function testGetUnknownService(): void
 	{
 		$this->analyse(
-			[__DIR__ . '/ExampleController.php'],
+			[__DIR__ . '/../../Symfony/data/ExampleController.php'],
 			[
 				[
 					'Service "service.not.found" is not registered in the container.',
-					21,
+					20,
 				],
 				[
 					'Service "PHPStan\Symfony\ServiceMap" is not registered in the container.',
-					27,
+					26,
 				],
 				[
 					'Service "service.PHPStan\Symfony\ServiceMap" is not registered in the container.',
-					39,
+					38,
 				],
 				[
-					'Service "PHPStan\Rules\Symfony\ExampleController" is not registered in the container.',
-					45,
+					'Service "PHPStan\Symfony\ExampleController" is not registered in the container.',
+					44,
 				],
 			]
 		);
diff --git a/tests/Symfony/ServiceMapTest.php b/tests/Symfony/ServiceMapTest.php
@@ -9,12 +9,8 @@
 use PhpParser\PrettyPrinter\Standard;
 use PHPStan\Analyser\Scope;
 use PHPStan\Analyser\ScopeContext;
-use PHPStan\Rules\Symfony\ExampleController;
 use PHPStan\Testing\TestCase;
 
-/**
- * @covers \PHPStan\Symfony\ServiceMap
- */
 final class ServiceMapTest extends TestCase
 {
 
@@ -48,8 +44,8 @@ public function testGetServiceIdFromNode(): void
 		$scope = new Scope($broker, $printer, $typeSpecifier, ScopeContext::create(''));
 
 		self::assertSame('foo', ServiceMap::getServiceIdFromNode(new String_('foo'), $scope));
-		self::assertSame('bar', ServiceMap::getServiceIdFromNode(new ClassConstFetch(new Name('PHPStan\Rules\Symfony\ExampleController'), 'BAR'), $scope));
-		self::assertSame('foobar', ServiceMap::getServiceIdFromNode(new Concat(new String_('foo'), new ClassConstFetch(new Name('PHPStan\Rules\Symfony\ExampleController'), 'BAR')), $scope));
+		self::assertSame('bar', ServiceMap::getServiceIdFromNode(new ClassConstFetch(new Name(ExampleController::class), 'BAR'), $scope));
+		self::assertSame('foobar', ServiceMap::getServiceIdFromNode(new Concat(new String_('foo'), new ClassConstFetch(new Name(ExampleController::class), 'BAR')), $scope));
 
 		$scope = $scope->enterClass($broker->getClass(ExampleController::class));
 		self::assertSame('bar', ServiceMap::getServiceIdFromNode(new ClassConstFetch(new Name('static'), 'BAR'), $scope));
diff --git a/tests/Symfony/ServiceTest.php b/tests/Symfony/ServiceTest.php
@@ -0,0 +1,20 @@
+<?php declare(strict_types = 1);
+
+namespace PHPStan\Symfony;
+
+use PHPUnit\Framework\TestCase;
+
+final class ServiceTest extends TestCase
+{
+
+	public function testGetSet(): void
+	{
+		$service = new Service('foo', 'Bar', true, true, 'alias');
+		self::assertSame('foo', $service->getId());
+		self::assertSame('Bar', $service->getClass());
+		self::assertTrue($service->isPublic());
+		self::assertTrue($service->isSynthetic());
+		self::assertSame('alias', $service->getAlias());
+	}
+
+}
diff --git a/tests/Symfony/data/ExampleController.php b/tests/Symfony/data/ExampleController.php
@@ -1,8 +1,7 @@
 <?php declare(strict_types = 1);
 
-namespace PHPStan\Rules\Symfony;
+namespace PHPStan\Symfony;
 
-use PHPStan\Symfony\ServiceMap;
 use Symfony\Bundle\FrameworkBundle\Controller\Controller;
 
 final class ExampleController extends Controller
diff --git a/tests/Type/Symfony/ContainerInterfaceDynamicReturnTypeExtensionTest.php b/tests/Type/Symfony/ContainerInterfaceDynamicReturnTypeExtensionTest.php
@@ -17,9 +17,6 @@
 use PHPStan\Type\Type;
 use PHPUnit\Framework\TestCase;
 
-/**
- * @covers \PHPStan\Type\Symfony\ContainerInterfaceDynamicReturnTypeExtension
- */
 final class ContainerInterfaceDynamicReturnTypeExtensionTest extends TestCase
 {
 
diff --git a/tests/Type/Symfony/ControllerDynamicReturnTypeExtensionTest.php b/tests/Type/Symfony/ControllerDynamicReturnTypeExtensionTest.php
@@ -17,9 +17,6 @@
 use PHPStan\Type\Type;
 use PHPUnit\Framework\TestCase;
 
-/**
- * @covers \PHPStan\Type\Symfony\ControllerDynamicReturnTypeExtension
- */
 final class ControllerDynamicReturnTypeExtensionTest extends TestCase
 {
 
diff --git a/tests/Type/Symfony/RequestDynamicReturnTypeExtensionTest.php b/tests/Type/Symfony/RequestDynamicReturnTypeExtensionTest.php
@@ -16,9 +16,6 @@
 use PHPStan\Type\Type;
 use PHPUnit\Framework\TestCase;
 
-/**
- * @covers \PHPStan\Type\Symfony\RequestDynamicReturnTypeExtension
- */
 final class RequestDynamicReturnTypeExtensionTest extends TestCase
 {
 

Original file line number	Diff line number	Diff line change
`@@ -28,7 +28,7 @@ public function getNodeType(): string`
`28`	`28`	`/**`
`29`	`29`	`* @param MethodCall $node`
`30`	`30`	`* @param Scope $scope`
`31`		`- * @return mixed[]`
	`31`	`+ * @return string[]`
`32`	`32`	`*/`
`33`	`33`	`public function processNode(Node $node, Scope $scope): array`
`34`	`34`	`{`
`@@ -50,8 +50,8 @@ public function processNode(Node $node, Scope $scope): array`
`50`	`50`	`$serviceId = ServiceMap::getServiceIdFromNode($node->args[0]->value, $scope);`
`51`	`51`	`if ($serviceId !== null) {`
`52`	`52`	`$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)];`
`55`	`55`	`}`
`56`	`56`	`}`
`57`	`57`
Original file line number	Diff line number	Diff line change
`@@ -46,8 +46,8 @@ public function getTypeFromMethodCall(`
`46`	`46`	`$serviceId = ServiceMap::getServiceIdFromNode($methodCall->args[0]->value, $scope);`
`47`	`47`	`if ($serviceId !== null) {`
`48`	`48`	`$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);`
`51`	`51`	`}`
`52`	`52`	`}`
`53`	`53`
Original file line number	Diff line number	Diff line change
`@@ -5,9 +5,6 @@`
`5`	`5`	`use PHPStan\Rules\Rule;`
`6`	`6`	`use PHPStan\Symfony\ServiceMap;`
`7`	`7`
`8`		`-/**`
`9`		`- * @covers \PHPStan\Rules\Symfony\ContainerInterfacePrivateServiceRule`
`10`		`- */`
`11`	`8`	`final class ContainerInterfacePrivateServiceRuleTest extends \PHPStan\Testing\RuleTestCase`
`12`	`9`	`{`
`13`	`10`
`@@ -21,11 +18,11 @@ protected function getRule(): Rule`
`21`	`18`	`public function testGetPrivateService(): void`
`22`	`19`	`{`
`23`	`20`	`$this->analyse(`
`24`		`- [__DIR__ . '/ExampleController.php'],`
	`21`	`+ [__DIR__ . '/../../Symfony/data/ExampleController.php'],`
`25`	`22`	`[`
`26`	`23`	`[`
`27`	`24`	`'Service "private" is private.',`
`28`		`- 15,`
	`25`	`+ 14,`
`29`	`26`	`],`
`30`	`27`	`]`
`31`	`28`	`);`