From 05ff6604927f1045d1051e9baaab5a5313a4f903 Mon Sep 17 00:00:00 2001 From: Linus Metzler Date: Thu, 5 Oct 2023 17:00:26 +0200 Subject: [PATCH] rector, PHPStan etc. --- composer.json | 9 ++++- phpstan-baseline.neon | 5 +++ phpstan.neon | 7 ++++ rector.php | 14 ++++++- src/Controller/FormController.php | 11 ------ .../Compiler/ExtensionCompilerPass.php | 4 +- .../Compiler/TransformerCompilerPass.php | 2 +- src/DependencyInjection/Configuration.php | 6 +-- .../ValanticPimcoreFormsExtension.php | 6 +-- src/Document/Areabrick/Form.php | 8 ++-- src/Document/Twig/Extension/Form.php | 8 ++-- src/Form/Builder.php | 14 ++----- src/Form/Extension/FormTypeExtension.php | 2 +- src/Form/FormErrorNormalizer.php | 16 ++------ .../InputHandler/AbstractInputHandler.php | 2 +- src/Form/Output/AssetOutput.php | 2 +- src/Form/Output/DataObjectOutput.php | 2 +- src/Form/Output/OutputScratchpad.php | 3 -- .../OverwriteAbstractTransformerTrait.php | 6 +-- src/Model/OutputResponse.php | 4 -- src/Repository/ConfigurationRepository.php | 13 +++---- src/Service/FormService.php | 37 ++++--------------- 22 files changed, 75 insertions(+), 106 deletions(-) diff --git a/composer.json b/composer.json index 460d5dd..13bcd0d 100644 --- a/composer.json +++ b/composer.json @@ -14,9 +14,12 @@ }, "require-dev": { "bamarni/composer-bin-plugin": "^1.8.2", + "phpstan/extension-installer": "^1.3.1", "phpstan/phpstan": "^1.10.37", "phpstan/phpstan-deprecation-rules": "^1.1.4", - "phpstan/extension-installer": "^1.3.1" + "phpstan/phpstan-strict-rules": "^1.5.1", + "rector/rector": "^0.18.5", + "roave/security-advisories": "dev-latest" }, "autoload": { "psr-4": { @@ -62,9 +65,13 @@ ], "php-cs-fixer-check": [ "vendor-bin/phpcs/vendor/bin/php-cs-fixer fix --diff --dry-run" + ], + "rector": [ + "./vendor/bin/rector process src" ] }, "config": { + "sort-packages": true, "allow-plugins": { "ocramius/package-versions": true, "bamarni/composer-bin-plugin": true, diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 516f45f..772339d 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -90,6 +90,11 @@ parameters: count: 1 path: src/Form/Transformer/FileTransformer.php + - + message: "#^Return type \\(string\\|null\\) of method Valantic\\\\PimcoreFormsBundle\\\\Model\\\\AbstractMessage\\:\\:key\\(\\) should be covariant with return type \\(string\\) of method Iterator\\\\:\\:key\\(\\)$#" + count: 1 + path: src/Model/AbstractMessage.php + - message: "#^Method Valantic\\\\PimcoreFormsBundle\\\\Service\\\\FormService\\:\\:errors\\(\\) should return array but returns array\\|ArrayObject\\|bool\\|float\\|int\\|string\\|null\\.$#" count: 1 diff --git a/phpstan.neon b/phpstan.neon index 2348d01..833ad6c 100644 --- a/phpstan.neon +++ b/phpstan.neon @@ -10,3 +10,10 @@ parameters: bootstrapFiles: - vendor/pimcore/pimcore/stubs/dynamic-constants.php + + treatPhpDocTypesAsCertain: false + + strictRules: + booleansInConditions: false + disallowedConstructs: false + noVariableVariables: false diff --git a/rector.php b/rector.php index 5b5ac1d..04ef83f 100644 --- a/rector.php +++ b/rector.php @@ -2,12 +2,15 @@ declare(strict_types=1); +use Rector\CodeQuality\Rector\Empty_\SimplifyEmptyCheckOnEmptyArrayRector; +use Rector\CodeQuality\Rector\Isset_\IssetOnPropertyObjectToPropertyExistsRector; use Rector\CodingStyle\Rector\FuncCall\CountArrayToEmptyArrayComparisonRector; use Rector\Config\RectorConfig; use Rector\Php55\Rector\String_\StringClassNameToClassConstantRector; -use Rector\Php80\Rector\FunctionLike\MixedTypeRector; +use Rector\Php81\Rector\FuncCall\NullToStrictStringFuncCallArgRector; use Rector\Set\ValueObject\LevelSetList; use Rector\Set\ValueObject\SetList; +use Rector\Strict\Rector\Empty_\DisallowedEmptyRuleFixerRector; return static function (RectorConfig $rectorConfig): void { $rectorConfig->paths([ @@ -15,6 +18,15 @@ ]); $rectorConfig->skip([ + SimplifyEmptyCheckOnEmptyArrayRector::class, + DisallowedEmptyRuleFixerRector::class, + NullToStrictStringFuncCallArgRector::class, + StringClassNameToClassConstantRector::class => [ + 'src/DependencyInjection/Compiler/ExtensionCompilerPass.php', + 'src/DependencyInjection/Compiler/TransformerCompilerPass.php', + ], + IssetOnPropertyObjectToPropertyExistsRector::class, + CountArrayToEmptyArrayComparisonRector::class ]); $rectorConfig->sets([ diff --git a/src/Controller/FormController.php b/src/Controller/FormController.php index 4d236c9..449ed0e 100644 --- a/src/Controller/FormController.php +++ b/src/Controller/FormController.php @@ -21,9 +21,6 @@ class FormController extends AbstractController /** * @Route("/ui/{name}") * - * @param string $name - * @param FormService $formService - * * @return Response */ public function uiAction(string $name, FormService $formService): Response @@ -36,9 +33,6 @@ public function uiAction(string $name, FormService $formService): Response /** * @Route("/html/{name}") * - * @param string $name - * @param FormService $formService - * * @return Response */ public function htmlAction(string $name, FormService $formService): Response @@ -51,11 +45,6 @@ public function htmlAction(string $name, FormService $formService): Response /** * @Route("/api/{name}") * - * @param string $name - * @param FormService $formService - * @param Request $request - * @param TranslatorInterface $translator - * * @throws SerializerException * * @return ApiResponse diff --git a/src/DependencyInjection/Compiler/ExtensionCompilerPass.php b/src/DependencyInjection/Compiler/ExtensionCompilerPass.php index 59af547..9763347 100644 --- a/src/DependencyInjection/Compiler/ExtensionCompilerPass.php +++ b/src/DependencyInjection/Compiler/ExtensionCompilerPass.php @@ -22,7 +22,7 @@ */ class ExtensionCompilerPass implements CompilerPassInterface { - public const EXTENSION_TAG = 'liform.extension'; + final public const EXTENSION_TAG = 'liform.extension'; /** * {@inheritDoc} @@ -35,7 +35,7 @@ public function process(ContainerBuilder $container): void $liform = $container->getDefinition('Limenius\Liform\Liform'); - foreach ($container->findTaggedServiceIds(self::EXTENSION_TAG) as $id => $attributes) { + foreach (array_keys($container->findTaggedServiceIds(self::EXTENSION_TAG)) as $id) { $extension = $container->getDefinition($id); $extensionClass = $extension->getClass(); diff --git a/src/DependencyInjection/Compiler/TransformerCompilerPass.php b/src/DependencyInjection/Compiler/TransformerCompilerPass.php index 14d0eff..4552856 100644 --- a/src/DependencyInjection/Compiler/TransformerCompilerPass.php +++ b/src/DependencyInjection/Compiler/TransformerCompilerPass.php @@ -23,7 +23,7 @@ */ class TransformerCompilerPass implements CompilerPassInterface { - public const TRANSFORMER_TAG = 'liform.transformer'; + final public const TRANSFORMER_TAG = 'liform.transformer'; /** * {@inheritDoc} diff --git a/src/DependencyInjection/Configuration.php b/src/DependencyInjection/Configuration.php index 23414d3..f86bc4d 100644 --- a/src/DependencyInjection/Configuration.php +++ b/src/DependencyInjection/Configuration.php @@ -19,8 +19,8 @@ */ class Configuration implements ConfigurationInterface { - public const SYMFONY_CONSTRAINTS_NAMESPACE = 'Symfony\\Component\\Validator\\Constraints\\'; - public const SYMFONY_FORMTYPES_NAMESPACE = 'Symfony\\Component\\Form\\Extension\\Core\\Type\\'; + final public const SYMFONY_CONSTRAINTS_NAMESPACE = 'Symfony\\Component\\Validator\\Constraints\\'; + final public const SYMFONY_FORMTYPES_NAMESPACE = 'Symfony\\Component\\Form\\Extension\\Core\\Type\\'; /** * {@inheritDoc} @@ -115,7 +115,7 @@ protected function buildFieldsNode(): ArrayNodeDefinition ->cannotBeEmpty() ->info('The type of this FormType') ->validate() - ->ifTrue(fn(string $type): bool => !(class_exists($type) || class_exists(self::SYMFONY_FORMTYPES_NAMESPACE . $type))) + ->ifTrue(fn(string $type): bool => !class_exists($type) && !class_exists(self::SYMFONY_FORMTYPES_NAMESPACE . $type)) ->thenInvalid('Invalid type class found. The type should either be a FQN or a subclass of ' . self::SYMFONY_FORMTYPES_NAMESPACE) ->end() ->example('TextType') diff --git a/src/DependencyInjection/ValanticPimcoreFormsExtension.php b/src/DependencyInjection/ValanticPimcoreFormsExtension.php index 33b91a9..08a1a6e 100644 --- a/src/DependencyInjection/ValanticPimcoreFormsExtension.php +++ b/src/DependencyInjection/ValanticPimcoreFormsExtension.php @@ -20,9 +20,9 @@ */ class ValanticPimcoreFormsExtension extends Extension { - public const TAG_OUTPUT = 'valantic.pimcore_forms.output'; - public const TAG_REDIRECT_HANDLER = 'valantic.pimcore_forms.redirect_handler'; - public const TAG_INPUT_HANDLER = 'valantic.pimcore_forms.input_handler'; + final public const TAG_OUTPUT = 'valantic.pimcore_forms.output'; + final public const TAG_REDIRECT_HANDLER = 'valantic.pimcore_forms.redirect_handler'; + final public const TAG_INPUT_HANDLER = 'valantic.pimcore_forms.input_handler'; /** * {@inheritDoc} diff --git a/src/Document/Areabrick/Form.php b/src/Document/Areabrick/Form.php index 1024123..bf8859c 100644 --- a/src/Document/Areabrick/Form.php +++ b/src/Document/Areabrick/Form.php @@ -13,11 +13,9 @@ class Form extends AbstractTemplateAreabrick implements EditableDialogBoxInterface { - protected ConfigurationRepository $configurationRepository; - - public function __construct(ConfigurationRepository $configurationRepository) - { - $this->configurationRepository = $configurationRepository; + public function __construct( + protected ConfigurationRepository $configurationRepository + ) { } public function getTemplateLocation(): string diff --git a/src/Document/Twig/Extension/Form.php b/src/Document/Twig/Extension/Form.php index b9517ae..0a01fe3 100644 --- a/src/Document/Twig/Extension/Form.php +++ b/src/Document/Twig/Extension/Form.php @@ -11,11 +11,9 @@ class Form extends AbstractExtension { - protected FormService $formService; - - public function __construct(FormService $formService) - { - $this->formService = $formService; + public function __construct( + protected FormService $formService + ) { } /** diff --git a/src/Form/Builder.php b/src/Form/Builder.php index 75f002f..aee7824 100644 --- a/src/Form/Builder.php +++ b/src/Form/Builder.php @@ -19,22 +19,14 @@ class Builder { - protected ContainerInterface $container; - protected UrlGeneratorInterface $urlGenerator; - protected TranslatorInterface $translator; - public function __construct( - ContainerInterface $container, - UrlGeneratorInterface $urlGenerator, - TranslatorInterface $translator + protected ContainerInterface $container, + protected UrlGeneratorInterface $urlGenerator, + protected TranslatorInterface $translator ) { - $this->container = $container; - $this->urlGenerator = $urlGenerator; - $this->translator = $translator; } /** - * @param string $name * @param array $config * * @return FormBuilderInterface diff --git a/src/Form/Extension/FormTypeExtension.php b/src/Form/Extension/FormTypeExtension.php index ff486d9..7aefb0c 100644 --- a/src/Form/Extension/FormTypeExtension.php +++ b/src/Form/Extension/FormTypeExtension.php @@ -56,7 +56,7 @@ public function apply(FormInterface $form, array $schema): array return $schema; } - $type = get_class($form->getConfig()->getType()->getInnerType()); + $type = $form->getConfig()->getType()->getInnerType()::class; $formType = null; diff --git a/src/Form/FormErrorNormalizer.php b/src/Form/FormErrorNormalizer.php index 0d86f37..8a78980 100644 --- a/src/Form/FormErrorNormalizer.php +++ b/src/Form/FormErrorNormalizer.php @@ -14,13 +14,10 @@ class FormErrorNormalizer implements NormalizerInterface { - protected TranslatorInterface $translator; - protected ConfigurationRepository $configurationRepository; - - public function __construct(TranslatorInterface $translator, ConfigurationRepository $configurationRepository) - { - $this->translator = $translator; - $this->configurationRepository = $configurationRepository; + public function __construct( + protected TranslatorInterface $translator, + protected ConfigurationRepository $configurationRepository + ) { } /** @@ -40,8 +37,6 @@ public function supportsNormalization($data, $format = null): bool } /** - * @param FormInterface $data - * * @return array> * * @see https://github.com/schmittjoh/serializer/blob/master/src/Handler/FormErrorHandler.php @@ -79,7 +74,6 @@ protected function convertFormToArray(FormInterface $data): array } /** - * @param FormError $error * @param string|null $customErrorMessageTemplate * * @return array @@ -112,8 +106,6 @@ protected function buildErrorEntry(FormError $error, ?string $customErrorMessage } /** - * @param FormError $error - * * @return string|null * * @see https://github.com/schmittjoh/serializer/blob/master/src/Handler/FormErrorHandler.php diff --git a/src/Form/InputHandler/AbstractInputHandler.php b/src/Form/InputHandler/AbstractInputHandler.php index 865a640..e00c61a 100644 --- a/src/Form/InputHandler/AbstractInputHandler.php +++ b/src/Form/InputHandler/AbstractInputHandler.php @@ -10,7 +10,7 @@ abstract class AbstractInputHandler implements InputHandlerInterface { protected FormInterface $form; - protected ?Request $request; + protected ?Request $request = null; public function initialize(FormInterface $form, ?Request $request): void { diff --git a/src/Form/Output/AssetOutput.php b/src/Form/Output/AssetOutput.php index 89be990..38cd050 100644 --- a/src/Form/Output/AssetOutput.php +++ b/src/Form/Output/AssetOutput.php @@ -22,7 +22,7 @@ public function handle(OutputResponse $outputResponse): OutputResponse { $path = Folder::getByPath($this->getPath()); - if ($path === null) { + if (!$path instanceof Folder) { throw new \InvalidArgumentException(sprintf('Path %s not found', $this->getPath())); } diff --git a/src/Form/Output/DataObjectOutput.php b/src/Form/Output/DataObjectOutput.php index fd96988..8f1b6e7 100644 --- a/src/Form/Output/DataObjectOutput.php +++ b/src/Form/Output/DataObjectOutput.php @@ -24,7 +24,7 @@ public function handle(OutputResponse $outputResponse): OutputResponse $path = Concrete::getByPath($this->getPath()); - if ($path === null) { + if (!$path instanceof Concrete) { throw new \InvalidArgumentException(sprintf('Path %s not found', $this->getPath())); } diff --git a/src/Form/Output/OutputScratchpad.php b/src/Form/Output/OutputScratchpad.php index 3b322a7..5420d0e 100644 --- a/src/Form/Output/OutputScratchpad.php +++ b/src/Form/Output/OutputScratchpad.php @@ -12,7 +12,6 @@ class OutputScratchpad protected static array $scratchpad = []; /** - * @param string $key * @param array $payload */ public static function set(string $key, array $payload): void @@ -21,8 +20,6 @@ public static function set(string $key, array $payload): void } /** - * @param string $key - * * @return array */ public static function get(string $key): array diff --git a/src/Form/Transformer/OverwriteAbstractTransformerTrait.php b/src/Form/Transformer/OverwriteAbstractTransformerTrait.php index 35faf8f..ce885c8 100644 --- a/src/Form/Transformer/OverwriteAbstractTransformerTrait.php +++ b/src/Form/Transformer/OverwriteAbstractTransformerTrait.php @@ -9,7 +9,6 @@ trait OverwriteAbstractTransformerTrait { /** - * @param FormInterface $form * @param array $schema * * @return array @@ -19,7 +18,9 @@ trait OverwriteAbstractTransformerTrait protected function addLabel(FormInterface $form, array $schema): array { $translationDomain = $form->getConfig()->getOption('translation_domain'); - if ($label = $form->getConfig()->getOption('label')) { + $label = $form->getConfig()->getOption('label'); + + if ($label !== null) { // translation is handled in \Valantic\PimcoreFormsBundle\Form\Builder::getOptions $schema['title'] = $label; } else { @@ -30,7 +31,6 @@ protected function addLabel(FormInterface $form, array $schema): array } /** - * @param FormInterface $form * @param array $schema * * @return array diff --git a/src/Model/OutputResponse.php b/src/Model/OutputResponse.php index 41fc538..48a4a23 100644 --- a/src/Model/OutputResponse.php +++ b/src/Model/OutputResponse.php @@ -37,8 +37,6 @@ public function setMessages(array $messages): self } /** - * @param AbstractMessage $message - * * @return $this */ public function addMessage(AbstractMessage $message): self @@ -61,8 +59,6 @@ public function getOverallStatus(): bool } /** - * @param bool $status - * * @return $this */ public function addStatus(bool $status): self diff --git a/src/Repository/ConfigurationRepository.php b/src/Repository/ConfigurationRepository.php index 71e9f57..88d1aea 100644 --- a/src/Repository/ConfigurationRepository.php +++ b/src/Repository/ConfigurationRepository.php @@ -8,14 +8,13 @@ class ConfigurationRepository { - public const CONTAINER_TAG = 'valantic.picmore_forms.config'; - public const EDITOR_STORAGE_DIRECTORY = PIMCORE_PRIVATE_VAR . '/bundles/valantic-forms'; - public const EDITOR_STORAGE_FILE = self::EDITOR_STORAGE_DIRECTORY . '/forms.yml'; - protected ParameterBagInterface $parameterBag; + final public const CONTAINER_TAG = 'valantic.picmore_forms.config'; + final public const EDITOR_STORAGE_DIRECTORY = PIMCORE_PRIVATE_VAR . '/bundles/valantic-forms'; + final public const EDITOR_STORAGE_FILE = self::EDITOR_STORAGE_DIRECTORY . '/forms.yml'; - public function __construct(ParameterBagInterface $parameterBag) - { - $this->parameterBag = $parameterBag; + public function __construct( + protected ParameterBagInterface $parameterBag + ) { } /** diff --git a/src/Service/FormService.php b/src/Service/FormService.php index 56c42b0..125ceb9 100644 --- a/src/Service/FormService.php +++ b/src/Service/FormService.php @@ -29,23 +29,16 @@ class FormService { - protected ConfigurationRepository $configurationRepository; - protected Builder $builder; protected Liform $liform; - protected OutputRepository $outputRepository; - protected RedirectHandlerRepository $redirectHandlerRepository; - protected FormErrorNormalizer $errorNormalizer; - protected InputHandlerRepository $inputHandlerRepository; - protected RequestStack $requestStack; public function __construct( - ConfigurationRepository $configurationRepository, - OutputRepository $outputRepository, - RedirectHandlerRepository $redirectHandlerRepository, - InputHandlerRepository $inputHandlerRepository, - Builder $builder, + protected ConfigurationRepository $configurationRepository, + protected OutputRepository $outputRepository, + protected RedirectHandlerRepository $redirectHandlerRepository, + protected InputHandlerRepository $inputHandlerRepository, + protected Builder $builder, Liform $liform, - FormErrorNormalizer $errorNormalizer, + protected FormErrorNormalizer $errorNormalizer, FormTypeExtension $formTypeExtension, FormNameExtension $formNameExtension, FormConstraintExtension $formConstraintExtension, @@ -53,15 +46,8 @@ public function __construct( ChoiceTypeExtension $choiceTypeExtension, HiddenTypeExtension $hiddenTypeExtension, FormDataExtension $formDataExtension, - RequestStack $requestStack + protected RequestStack $requestStack ) { - $this->builder = $builder; - $this->configurationRepository = $configurationRepository; - $this->redirectHandlerRepository = $redirectHandlerRepository; - $this->inputHandlerRepository = $inputHandlerRepository; - $this->outputRepository = $outputRepository; - $this->errorNormalizer = $errorNormalizer; - $liform->addExtension($formTypeExtension); $liform->addExtension($formNameExtension); $liform->addExtension($formConstraintExtension); @@ -70,7 +56,6 @@ public function __construct( $liform->addExtension($hiddenTypeExtension); $liform->addExtension($formDataExtension); $this->liform = $liform; - $this->requestStack = $requestStack; } public function build(string $name): FormBuilderInterface @@ -110,8 +95,6 @@ public function build(string $name): FormBuilderInterface } /** - * @param FormInterface $form - * * @return array */ public function json(FormInterface $form): array @@ -123,8 +106,6 @@ public function json(FormInterface $form): array } /** - * @param string $name - * * @return array */ public function buildJson(string $name): array @@ -154,8 +135,6 @@ public function buildForm(string $name): FormInterface * Sample for a valid template string: '(%2$s) %1$s' * Sample result for example in German: '(Dateiupload) Die Datei ist gross (12MB), die maximal zulässige Grösse beträgt 10MB.' * - * @param FormInterface $form - * * @throws SerializerException * * @return array @@ -199,8 +178,6 @@ public function getRedirectUrl(FormInterface $form, bool $success): ?string } /** - * @param string $name - * * @return array */ protected function getConfig(string $name): array