diff --git a/app/Factory/TransactionFactory.php b/app/Factory/TransactionFactory.php index 35a435faa7..7f328ac270 100644 --- a/app/Factory/TransactionFactory.php +++ b/app/Factory/TransactionFactory.php @@ -113,7 +113,7 @@ class TransactionFactory $destinationAccount = $this->findAccount($destinationType, $data['destination_id'], $data['destination_name']); if (null === $sourceAccount || null === $destinationAccount) { - throw new FireflyException('Could not determine source or destination account.'); + throw new FireflyException('Could not determine source or destination account.', $data); } Log::debug(sprintf('Source type is "%s", destination type is "%s"', $sourceAccount->accountType->type, $destinationAccount->accountType->type)); diff --git a/app/Import/Converter/Amount.php b/app/Import/Converter/Amount.php index 070842a050..1b828c6663 100644 --- a/app/Import/Converter/Amount.php +++ b/app/Import/Converter/Amount.php @@ -33,12 +33,12 @@ class Amount implements ConverterInterface * Some people, when confronted with a problem, think "I know, I'll use regular expressions." Now they have two problems. * - Jamie Zawinski. * - * + * @SuppressWarnings(PHPMD.ExcessiveMethodLength) + * @SuppressWarnings(PHPMD.NPathComplexity) + * @SuppressWarnings(PHPMD.CyclomaticComplexity) * @param $value * * @return string - * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function convert($value): string { @@ -46,59 +46,43 @@ class Amount implements ConverterInterface return '0'; } Log::debug(sprintf('Start with amount "%s"', $value)); - $original = $value; - $value = (string)$value; - $value = $this->stripAmount($value); - $len = \strlen($value); - $decimalPosition = $len - 3; - $altPosition = $len - 2; - $decimal = null; + $original = $value; + $value = $this->stripAmount((string)$value); + $decimal = null; - if (($len > 2 && '.' === $value[$decimalPosition]) || ($len > 2 && strpos($value, '.') > $decimalPosition)) { + if ($this->decimalIsDot($value)) { $decimal = '.'; Log::debug(sprintf('Decimal character in "%s" seems to be a dot.', $value)); } - if ($len > 2 && ',' === $value[$decimalPosition]) { + + if ($this->decimalIsComma($value)) { $decimal = ','; Log::debug(sprintf('Decimal character in "%s" seems to be a comma.', $value)); } + // decimal character is null? find out if "0.1" or ".1" or "0,1" or ",1" - if ($len > 1 && ('.' === $value[$altPosition] || ',' === $value[$altPosition])) { - $decimal = $value[$altPosition]; - Log::debug(sprintf('Alternate search resulted in "%s" for decimal sign.', $decimal)); + if ($this->alternativeDecimalSign($value)) { + $decimal = $this->getAlternativeDecimalSign($value); } // decimal character still null? Search from the left for '.',',' or ' '. if (null === $decimal) { - Log::debug('Decimal is still NULL, probably number with >2 decimals. Search for a dot.'); - $res = strrpos($value, '.'); - if (!(false === $res)) { - // blandly assume this is the one. - Log::debug(sprintf('Searched from the left for "." in amount "%s", assume this is the decimal sign.', $value)); - $decimal = '.'; - } - unset($res); + $decimal = $this->findFromLeft($value); } - // if decimal is dot, replace all comma's and spaces with nothing. then parse as float (round to 4 pos) - if ('.' === $decimal) { - $search = [',', ' ']; - $value = str_replace($search, '', $value); - Log::debug(sprintf('Converted amount from "%s" to "%s".', $original, $value)); - } - if (',' === $decimal) { - $search = ['.', ' ']; - $value = str_replace($search, '', $value); - $value = str_replace(',', '.', $value); + // if decimal is dot, replace all comma's and spaces with nothing + if (null !== $decimal) { + $value = $this->replaceDecimal($decimal, $value); Log::debug(sprintf('Converted amount from "%s" to "%s".', $original, $value)); } + if (null === $decimal) { // replace all: $search = ['.', ' ', ',']; $value = str_replace($search, '', $value); Log::debug(sprintf('No decimal character found. Converted amount from "%s" to "%s".', $original, $value)); } - if ($value{0} === '.') { + if ('.' === $value{0}) { $value = '0' . $value; } @@ -114,18 +98,113 @@ class Amount implements ConverterInterface return $formatted; } - private function bcround($number, $scale = 0) + /** + * Check if the value has a dot or comma on an alternative place, + * catching strings like ",1" or ".5". + * + * @param string $value + * + * @return bool + */ + private function alternativeDecimalSign(string $value): bool { - $fix = "5"; - for ($i = 0; $i < $scale; $i++) { - $fix = "0$fix"; - } - $number = bcadd($number, "0.$fix", $scale + 1); + $length = \strlen($value); + $altPosition = $length - 2; - return bcdiv($number, "1.0", $scale); + return $length > 1 && ('.' === $value[$altPosition] || ',' === $value[$altPosition]); } /** + * Helper function to see if the decimal separator is a comma. + * + * @param string $value + * + * @return bool + */ + private function decimalIsComma(string $value): bool + { + $length = \strlen($value); + $decimalPosition = $length - 3; + + return $length > 2 && ',' === $value[$decimalPosition]; + } + + /** + * Helper function to see if the decimal separator is a dot. + * + * @param string $value + * + * @return bool + */ + private function decimalIsDot(string $value): bool + { + $length = \strlen($value); + $decimalPosition = $length - 3; + + return ($length > 2 && '.' === $value[$decimalPosition]) || ($length > 2 && strpos($value, '.') > $decimalPosition); + } + + /** + * Search from the left for decimal sign. + * + * @param string $value + * + * @return string + */ + private function findFromLeft(string $value): ?string + { + $decimal = null; + Log::debug('Decimal is still NULL, probably number with >2 decimals. Search for a dot.'); + $res = strrpos($value, '.'); + if (!(false === $res)) { + // blandly assume this is the one. + Log::debug(sprintf('Searched from the left for "." in amount "%s", assume this is the decimal sign.', $value)); + $decimal = '.'; + } + + return $decimal; + } + + /** + * Returns the alternative decimal point used, such as a dot or a comma, + * from strings like ",1" or "0.5". + * + * @param string $value + * + * @return string + */ + private function getAlternativeDecimalSign(string $value): string + { + $length = \strlen($value); + $altPosition = $length - 2; + + return $value[$altPosition]; + + } + + /** + * @param string $decimal + * @param string $value + * + * @return string + */ + private function replaceDecimal(string $decimal, string $value): string + { + $search = [',', ' ']; // default when decimal sign is a dot. + if (',' === $decimal) { + $search = ['.', ' ']; + } + $value = str_replace($search, '', $value); + + /** @noinspection CascadeStringReplacementInspection */ + $value = str_replace(',', '.', $value); + + return $value; + } + + /** + * Strip amount from weird characters. + * * @param string $value * * @return string diff --git a/app/Import/Converter/INGDebitCredit.php b/app/Import/Converter/INGDebitCredit.php index 4779cecca6..4148c7d2ea 100644 --- a/app/Import/Converter/INGDebitCredit.php +++ b/app/Import/Converter/INGDebitCredit.php @@ -34,7 +34,7 @@ class INGDebitCredit implements ConverterInterface * * @return int */ - public function convert($value) + public function convert($value): int { Log::debug('Going to convert ing debit credit', ['value' => $value]); diff --git a/app/Import/JobConfiguration/FakeJobConfiguration.php b/app/Import/JobConfiguration/FakeJobConfiguration.php index 2003a2b230..751cacf721 100644 --- a/app/Import/JobConfiguration/FakeJobConfiguration.php +++ b/app/Import/JobConfiguration/FakeJobConfiguration.php @@ -40,20 +40,22 @@ class FakeJobConfiguration implements JobConfigurationInterface /** * Returns true when the initial configuration for this job is complete. + * configuration array of job must have two values: + * 'artist' must be 'david bowie', case insensitive + * 'song' must be 'golden years', case insensitive. + * if stage is not "new", then album must be 'station to station' * * @return bool + * @SuppressWarnings(PHPMD.CyclomaticComplexity) */ public function configurationComplete(): bool { - // configuration array of job must have two values: - // 'artist' must be 'david bowie', case insensitive - // 'song' must be 'golden years', case insensitive. - // if stage is not "new", then album must be 'station to station' $config = $this->importJob->configuration; - if ($this->importJob->stage === 'new') { - return (isset($config['artist']) && 'david bowie' === strtolower($config['artist'])) - && (isset($config['song']) && 'golden years' === strtolower($config['song'])) - && isset($config['apply-rules']); + if ('new' === $this->importJob->stage) { + return + isset($config['artist'], $config['song'], $config['apply-rules']) + && 'david bowie' === strtolower($config['artist']) + && 'golden years' === strtolower($config['song']); } return isset($config['album']) && 'station to station' === strtolower($config['album']); @@ -80,12 +82,12 @@ class FakeJobConfiguration implements JobConfigurationInterface $configuration['artist'] = $artist; } - if ($song === 'golden years') { + if ('golden years' === $song) { // store song $configuration['song'] = $song; } - if ($album === 'station to station') { + if ('station to station' === $album) { // store album $configuration['album'] = $album; } @@ -96,8 +98,7 @@ class FakeJobConfiguration implements JobConfigurationInterface $this->repository->setConfiguration($this->importJob, $configuration); $messages = new MessageBag(); - if (\count($configuration) !== 3) { - + if (3 !== \count($configuration)) { $messages->add('some_key', 'Ignore this error: ' . \count($configuration)); } @@ -136,13 +137,13 @@ class FakeJobConfiguration implements JobConfigurationInterface if (null === $applyRules) { return 'import.fake.apply-rules'; } - if (strtolower($artist) !== 'david bowie') { + if ('david bowie' !== strtolower($artist)) { return 'import.fake.enter-artist'; } - if (strtolower($song) !== 'golden years') { + if ('golden years' !== strtolower($song)) { return 'import.fake.enter-song'; } - if (strtolower($album) !== 'station to station' && $this->importJob->stage !== 'new') { + if ('new' !== $this->importJob->stage && 'station to station' !== strtolower($album)) { return 'import.fake.enter-album'; } diff --git a/app/Import/JobConfiguration/FileJobConfiguration.php b/app/Import/JobConfiguration/FileJobConfiguration.php index 5820b38951..a40bc47bf7 100644 --- a/app/Import/JobConfiguration/FileJobConfiguration.php +++ b/app/Import/JobConfiguration/FileJobConfiguration.php @@ -52,7 +52,7 @@ class FileJobConfiguration implements JobConfigurationInterface */ public function configurationComplete(): bool { - return $this->importJob->stage === 'ready_to_run'; + return 'ready_to_run' === $this->importJob->stage; } /** diff --git a/app/Import/Mapper/AssetAccountIbans.php b/app/Import/Mapper/AssetAccountIbans.php index 3d166184b7..ce9585fc44 100644 --- a/app/Import/Mapper/AssetAccountIbans.php +++ b/app/Import/Mapper/AssetAccountIbans.php @@ -49,7 +49,7 @@ class AssetAccountIbans implements MapperInterface if (\strlen($iban) > 0) { $topList[$accountId] = $account->iban . ' (' . $account->name . ')'; } - if (0 === \strlen($iban)) { + if ('' === $iban) { $list[$accountId] = $account->name; } } diff --git a/app/Import/Mapper/OpposingAccountIbans.php b/app/Import/Mapper/OpposingAccountIbans.php index a8b3d4eb99..3eae583064 100644 --- a/app/Import/Mapper/OpposingAccountIbans.php +++ b/app/Import/Mapper/OpposingAccountIbans.php @@ -55,10 +55,11 @@ class OpposingAccountIbans implements MapperInterface if (\strlen($iban) > 0) { $topList[$accountId] = $account->iban . ' (' . $account->name . ')'; } - if (0 === \strlen($iban)) { + if ('' === $iban) { $list[$accountId] = $account->name; } } + /** @noinspection AdditionOperationOnArraysInspection */ $list = $topList + $list; asort($list); $list = [0 => (string)trans('import.map_do_not_map')] + $list; diff --git a/app/Import/Prerequisites/FakePrerequisites.php b/app/Import/Prerequisites/FakePrerequisites.php index 86642e8066..a5565d9c5d 100644 --- a/app/Import/Prerequisites/FakePrerequisites.php +++ b/app/Import/Prerequisites/FakePrerequisites.php @@ -58,7 +58,7 @@ class FakePrerequisites implements PrerequisitesInterface $apiKey = app('preferences')->getForUser($this->user, 'fake_api_key', null)->data; } $oldKey = (string)\request()->old('api_key'); - if ($oldKey !== '') { + if ('' !== $oldKey) { $apiKey = \request()->old('api_key'); // @codeCoverageIgnore } @@ -118,7 +118,7 @@ class FakePrerequisites implements PrerequisitesInterface if (null === $apiKey->data) { return false; } - if (\strlen((string)$apiKey->data) === 32) { + if (32 === \strlen((string)$apiKey->data)) { return true; } diff --git a/app/Import/Routine/FakeRoutine.php b/app/Import/Routine/FakeRoutine.php index bd609706c5..497fcb93dd 100644 --- a/app/Import/Routine/FakeRoutine.php +++ b/app/Import/Routine/FakeRoutine.php @@ -55,7 +55,7 @@ class FakeRoutine implements RoutineInterface public function run(): void { Log::debug(sprintf('Now in run() for fake routine with status: %s', $this->importJob->status)); - if ($this->importJob->status !== 'ready_to_run') { + if ('ready_to_run' !== $this->importJob->status) { throw new FireflyException(sprintf('Fake job should have status "ready_to_run", not "%s"', $this->importJob->status)); // @codeCoverageIgnore } @@ -95,7 +95,6 @@ class FakeRoutine implements RoutineInterface /** * @param ImportJob $importJob * - * @return */ public function setImportJob(ImportJob $importJob): void { diff --git a/app/Import/Routine/SpectreRoutine.php b/app/Import/Routine/SpectreRoutine.php index 83aea2664a..a35f65b9f4 100644 --- a/app/Import/Routine/SpectreRoutine.php +++ b/app/Import/Routine/SpectreRoutine.php @@ -66,7 +66,7 @@ class SpectreRoutine implements RoutineInterface $handler->run(); // if count logins is zero, go to authenticate stage - if ($handler->getCountLogins() === 0) { + if (0 === $handler->getCountLogins()) { $this->repository->setStage($this->importJob, 'do-authenticate'); $this->repository->setStatus($this->importJob, 'ready_to_run'); diff --git a/app/Import/Specifics/AbnAmroDescription.php b/app/Import/Specifics/AbnAmroDescription.php index dcdb8600c3..8eab9c29f3 100644 --- a/app/Import/Specifics/AbnAmroDescription.php +++ b/app/Import/Specifics/AbnAmroDescription.php @@ -122,8 +122,6 @@ class AbnAmroDescription implements SpecificInterface /** * Parses the current description in SEPA format. * - * @SuppressWarnings(PHPMD.CyclomaticComplexity) - * * @return bool true if the description is SEPA format, false otherwise */ protected function parseSepaDescription(): bool @@ -165,7 +163,7 @@ class AbnAmroDescription implements SpecificInterface // Set a new description for the current transaction. If none was given // set the description to type, name and reference $this->row[7] = $newDescription; - if (0 === \strlen($newDescription)) { + if ('' === $newDescription) { $this->row[7] = sprintf('%s - %s (%s)', $type, $name, $reference); } @@ -220,7 +218,7 @@ class AbnAmroDescription implements SpecificInterface // Set a new description for the current transaction. If none was given // set the description to type, name and reference $this->row[7] = $newDescription; - if (0 === \strlen($newDescription)) { + if ('' === $newDescription) { $this->row[7] = sprintf('%s - %s (%s)', $type, $name, $reference); } } diff --git a/app/Import/Specifics/PresidentsChoice.php b/app/Import/Specifics/PresidentsChoice.php index b01fdb28a5..2b067305d1 100644 --- a/app/Import/Specifics/PresidentsChoice.php +++ b/app/Import/Specifics/PresidentsChoice.php @@ -55,7 +55,7 @@ class PresidentsChoice implements SpecificInterface $row = array_values($row); // first, if column 2 is empty and 3 is not, do nothing. // if column 3 is empty and column 2 is not, move amount to column 3, *-1 - if (isset($row[3]) && 0 === \strlen($row[3])) { + if (isset($row[3]) && '' === $row[3]) { $row[3] = bcmul($row[2], '-1'); } if (isset($row[1])) { diff --git a/app/Import/Specifics/RabobankDescription.php b/app/Import/Specifics/RabobankDescription.php index 3927fedf27..38b9e33dd3 100644 --- a/app/Import/Specifics/RabobankDescription.php +++ b/app/Import/Specifics/RabobankDescription.php @@ -60,7 +60,7 @@ class RabobankDescription implements SpecificInterface $oppositeName = isset($row[6]) ? trim($row[6]) : ''; $alternateName = isset($row[10]) ? trim($row[10]) : ''; - if (\strlen($oppositeAccount) < 1 && \strlen($oppositeName) < 1) { + if ('' === $oppositeAccount && '' === $oppositeName) { Log::debug( sprintf( 'Rabobank specific: Opposite account and opposite name are' . @@ -71,7 +71,7 @@ class RabobankDescription implements SpecificInterface $row[6] = $alternateName; $row[10] = ''; } - if (!(\strlen($oppositeAccount) < 1 && \strlen($oppositeName) < 1)) { + if (!('' === $oppositeAccount && '' === $oppositeName)) { Log::debug('Rabobank specific: either opposite account or name are filled.'); } diff --git a/app/Import/Storage/ImportArrayStorage.php b/app/Import/Storage/ImportArrayStorage.php index 551bec3d14..e71e4986b1 100644 --- a/app/Import/Storage/ImportArrayStorage.php +++ b/app/Import/Storage/ImportArrayStorage.php @@ -102,7 +102,7 @@ class ImportArrayStorage // run rules, if configured to. $config = $this->importJob->configuration; - if (isset($config['apply-rules']) && $config['apply-rules'] === true) { + if (isset($config['apply-rules']) && true === $config['apply-rules']) { $this->setStatus('applying_rules'); $this->applyRules($collection); $this->setStatus('rules_applied'); @@ -118,7 +118,6 @@ class ImportArrayStorage * * @param Collection $collection * - * @throws FireflyException */ private function applyRules(Collection $collection): void { @@ -153,7 +152,7 @@ class ImportArrayStorage foreach ($array as $index => $transaction) { if (strtolower(TransactionType::TRANSFER) === $transaction['type']) { $count++; - Log::debug(sprintf('Row #%d is a transfer, increase count to %d', ($index + 1), $count)); + Log::debug(sprintf('Row #%d is a transfer, increase count to %d', $index + 1, $count)); } } if (0 === $count) { @@ -179,7 +178,7 @@ class ImportArrayStorage { unset($transaction['importHashV2']); $json = json_encode($transaction); - if ($json === false) { + if (false === $json) { throw new FireflyException('Could not encode import array. Please see the logs.', $transaction); // @codeCoverageIgnore } $hash = hash('sha256', $json, false); @@ -231,7 +230,6 @@ class ImportArrayStorage * @param string $hash * * @return int|null - * @throws FireflyException */ private function hashExists(string $hash): ?int { @@ -253,7 +251,7 @@ class ImportArrayStorage */ private function linkToTag(Collection $collection): void { - if ($collection->count() === 0) { + if (0 === $collection->count()) { return; } /** @var TagRepositoryInterface $repository */ @@ -321,7 +319,7 @@ class ImportArrayStorage [ 'description' => $transaction['description'] ?? '', 'amount' => $transaction['transactions'][0]['amount'] ?? 0, - 'date' => isset($transaction['date']) ? $transaction['date'] : '', + 'date' => $transaction['date'] ?? '', ] ); } @@ -365,24 +363,22 @@ class ImportArrayStorage ); continue; } - if ($this->checkForTransfers) { - if ($this->transferExists($transaction)) { - $this->logDuplicateTransfer($transaction); - $this->repository->addErrorMessage( - $this->importJob, sprintf( - 'Row #%d ("%s") could not be imported. Such a transfer already exists.', - $index, - $transaction['description'] - ) - ); - continue; - } + if ($this->checkForTransfers && $this->transferExists($transaction)) { + $this->logDuplicateTransfer($transaction); + $this->repository->addErrorMessage( + $this->importJob, sprintf( + 'Row #%d ("%s") could not be imported. Such a transfer already exists.', + $index, + $transaction['description'] + ) + ); + continue; } $transaction['importHashV2'] = $hash; $toStore[] = $transaction; } $count = \count($toStore); - if ($count === 0) { + if (0 === $count) { Log::info('No transactions to store left!'); return new Collection; @@ -411,7 +407,7 @@ class ImportArrayStorage Log::debug(sprintf('Going to store entry %d of %d', $index + 1, $count)); // convert the date to an object: $store['date'] = Carbon::createFromFormat('Y-m-d', $store['date']); - $store['description'] = $store['description'] === '' ? '(empty description)' : $store['description']; + $store['description'] = '' === $store['description'] ? '(empty description)' : $store['description']; // store the journal. try { $journal = $this->journalRepos->store($store); @@ -445,7 +441,7 @@ class ImportArrayStorage return false; } // how many hits do we need? - $requiredHits = count($transaction['transactions']) * 4; + $requiredHits = \count($transaction['transactions']) * 4; $totalHits = 0; Log::debug(sprintf('Required hits for transfer comparison is %d', $requiredHits)); Log::debug(sprintf('Array has %d transactions.', \count($transaction['transactions']))); @@ -504,6 +500,7 @@ class ImportArrayStorage // compare source and destination id's $transferSourceIDs = [(int)$transfer->account_id, (int)$transfer->opposing_account_id]; sort($transferSourceIDs); + /** @noinspection DisconnectedForeachInstructionInspection */ Log::debug('Comparing current transaction source+dest IDs', $currentSourceIDs); Log::debug('.. with current transfer source+dest IDs', $transferSourceIDs); if ($currentSourceIDs === $transferSourceIDs) { @@ -515,6 +512,7 @@ class ImportArrayStorage // compare source and destination names $transferSource = [(string)$transfer->account_name, (int)$transfer->opposing_account_name]; sort($transferSource); + /** @noinspection DisconnectedForeachInstructionInspection */ Log::debug('Comparing current transaction source+dest names', $currentSourceNames); Log::debug('.. with current transfer source+dest names', $transferSource); if ($currentSourceNames === $transferSource) {