From 9c248fc529dd69c63ed705e7e1387b1a39bf2cd8 Mon Sep 17 00:00:00 2001 From: Tomas Votruba Date: Wed, 23 Sep 2026 11:48:44 +0200 Subject: [PATCH] [symfony] remove PreferAutowireAttributeOverConfigParamRule, too opinionated --- README.md | 40 ---- config/symfony-config-rules.neon | 3 - .../RuleIdentifier/SymfonyRuleIdentifier.php | 2 - src/Enum/SymfonyClass.php | 2 - src/Enum/SymfonyFunctionName.php | 2 - ...erAutowireAttributeOverConfigParamRule.php | 182 ------------------ .../Fixture/ParameterPercentReference.php | 13 -- .../Fixture/SkipNoParameterReference.php | 13 -- .../Fixture/SomeConfigWithInvalidSet.php | 14 -- ...towireAttributeOverConfigParamRuleTest.php | 61 ------ .../Source/SomeSetServiceWithConstructor.php | 10 - .../config/configured_rule.neon | 2 - 12 files changed, 344 deletions(-) delete mode 100644 src/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule.php delete mode 100644 tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/ParameterPercentReference.php delete mode 100644 tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SkipNoParameterReference.php delete mode 100644 tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SomeConfigWithInvalidSet.php delete mode 100644 tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/PreferAutowireAttributeOverConfigParamRuleTest.php delete mode 100644 tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Source/SomeSetServiceWithConstructor.php delete mode 100644 tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/config/configured_rule.neon diff --git a/README.md b/README.md index b04b577c0..aa2b6e5a6 100644 --- a/README.md +++ b/README.md @@ -1912,46 +1912,6 @@ return static function (ContainerConfigurator $containerConfigurator): void {
-### PreferAutowireAttributeOverConfigParamRule - -Instead of parameter reference in config, add #[Autowire(param: ...)] in the "%s" class constructor - -```yaml -rules: - - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule -``` - -```php -use Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator; - -return static function (ContainerConfigurator $containerConfigurator): void { - $services = $containerConfigurator->services(); - - $services->set(SomeService::class)->args(['%some_param%']); -}; -``` - -:x: - -
- -```php -use Symfony\Component\DependencyInjection\Attribute\Autowire; - -final class SomeService -{ - public function __construct( - #[Autowire(param: 'some_param')] - private string $someParam - ) { - } -} -``` - -:+1: - -
- ### NoDuplicateArgsAutowireByTypeRule Instead of passing "%s" to args(), remove the line and let autowiring handle it diff --git a/config/symfony-config-rules.neon b/config/symfony-config-rules.neon index 6156bf52d..37b65afe0 100644 --- a/config/symfony-config-rules.neon +++ b/config/symfony-config-rules.neon @@ -14,9 +14,6 @@ rules: - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoDuplicateArgsAutowireByTypeRule - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoDuplicateArgAutowireByTypeRule - # #[Autowire() in-class attribute over param() in config - - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule - # $services->set('X', 'X') - Symplify\PHPStanRules\Rules\Symfony\ConfigClosure\NoServiceSameNameSetClassRule diff --git a/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php b/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php index 2b648e89b..18f10a05d 100644 --- a/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php +++ b/src/Enum/RuleIdentifier/SymfonyRuleIdentifier.php @@ -52,8 +52,6 @@ final class SymfonyRuleIdentifier public const string REQUIRED_IS_GRANTED_ENUM = 'symfony.requiredIsGrantedEnum'; - public const string PREFER_AUTOWIRE_ATTRIBUTE_OVER_CONFIG_PARAM = 'symfony.preferAutowireAttributeOverConfigParam'; - public const string RULE_IDENTIFIER = 'symfony.noServiceAutowireDuplicate'; public const string NO_SET_CLASS_SERVICE_DUPLICATE = 'symfony.noSetClassServiceDuplicate'; diff --git a/src/Enum/SymfonyClass.php b/src/Enum/SymfonyClass.php index 2fbfe6fef..78b657f3b 100644 --- a/src/Enum/SymfonyClass.php +++ b/src/Enum/SymfonyClass.php @@ -41,6 +41,4 @@ final class SymfonyClass public const string CONTAINER_CONFIGURATOR = 'Symfony\Component\DependencyInjection\Loader\Configurator\ContainerConfigurator'; public const string IS_GRANTED = 'Symfony\Component\Security\Http\Attribute\IsGranted'; - - public const string ATTRIBUTE = 'Symfony\Component\DependencyInjection\Attribute\Autowire'; } diff --git a/src/Enum/SymfonyFunctionName.php b/src/Enum/SymfonyFunctionName.php index bfc5b61c1..dcc302a71 100644 --- a/src/Enum/SymfonyFunctionName.php +++ b/src/Enum/SymfonyFunctionName.php @@ -9,6 +9,4 @@ final class SymfonyFunctionName public const string REF = 'Symfony\Component\DependencyInjection\Loader\Configurator\ref'; public const string SERVICE = 'Symfony\Component\DependencyInjection\Loader\Configurator\service'; - - public const string PARAM = 'Symfony\Component\DependencyInjection\Loader\Configurator\param'; } diff --git a/src/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule.php b/src/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule.php deleted file mode 100644 index 87d4c202e..000000000 --- a/src/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule.php +++ /dev/null @@ -1,182 +0,0 @@ - - * - * @see \Symplify\PHPStanRules\Tests\Rules\Symfony\ConfigClosure\PreferAutowireAttributeOverConfigParamRule\PreferAutowireAttributeOverConfigParamRuleTest - */ -final readonly class PreferAutowireAttributeOverConfigParamRule implements Rule -{ - /** - * @api used in tests - */ - public const string ERROR_MESSAGE = 'Instead of parameter reference in config, add #[Autowire(param: ...)] in the "%s" class constructor'; - - private NodeFinder $nodeFinder; - - public function __construct( - private ReflectionProvider $reflectionProvider, - ) { - $this->nodeFinder = new NodeFinder(); - } - - public function getNodeType(): string - { - return Closure::class; - } - - /** - * @param Closure $node - */ - public function processNode(Node $node, Scope $scope): array - { - // enable this rule only if the autowire attribute is present - if (! $this->reflectionProvider->hasClass(SymfonyClass::ATTRIBUTE)) { - return []; - } - - if (! SymfonyClosureDetector::detect($node)) { - return []; - } - - $methodCalls = $this->nodeFinder->findInstanceOf($node, MethodCall::class); - - $ruleErrors = []; - - foreach ($methodCalls as $methodCall) { - if ($methodCall->isFirstClassCallable()) { - continue; - } - - if (! NamingHelper::isNames($methodCall->name, ['arg', 'args'])) { - continue; - } - - // find param() func call or string with '%' in it - if (! $this->hasPossibleParameterInject($methodCall)) { - continue; - } - - // find out parent class! if in /vendor, let's skip it - $serviceClassName = $this->resolveRegisteredServiceClassName($methodCall, $scope); - if (! is_string($serviceClassName)) { - continue; - } - - // let's skip, as /vendor service that cannot be edited - if ($this->isVendorClass($serviceClassName)) { - continue; - } - - $errorMessage = sprintf(self::ERROR_MESSAGE, $serviceClassName); - - $identifierRuleError = RuleErrorBuilder::message($errorMessage) - ->identifier(SymfonyRuleIdentifier::PREFER_AUTOWIRE_ATTRIBUTE_OVER_CONFIG_PARAM) - ->line($methodCall->getStartLine()) - ->build(); - - $ruleErrors[] = $identifierRuleError; - } - - return $ruleErrors; - } - - private function hasPossibleParameterInject(MethodCall $methodCall): bool - { - return array_any($methodCall->getArgs(), fn (Arg $arg): bool => $this->isParamFuncOrString($arg->value)); - } - - private function isParamFuncOrString(Expr $expr): bool - { - $nodeFinder = new NodeFinder(); - - /** @var FuncCall[] $funcCalls */ - $funcCalls = $nodeFinder->findInstanceOf($expr, FuncCall::class); - - foreach ($funcCalls as $funcCall) { - if (NamingHelper::isName($funcCall->name, SymfonyFunctionName::PARAM)) { - return true; - } - } - - /** @var String_[] $strings */ - $strings = $nodeFinder->findInstanceOf($expr, String_::class); - foreach ($strings as $string) { - if (str_starts_with($string->value, '%')) { - return true; - } - } - - return false; - } - - private function isVendorClass(string $className): bool - { - if (! $this->reflectionProvider->hasClass($className)) { - return false; - } - - $serviceClassReflection = $this->reflectionProvider->getClass($className); - return str_contains((string) $serviceClassReflection->getFileName(), '/vendor/'); - } - - private function resolveClassNameFromServiceSetMethodCall(MethodCall $setMethodCall, Scope $scope): ?string - { - $serviceSetMethodCallArgs = $setMethodCall->getArgs(); - - // two params? then service class is the 2nd arg - $serviceArg = $serviceSetMethodCallArgs[1] ?? $serviceSetMethodCallArgs[0]; - $serviceClassNameType = $scope->getType($serviceArg->value); - - if (! $serviceClassNameType instanceof ConstantStringType) { - return null; - } - - $serviceClassName = $serviceClassNameType->getValue(); - - // probably only another service override - if (! $this->reflectionProvider->hasClass($serviceClassName)) { - return null; - } - - return $serviceClassName; - } - - private function resolveRegisteredServiceClassName(MethodCall $methodCall, Scope $scope): ?string - { - $currentMethodCall = $methodCall; - - while ($currentMethodCall instanceof MethodCall) { - if (NamingHelper::isName($currentMethodCall->name, 'set')) { - return $this->resolveClassNameFromServiceSetMethodCall($currentMethodCall, $scope); - } - - $currentMethodCall = $currentMethodCall->var; - } - - return null; - } -} diff --git a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/ParameterPercentReference.php b/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/ParameterPercentReference.php deleted file mode 100644 index b4d94e68a..000000000 --- a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/ParameterPercentReference.php +++ /dev/null @@ -1,13 +0,0 @@ -services(); - - $services->set(SomeSetServiceWithConstructor::class) - ->arg('$key', '%parameter_name%'); -}; diff --git a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SkipNoParameterReference.php b/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SkipNoParameterReference.php deleted file mode 100644 index d28d055bd..000000000 --- a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SkipNoParameterReference.php +++ /dev/null @@ -1,13 +0,0 @@ -services(); - - $services->set(SomeSetServiceWithConstructor::class) - ->call('some', ['%parameter_name%']); -}; diff --git a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SomeConfigWithInvalidSet.php b/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SomeConfigWithInvalidSet.php deleted file mode 100644 index e049508b4..000000000 --- a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Fixture/SomeConfigWithInvalidSet.php +++ /dev/null @@ -1,14 +0,0 @@ -services(); - - $services->set(SomeSetServiceWithConstructor::class) - ->arg('$key', param('parameter_name')); -}; diff --git a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/PreferAutowireAttributeOverConfigParamRuleTest.php b/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/PreferAutowireAttributeOverConfigParamRuleTest.php deleted file mode 100644 index bfd55e7c2..000000000 --- a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/PreferAutowireAttributeOverConfigParamRuleTest.php +++ /dev/null @@ -1,61 +0,0 @@ -> $expectedErrorMessagesWithLines - */ - #[DataProvider('provideData')] - public function testRule(string $filePath, array $expectedErrorMessagesWithLines): void - { - $this->analyse([$filePath], $expectedErrorMessagesWithLines); - } - - /** - * @return Iterator, mixed>> - */ - public static function provideData(): Iterator - { - yield [__DIR__ . '/Fixture/SomeConfigWithInvalidSet.php', [ - [ - sprintf(PreferAutowireAttributeOverConfigParamRule::ERROR_MESSAGE, SomeSetServiceWithConstructor::class), - 12, - ], - ]]; - - yield [__DIR__ . '/Fixture/ParameterPercentReference.php', [ - [ - sprintf(PreferAutowireAttributeOverConfigParamRule::ERROR_MESSAGE, SomeSetServiceWithConstructor::class), - 11, - ], - ]]; - - yield [__DIR__ . '/Fixture/SkipNoParameterReference.php', []]; - } - - /** - * @return array - */ - #[Override] - public static function getAdditionalConfigFiles(): array - { - return [__DIR__ . '/config/configured_rule.neon']; - } - - protected function getRule(): Rule - { - return self::getContainer()->getByType(PreferAutowireAttributeOverConfigParamRule::class); - } -} diff --git a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Source/SomeSetServiceWithConstructor.php b/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Source/SomeSetServiceWithConstructor.php deleted file mode 100644 index dd7fa84ad..000000000 --- a/tests/Rules/Symfony/ConfigClosure/PreferAutowireAttributeOverConfigParamRule/Source/SomeSetServiceWithConstructor.php +++ /dev/null @@ -1,10 +0,0 @@ -