diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ad20c62..cc71592f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/), For a full diff see [`0.11.0...master`](https://github.com/localheinz/phpstan-rules/compare/0.11.0...master). +### Added + +* Added `Methods\NoParameterWithContainerTypeDeclarationRule`, which reports an error when a method has a type declaration that corresponds to a known dependency injection container or service locator ([#122](https://github.com/localheinz/phpstan-rules/pull/122)), by [@dmecke](https://github.com/dmecke) + ## [`0.11.0`](https://github.com/localheinz/phpstan-rules/releases/tag/0.10.0) For a full diff see [`0.10.0...0.11.0`](https://github.com/localheinz/phpstan-rules/compare/0.10.0...0.11.0). diff --git a/README.md b/README.md index 96f71c98..a246d449 100644 --- a/README.md +++ b/README.md @@ -54,6 +54,7 @@ This package provides the following rules for use with [`phpstan/phpstan`](https * [`Localheinz\PHPStan\Rules\Functions\NoParameterWithNullDefaultValueRule`](https://github.com/localheinz/phpstan-rules#functionsnoparameterwithnulldefaultvaluerule) * [`Localheinz\PHPStan\Rules\Methods\NoConstructorParameterWithDefaultValueRule`](https://github.com/localheinz/phpstan-rules#methodsnoconstructorparameterwithdefaultvaluerule) * [`Localheinz\PHPStan\Rules\Methods\NoNullableReturnTypeDeclarationRule`](https://github.com/localheinz/phpstan-rules#methodsnonullablereturntypedeclarationrule) +* [`Localheinz\PHPStan\Rules\Methods\NoParameterWithContainerTypeDeclarationRule`](https://github.com/localheinz/phpstan-rules#methodsnoparamterwithcontainertypedeclarationrule) * [`Localheinz\PHPStan\Rules\Methods\NoParameterWithNullableTypeDeclarationRule`](https://github.com/localheinz/phpstan-rules#methodsnoparameterwithnullabletypedeclarationrule) * [`Localheinz\PHPStan\Rules\Methods\NoParameterWithNullDefaultValueRule`](https://github.com/localheinz/phpstan-rules#methodsnoparameterwithnulldefaultvaluerule) * [`Localheinz\PHPStan\Rules\Statements\NoSwitchRule`](https://github.com/localheinz/phpstan-rules#statementsnoswitchrule) @@ -184,6 +185,29 @@ This rule reports an error when a method declared in uses a nullable return type declaration. +#### `Methods\NoParameterWithContainerTypeDeclarationRule` + +This rule reports an error when a method has a type declaration for a known dependency injection container or service locator. + +##### Defaults + +By default, this rule disallows the use of type declarations indicating an implementation of + +* [`Psr\Container\ContainerInterface`](https://github.com/php-fig/container/blob/1.0.0/src/ContainerInterface.php) + +is expected to be injected into a method. + +##### Configuring container interfaces + +If you want to configure the list of interfaces implemented by dependency injection containers and service locators yourself, you can set the `interfacesImplementedByContainers` parameter to a list of interface names: + +```neon +parameters: + interfacesImplementedByContainers: + - Fancy\DependencyInjection\ContainerInterface + - Other\ServiceLocatorInterface +``` + #### `Methods\NoParameterWithNullableTypeDeclarationRule` This rule reports an error when a method declared in diff --git a/composer.json b/composer.json index 275f49fe..0dce793c 100644 --- a/composer.json +++ b/composer.json @@ -27,7 +27,9 @@ "localheinz/test-util": "~0.7.0", "phpstan/phpstan-deprecation-rules": "~0.11.2", "phpstan/phpstan-strict-rules": "~0.11.1", - "phpunit/phpunit": "^7.5.15" + "phpunit/phpunit": "^7.5.15", + "psr/container": "^1.0.0", + "zendframework/zend-servicemanager": "^2.0.0" }, "config": { "preferred-install": "dist", diff --git a/composer.lock b/composer.lock index 8d40a697..a45d1e8e 100644 --- a/composer.lock +++ b/composer.lock @@ -4,7 +4,7 @@ "Read more about it at https://getcomposer.org/doc/01-basic-usage.md#installing-dependencies", "This file is @generated automatically" ], - "content-hash": "1509209436d2ef89f89a6945ea8728a2", + "content-hash": "2dfa168e6d55f72f311405e51e6f63a8", "packages": [ { "name": "composer/xdebug-handler", @@ -1362,6 +1362,37 @@ ], "time": "2019-03-19T17:25:45+00:00" }, + { + "name": "container-interop/container-interop", + "version": "1.2.0", + "source": { + "type": "git", + "url": "https://github.com/container-interop/container-interop.git", + "reference": "79cbf1341c22ec75643d841642dd5d6acd83bdb8" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/container-interop/container-interop/zipball/79cbf1341c22ec75643d841642dd5d6acd83bdb8", + "reference": "79cbf1341c22ec75643d841642dd5d6acd83bdb8", + "shasum": "" + }, + "require": { + "psr/container": "^1.0" + }, + "type": "library", + "autoload": { + "psr-4": { + "Interop\\Container\\": "src/Interop/Container/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "MIT" + ], + "description": "Promoting the interoperability of container objects (DIC, SL, etc.)", + "homepage": "https://github.com/container-interop/container-interop", + "time": "2017-02-14T19:40:03+00:00" + }, { "name": "doctrine/annotations", "version": "v1.7.0", @@ -2500,18 +2531,18 @@ "authors": [ { "name": "Arne Blankerts", - "role": "Developer", - "email": "arne@blankerts.de" + "email": "arne@blankerts.de", + "role": "Developer" }, { "name": "Sebastian Heuer", - "role": "Developer", - "email": "sebastian@phpeople.de" + "email": "sebastian@phpeople.de", + "role": "Developer" }, { "name": "Sebastian Bergmann", - "role": "Developer", - "email": "sebastian@phpunit.de" + "email": "sebastian@phpunit.de", + "role": "Developer" } ], "description": "Component for reading phar.io manifest information from a PHP Archive (PHAR)", @@ -4535,6 +4566,58 @@ "validate" ], "time": "2019-08-24T08:43:50+00:00" + }, + { + "name": "zendframework/zend-servicemanager", + "version": "2.7.11", + "source": { + "type": "git", + "url": "https://github.com/zendframework/zend-servicemanager.git", + "reference": "99ec9ed5d0f15aed9876433c74c2709eb933d4c7" + }, + "dist": { + "type": "zip", + "url": "https://api.github.com/repos/zendframework/zend-servicemanager/zipball/99ec9ed5d0f15aed9876433c74c2709eb933d4c7", + "reference": "99ec9ed5d0f15aed9876433c74c2709eb933d4c7", + "shasum": "" + }, + "require": { + "container-interop/container-interop": "~1.0", + "php": "^5.5 || ^7.0" + }, + "require-dev": { + "athletic/athletic": "dev-master", + "fabpot/php-cs-fixer": "1.7.*", + "phpunit/phpunit": "~4.0", + "zendframework/zend-di": "~2.5", + "zendframework/zend-mvc": "~2.5" + }, + "suggest": { + "ocramius/proxy-manager": "ProxyManager 0.5.* to handle lazy initialization of services", + "zendframework/zend-di": "Zend\\Di component" + }, + "type": "library", + "extra": { + "branch-alias": { + "dev-master": "2.7-dev", + "dev-develop": "3.0-dev" + } + }, + "autoload": { + "psr-4": { + "Zend\\ServiceManager\\": "src/" + } + }, + "notification-url": "https://packagist.org/downloads/", + "license": [ + "BSD-3-Clause" + ], + "homepage": "https://github.com/zendframework/zend-servicemanager", + "keywords": [ + "servicemanager", + "zf2" + ], + "time": "2018-06-22T14:49:54+00:00" } ], "aliases": [], diff --git a/rules.neon b/rules.neon index 8fb806b2..1c3ea7b1 100644 --- a/rules.neon +++ b/rules.neon @@ -2,11 +2,14 @@ parameters: allowAbstractClasses: true classesAllowedToBeExtended: [] classesNotRequiredToBeAbstractOrFinal: [] + interfacesImplementedByContainers: + - Psr\Container\ContainerInterface parametersSchema: allowAbstractClasses: bool() classesAllowedToBeExtended: listOf(string()) classesNotRequiredToBeAbstractOrFinal: listOf(string()) + interfacesImplementedByContainers: listOf(string()) rules: - Localheinz\PHPStan\Rules\Closures\NoNullableReturnTypeDeclarationRule @@ -40,3 +43,10 @@ services: classesAllowedToBeExtended: %classesAllowedToBeExtended% tags: - phpstan.rules.rule + + - + class: Localheinz\PHPStan\Rules\Methods\NoParameterWithContainerTypeDeclarationRule + arguments: + interfacesImplementedByContainers: %interfacesImplementedByContainers% + tags: + - phpstan.rules.rule diff --git a/src/Methods/NoParameterWithContainerTypeDeclarationRule.php b/src/Methods/NoParameterWithContainerTypeDeclarationRule.php new file mode 100644 index 00000000..e275ba17 --- /dev/null +++ b/src/Methods/NoParameterWithContainerTypeDeclarationRule.php @@ -0,0 +1,148 @@ +broker = $broker; + $this->interfacesImplementedByContainers = \array_filter( + \array_map(static function (string $interfaceImplementedByContainers): string { + return $interfaceImplementedByContainers; + }, $interfacesImplementedByContainers), + static function (string $interfaceImplementedByContainer): bool { + return \interface_exists($interfaceImplementedByContainer); + } + ); + } + + public function getNodeType(): string + { + return Node\Stmt\ClassMethod::class; + } + + public function processNode(Node $node, Scope $scope): array + { + if (!$node instanceof Node\Stmt\ClassMethod) { + throw new ShouldNotHappenException(\sprintf( + 'Expected node to be instance of "%s", but got instance of "%s" instead.', + Node\Stmt\ClassMethod::class, + \get_class($node) + )); + } + + if (0 === \count($this->interfacesImplementedByContainers)) { + return []; + } + + if (0 === \count($node->params)) { + return []; + } + + $methodName = $node->name->toString(); + + /** @var Reflection\ClassReflection $containingClass */ + $containingClass = $scope->getClassReflection(); + + return \array_reduce( + $node->params, + function (array $errors, Node\Param $node) use ($containingClass, $methodName) { + $type = $node->type; + + if (!$type instanceof Node\Name) { + return $errors; + } + + /** @var Node\Expr\Variable $variable */ + $variable = $node->var; + + /** @var string $parameterName */ + $parameterName = $variable->name; + + $classUsedInTypeDeclaration = $this->broker->getClass($type->toCodeString()); + + if ($classUsedInTypeDeclaration->isInterface()) { + foreach ($this->interfacesImplementedByContainers as $interfaceImplementedByContainer) { + if ($classUsedInTypeDeclaration->getNativeReflection()->isSubclassOf($interfaceImplementedByContainer)) { + $errors[] = self::createError( + $containingClass, + $methodName, + $parameterName, + $classUsedInTypeDeclaration + ); + + return $errors; + } + } + } + + foreach ($this->interfacesImplementedByContainers as $interfaceImplementedByContainer) { + if ($classUsedInTypeDeclaration->getNativeReflection()->implementsInterface($interfaceImplementedByContainer)) { + $errors[] = self::createError( + $containingClass, + $methodName, + $parameterName, + $classUsedInTypeDeclaration + ); + + return $errors; + } + } + + return $errors; + }, + [] + ); + } + + private static function createError( + Reflection\ClassReflection $classReflection, + string $methodName, + string $parameterName, + Reflection\ClassReflection $classUsedInTypeDeclaration + ): string { + if ($classReflection->isAnonymous()) { + return \sprintf( + 'Method %s() in anonymous class has a parameter $%s with a type declaration of %s, but containers should not be injected.', + $methodName, + $parameterName, + $classUsedInTypeDeclaration->getName() + ); + } + + return \sprintf( + 'Method %s::%s() has a parameter $%s with a type declaration of %s, but containers should not be injected.', + $classReflection->getName(), + $methodName, + $parameterName, + $classUsedInTypeDeclaration->getName() + ); + } +} diff --git a/test/Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/ClassImplementingContainerInterface.php b/test/Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/ClassImplementingContainerInterface.php new file mode 100644 index 00000000..98c345ef --- /dev/null +++ b/test/Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/ClassImplementingContainerInterface.php @@ -0,0 +1,18 @@ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Success/anonymous-class-with-method-with-parameter-with-other-type-declaration.php', + 'anonymous-class-with-method-without-parameters' => __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Success/anonymous-class-with-method-without-parameter.php', + 'class-with-method-with-parameter-with-other-type-declaration' => __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Success/ClassWithMethodWithParameterWithOtherTypeDeclaration.php', + 'class-with-method-without-parameter' => __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Success/ClassWithMethodWithoutParameter.php', + 'interface-with-method-with-parameter-with-other-type-declaration' => __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Success/InterfaceWithMethodWithParameterWithOtherTypeDeclaration.php', + 'interface-with-method-without-parameter' => __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Success/InterfaceWithMethodWithoutParameter.php', + ]; + + foreach ($paths as $description => $path) { + yield $description => [ + $path, + ]; + } + } + + public function providerAnalysisFails(): iterable + { + $paths = [ + 'anonymous-class-with-method-with-parameter-with-class-implementing-container-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/anonymous-class-with-method-with-parameter-with-class-implementing-container-interface-as-type-declaration.php', + [ + \sprintf( + 'Method __construct() in anonymous class has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\ClassImplementingContainerInterface::class + ), + 9, + ], + ], + 'anonymous-class-with-method-with-parameter-with-container-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/anonymous-class-with-method-with-parameter-with-container-interface-as-type-declaration.php', + [ + \sprintf( + 'Method __construct() in anonymous class has a parameter $container with a type declaration of %s, but containers should not be injected.', + Container\ContainerInterface::class + ), + 11, + ], + ], + 'anonymous-class-with-method-with-parameter-with-interface-extending-container-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/anonymous-class-with-method-with-parameter-with-interface-extending-container-interface-as-type-declaration.php', + [ + \sprintf( + 'Method __construct() in anonymous class has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\InterfaceExtendingContainerInterface::class + ), + 9, + ], + ], + 'anonymous-class-with-method-with-parameter-with-service-locator-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/anonymous-class-with-method-with-parameter-with-service-locator-interface-as-type-declaration.php', + [ + \sprintf( + 'Method __construct() in anonymous class has a parameter $container with a type declaration of %s, but containers should not be injected.', + ServiceManager\ServiceLocatorInterface::class + ), + 11, + ], + ], + 'class-with-method-with-parameter-with-class-implementing-container-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/ClassWithMethodWithParameterWithClassImplementingContainerInterfaceAsTypeDeclaration.php', + [ + \sprintf( + 'Method %s::method() has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\ClassWithMethodWithParameterWithClassImplementingContainerInterfaceAsTypeDeclaration::class, + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\ClassImplementingContainerInterface::class + ), + 9, + ], + ], + 'class-with-method-with-parameter-with-container-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/ClassWithMethodWithParameterWithContainerInterfaceAsTypeDeclaration.php', + [ + \sprintf( + 'Method %s::method() has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\ClassWithMethodWithParameterWithContainerInterfaceAsTypeDeclaration::class, + Container\ContainerInterface::class + ), + 11, + ], + ], + 'class-with-method-with-parameter-with-interface-extending-container-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/ClassWithMethodWithParameterWithInterfaceExtendingContainerInterfaceAsTypeDeclaration.php', + [ + \sprintf( + 'Method %s::method() has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\ClassWithMethodWithParameterWithInterfaceExtendingContainerInterfaceAsTypeDeclaration::class, + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\InterfaceExtendingContainerInterface::class + ), + 9, + ], + ], + 'class-with-method-with-parameter-with-service-locator-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/ClassWithMethodWithParameterWithServiceLocatorInterfaceAsTypeDeclaration.php', + [ + \sprintf( + 'Method %s::method() has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\ClassWithMethodWithParameterWithServiceLocatorInterfaceAsTypeDeclaration::class, + ServiceManager\ServiceLocatorInterface::class + ), + 11, + ], + ], + 'interface-with-method-with-parameter-with-container-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/InterfaceWithMethodWithParameterWithContainerInterfaceAsTypeDeclaration.php', + [ + \sprintf( + 'Method %s::method() has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\InterfaceWithMethodWithParameterWithContainerInterfaceAsTypeDeclaration::class, + Container\ContainerInterface::class + ), + 11, + ], + ], + 'interface-with-method-with-parameter-with-service-locator-interface-as-type-declaration' => [ + __DIR__ . '/../../Fixture/Methods/NoParameterWithContainerTypeDeclarationRule/Failure/InterfaceWithMethodWithParameterWithServiceLocatorInterfaceAsTypeDeclaration.php', + [ + \sprintf( + 'Method %s::method() has a parameter $container with a type declaration of %s, but containers should not be injected.', + Fixture\Methods\NoParameterWithContainerTypeDeclarationRule\Failure\InterfaceWithMethodWithParameterWithServiceLocatorInterfaceAsTypeDeclaration::class, + ServiceManager\ServiceLocatorInterface::class + ), + 11, + ], + ], + ]; + + foreach ($paths as $description => [$path, $error]) { + yield $description => [ + $path, + $error, + ]; + } + } + + protected function getRule(): Rule + { + return new NoParameterWithContainerTypeDeclarationRule( + $this->createBroker(), + [ + Container\ContainerInterface::class, + ServiceManager\ServiceLocatorInterface::class, + ] + ); + } +}