From ef7db637be9b85fa00278fc3477ac66abe8eb7d1 Mon Sep 17 00:00:00 2001 From: Vincent Langlet Date: Wed, 10 Jan 2024 11:46:57 +0100 Subject: [PATCH 1/2] Infer type for builder getData --- extension.neon | 5 +++++ stubs/Symfony/Component/Form/AbstractType.stub | 1 + .../Component/Form/FormBuilderInterface.stub | 12 ++++++++++-- .../Form/FormConfigBuilderInterface.stub | 13 +++++++++++++ .../Component/Form/FormConfigInterface.stub | 16 ++++++++++++++++ .../Form/FormTypeExtensionInterface.stub | 1 + .../Component/Form/FormTypeInterface.stub | 1 + tests/Type/Symfony/data/form_data_type.php | 3 +++ 8 files changed, 50 insertions(+), 2 deletions(-) create mode 100644 stubs/Symfony/Component/Form/FormConfigBuilderInterface.stub create mode 100644 stubs/Symfony/Component/Form/FormConfigInterface.stub diff --git a/extension.neon b/extension.neon index af69d8bf..dec41999 100644 --- a/extension.neon +++ b/extension.neon @@ -14,6 +14,9 @@ parameters: featureToggles: skipCheckGenericClasses: - Symfony\Component\Form\AbstractType + - Symfony\Component\Form\FormBuilderInterface + - Symfony\Component\Form\FormConfigBuilderInterface + - Symfony\Component\Form\FormConfigInterface - Symfony\Component\Form\FormInterface - Symfony\Component\Form\FormTypeExtensionInterface - Symfony\Component\Form\FormTypeInterface @@ -47,6 +50,8 @@ parameters: - stubs/Symfony/Component/Form/Exception/TransformationFailedException.stub - stubs/Symfony/Component/Form/DataTransformerInterface.stub - stubs/Symfony/Component/Form/FormBuilderInterface.stub + - stubs/Symfony/Component/Form/FormConfigBuilderInterface.stub + - stubs/Symfony/Component/Form/FormConfigInterface.stub - stubs/Symfony/Component/Form/FormInterface.stub - stubs/Symfony/Component/Form/FormFactoryInterface.stub - stubs/Symfony/Component/Form/FormTypeExtensionInterface.stub diff --git a/stubs/Symfony/Component/Form/AbstractType.stub b/stubs/Symfony/Component/Form/AbstractType.stub index da2b1439..e99b746c 100644 --- a/stubs/Symfony/Component/Form/AbstractType.stub +++ b/stubs/Symfony/Component/Form/AbstractType.stub @@ -11,6 +11,7 @@ abstract class AbstractType implements FormTypeInterface { /** + * @param FormBuilderInterface $builder * @param array $options */ public function buildForm(FormBuilderInterface $builder, array $options): void; diff --git a/stubs/Symfony/Component/Form/FormBuilderInterface.stub b/stubs/Symfony/Component/Form/FormBuilderInterface.stub index 5173bb64..fe578c50 100644 --- a/stubs/Symfony/Component/Form/FormBuilderInterface.stub +++ b/stubs/Symfony/Component/Form/FormBuilderInterface.stub @@ -3,9 +3,17 @@ namespace Symfony\Component\Form; /** - * @extends \Traversable + * @template TData + * + * @extends \Traversable> + * @extends FormConfigBuilderInterface */ -interface FormBuilderInterface extends \Traversable +interface FormBuilderInterface extends \Traversable, \Countable, FormConfigBuilderInterface { + /** + * @return FormInterface + */ + public function getForm(): FormInterface; + } diff --git a/stubs/Symfony/Component/Form/FormConfigBuilderInterface.stub b/stubs/Symfony/Component/Form/FormConfigBuilderInterface.stub new file mode 100644 index 00000000..a167ce43 --- /dev/null +++ b/stubs/Symfony/Component/Form/FormConfigBuilderInterface.stub @@ -0,0 +1,13 @@ + + */ +interface FormConfigBuilderInterface extends FormConfigInterface +{ + +} diff --git a/stubs/Symfony/Component/Form/FormConfigInterface.stub b/stubs/Symfony/Component/Form/FormConfigInterface.stub new file mode 100644 index 00000000..942d467b --- /dev/null +++ b/stubs/Symfony/Component/Form/FormConfigInterface.stub @@ -0,0 +1,16 @@ + $builder * @param array $options */ public function buildForm(FormBuilderInterface $builder, array $options): void; diff --git a/stubs/Symfony/Component/Form/FormTypeInterface.stub b/stubs/Symfony/Component/Form/FormTypeInterface.stub index 2f745283..8536656a 100644 --- a/stubs/Symfony/Component/Form/FormTypeInterface.stub +++ b/stubs/Symfony/Component/Form/FormTypeInterface.stub @@ -8,6 +8,7 @@ namespace Symfony\Component\Form; interface FormTypeInterface { /** + * @param FormBuilderInterface $builder * @param array $options */ public function buildForm(FormBuilderInterface $builder, array $options): void; diff --git a/tests/Type/Symfony/data/form_data_type.php b/tests/Type/Symfony/data/form_data_type.php index 524a5b7c..e0cefb88 100644 --- a/tests/Type/Symfony/data/form_data_type.php +++ b/tests/Type/Symfony/data/form_data_type.php @@ -29,6 +29,9 @@ class DataClassType extends AbstractType public function buildForm(FormBuilderInterface $builder, array $options): void { + assertType('GenericFormDataType\DataClass|null', $builder->getData()); + assertType('GenericFormDataType\DataClass|null', $builder->getForm()->getData()); + $builder ->add('foo', NumberType::class) ->add('bar', TextType::class) From d8a0bc03a68d95288b6471c37d435647fbdaff1a Mon Sep 17 00:00:00 2001 From: Robert Meijers Date: Thu, 29 Feb 2024 14:44:49 +0100 Subject: [PATCH 2/2] Narrow type for Extension::getConfiguration if class exists The base extension class automatically creates a Configuration instance when a Configuration class exists in the namespace of the extension. But PHPStan obviously doesn't understand this behaviour and always assumes that `getConfiguration()` returns `ConfigurationInterface|null` meaning that the default pattern to get and parse the configuration reports an error. I.e.: ```php namespace Foo; class SomeExtension extends Extension { public function load(array $configs, ContainerBuilder $container): void { $configuration = $this->getConfiguration($configs, $container); $config = $this->processConfiguration($configuration, $configs); } } ``` results in an error because `processConfiguration()` doesn't accept `ConfigurationInterface|null`. But when a `Configuration` class exists in the same namespace as the `Extension` class (so `Foo\Extension`) an instance of it is returned. This `DynamicReturnTypeExtension` overrides the return type of `Extension::getConfiguration()` so it automatically narrows the return type in case `getConfiguration()` is not overriden and a `Configuration` class exists. So that in the given example `getConfiguration()` doesn't return `ConfigurationInterface|null` anymore but `Foo\Configuration` and there is no error on calling `processConfiguration()`. --- extension.neon | 5 + ...ionGetConfigurationReturnTypeExtension.php | 105 ++++++++++++++++++ tests/Type/Symfony/ExtensionTest.php | 9 ++ .../anonymous/AnonymousExtension.php | 17 +++ .../IgnoreImplementedExtension.php | 23 ++++ .../multiple-types/MultipleTypes.php | 18 +++ .../Configuration.php | 16 +++ ...WithConstructorOptionalParamsExtension.php | 17 +++ .../Configuration.php | 16 +++ ...WithConstructorRequiredParamsExtension.php | 17 +++ .../Configuration.php | 16 +++ ...hConfigurationWithConstructorExtension.php | 17 +++ .../with-configuration/Configuration.php | 12 ++ .../WithConfigurationExtension.php | 17 +++ .../WithoutConfigurationExtension.php | 17 +++ 15 files changed, 322 insertions(+) create mode 100644 src/Type/Symfony/ExtensionGetConfigurationReturnTypeExtension.php create mode 100644 tests/Type/Symfony/data/extension/anonymous/AnonymousExtension.php create mode 100644 tests/Type/Symfony/data/extension/ignore-implemented/IgnoreImplementedExtension.php create mode 100644 tests/Type/Symfony/data/extension/multiple-types/MultipleTypes.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration-with-constructor-optional-params/Configuration.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration-with-constructor-optional-params/WithConfigurationWithConstructorOptionalParamsExtension.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration-with-constructor-required-params/Configuration.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration-with-constructor-required-params/WithConfigurationWithConstructorRequiredParamsExtension.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration-with-constructor/Configuration.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration-with-constructor/WithConfigurationWithConstructorExtension.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration/Configuration.php create mode 100644 tests/Type/Symfony/data/extension/with-configuration/WithConfigurationExtension.php create mode 100644 tests/Type/Symfony/data/extension/without-configuration/WithoutConfigurationExtension.php diff --git a/extension.neon b/extension.neon index dec41999..531eb7df 100644 --- a/extension.neon +++ b/extension.neon @@ -346,3 +346,8 @@ services: - factory: PHPStan\Type\Symfony\CacheInterfaceGetDynamicReturnTypeExtension tags: [phpstan.broker.dynamicMethodReturnTypeExtension] + + # Extension::getConfiguration() return type + - + factory: PHPStan\Type\Symfony\ExtensionGetConfigurationReturnTypeExtension + tags: [phpstan.broker.dynamicMethodReturnTypeExtension] diff --git a/src/Type/Symfony/ExtensionGetConfigurationReturnTypeExtension.php b/src/Type/Symfony/ExtensionGetConfigurationReturnTypeExtension.php new file mode 100644 index 00000000..7af50773 --- /dev/null +++ b/src/Type/Symfony/ExtensionGetConfigurationReturnTypeExtension.php @@ -0,0 +1,105 @@ +reflectionProvider = $reflectionProvider; + } + + public function getClass(): string + { + return 'Symfony\Component\DependencyInjection\Extension\Extension'; + } + + public function isMethodSupported(MethodReflection $methodReflection): bool + { + return $methodReflection->getName() === 'getConfiguration' + && $methodReflection->getDeclaringClass()->getName() === 'Symfony\Component\DependencyInjection\Extension\Extension'; + } + + public function getTypeFromMethodCall( + MethodReflection $methodReflection, + MethodCall $methodCall, + Scope $scope + ): ?Type + { + $types = []; + $extensionType = $scope->getType($methodCall->var); + $classes = $extensionType->getObjectClassNames(); + + foreach ($classes as $extensionName) { + if (str_contains($extensionName, "\0")) { + $types[] = new NullType(); + continue; + } + + $lastBackslash = strrpos($extensionName, '\\'); + if ($lastBackslash === false) { + $types[] = new NullType(); + continue; + } + + $configurationName = substr_replace($extensionName, '\Configuration', $lastBackslash); + if (!$this->reflectionProvider->hasClass($configurationName)) { + $types[] = new NullType(); + continue; + } + + $reflection = $this->reflectionProvider->getClass($configurationName); + if ($this->hasRequiredConstructor($reflection)) { + $types[] = new NullType(); + continue; + } + + $types[] = new ObjectType($configurationName); + } + + return TypeCombinator::union(...$types); + } + + private function hasRequiredConstructor(ClassReflection $class): bool + { + if (!$class->hasConstructor()) { + return false; + } + + $constructor = $class->getConstructor(); + foreach ($constructor->getVariants() as $variant) { + $anyRequired = false; + foreach ($variant->getParameters() as $parameter) { + if (!$parameter->isOptional()) { + $anyRequired = true; + break; + } + } + + if (!$anyRequired) { + return false; + } + } + + return true; + } + +} diff --git a/tests/Type/Symfony/ExtensionTest.php b/tests/Type/Symfony/ExtensionTest.php index 879abb5d..73eb49b7 100644 --- a/tests/Type/Symfony/ExtensionTest.php +++ b/tests/Type/Symfony/ExtensionTest.php @@ -57,6 +57,15 @@ public function dataFileAsserts(): iterable yield from $this->gatherAssertTypes(__DIR__ . '/data/FormInterface_getErrors.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/cache.php'); yield from $this->gatherAssertTypes(__DIR__ . '/data/form_data_type.php'); + + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/with-configuration/WithConfigurationExtension.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/without-configuration/WithoutConfigurationExtension.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/anonymous/AnonymousExtension.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/ignore-implemented/IgnoreImplementedExtension.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/multiple-types/MultipleTypes.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/with-configuration-with-constructor/WithConfigurationWithConstructorExtension.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/with-configuration-with-constructor-optional-params/WithConfigurationWithConstructorOptionalParamsExtension.php'); + yield from $this->gatherAssertTypes(__DIR__ . '/data/extension/with-configuration-with-constructor-required-params/WithConfigurationWithConstructorRequiredParamsExtension.php'); } /** diff --git a/tests/Type/Symfony/data/extension/anonymous/AnonymousExtension.php b/tests/Type/Symfony/data/extension/anonymous/AnonymousExtension.php new file mode 100644 index 00000000..1e33cb37 --- /dev/null +++ b/tests/Type/Symfony/data/extension/anonymous/AnonymousExtension.php @@ -0,0 +1,17 @@ +getConfiguration($configs, $container) + ); + } +}; diff --git a/tests/Type/Symfony/data/extension/ignore-implemented/IgnoreImplementedExtension.php b/tests/Type/Symfony/data/extension/ignore-implemented/IgnoreImplementedExtension.php new file mode 100644 index 00000000..614431f2 --- /dev/null +++ b/tests/Type/Symfony/data/extension/ignore-implemented/IgnoreImplementedExtension.php @@ -0,0 +1,23 @@ +getConfiguration($configs, $container) + ); + } + + public function getConfiguration(array $config, ContainerBuilder $container): ?ConfigurationInterface + { + return null; + } +} diff --git a/tests/Type/Symfony/data/extension/multiple-types/MultipleTypes.php b/tests/Type/Symfony/data/extension/multiple-types/MultipleTypes.php new file mode 100644 index 00000000..77c44003 --- /dev/null +++ b/tests/Type/Symfony/data/extension/multiple-types/MultipleTypes.php @@ -0,0 +1,18 @@ +getConfiguration($configs, $container) + ); +} diff --git a/tests/Type/Symfony/data/extension/with-configuration-with-constructor-optional-params/Configuration.php b/tests/Type/Symfony/data/extension/with-configuration-with-constructor-optional-params/Configuration.php new file mode 100644 index 00000000..4ff16c39 --- /dev/null +++ b/tests/Type/Symfony/data/extension/with-configuration-with-constructor-optional-params/Configuration.php @@ -0,0 +1,16 @@ +getConfiguration($configs, $container) + ); + } +} diff --git a/tests/Type/Symfony/data/extension/with-configuration-with-constructor-required-params/Configuration.php b/tests/Type/Symfony/data/extension/with-configuration-with-constructor-required-params/Configuration.php new file mode 100644 index 00000000..b9d5bcc1 --- /dev/null +++ b/tests/Type/Symfony/data/extension/with-configuration-with-constructor-required-params/Configuration.php @@ -0,0 +1,16 @@ +getConfiguration($configs, $container) + ); + } +} diff --git a/tests/Type/Symfony/data/extension/with-configuration-with-constructor/Configuration.php b/tests/Type/Symfony/data/extension/with-configuration-with-constructor/Configuration.php new file mode 100644 index 00000000..8eea9eb9 --- /dev/null +++ b/tests/Type/Symfony/data/extension/with-configuration-with-constructor/Configuration.php @@ -0,0 +1,16 @@ +getConfiguration($configs, $container) + ); + } +} diff --git a/tests/Type/Symfony/data/extension/with-configuration/Configuration.php b/tests/Type/Symfony/data/extension/with-configuration/Configuration.php new file mode 100644 index 00000000..4e8c51b5 --- /dev/null +++ b/tests/Type/Symfony/data/extension/with-configuration/Configuration.php @@ -0,0 +1,12 @@ +getConfiguration($configs, $container) + ); + } +} diff --git a/tests/Type/Symfony/data/extension/without-configuration/WithoutConfigurationExtension.php b/tests/Type/Symfony/data/extension/without-configuration/WithoutConfigurationExtension.php new file mode 100644 index 00000000..dccec3e2 --- /dev/null +++ b/tests/Type/Symfony/data/extension/without-configuration/WithoutConfigurationExtension.php @@ -0,0 +1,17 @@ +getConfiguration($configs, $container) + ); + } +}