Various code cleanup.

app/Rules/BelongsUser.php

@@ -65,8 +65,11 @@ class BelongsUser implements Rule
      *
      * @return bool
      * @throws FireflyException
+     *
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     * @SuppressWarnings(PHPMD.ExcessiveMethodLength)
      */
-    public function passes($attribute, $value)
+    public function passes($attribute, $value): bool
     {
         $attribute = $this->parseAttribute($attribute);
         if (!auth()->check()) {
@@ -75,48 +78,22 @@ class BelongsUser implements Rule
         $attribute = (string)$attribute;
         switch ($attribute) {
             case 'piggy_bank_id':
-                $count = PiggyBank::leftJoin('accounts', 'accounts.id', '=', 'piggy_banks.account_id')
-                                  ->where('piggy_banks.id', '=', (int)$value)
-                                  ->where('accounts.user_id', '=', auth()->user()->id)->count();
-
-                return $count === 1;
-                break;
+                return $this->validatePiggyBankId((int)$value);
             case 'piggy_bank_name':
-                $count = $this->countField(PiggyBank::class, 'name', $value);
-
-                return $count === 1;
-                break;
+                return $this->validatePiggyBankName($value);
             case 'bill_id':
-                $count = Bill::where('id', '=', (int)$value)->where('user_id', '=', auth()->user()->id)->count();
-
-                return $count === 1;
+                return $this->validateBillId((int)$value);
             case 'bill_name':
-                $count = $this->countField(Bill::class, 'name', $value);
-
-                return $count === 1;
-                break;
+                return $this->validateBillName($value);
             case 'budget_id':
-                $count = Budget::where('id', '=', (int)$value)->where('user_id', '=', auth()->user()->id)->count();
-
-                return $count === 1;
-                break;
+                return $this->validateBudgetId((int)$value);
             case 'category_id':
-                $count = Category::where('id', '=', (int)$value)->where('user_id', '=', auth()->user()->id)->count();
-
-                return $count === 1;
-                break;
+                return $this->validateCategoryId((int)$value);
             case 'budget_name':
-                $count = $this->countField(Budget::class, 'name', $value);
-
-                return $count === 1;
-                break;
+                return $this->validateBudgetName($value);
             case 'source_id':
             case 'destination_id':
-                $count = Account::where('id', '=', (int)$value)->where('user_id', '=', auth()->user()->id)->count();
-
-                return $count === 1;
-                break;
-
+                return $this->validateAccountId((int)$value);
             default:
                 throw new FireflyException(sprintf('Rule BelongUser cannot handle "%s"', $attribute)); // @codeCoverageIgnore
         }
@@ -128,18 +105,20 @@ class BelongsUser implements Rule
      * @param string $value
      *
      * @return int
+     *
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     protected function countField(string $class, string $field, string $value): int
     {
+        $objects = [];
         // get all objects belonging to user:
-        switch ($class) {
-            case PiggyBank::class:
-                $objects = PiggyBank::leftJoin('accounts', 'accounts.id', '=', 'piggy_banks.account_id')
-                                    ->where('accounts.user_id', '=', auth()->user()->id)->get(['piggy_banks.*']);
-                break;
-            default:
-                $objects = $class::where('user_id', '=', auth()->user()->id)->get();
-                break;
+        if (PiggyBank::class === $class) {
+            $objects = PiggyBank::leftJoin('accounts', 'accounts.id', '=', 'piggy_banks.account_id')
+                                ->where('accounts.user_id', '=', auth()->user()->id)->get(['piggy_banks.*']);
+
+        }
+        if (PiggyBank::class !== $class) {
+            $objects = $class::where('user_id', '=', auth()->user()->id)->get();
         }
         $count = 0;
         foreach ($objects as $object) {
@@ -159,13 +138,111 @@ class BelongsUser implements Rule
     private function parseAttribute(string $attribute): string
     {
         $parts = explode('.', $attribute);
-        if (\count($parts) === 1) {
+        if (1 === \count($parts)) {
             return $attribute;
         }
-        if (\count($parts) === 3) {
+        if (3 === \count($parts)) {
             return $parts[2];
         }
 
         return $attribute; // @codeCoverageIgnore
     }
+
+    /**
+     * @param int $value
+     *
+     * @return bool
+     */
+    private function validateAccountId(int $value): bool
+    {
+        $count = Account::where('id', '=', (int)$value)->where('user_id', '=', auth()->user()->id)->count();
+
+        return 1 === $count;
+    }
+
+    /**
+     * @param int $value
+     *
+     * @return bool
+     */
+    private function validateBillId(int $value): bool
+    {
+        $count = Bill::where('id', '=', $value)->where('user_id', '=', auth()->user()->id)->count();
+
+        return 1 === $count;
+    }
+
+    /**
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function validateBillName(string $value): bool
+    {
+        $count = $this->countField(Bill::class, 'name', $value);
+
+        return 1 === $count;
+    }
+
+    /**
+     * @param int $value
+     *
+     * @return bool
+     */
+    private function validateBudgetId(int $value): bool
+    {
+        $count = Budget::where('id', '=', $value)->where('user_id', '=', auth()->user()->id)->count();
+
+        return 1 === $count;
+    }
+
+    /**
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function validateBudgetName(string $value): bool
+    {
+        $count = $this->countField(Budget::class, 'name', $value);
+
+        return 1 === $count;
+    }
+
+    /**
+     * @param int $value
+     *
+     * @return bool
+     */
+    private function validateCategoryId(int $value): bool
+    {
+        $count = Category::where('id', '=', $value)->where('user_id', '=', auth()->user()->id)->count();
+
+        return 1 === $count;
+    }
+
+    /**
+     * @param int $value
+     *
+     * @return bool
+     */
+    private function validatePiggyBankId(int $value): bool
+    {
+        $count = PiggyBank::leftJoin('accounts', 'accounts.id', '=', 'piggy_banks.account_id')
+                          ->where('piggy_banks.id', '=', $value)
+                          ->where('accounts.user_id', '=', auth()->user()->id)->count();
+
+        return 1 === $count;
+    }
+
+    /**
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function validatePiggyBankName(string $value): bool
+    {
+        $count = $this->countField(PiggyBank::class, 'name', $value);
+
+        return 1 === $count;
+    }
 }

app/Rules/IsAssetAccountId.php

@@ -49,6 +49,8 @@ class IsAssetAccountId implements Rule
      * @param  mixed  $value
      *
      * @return bool
+     *
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
     public function passes($attribute, $value): bool
     {

app/Rules/IsValidAttachmentModel.php

@@ -24,7 +24,6 @@ declare(strict_types=1);
 namespace FireflyIII\Rules;
 
 
-use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Models\TransactionJournal;
 use FireflyIII\Repositories\Journal\JournalRepositoryInterface;
 use Illuminate\Contracts\Validation\Rule;
@@ -64,25 +63,25 @@ class IsValidAttachmentModel implements Rule
      * @param  mixed  $value
      *
      * @return bool
-     * @throws FireflyException
+     *
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
     public function passes($attribute, $value): bool
     {
         if (!auth()->check()) {
             return false;
         }
-        $user = auth()->user();
-        switch ($this->model) {
-            default:
-                throw new FireflyException(sprintf('Model "%s" cannot be validated.', $this->model));
-            case TransactionJournal::class:
-                /** @var JournalRepositoryInterface $repository */
-                $repository = app(JournalRepositoryInterface::class);
-                $repository->setUser($user);
-                $result = $repository->findNull((int)$value);
 
-                return null !== $result;
-                break;
+
+        if (TransactionJournal::class === $this->model) {
+            $repository = app(JournalRepositoryInterface::class);
+            $user       = auth()->user();
+            $repository->setUser($user);
+            $result = $repository->findNull((int)$value);
+
+            return null !== $result;
         }
+
+        return false;
     }
 }

app/Rules/UniqueIban.php

@@ -23,7 +23,6 @@ declare(strict_types=1);
 
 namespace FireflyIII\Rules;
 
-use FireflyIII\Exceptions\FireflyException;
 use FireflyIII\Models\Account;
 use FireflyIII\Models\AccountType;
 use Illuminate\Contracts\Validation\Rule;
@@ -57,7 +56,7 @@ class UniqueIban implements Rule
      *
      * @return string
      */
-    public function message()
+    public function message(): string
     {
         return (string)trans('validation.unique_iban_for_user');
     }
@@ -69,9 +68,10 @@ class UniqueIban implements Rule
      * @param  mixed  $value
      *
      * @return bool
-     * @throws FireflyException
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
-    public function passes($attribute, $value)
+    public function passes($attribute, $value): bool
     {
         if (!auth()->check()) {
             return true; // @codeCoverageIgnore
@@ -79,50 +79,11 @@ class UniqueIban implements Rule
         if (null === $this->expectedType) {
             return true;
         }
-        $maxCounts = [
-            AccountType::ASSET   => 0,
-            AccountType::EXPENSE => 0,
-            AccountType::REVENUE => 0,
-        ];
-        switch ($this->expectedType) {
-            case 'asset':
-            case AccountType::ASSET:
-                // iban should be unique amongst asset accounts
-                // should not be in use with expense or revenue accounts.
-                // ie: must be totally unique.
-                break;
-            case 'expense':
-            case AccountType::EXPENSE:
-                // should be unique amongst expense and asset accounts.
-                // may appear once in revenue accounts
-                $maxCounts[AccountType::REVENUE] = 1;
-                break;
-            case 'revenue':
-            case AccountType::REVENUE:
-                // should be unique amongst revenue and asset accounts.
-                // may appear once in expense accounts
-                $maxCounts[AccountType::EXPENSE] = 1;
-                break;
-            default:
-
-                throw new FireflyException(sprintf('UniqueIban cannot handle type "%s"', $this->expectedType));
-        }
+        $maxCounts = $this->getMaxOccurrences();
 
         foreach ($maxCounts as $type => $max) {
-            $count = 0;
-            $query = auth()->user()
-                           ->accounts()
-                           ->leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id')
-                           ->where('account_types.type', $type);
-            if (null !== $this->account) {
-                $query->where('accounts.id', '!=', $this->account->id);
-            }
-            $result = $query->get(['accounts.*']);
-            foreach ($result as $account) {
-                if ($account->iban === $value) {
-                    $count++;
-                }
-            }
+            $count = $this->countHits($type, $value);
+
             if ($count > $max) {
                 Log::debug(
                     sprintf(
@@ -137,4 +98,57 @@ class UniqueIban implements Rule
 
         return true;
     }
+
+    /**
+     * @param string $type
+     * @param string $iban
+     *
+     * @return int
+     */
+    private function countHits(string $type, string $iban): int
+    {
+        $count = 0;
+        /** @noinspection NullPointerExceptionInspection */
+        $query = auth()->user()
+                       ->accounts()
+                       ->leftJoin('account_types', 'account_types.id', '=', 'accounts.account_type_id')
+                       ->where('account_types.type', $type);
+        if (null !== $this->account) {
+            $query->where('accounts.id', '!=', $this->account->id);
+        }
+        $result = $query->get(['accounts.*']);
+        foreach ($result as $account) {
+            if ($account->iban === $iban) {
+                $count++;
+            }
+        }
+
+        return $count;
+    }
+
+    /**
+     * @return array
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     */
+    private function getMaxOccurrences(): array
+    {
+        $maxCounts = [
+            AccountType::ASSET   => 0,
+            AccountType::EXPENSE => 0,
+            AccountType::REVENUE => 0,
+        ];
+
+        if ('expense' === $this->expectedType || AccountType::EXPENSE === $this->expectedType) {
+            // IBAN should be unique amongst expense and asset accounts.
+            // may appear once in revenue accounts
+            $maxCounts[AccountType::REVENUE] = 1;
+        }
+        if ('revenue' === $this->expectedType || AccountType::EXPENSE === $this->expectedType) {
+            // IBAN should be unique amongst revenue and asset accounts.
+            // may appear once in expense accounts
+            $maxCounts[AccountType::EXPENSE] = 1;
+        }
+
+        return $maxCounts;
+    }
 }

app/Rules/ValidRecurrenceRepetitionType.php

@@ -48,11 +48,13 @@ class ValidRecurrenceRepetitionType implements Rule
      * @param  mixed  $value
      *
      * @return bool
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function passes($attribute, $value): bool
     {
         $value = (string)$value;
-        if ($value === 'daily') {
+        if ('daily' === $value) {
             return true;
         }
         //monthly,17

app/Rules/ValidRecurrenceRepetitionValue.php

@@ -27,6 +27,7 @@ namespace FireflyIII\Rules;
 use Carbon\Carbon;
 use Illuminate\Contracts\Validation\Rule;
 use InvalidArgumentException;
+use Log;
 
 /**
  * Class ValidRecurrenceRepetitionValue
@@ -51,57 +52,103 @@ class ValidRecurrenceRepetitionValue implements Rule
      * @param  mixed  $value
      *
      * @return bool
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
      */
     public function passes($attribute, $value): bool
     {
         $value = (string)$value;
 
-        if ($value === 'daily') {
+        if ('daily' === $value) {
             return true;
         }
 
         if (0 === strpos($value, 'monthly')) {
-            $dayOfMonth = (int)substr($value, 8);
-
-            return $dayOfMonth > 0 && $dayOfMonth < 32;
+            return $this->validateMonthly($value);
         }
 
         //ndom,3,7
         // nth x-day of the month.
         if (0 === strpos($value, 'ndom')) {
-            $parameters = explode(',', substr($value, 5));
-            if (\count($parameters) !== 2) {
-                return false;
-            }
-            $nthDay    = (int)($parameters[0] ?? 0.0);
-            $dayOfWeek = (int)($parameters[1] ?? 0.0);
-            if ($nthDay < 1 || $nthDay > 5) {
-                return false;
-            }
-
-            return $dayOfWeek > 0 && $dayOfWeek < 8;
+            return $this->validateNdom($value);
         }
 
         //weekly,7
         if (0 === strpos($value, 'weekly')) {
-            $dayOfWeek = (int)substr($value, 7);
-
-            return $dayOfWeek > 0 && $dayOfWeek < 8;
+            return $this->validateWeekly($value);
         }
 
         //yearly,2018-01-01
         if (0 === strpos($value, 'yearly')) {
-            // rest of the string must be valid date:
-            $dateString = substr($value, 7);
-            try {
-                $date = Carbon::createFromFormat('Y-m-d', $dateString);
-            } catch (InvalidArgumentException $e) {
-                return false;
-            }
-
-            return true;
+            return $this->validateYearly($value);
         }
 
         return false;
     }
+
+    /**
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function validateMonthly(string $value): bool
+    {
+        $dayOfMonth = (int)substr($value, 8);
+
+        return $dayOfMonth > 0 && $dayOfMonth < 32;
+    }
+
+    /**
+     * @param string $value
+     *
+     * @return bool
+     *
+     * @SuppressWarnings(PHPMD.CyclomaticComplexity)
+     */
+    private function validateNdom(string $value): bool
+    {
+        $parameters = explode(',', substr($value, 5));
+        if (2 !== \count($parameters)) {
+            return false;
+        }
+        $nthDay    = (int)($parameters[0] ?? 0.0);
+        $dayOfWeek = (int)($parameters[1] ?? 0.0);
+        if ($nthDay < 1 || $nthDay > 5) {
+            return false;
+        }
+
+        return $dayOfWeek > 0 && $dayOfWeek < 8;
+    }
+
+    /**
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function validateWeekly(string $value): bool
+    {
+        $dayOfWeek = (int)substr($value, 7);
+
+        return $dayOfWeek > 0 && $dayOfWeek < 8;
+    }
+
+    /**
+     * @param string $value
+     *
+     * @return bool
+     */
+    private function validateYearly(string $value): bool
+    {
+        // rest of the string must be valid date:
+        $dateString = substr($value, 7);
+        try {
+            Carbon::createFromFormat('Y-m-d', $dateString);
+        } catch (InvalidArgumentException $e) {
+            Log::debug(sprintf('Could not parse date %s: %s', $dateString, $e->getMessage()));
+
+            return false;
+        }
+
+        return true;
+    }
 }

app/Rules/ValidTransactions.php

@@ -49,7 +49,7 @@ class ValidTransactions implements Rule
      *
      * @return string
      */
-    public function message()
+    public function message(): string
     {
         return (string)trans('validation.invalid_selection');
     }
@@ -61,8 +61,9 @@ class ValidTransactions implements Rule
      * @param  mixed  $value
      *
      * @return bool
+     * @SuppressWarnings(PHPMD.UnusedFormalParameter)
      */
-    public function passes($attribute, $value)
+    public function passes($attribute, $value): bool
     {
         Log::debug('In ValidTransactions::passes');
         if (!\is_array($value)) {
@@ -73,7 +74,7 @@ class ValidTransactions implements Rule
             $count = Transaction::where('transactions.id', $transactionId)
                                 ->leftJoin('accounts', 'accounts.id', '=', 'transactions.account_id')
                                 ->where('accounts.user_id', $userId)->count();
-            if ($count === 0) {
+            if (0 === $count) {
                 Log::debug(sprintf('Count for transaction #%d and user #%d is zero! Return FALSE', $transactionId, $userId));
 
                 return false;