PHP Code Sniffer and PHPStan fixes

CRM/Twingle/Form/Profile.php

@@ -16,8 +16,9 @@
 declare(strict_types = 1);
 
 use CRM_Twingle_ExtensionUtil as E;
-use Civi\Twingle\Exceptions\ProfileException;
 use Civi\Twingle\Exceptions\BaseException;
+use Civi\Twingle\Exceptions\ProfileException;
+use Civi\Twingle\Exceptions\ProfileValidationError;
 
 /**
  * Form controller class
@@ -168,7 +169,7 @@ class CRM_Twingle_Form_Profile extends CRM_Core_Form {
 
     switch ($this->_op) {
       case 'delete':
-        if ($this->profile) {
+        if (isset($this->profile)) {
           CRM_Utils_System::setTitle(E::ts('Delete Twingle API profile <em>%1</em>', [1 => $this->profile->getName()]));
           $this->addButtons([
             [
@@ -185,23 +186,26 @@ class CRM_Twingle_Form_Profile extends CRM_Core_Form {
         // Retrieve the source profile name.
         $source_id = CRM_Utils_Request::retrieve('source_id', 'Int', $this);
         // When copying without a valid profile id, copy the default profile.
-        if (!$source_id) {
+        if (!is_int($source_id)) {
           $this->profile = CRM_Twingle_Profile::createDefaultProfile();
-        } else {
+        }
+        else {
           try {
             $source_profile = CRM_Twingle_Profile::getProfile($source_id);
             $this->profile = $source_profile->copy();
             $this->profile->validate();
-          } catch (ProfileValidationError $e) {
+          }
-            if ($e->getErrorCode() == ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_FAILED) {
+          catch (ProfileValidationError $exception) {
-              Civi::log()->error($e->getLogMessage());
+            if ($exception->getErrorCode() == ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_FAILED) {
+              Civi::log()->error($exception->getLogMessage());
               CRM_Core_Session::setStatus(E::ts('The profile is invalid and cannot be copied.'), E::ts('Error'));
               CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/admin/settings/twingle/profiles', 'reset=1'));
               return;
             }
-          } catch (ProfileException $e) {
+          }
-            if ($e->getErrorCode() == ProfileException::ERROR_CODE_PROFILE_NOT_FOUND) {
+          catch (ProfileException $exception) {
-              Civi::log()->error($e->getLogMessage());
+            if ($exception->getErrorCode() == ProfileException::ERROR_CODE_PROFILE_NOT_FOUND) {
+              Civi::log()->error($exception->getLogMessage());
               CRM_Core_Session::setStatus(E::ts('The profile to be copied could not be found.'), E::ts('Error'));
               CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/admin/settings/twingle/profiles', 'reset=1'));
               return;
@@ -209,7 +213,10 @@ class CRM_Twingle_Form_Profile extends CRM_Core_Form {
           }
           catch (Civi\Core\Exception\DBQueryException $e) {
             Civi::log()->error($e->getMessage());
-            CRM_Core_Session::setStatus(E::ts('A database error has occurred. See the log for details.'), E::ts('Error'));
+            CRM_Core_Session::setStatus(
+              E::ts('A database error has occurred. See the log for details.'),
+              E::ts('Error')
+            );
             CRM_Utils_System::redirect(CRM_Utils_System::url('civicrm/admin/settings/twingle/profiles', 'reset=1'));
             return;
           }
@@ -218,7 +225,9 @@ class CRM_Twingle_Form_Profile extends CRM_Core_Form {
         break;
 
       case 'edit':
-        CRM_Utils_System::setTitle(E::ts('Edit Twingle API profile <em>%1</em>', [1 => $this->profile->getName()]));
+        CRM_Utils_System::setTitle(
+          E::ts('Edit Twingle API profile <em>%1</em>', [1 => $this->profile->getName()])
+        );
         break;
 
       case 'create':
@@ -554,8 +563,8 @@ class CRM_Twingle_Form_Profile extends CRM_Core_Form {
    */
   public function validate() {
 
-    if (in_array($this->_op, ['create', 'edit', 'copy'])) {
+    if (in_array($this->_op, ['create', 'edit', 'copy'], TRUE)) {
-      // Create profile with new values
+      // Create profile with new values.
       $profile_values = $this->exportValues();
       $profile = new CRM_Twingle_Profile(
         $profile_values['name'],
@@ -572,6 +581,7 @@ class CRM_Twingle_Form_Profile extends CRM_Core_Form {
           case ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_FAILED:
             $this->setElementError($e->getAffectedFieldName(), $e->getMessage());
             break;
+
           case ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_WARNING:
             CRM_Core_Session::setStatus($e->getMessage(), E::ts('Warning'));
         }

CRM/Twingle/Profile.php

@@ -17,6 +17,7 @@ declare(strict_types = 1);
 
 use CRM_Twingle_ExtensionUtil as E;
 use Civi\Twingle\Exceptions\ProfileException as ProfileException;
+use Civi\Twingle\Exceptions\ProfileValidationError;
 
 /**
  * Profiles define how incoming submissions from the Twingle API are
@@ -25,16 +26,16 @@ use Civi\Twingle\Exceptions\ProfileException as ProfileException;
 class CRM_Twingle_Profile {
 
   /**
-   * @var int $id
+   * @var int
    *   The id of the profile.
    */
-  protected $id = NULL;
+  protected ?int $id;
 
   /**
-   * @var string $name
+   * @var string
    *   The name of the profile.
    */
-  protected $name;
+  protected string $name;
 
   /**
    * @var array<string, mixed>
@@ -99,7 +100,7 @@ class CRM_Twingle_Profile {
    * @return bool
    */
   public function matches($project_id) {
-    return in_array($project_id, $this->getProjectIds());
+    return in_array($project_id, $this->getProjectIds(), TRUE);
   }
 
   /**
@@ -141,7 +142,7 @@ class CRM_Twingle_Profile {
    *
    * @return int
    */
-  public function getId() {
+  public function getId(): ?int {
     return $this->id;
   }
 
@@ -150,7 +151,7 @@ class CRM_Twingle_Profile {
    *
    * @param int $id
    */
-  public function setId(int $id) {
+  public function setId(int $id): void {
     $this->id = $id;
   }
 
@@ -159,7 +160,7 @@ class CRM_Twingle_Profile {
    *
    * @return string
    */
-  public function getName() {
+  public function getName(): string {
     return $this->name;
   }
 
@@ -177,21 +178,21 @@ class CRM_Twingle_Profile {
    *
    * @return bool
    */
-  public function is_default() {
+  public function is_default(): bool {
-    return $this->name == 'default';
+    return $this->name === 'default';
   }
 
   /**
    * Retrieves the profile's project IDs.
    *
-   * @return array
+   * @return array<string>
    */
   public function getProjectIds(): array {
     return array_map(
       function($project_id) {
         return trim($project_id);
       },
-      explode(',', $this->getAttribute("selector"))
+      explode(',', $this->getAttribute('selector') ?? '')
     );
   }
 
@@ -242,14 +243,14 @@ class CRM_Twingle_Profile {
    * Verifies whether the profile is valid (i.e. consistent and not colliding
    * with other profiles).
    *
-   * @throws \CRM_Twingle_Exceptions_ProfileValidationError
+   * @throws \Civi\Twingle\Exceptions\ProfileValidationError
    * @throws \Civi\Core\Exception\DBQueryException
    *   When the profile could not be successfully validated.
    */
-  public function validate() {
+  public function validate(): void {
 
     // Name cannot be empty
-    if (empty($this->getName())) {
+    if ('' === $this->getName()) {
       throw new ProfileValidationError(
         'name',
         E::ts('Profile name cannot be empty.'),
@@ -258,7 +259,7 @@ class CRM_Twingle_Profile {
     }
 
     // Restrict profile names to alphanumeric characters, space and the underscore.
-    $contains_illegal_characters = preg_match("/[^A-Za-z0-9_\s]/", $this->getName());
+    $contains_illegal_characters = (1 !== preg_match('/[^A-Za-z0-9_\s]/', $this->getName()));
     if ($contains_illegal_characters) {
       throw new ProfileValidationError(
         'name',
@@ -269,10 +270,11 @@ class CRM_Twingle_Profile {
 
     // Check if profile name is already used for other profile
     $profile_name_duplicates = array_filter(
-      CRM_Twingle_Profile::getProfiles(), function($profile) {
+      CRM_Twingle_Profile::getProfiles(),
+      function($profile) {
         return $profile->getName() == $this->getName() && $this->getId() != $profile->getId();
       });
-    if (!empty($profile_name_duplicates)) {
+    if ([] !== $profile_name_duplicates) {
       throw new ProfileValidationError(
         'name',
         E::ts("A profile with the name '%1' already exists.", [1 => $this->getName()]),
@@ -283,17 +285,19 @@ class CRM_Twingle_Profile {
     // Check if project_id is already used in other profile
     $profiles = $this::getProfiles();
     foreach ($profiles as $profile) {
-      if ($profile->getId() == $this->getId() || $profile->is_default()) continue;
+      if ($profile->getId() == $this->getId() || $profile->is_default()) {
+        continue;
+      };
       $project_ids = $this->getProjectIds();
       $id_duplicates = array_intersect($profile->getProjectIds(), $project_ids);
-      if (!empty($id_duplicates)) {
+      if ([] !== $id_duplicates) {
         throw new ProfileValidationError(
           'selector',
           E::ts(
             "Project ID(s) [%1] already used in profile '%2'.",
             [
-              1 => implode(", ", $id_duplicates),
+              1 => implode(', ', $id_duplicates),
-              2 => $profile->getName()
+              2 => $profile->getName(),
             ]
           ),
           ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_WARNING
@@ -302,8 +306,8 @@ class CRM_Twingle_Profile {
     }
 
     // Validate custom field mapping.
-    $custom_field_mapping = $this->getAttribute('custom_field_mapping', FALSE);
+    $custom_field_mapping = $this->getAttribute('custom_field_mapping');
-    if ($custom_field_mapping) {
+    if (is_string($custom_field_mapping)) {
       $custom_field_mapping = preg_split('/\r\n|\r|\n/', $custom_field_mapping, -1, PREG_SPLIT_NO_EMPTY);
       $parsing_error = new ProfileValidationError(
         'custom_field_mapping',
@@ -314,7 +318,7 @@ class CRM_Twingle_Profile {
         throw $parsing_error;
       }
       foreach ($custom_field_mapping as $custom_field_map) {
-        $custom_field_map = explode("=", $custom_field_map);
+        $custom_field_map = explode('=', $custom_field_map);
         if (count($custom_field_map) !== 2) {
           throw $parsing_error;
         }
@@ -323,18 +327,22 @@ class CRM_Twingle_Profile {
 
         // Check for custom field existence
         try {
+          /**
+           * @phpstan-var array<string, mixed> $custom_field
+           */
           $custom_field = civicrm_api3(
             'CustomField', 'getsingle', ['id' => $custom_field_id]
           );
         }
-        catch (CiviCRM_API3_Exception $exception) {
+        catch (CRM_Core_Exception $exception) {
           throw new ProfileValidationError(
             'custom_field_mapping',
             E::ts(
               'Custom field custom_%1 does not exist.',
               [1 => $custom_field_id]
             ),
-            ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_FAILED
+            ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_FAILED,
+            $exception
           );
         }
 
@@ -353,14 +361,16 @@ class CRM_Twingle_Profile {
                 ],
               ],
             ]);
-        } catch (CiviCRM_API3_Exception $exception) {
+        }
+        catch (CRM_Core_Exception $exception) {
           throw new ProfileValidationError(
             'custom_field_mapping',
             E::ts(
               'Custom field custom_%1 is not in a CustomGroup that extends one of the supported CiviCRM entities.',
               [1 => $custom_field['id']]
             ),
-            ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_FAILED
+            ProfileValidationError::ERROR_CODE_PROFILE_VALIDATION_FAILED,
+            $exception
           );
         }
       }
@@ -370,15 +380,14 @@ class CRM_Twingle_Profile {
   /**
    * Persists the profile within the database.
    *
-   * @throws \CRM_Twingle_Exceptions_ProfileException
+   * @throws \Civi\Twingle\Exceptions\ProfileException
    */
-  public function saveProfile() {
+  public function saveProfile(): void {
-
     try {
-      if ($this->id !== NULL) {
+      if (isset($this->id)) {
         // existing profile -> just update the config
         CRM_Core_DAO::executeQuery(
-          "UPDATE civicrm_twingle_profile SET config = %2, name = %3 WHERE id = %1",
+          'UPDATE civicrm_twingle_profile SET config = %2, name = %3 WHERE id = %1',
           [
             1 => [$this->id, 'String'],
             2 => [json_encode($this->data), 'String'],
@@ -388,17 +397,20 @@ class CRM_Twingle_Profile {
       else {
         // new profile -> add new entry to the DB
         CRM_Core_DAO::executeQuery(
-          "INSERT IGNORE INTO civicrm_twingle_profile(name,config,last_access,access_counter) VALUES (%1, %2, null, 0)",
+          <<<SQL
+          INSERT IGNORE INTO civicrm_twingle_profile(name,config,last_access,access_counter) VALUES (%1, %2, null, 0)
+          SQL,
           [
             1 => [$this->name, 'String'],
             2 => [json_encode($this->data), 'String'],
           ]);
       }
     }
-    catch (Exception $e) {
+    catch (Exception $exception) {
       throw new ProfileException(
-        E::ts("Could not save/update profile: %1", [1 => $e->getMessage()]),
+        E::ts('Could not save/update profile: %1', [1 => $exception->getMessage()]),
-        ProfileException::ERROR_CODE_COULD_NOT_SAVE_PROFILE
+        ProfileException::ERROR_CODE_COULD_NOT_SAVE_PROFILE,
+        $exception
       );
     }
   }
@@ -408,7 +420,7 @@ class CRM_Twingle_Profile {
    *
    * @throws \Civi\Twingle\Exceptions\ProfileException
    */
-  public function deleteProfile() {
+  public function deleteProfile(): void {
     // Do only reset default profile
     if ($this->getName() == 'default') {
       try {
@@ -417,31 +429,31 @@ class CRM_Twingle_Profile {
         $default_profile->saveProfile();
 
         // Reset counter
-        CRM_Core_DAO::executeQuery("UPDATE civicrm_twingle_profile SET access_counter = 0, last_access = NULL WHERE id = %1", [
+        CRM_Core_DAO::executeQuery(
-          1 => [
+          'UPDATE civicrm_twingle_profile SET access_counter = 0, last_access = NULL WHERE id = %1',
-            $this->id,
+          [1 => [$this->id, 'Integer']]
-            'Integer'
+        );
-          ]
+      }
-        ]);
+      catch (Exception $exception) {
-      } catch (Exception $e) {
         throw new ProfileException(
-          E::ts("Could not reset default profile: %1", [1 => $e->getMessage()]),
+          E::ts('Could not reset default profile: %1', [1 => $exception->getMessage()]),
-          ProfileException::ERROR_CODE_COULD_NOT_RESET_PROFILE
+          ProfileException::ERROR_CODE_COULD_NOT_RESET_PROFILE,
+          $exception
         );
       }
     }
     else {
       try {
-        CRM_Core_DAO::executeQuery("DELETE FROM civicrm_twingle_profile WHERE id = %1", [
+        CRM_Core_DAO::executeQuery(
-          1 => [
+          'DELETE FROM civicrm_twingle_profile WHERE id = %1',
-            $this->id,
+          [1 => [$this->id, 'Integer']]
-            'Integer'
+        );
-          ]
+      }
-        ]);
+      catch (Exception $exception) {
-      } catch (Exception $e) {
         throw new ProfileException(
-          E::ts("Could not delete profile: %1", [1 => $e->getMessage()]),
+          E::ts('Could not delete profile: %1', [1 => $exception->getMessage()]),
-          ProfileException::ERROR_CODE_COULD_NOT_DELETE_PROFILE
+          ProfileException::ERROR_CODE_COULD_NOT_DELETE_PROFILE,
+          $exception
         );
       }
     }
@@ -620,11 +632,20 @@ class CRM_Twingle_Profile {
    * @throws \Civi\Twingle\Exceptions\ProfileException
    */
   public static function getProfile(int $id = NULL) {
-    if (!empty($id)) {
+    if (isset($id)) {
-      $profile_data = CRM_Core_DAO::executeQuery("SELECT id, name, config FROM civicrm_twingle_profile WHERE id = %1",
+      /**
-        [1 => [$id, 'Integer']]);
+       * @var CRM_Core_DAO $profile_data
+       */
+      $profile_data = CRM_Core_DAO::executeQuery(
+        'SELECT id, name, config FROM civicrm_twingle_profile WHERE id = %1',
+        [1 => [$id, 'Integer']]
+      );
       if ($profile_data->fetch()) {
-        return new CRM_Twingle_Profile($profile_data->name, json_decode($profile_data->config, 1), (int) $profile_data->id);
+        return new CRM_Twingle_Profile(
+          $profile_data->name,
+          json_decode($profile_data->config, TRUE),
+          (int) $profile_data->id
+        );
       }
     }
     throw new ProfileException('Profile not found.', ProfileException::ERROR_CODE_PROFILE_NOT_FOUND);
@@ -634,16 +655,23 @@ class CRM_Twingle_Profile {
    * Retrieves the list of all profiles persisted within the current CiviCRM
    * settings, including the default profile.
    *
-   * @return array<string, \CRM_Twingle_Profile>
+   * @return array<int, \CRM_Twingle_Profile>
-   *   profile_name => CRM_Twingle_Profile
+   *   An array of profiles with profile IDs as keys and profile objects as values.
    * @throws \Civi\Core\Exception\DBQueryException
    */
-  public static function getProfiles() {
+  public static function getProfiles(): array {
     // todo: cache?
     $profiles = [];
-    $profile_data = CRM_Core_DAO::executeQuery("SELECT id, name, config FROM civicrm_twingle_profile");
+    /**
+     * @var CRM_Core_DAO $profile_data
+     */
+    $profile_data = CRM_Core_DAO::executeQuery('SELECT id, name, config FROM civicrm_twingle_profile');
     while ($profile_data->fetch()) {
-      $profiles[$profile_data->id] = new CRM_Twingle_Profile($profile_data->name, json_decode($profile_data->config, 1), (int) $profile_data->id);
+      $profiles[(int) $profile_data->id] = new CRM_Twingle_Profile(
+        $profile_data->name,
+        json_decode($profile_data->config, TRUE),
+        (int) $profile_data->id
+      );
     }
     return $profiles;
   }
@@ -651,17 +679,20 @@ class CRM_Twingle_Profile {
   /**
    * Get the stats (access_count, last_access) for all twingle profiles
    *
-   * @return CRM_Twingle_Profile[]
+   * @return array<string, array<string, mixed>>
    * @throws \Civi\Core\Exception\DBQueryException
    */
   public static function getProfileStats() {
     $stats = [];
+    /**
+     * @var CRM_Core_DAO $profile_data
+     */
     $profile_data = CRM_Core_DAO::executeQuery(
       'SELECT name, last_access, access_counter FROM civicrm_twingle_profile'
     );
     while ($profile_data->fetch()) {
       // phpcs:disable Drupal.Arrays.Array.ArrayIndentation
-      $stats[$profile_data->name] = [
+      $stats[(string) $profile_data->name] = [
         'name' => $profile_data->name,
         'last_access' => $profile_data->last_access,
         'last_access_txt' => $profile_data->last_access

Civi/Twingle/Exceptions/BaseException.php

@@ -28,21 +28,21 @@ use CRM_Twingle_ExtensionUtil as E;
 class BaseException extends \Exception {
 
   /**
-   * @var int|string
+   * @var string
    */
   protected $code;
   protected string $log_message;
 
   /**
    * BaseException Constructor
-   * @param string|null $message
+   * @param string $message
    *   Error message
-   * @param string|null $error_code
+   * @param string $error_code
    *   A meaningful error code
-   * @param \Throwable|null $previous
+   * @param \Throwable $previous
    *   A previously thrown exception to include.
    */
-  public function __construct(?string $message = '', ?string $error_code = '', ?\Throwable $previous = NULL) {
+  public function __construct(string $message = '', string $error_code = '', \Throwable $previous = NULL) {
     parent::__construct($message, 1, $previous);
     $this->log_message = '' !== $message ? E::LONG_NAME . ': ' . $message : '';
     $this->code = $error_code;
@@ -61,7 +61,7 @@ class BaseException extends \Exception {
    * @return string
    */
   public function getErrorCode() {
-    return (string) $this->code;
+    return $this->code;
   }
 
 }

Civi/Twingle/Exceptions/ProfileValidationError.php

@@ -38,8 +38,13 @@ class ProfileValidationError extends BaseException {
    * @param string $error_code
    *  A meaningful error code
    */
-  public function __construct(string $affected_field_name, string $message = '', string $error_code = '') {
+  public function __construct(
-    parent::__construct($message, $error_code);
+    string $affected_field_name,
+    string $message = '',
+    string $error_code = '',
+    ?\Throwable $previous = NULL
+  ) {
+    parent::__construct($message, $error_code, $previous);
     $this->affected_field_name = $affected_field_name;
   }