[rector] apply SetList::DEAD_CODE, better type checking (see msg)

This commit includes a variety of changes that improve the codebase's quality and consistency. The following changes were made: - Replaced `==` with `===` for strict comparisons. - Removed redundant `break` statements after `return` in switch-case structures. - Fixed variable naming consistency and unnecessary variable assignments. - Simplified conditional statements and removed unused code. - Corrected minor typos and formatted code for better readability. - Enhanced type safety by casting variables explicitly and adding type hints. - Improved function and variable documentation with consistent comments. - Refactored repeated logic to improve maintainability. - Removed redundant checks for null values and unnecessary extractions.
ChurchCRM · May 8, 2024 · 2236ac8 · 2236ac8
1 parent ee4855d
commit 2236ac8
Show file tree

Hide file tree

Showing 84 changed files with 355 additions and 756 deletions.
diff --git a/src/AddDonors.php b/src/AddDonors.php
@@ -61,7 +61,7 @@
     $sSQL = "SELECT pn_per_id FROM paddlenum_pn WHERE pn_per_id='$donorID' AND pn_FR_ID = '$iFundRaiserID'";
     $rsBuyer = RunQuery($sSQL);
 
-    if ($donorID > 0 && mysqli_num_rows($rsBuyer) == 0) {
+    if ($donorID > 0 && mysqli_num_rows($rsBuyer) === 0) {
         $sSQL = "INSERT INTO paddlenum_pn (pn_Num, pn_fr_ID, pn_per_ID)
 		                VALUES ('$extraPaddleNum', '$iFundRaiserID', '$donorID')";
         RunQuery($sSQL);

diff --git a/src/CanvassEditor.php b/src/CanvassEditor.php
@@ -26,11 +26,11 @@
 
 $iCanvassID = 0;
 if (array_key_exists('CanvassID', $_GET)) {
-    $iCanvassID = InputUtils::legacyFilterInput($_GET['CanvassID'], 'int');
+    $iCanvassID = (int) InputUtils::legacyFilterInput($_GET['CanvassID'], 'int');
 }
 $linkBack = InputUtils::legacyFilterInput($_GET['linkBack']);
-$iFamily = InputUtils::legacyFilterInput($_GET['FamilyID']);
-$iFYID = InputUtils::legacyFilterInput($_GET['FYID']);
+$iFamily = (int) InputUtils::legacyFilterInput($_GET['FamilyID'], 'int');
+$iFYID = (int) InputUtils::legacyFilterInput($_GET['FYID'], 'int');
 
 $sDateError = '';
 $bNotInterested = false;
@@ -58,54 +58,56 @@
     $tFinancial = InputUtils::legacyFilterInput($_POST['Financial']);
     $tSuggestion = InputUtils::legacyFilterInput($_POST['Suggestion']);
     $bNotInterested = isset($_POST['NotInterested']);
-    if ($bNotInterested == '') {
-        $bNotInterested = 0;
-    }
     $tWhyNotInterested = InputUtils::legacyFilterInput($_POST['WhyNotInterested']);
 
     // New canvas input (add)
-    if ($iCanvassID < 1) {
-        $canvassData = new CanvassData();
-        $canvassData
-            ->setFamilyId($iFamily)
-            ->setCanvasser($iCanvasser)
-            ->setFyid($iFYID)
-            ->setDate($dDate)
-            ->setPositive($tPositive)
-            ->setCritical($tCritical)
-            ->setInsightful($tInsightful)
-            ->setFinancial($tFinancial)
-            ->setSuggestion($tSuggestion)
-            ->setNotInterested($bNotInterested)
-            ->setWhyNotInterested($tWhyNotInterested);
-        $canvassData->save();
-        $canvassData->reload();
-        $iCanvassID = $canvassData->getId();
+    $newCanvas = $iCanvassID < 1;
+    $canvassData = new CanvassData();
+    if ($newCanvas) {
+        $canvassData->setFamilyId($iFamily);
     } else {
         $canvassData = CanvassDataQuery::create()->findOneByFamilyId($iFamily);
-        $canvassData
-            ->setCanvasser($iCanvasser)
-            ->setFyid($iFYID)
-            ->setDate($dDate)
-            ->setPositive($tPositive)
-            ->setCritical($tCritical)
-            ->setInsightful($tInsightful)
-            ->setFinancial($tFinancial)
-            ->setSuggestion($tSuggestion)
-            ->setNotInterested($bNotInterested)
-            ->setWhyNotInterested($tWhyNotInterested);
-        $canvassData->save();
     }
 
+    $canvassData
+        ->setCanvasser($iCanvasser)
+        ->setFyid($iFYID)
+        ->setDate($dDate)
+        ->setPositive($tPositive)
+        ->setCritical($tCritical)
+        ->setInsightful($tInsightful)
+        ->setFinancial($tFinancial)
+        ->setSuggestion($tSuggestion)
+        ->setNotInterested($bNotInterested)
+        ->setWhyNotInterested($tWhyNotInterested);
+    $canvassData->save();
+
+    if ($newCanvas) {
+        $canvassData->reload();
+        $iCanvassID = $canvassData->getId();
+    }
     if (isset($_POST['Submit'])) {
         // Check for redirection to another page after saving information: (ie. PledgeEditor.php?previousPage=prev.php?a=1;b=2;c=3)
-        if ($linkBack != '') {
+        if (!empty($linkBack)) {
             RedirectUtils::redirect($linkBack);
         } else {
             RedirectUtils::redirect('CanvassEditor.php?FamilyID=' . $iFamily . '&FYID=' . $iFYID . '&CanvassID=' . $iCanvassID . '&linkBack=', $linkBack);
         }
     }
 } else {
+    // Set some default values
+    $iCanvasser = AuthenticationManager::getCurrentUser()->getId();
+    $dDate = date('Y-m-d');
+
+    $dDate = '';
+    $tPositive = '';
+    $tCritical = '';
+    $tInsightful = '';
+    $tFinancial = '';
+    $tSuggestion = '';
+    $bNotInterested = false;
+    $tWhyNotInterested = '';
+
     $sSQL = 'SELECT * FROM canvassdata_can WHERE can_famID = ' . $iFamily . ' AND can_FYID=' . $iFYID;
     $rsCanvass = RunQuery($sSQL);
     if (mysqli_num_rows($rsCanvass) > 0) {
@@ -122,19 +124,6 @@
         $tSuggestion = $can_Suggestion;
         $bNotInterested = $can_NotInterested;
         $tWhyNotInterested = $can_WhyNotInterested;
-    } else {
-        // Set some default values
-        $iCanvasser = AuthenticationManager::getCurrentUser()->getId();
-        $dDate = date('Y-m-d');
-
-        $dDate = '';
-        $tPositive = '';
-        $tCritical = '';
-        $tInsightful = '';
-        $tFinancial = '';
-        $tSuggestion = '';
-        $bNotInterested = false;
-        $tWhyNotInterested = '';
     }
 }
 

diff --git a/src/ChurchCRM/Authentication/AuthenticationManager.php b/src/ChurchCRM/Authentication/AuthenticationManager.php
@@ -61,9 +61,7 @@ public static function endSession(bool $preventRedirect = false): void
         $currentSessionUserName = 'Unknown';
 
         try {
-            if (self::getCurrentUser() instanceof User) {
-                $currentSessionUserName = self::getCurrentUser()->getName();
-            }
+            $currentSessionUserName = self::getCurrentUser()->getName();
         } catch (\Exception $e) {
             //unable to get name of user logging out. Don't really care.
         }

diff --git a/src/ChurchCRM/Backup/BackupDownloader.php b/src/ChurchCRM/Backup/BackupDownloader.php
@@ -32,9 +32,6 @@ public static function downloadBackup(string $filename): void
             $ext = strtolower($path_parts['extension']);
             switch ($ext) {
                 case 'gz':
-                    header('Content-type: application/x-gzip');
-                    header('Content-Disposition: attachment; filename="' . $path_parts['basename'] . '"');
-                    break;
                 case 'tar.gz':
                     header('Content-type: application/x-gzip');
                     header('Content-Disposition: attachment; filename="' . $path_parts['basename'] . '"');

diff --git a/src/ChurchCRM/Backup/BackupJob.php b/src/ChurchCRM/Backup/BackupJob.php
@@ -17,32 +17,13 @@
 class BackupJob extends JobBase
 {
     private string $BackupFileBaseName;
-
     private ?\SplFileInfo $BackupFile = null;
-    /**
-     * @var bool
-     */
-    private $IncludeExtraneousFiles;
-    /**
-     * @var string
-     */
-    public $BackupDownloadFileName;
-    /**
-     * @var bool
-     */
-    public $shouldEncrypt;
-
-    /**
-     * @var string
-     */
-    public $BackupPassword;
-
-    /**
-     * @param string     $BaseName
-     * @param BackupType $BackupType
-     * @param bool       $IncludeExtraneousFiles
-     */
-    public function __construct(string $BaseName, $BackupType, $IncludeExtraneousFiles, $EncryptBackup, $BackupPassword)
+    private bool $IncludeExtraneousFiles;
+    public string $BackupDownloadFileName;
+    public bool $shouldEncrypt;
+    public string $BackupPassword;
+
+    public function __construct(string $BaseName, string $BackupType, bool $IncludeExtraneousFiles, bool $EncryptBackup, string $BackupPassword)
     {
         $this->BackupType = $BackupType;
         $this->TempFolder = $this->createEmptyTempFolder();

diff --git a/src/ChurchCRM/Backup/JobBase.php b/src/ChurchCRM/Backup/JobBase.php
@@ -8,13 +8,8 @@
 
 class JobBase
 {
-    /** @var BackupType */
-    protected $BackupType;
-
-    /**
-     * @var string
-     */
-    protected $TempFolder;
+    protected string $BackupType;
+    protected string $TempFolder;
 
     protected function createEmptyTempFolder(): string
     {

diff --git a/src/ChurchCRM/Backup/RestoreJob.php b/src/ChurchCRM/Backup/RestoreJob.php
@@ -9,6 +9,7 @@
 use ChurchCRM\SQLUtils;
 use ChurchCRM\Utils\InputUtils;
 use ChurchCRM\Utils\LoggerUtils;
+use Defuse\Crypto\Exception\WrongKeyOrModifiedCiphertextException;
 use Defuse\Crypto\File;
 use Exception;
 use PharData;
@@ -17,15 +18,7 @@
 class RestoreJob extends JobBase
 {
     private \SplFileInfo $RestoreFile;
-
-    /**
-     * @var array
-     */
-    public $Messages = [];
-    /**
-     * @var bool
-     */
-    private $IsBackupEncrypted;
+    public array $Messages = [];
     private ?string $restorePassword = null;
 
     private function isIncomingFileFailed(): bool
@@ -64,7 +57,7 @@ private function decryptBackup(): void
             File::decryptFileWithPassword($this->RestoreFile, $tempfile, $this->restorePassword);
             rename($tempfile, $this->RestoreFile);
             LoggerUtils::getAppLogger()->info('File decrypted');
-        } catch (\Defuse\Crypto\Exception\WrongKeyOrModifiedCiphertextException $ex) {
+        } catch (WrongKeyOrModifiedCiphertextException $ex) {
             if ($ex->getMessage() == 'Bad version header.') {
                 LoggerUtils::getAppLogger()->info("Bad version header; this file probably wasn't encrypted");
             } else {

diff --git a/src/ChurchCRM/Bootstrapper.php b/src/ChurchCRM/Bootstrapper.php
@@ -198,9 +198,9 @@ private static function isDatabaseEmpty(): bool
         $connection = Propel::getConnection();
         $query = "SHOW TABLES FROM `" . self::$databaseName . "`";
         $statement = $connection->prepare($query);
-        $resultset = $statement->execute();
+        $statement->execute();
         $results = $statement->fetchAll(\PDO::FETCH_ASSOC);
-        if (count($results) == 0) {
+        if (count($results) === 0) {
             self::$bootStrapLogger->debug("No database tables found");
             return true;
         }

diff --git a/src/ChurchCRM/Config/Menu/MenuItem.php b/src/ChurchCRM/Config/Menu/MenuItem.php
@@ -65,9 +65,6 @@ public function getName()
         return $this->name;
     }
 
-    /**
-     * @return bool
-     */
     public function isExternal(): bool
     {
         return $this->external;

diff --git a/src/ChurchCRM/Dashboard/EventsMenuItems.php b/src/ChurchCRM/Dashboard/EventsMenuItems.php
@@ -17,13 +17,11 @@ public static function getDashboardItemName(): string
 
     public static function getDashboardItemValue(): array
     {
-        $activeEvents = [
+        return [
             'Events'        => self::getNumberEventsOfToday(),
             'Birthdays'     => self::getNumberBirthDates(),
             'Anniversaries' => self::getNumberAnniversaries(),
         ];
-
-        return $activeEvents;
     }
 
     public static function shouldInclude(string $PageName): bool
@@ -48,8 +46,6 @@ private static function getNumberBirthDates()
             ->filterByBirthMonth(date('m'))
             ->filterByBirthDay(date('d'))
             ->count();
-
-        return $peopleWithBirthDays;
     }
 
     private static function getNumberAnniversaries()

diff --git a/src/ChurchCRM/MICRFunctions.php b/src/ChurchCRM/MICRFunctions.php
@@ -78,7 +78,7 @@ public function findRouteAndAccount($micr)
         }
     }
 
-    public function findCheckNo($micr)
+    public function findCheckNo($micr): string
     {
         $formatID = $this->identifyFormat($micr);
         if ($formatID == $this->CHECKNO_FIRST) {

diff --git a/src/ChurchCRM/Reports/ChurchInfoReport.php b/src/ChurchCRM/Reports/ChurchInfoReport.php
@@ -63,15 +63,13 @@ public function printRightJustified($x, $y, $str): void
     public function printRightJustifiedCell($x, $y, $wid, $str): void
     {
         $strconv = iconv('UTF-8', 'ISO-8859-1', $str);
-        $iLen = strlen($strconv);
         $this->SetXY($x, $y);
         $this->Cell($wid, SystemConfig::getValue('incrementY'), $strconv, 1, 0, 'R');
     }
 
     public function printCenteredCell($x, $y, $wid, $str): void
     {
         $strconv = iconv('UTF-8', 'ISO-8859-1', $str);
-        $iLen = strlen($strconv);
         $this->SetXY($x, $y);
         $this->Cell($wid, SystemConfig::getValue('incrementY'), $strconv, 1, 0, 'C');
     }
@@ -131,10 +129,9 @@ public function startLetterPage($fam_ID, $fam_Name, $fam_Address1, $fam_Address2
         if ($fam_Country != '' && $fam_Country != SystemConfig::getValue('sDefaultCountry')) {
             $this->writeAt(SystemConfig::getValue('leftX'), $curY, $fam_Country);
             $curY += SystemConfig::getValue('incrementY');
-        }
-        $curY += 5.0; // mm to get away from the second window
+        } // mm to get away from the second window
 
-        return $curY;
+        return $curY + 5.0;
     }
 
     public function makeSalutation($famID): string

diff --git a/src/ChurchCRM/Reports/PDF_AddressReport.php b/src/ChurchCRM/Reports/PDF_AddressReport.php
@@ -9,10 +9,7 @@ class PdfAddressReport extends ChurchInfoReport
     private int $_Margin_Top = 12;         // Top margin
     private int $_Char_Size = 12;        // Character size
     private int $_CurLine = 2;
-    private int $_Column = 0;
     private string $_Font = 'Times';
-    private $sFamily;
-    private $sLastName;
 
     private function numLinesInFpdfCell(int $w, $txt): int
     {

diff --git a/src/ChurchCRM/Reports/PDF_Attendance.php b/src/ChurchCRM/Reports/PDF_Attendance.php
@@ -65,7 +65,6 @@ public function drawAttendanceCalendar(
         }
 
         $yTitle = 20;
-        $yTeachers = $yTitle + $yIncrement;
         $nameX = 10 + $yIncrement / 2;
         $numMembers = 0;
         $aNameCount = 0;

diff --git a/src/ChurchCRM/SQLUtils.php b/src/ChurchCRM/SQLUtils.php
@@ -119,8 +119,6 @@ private static function clearSQL($sql, &$isMultiComment)
      *
      * @param int    $offset
      * @param string $text
-     *
-     * @return bool
      */
     private static function isQuoted($offset, $text): bool
     {

diff --git a/src/ChurchCRM/Search/BaseSearchResultProvider.php b/src/ChurchCRM/Search/BaseSearchResultProvider.php
@@ -4,28 +4,20 @@
 
 abstract class BaseSearchResultProvider
 {
-    /* @var string */
-    protected $pluralNoun;
-    /* @var ChurchCRM\Search\SearchResult[] */
-    protected $searchResults = [];
+    protected string $pluralNoun;
+
+    /* @var SearchResult[] */
+    protected array $searchResults = [];
 
     abstract public function getSearchResults(string $SearchQuery);
 
-    protected function formatSearchGroup()
+    protected function formatSearchGroup(): SearchResultGroup
     {
-        if (!empty($this->searchResults)) {
-            return new SearchResultGroup(gettext($this->pluralNoun) . ' (' . count($this->searchResults) . ')', $this->searchResults);
-        }
-
-        return [];
+        return new SearchResultGroup(gettext($this->pluralNoun) . ' (' . count($this->searchResults) . ')', $this->searchResults);
     }
 
-    protected function addSearchResults(array $results)
+    protected function addSearchResults(array $results): void
     {
         $this->searchResults = array_merge($this->searchResults, $results);
     }
-
-    protected function __construct()
-    {
-    }
 }
diff --git a/src/ChurchCRM/Search/FamilySearchResultProvider.php b/src/ChurchCRM/Search/FamilySearchResultProvider.php
@@ -18,7 +18,6 @@ public function __construct()
 
     public function getSearchResults(string $SearchQuery)
     {
-        $searchResults = [];
         if (SystemConfig::getBooleanValue('bSearchIncludeFamilies')) {
             $this->addSearchResults($this->getFamilySearchResultsByPartialName($SearchQuery));
         }