From 5a85d497ab5346d590c6720b22df11f02f73bd98 Mon Sep 17 00:00:00 2001 From: Pol Dellaiera Date: Tue, 16 Nov 2021 15:12:31 +0100 Subject: [PATCH] fix: SA: Fix "...Switch condition type..." rule. Also fix a critical bug in `ReportList.php`. SA stands for Static Analysis. --- phpstan-baseline.neon | 40 ------ .../Export/Export/ReportList.php | 133 ++++++------------ 2 files changed, 46 insertions(+), 127 deletions(-) diff --git a/phpstan-baseline.neon b/phpstan-baseline.neon index 457367a5c..cab6a5eeb 100644 --- a/phpstan-baseline.neon +++ b/phpstan-baseline.neon @@ -160,46 +160,6 @@ parameters: count: 1 path: src/Bundle/ChillPersonBundle/Repository/SocialWork/ResultRepository.php - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'person_address_country_name' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'person_birthdate' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'person_countryOfBirth' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'person_gender' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'person_nationality' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'report_date' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'report_scope' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - - - message: "#^Switch condition type \\(Chill\\\\ReportBundle\\\\Export\\\\Export\\\\type\\) does not match case condition 'report_user' \\(string\\)\\.$#" - count: 1 - path: src/Bundle/ChillReportBundle/Export/Export/ReportList.php - - message: "#^Chill\\\\TaskBundle\\\\Entity\\\\RecurringTask\\:\\:__construct\\(\\) does not call parent constructor from Chill\\\\TaskBundle\\\\Entity\\\\AbstractTask\\.$#" count: 1 diff --git a/src/Bundle/ChillReportBundle/Export/Export/ReportList.php b/src/Bundle/ChillReportBundle/Export/Export/ReportList.php index 9524f860a..432cb8e3f 100644 --- a/src/Bundle/ChillReportBundle/Export/Export/ReportList.php +++ b/src/Bundle/ChillReportBundle/Export/Export/ReportList.php @@ -1,7 +1,7 @@ - */ class ReportList implements ListInterface, ExportElementValidatedInterface { - /** - * - * @var CustomFieldsGroup - */ - protected $customfieldsGroup; + protected CustomFieldsGroup $customfieldsGroup; - /** - * - * @var TranslatableStringHelper - */ - protected $translatableStringHelper; + protected TranslatableStringHelper $translatableStringHelper; - /** - * - * @var TranslatorInterface - */ - protected $translator; + protected TranslatorInterface $translator; - /** - * - * @var CustomFieldProvider - */ - protected $customFieldProvider; + protected CustomFieldProvider $customFieldProvider; - protected $em; + protected EntityManagerInterface $em; - protected $fields = array( + protected array $fields = [ 'person_id', 'person_firstName', 'person_lastName', 'person_birthdate', 'person_placeOfBirth', 'person_gender', 'person_memo', 'person_email', 'person_phonenumber', 'person_countryOfBirth', 'person_nationality', 'person_address_street_address_1', 'person_address_street_address_2', 'person_address_valid_from', 'person_address_postcode_label', 'person_address_postcode_code', 'person_address_country_name', 'person_address_country_code', 'report_id', 'report_user', 'report_date', 'report_scope' - ); + ]; - protected $slugs = []; + protected array $slugs = []; function __construct( CustomFieldsGroup $customfieldsGroup, @@ -85,7 +65,7 @@ class ReportList implements ListInterface, ExportElementValidatedInterface } - public function buildForm(\Symfony\Component\Form\FormBuilderInterface $builder) + public function buildForm(FormBuilderInterface $builder) { $choices = array_combine($this->fields, $this->fields); @@ -97,7 +77,7 @@ class ReportList implements ListInterface, ExportElementValidatedInterface } // Add a checkbox to select fields - $builder->add('fields', ChoiceType::class, array( + $builder->add('fields', ChoiceType::class, [ 'multiple' => true, 'expanded' => true, 'choices' => $choices, @@ -106,11 +86,11 @@ class ReportList implements ListInterface, ExportElementValidatedInterface // add a 'data-display-target' for address fields if (substr($val, 0, 8) === 'address_') { return ['data-display-target' => 'address_date']; - } else { - return []; } + + return []; }, - 'choice_label' => function($key, $label) { + 'choice_label' => function(string $key, string $label): string { switch (\substr($key, 0, 7)) { case 'person_': return $this->translator->trans(\substr($key, 7, \strlen($key) - 7)). @@ -123,7 +103,7 @@ class ReportList implements ListInterface, ExportElementValidatedInterface ' ('.$this->translator->trans("Report's question").')';; } }, - 'constraints' => [new Callback(array( + 'constraints' => [new Callback([ 'callback' => function($selected, ExecutionContextInterface $context) { if (count($selected) === 0) { $context->buildViolation('You must select at least one element') @@ -131,24 +111,25 @@ class ReportList implements ListInterface, ExportElementValidatedInterface ->addViolation(); } } - ))] - )); + ])] + ]); // add a date field for addresses - $builder->add('address_date', ChillDateType::class, array( + $builder->add('address_date', ChillDateType::class, [ 'label' => "Address valid at this date", 'data' => new \DateTime(), 'required' => false, 'block_name' => 'list_export_form_address_date' - )); + ]); } public function validateForm($data, ExecutionContextInterface $context) { // get the field starting with address_ - $addressFields = array_filter(function($el) { - return substr($el, 0, 8) === 'address_'; - }, $this->fields); + $addressFields = array_filter( + $this->fields, + static fn(string $el): bool => substr($el, 0, 8) === 'address_' + ); // check if there is one field starting with address in data if (count(array_intersect($data['fields'], $addressFields)) > 0) { @@ -177,7 +158,7 @@ class ReportList implements ListInterface, ExportElementValidatedInterface public function getAllowedFormattersTypes() { - return array(FormatterInterface::TYPE_LIST); + return [FormatterInterface::TYPE_LIST]; } public function getDescription() @@ -190,13 +171,6 @@ class ReportList implements ListInterface, ExportElementValidatedInterface ); } - /** - * {@inheritDoc} - * - * @param type $key - * @param array $values - * @param type $data - */ public function getLabels($key, array $values, $data): \Closure { switch ($key) { @@ -284,7 +258,7 @@ class ReportList implements ListInterface, ExportElementValidatedInterface ->getRepository('ChillMainBundle:Country'); // load all countries in a single query - $countryRepository->findBy(array('countryCode' => $values)); + $countryRepository->findBy(['countryCode' => $values]); return function($value) use ($key, $countryRepository) { if ($value === '_header') { return \strtolower($key); } @@ -317,9 +291,9 @@ class ReportList implements ListInterface, ExportElementValidatedInterface return $value; }; - } else { - return $this->getLabelForCustomField($key, $values, $data); } + + return $this->getLabelForCustomField($key, $values, $data); } } @@ -330,7 +304,7 @@ class ReportList implements ListInterface, ExportElementValidatedInterface /* @var $cf CustomField */ $cf = $this->em ->getRepository(CustomField::class) - ->findOneBy(array('slug' => $this->DQLToSlug($key))); + ->findOneBy(['slug' => $this->DQLToSlug($key)]); $cfType = $this->customFieldProvider->getCustomFieldByType($cf->getType()); $defaultFunction = function($value) use ($cf) { @@ -358,19 +332,19 @@ class ReportList implements ListInterface, ExportElementValidatedInterface if ($slugChoice === '_other' and $cfType->isChecked($cf, $choiceSlug, $decoded)) { return $cfType->extractOtherValue($cf, $decoded); - } else { - return $cfType->isChecked($cf, $slugChoice, $decoded); } + + return $cfType->isChecked($cf, $slugChoice, $decoded); }; - } else { - return $defaultFunction; } + + return $defaultFunction; } public function getQueryKeys($data) { - $fields = array(); + $fields = []; foreach ($data['fields'] as $key) { if (in_array($key, $this->fields)) { @@ -382,16 +356,9 @@ class ReportList implements ListInterface, ExportElementValidatedInterface return \array_merge($fields, \array_keys($this->slugs)); } - /** - * clean a slug to be usable by DQL - * - * @param string $slugsanitize - * @param string $type the type of the customfield, if required (currently only for choices) - * @return string - */ private function slugToDQL($slug, $type = "default", array $additionalInfos = []) { - $uid = 'slug_'.\uniqid(); + $uid = 'slug_' . \uniqid('', true); $this->slugs[$uid] = [ 'slug' => $slug, @@ -407,11 +374,6 @@ class ReportList implements ListInterface, ExportElementValidatedInterface return $this->slugs[$cleanedSlug]['slug']; } - /** - * - * @param type $cleanedSlug - * @return an array with keys = 'slug', 'type', 'additionnalInfo' - */ private function extractInfosFromSlug($slug) { return $this->slugs[$slug]; @@ -437,7 +399,7 @@ class ReportList implements ListInterface, ExportElementValidatedInterface return 'report'; } - public function initiateQuery(array $requiredModifiers, array $acl, array $data = array()) + public function initiateQuery(array $requiredModifiers, array $acl, array $data = []) { $centers = array_map(function($el) { return $el['center']; }, $acl); @@ -533,17 +495,14 @@ class ReportList implements ListInterface, ExportElementValidatedInterface } } - $qb - ->from(Report::class, 'report') - ->leftJoin('report.person', 'person') - ->join('person.center', 'center') - ->andWhere($qb->expr()->eq('report.cFGroup', ':cFGroup')) - ->setParameter('cFGroup', $this->customfieldsGroup) - ->andWhere('center IN (:authorized_centers)') - ->setParameter('authorized_centers', $centers); - ; - - return $qb; + return $qb + ->from(Report::class, 'report') + ->leftJoin('report.person', 'person') + ->join('person.center', 'center') + ->andWhere($qb->expr()->eq('report.cFGroup', ':cFGroup')) + ->setParameter('cFGroup', $this->customfieldsGroup) + ->andWhere('center IN (:authorized_centers)') + ->setParameter('authorized_centers', $centers); } public function requiredRole()