From 1eb4ae3a2cd39cb08b7ccbce04096916913e51ed Mon Sep 17 00:00:00 2001 From: James Cole Date: Wed, 18 Mar 2026 05:26:50 +0100 Subject: [PATCH] Fix a variety of Mago issues. --- app/Exceptions/Handler.php | 1 + .../Report/Standard/YearReportGenerator.php | 10 +- .../Report/Tag/MonthReportGenerator.php | 16 ++- .../Admin/NotificationController.php | 2 +- .../Controllers/Auth/RegisterController.php | 4 +- .../Auth/ResetPasswordController.php | 2 +- .../Budget/BudgetLimitController.php | 10 +- .../PiggyBank/AmountController.php | 2 +- .../Controllers/PreferencesController.php | 131 +++++++++--------- .../Transaction/ConvertController.php | 2 +- .../Controllers/UserGroup/EditController.php | 2 +- mago.toml | 24 ++-- 12 files changed, 108 insertions(+), 98 deletions(-) diff --git a/app/Exceptions/Handler.php b/app/Exceptions/Handler.php index 339223ac27..0bba680727 100644 --- a/app/Exceptions/Handler.php +++ b/app/Exceptions/Handler.php @@ -250,6 +250,7 @@ class Handler extends ExceptionHandler 'json' => request()->acceptsJson(), 'method' => request()->method(), 'headers' => $headers, + // @mago-expect lint:no-request-all 'post' => 'POST' === request()->method() ? json_encode(request()->all()) : '', ]; diff --git a/app/Generator/Report/Standard/YearReportGenerator.php b/app/Generator/Report/Standard/YearReportGenerator.php index 2605cc06b8..9729830982 100644 --- a/app/Generator/Report/Standard/YearReportGenerator.php +++ b/app/Generator/Report/Standard/YearReportGenerator.php @@ -56,9 +56,13 @@ class YearReportGenerator implements ReportGeneratorInterface $reportType = 'default'; try { - $result = view('reports.default.year', ['accountIds' => $accountIds, 'reportType' => $reportType]) - ->with('start', $this->start) - ->with('end', $this->end) + $result = view('reports.default.year', + [ + 'accountIds' => $accountIds, + 'reportType' => $reportType, + 'start' => $this->start, + 'end' => $this->end, + ]) ->render() ; } catch (Throwable $e) { diff --git a/app/Generator/Report/Tag/MonthReportGenerator.php b/app/Generator/Report/Tag/MonthReportGenerator.php index 4e439dd0c2..ce88337267 100644 --- a/app/Generator/Report/Tag/MonthReportGenerator.php +++ b/app/Generator/Report/Tag/MonthReportGenerator.php @@ -63,13 +63,15 @@ class MonthReportGenerator implements ReportGeneratorInterface // render! try { - $result = view('reports.tag.month', ['accountIds' => $accountIds, 'reportType' => $reportType, 'tagIds' => $tagIds]) - ->with('start', $this->start) - ->with('end', $this->end) - ->with('tags', $this->tags) - ->with('accounts', $this->accounts) - ->render() - ; + $result = view('reports.tag.month', [ + 'accountIds' => $accountIds, + 'reportType' => $reportType, + 'tagIds' => $tagIds, + 'start' => $this->start, + 'end' => $this->end, + 'tags' => $this->tags, + 'accounts' => $this->accounts, + ])->render(); } catch (Throwable $e) { Log::error(sprintf('Cannot render reports.tag.month: %s', $e->getMessage())); Log::error($e->getTraceAsString()); diff --git a/app/Http/Controllers/Admin/NotificationController.php b/app/Http/Controllers/Admin/NotificationController.php index 2c78c6eead..986035a3b0 100644 --- a/app/Http/Controllers/Admin/NotificationController.php +++ b/app/Http/Controllers/Admin/NotificationController.php @@ -132,7 +132,7 @@ final class NotificationController extends Controller return redirect(route('settings.notification.index')); } - $all = $request->all(); + $all = $request->only(['channel']); $channel = $all['test_submit'] ?? ''; switch ($channel) { diff --git a/app/Http/Controllers/Auth/RegisterController.php b/app/Http/Controllers/Auth/RegisterController.php index f21f87a102..72f7b23c64 100644 --- a/app/Http/Controllers/Auth/RegisterController.php +++ b/app/Http/Controllers/Auth/RegisterController.php @@ -83,8 +83,8 @@ final class RegisterController extends Controller throw new FireflyException('Registration is currently not available :('); } - $this->validator($request->all())->validate(); - $user = $this->createUser($request->all()); + $this->validator($request->only(['email', 'password']))->validate(); + $user = $this->createUser($request->only(['email', 'password'])); Log::info(sprintf('Registered new user %s', $user->email)); $owner = new OwnerNotifiable(); event(new NewUserRegistered($owner, $user)); diff --git a/app/Http/Controllers/Auth/ResetPasswordController.php b/app/Http/Controllers/Auth/ResetPasswordController.php index fa2c8f15d9..259abceeb2 100644 --- a/app/Http/Controllers/Auth/ResetPasswordController.php +++ b/app/Http/Controllers/Auth/ResetPasswordController.php @@ -132,7 +132,7 @@ final class ResetPasswordController extends Controller $allowRegistration = false; } - return view('auth.passwords.reset')->with([ + return view('auth.passwords.reset', [ 'token' => $token, 'email' => $request->email, 'allowRegistration' => $allowRegistration, diff --git a/app/Http/Controllers/Budget/BudgetLimitController.php b/app/Http/Controllers/Budget/BudgetLimitController.php index 3891ef1ae6..e26a40b841 100644 --- a/app/Http/Controllers/Budget/BudgetLimitController.php +++ b/app/Http/Controllers/Budget/BudgetLimitController.php @@ -137,15 +137,15 @@ final class BudgetLimitController extends Controller */ public function store(Request $request): JsonResponse|RedirectResponse { - Log::debug('Going to store new budget-limit.', $request->all()); + Log::debug('Going to store new budget-limit.'); // first search for existing one and update it if necessary. - $currency = $this->currencyRepos->find((int) $request->get('transaction_currency_id')); - $budget = $this->repository->find((int) $request->get('budget_id')); + $currency = $this->currencyRepos->find((int) $request->input('transaction_currency_id')); + $budget = $this->repository->find((int) $request->input('budget_id')); if (!$currency instanceof TransactionCurrency || !$budget instanceof Budget) { throw new FireflyException('No valid currency or budget.'); } - $start = Carbon::createFromFormat('Y-m-d', $request->get('start')); - $end = Carbon::createFromFormat('Y-m-d', $request->get('end')); + $start = Carbon::createFromFormat('Y-m-d', $request->input('start')); + $end = Carbon::createFromFormat('Y-m-d', $request->input('end')); if (!$start instanceof Carbon || !$end instanceof Carbon) { return response()->json(); diff --git a/app/Http/Controllers/PiggyBank/AmountController.php b/app/Http/Controllers/PiggyBank/AmountController.php index e78956e733..5acbee04be 100644 --- a/app/Http/Controllers/PiggyBank/AmountController.php +++ b/app/Http/Controllers/PiggyBank/AmountController.php @@ -135,7 +135,7 @@ final class AmountController extends Controller */ public function postAdd(Request $request, PiggyBank $piggyBank): RedirectResponse { - $data = $request->all(); + $data = $request->only(['amount']); $amounts = $data['amount'] ?? []; $total = '0'; Log::debug('Start with loop.'); diff --git a/app/Http/Controllers/PreferencesController.php b/app/Http/Controllers/PreferencesController.php index f20cea2280..a04cccf52a 100644 --- a/app/Http/Controllers/PreferencesController.php +++ b/app/Http/Controllers/PreferencesController.php @@ -44,7 +44,6 @@ use Illuminate\Support\Facades\Log; use Illuminate\View\View; use JsonException; use Safe\Exceptions\FilesystemException; - use function Safe\file_get_contents; use function Safe\json_decode; @@ -61,7 +60,7 @@ final class PreferencesController extends Controller parent::__construct(); $this->middleware(static function ($request, $next) { - app('view')->share('title', (string) trans('firefly.preferences')); + app('view')->share('title', (string)trans('firefly.preferences')); app('view')->share('mainTitleIcon', 'fa-gear'); return $next($request); @@ -76,22 +75,22 @@ final class PreferencesController extends Controller * @throws FireflyException * @throws FilesystemException */ - public function index(AccountRepositoryInterface $repository): Factory|\Illuminate\Contracts\View\View + public function index(AccountRepositoryInterface $repository): Factory | \Illuminate\Contracts\View\View { - $accounts = $repository->getAccountsByType([ - AccountTypeEnum::DEFAULT->value, - AccountTypeEnum::ASSET->value, - AccountTypeEnum::LOAN->value, - AccountTypeEnum::DEBT->value, - AccountTypeEnum::MORTGAGE->value, - ]); - $isDocker = config('firefly.is_docker'); - $groupedAccounts = []; + $accounts = $repository->getAccountsByType([ + AccountTypeEnum::DEFAULT->value, + AccountTypeEnum::ASSET->value, + AccountTypeEnum::LOAN->value, + AccountTypeEnum::DEBT->value, + AccountTypeEnum::MORTGAGE->value, + ]); + $isDocker = config('firefly.is_docker'); + $groupedAccounts = []; /** @var Account $account */ foreach ($accounts as $account) { - $type = $account->accountType->type; - $role = sprintf('opt_group_%s', $repository->getMetaValue($account, 'account_role')); + $type = $account->accountType->type; + $role = sprintf('opt_group_%s', $repository->getMetaValue($account, 'account_role')); if (in_array($type, [AccountTypeEnum::MORTGAGE->value, AccountTypeEnum::DEBT->value, AccountTypeEnum::LOAN->value], true)) { $role = sprintf('opt_group_l_%s', $type); @@ -100,48 +99,48 @@ final class PreferencesController extends Controller if ('opt_group_' === $role) { $role = 'opt_group_defaultAsset'; } - $groupedAccounts[(string) trans(sprintf('firefly.%s', $role))][$account->id] = $account->name; + $groupedAccounts[(string)trans(sprintf('firefly.%s', $role))][$account->id] = $account->name; } ksort($groupedAccounts); /** @var array $accountIds */ - $accountIds = $accounts->pluck('id')->toArray(); - $viewRange = Navigation::getViewRange(false); - $frontpageAccountsPref = Preferences::get('frontpageAccounts', $accountIds); - $frontpageAccounts = $frontpageAccountsPref->data; + $accountIds = $accounts->pluck('id')->toArray(); + $viewRange = Navigation::getViewRange(false); + $frontpageAccountsPref = Preferences::get('frontpageAccounts', $accountIds); + $frontpageAccounts = $frontpageAccountsPref->data; if (!is_array($frontpageAccounts)) { $frontpageAccounts = $accountIds; } - $language = Steam::getLanguage(); - $languages = config('firefly.languages'); - $locale = Preferences::get('locale', config('firefly.default_locale', 'equal'))->data; - $listPageSize = Preferences::get('listPageSize', 50)->data; - $darkMode = Preferences::get('darkMode', 'browser')->data; - $customFiscalYear = Preferences::get('customFiscalYear', 0)->data; - $fiscalYearStartStr = Preferences::get('fiscalYearStart', '01-01')->data; - $convertToPrimary = $this->convertToPrimary; + $language = Steam::getLanguage(); + $languages = config('firefly.languages'); + $locale = Preferences::get('locale', config('firefly.default_locale', 'equal'))->data; + $listPageSize = Preferences::get('listPageSize', 50)->data; + $darkMode = Preferences::get('darkMode', 'browser')->data; + $customFiscalYear = Preferences::get('customFiscalYear', 0)->data; + $fiscalYearStartStr = Preferences::get('fiscalYearStart', '01-01')->data; + $convertToPrimary = $this->convertToPrimary; if (is_array($fiscalYearStartStr)) { $fiscalYearStartStr = '01-01'; } - $fiscalYearStart = sprintf('%s-%s', Carbon::now()->format('Y'), (string) $fiscalYearStartStr); - $tjOptionalFields = Preferences::get('transaction_journal_optional_fields', [])->data; - $availableDarkModes = config('firefly.available_dark_modes'); + $fiscalYearStart = sprintf('%s-%s', Carbon::now()->format('Y'), (string)$fiscalYearStartStr); + $tjOptionalFields = Preferences::get('transaction_journal_optional_fields', [])->data; + $availableDarkModes = config('firefly.available_dark_modes'); // notifications settings - $slackUrl = Preferences::getEncrypted('slack_webhook_url', '')->data; - $pushoverAppToken = (string) Preferences::getEncrypted('pushover_app_token', '')->data; - $pushoverUserToken = (string) Preferences::getEncrypted('pushover_user_token', '')->data; - $ntfyServer = Preferences::getEncrypted('ntfy_server', 'https://ntfy.sh')->data; - $ntfyTopic = (string) Preferences::getEncrypted('ntfy_topic', '')->data; - $ntfyAuth = '1' === Preferences::get('ntfy_auth', false)->data; - $ntfyUser = Preferences::getEncrypted('ntfy_user', '')->data; - $ntfyPass = (string) Preferences::getEncrypted('ntfy_pass', '')->data; - $channels = config('notifications.channels'); - $forcedAvailability = []; - $anonymous = Steam::anonymous(); + $slackUrl = Preferences::getEncrypted('slack_webhook_url', '')->data; + $pushoverAppToken = (string)Preferences::getEncrypted('pushover_app_token', '')->data; + $pushoverUserToken = (string)Preferences::getEncrypted('pushover_user_token', '')->data; + $ntfyServer = Preferences::getEncrypted('ntfy_server', 'https://ntfy.sh')->data; + $ntfyTopic = (string)Preferences::getEncrypted('ntfy_topic', '')->data; + $ntfyAuth = '1' === Preferences::get('ntfy_auth', false)->data; + $ntfyUser = Preferences::getEncrypted('ntfy_user', '')->data; + $ntfyPass = (string)Preferences::getEncrypted('ntfy_pass', '')->data; + $channels = config('notifications.channels'); + $forcedAvailability = []; + $anonymous = Steam::anonymous(); // notification preferences - $notifications = []; + $notifications = []; foreach (config('notifications.notifications.user') as $key => $info) { if (true === $info['enabled']) { $notifications[$key] = [ @@ -167,7 +166,7 @@ final class PreferencesController extends Controller Log::error($e->getMessage()); $locales = []; } - $locales = ['equal' => (string) trans('firefly.equal_to_language')] + $locales; + $locales = ['equal' => (string)trans('firefly.equal_to_language')] + $locales; // an important fallback is that the frontPageAccount array gets refilled automatically // when it turns up empty. if (0 === count($frontpageAccounts)) { @@ -231,16 +230,17 @@ final class PreferencesController extends Controller Log::debug('postIndex for preferences.'); // front page accounts $frontpageAccounts = []; - if (is_array($request->get('frontpageAccounts')) && count($request->get('frontpageAccounts')) > 0) { - foreach ($request->get('frontpageAccounts') as $id) { - $frontpageAccounts[] = (int) $id; + if (is_array($request->input('frontpageAccounts')) && count($request->input('frontpageAccounts')) > 0) { + foreach ($request->input('frontpageAccounts') as $id) { + $frontpageAccounts[] = (int)$id; } Log::debug('Update frontpageAccounts', $frontpageAccounts); Preferences::set('frontpageAccounts', $frontpageAccounts); } // extract notifications: - $all = $request->all(); + $keys = array_map(function(string $value): string {return sprintf('notification_%s', $value);}, array_keys(config('notifications.notifications.user'))); + $all = $request->only($keys); foreach (config('notifications.notifications.user') as $key => $info) { $key = sprintf('notification_%s', $key); if (array_key_exists($key, $all)) { @@ -252,10 +252,11 @@ final class PreferencesController extends Controller Preferences::set($key, false); } } + unset($all); // view range: - Log::debug(sprintf('Let viewRange to "%s"', $request->get('viewRange'))); - Preferences::set('viewRange', $request->get('viewRange')); + Log::debug(sprintf('Let viewRange to "%s"', $request->input('viewRange'))); + Preferences::set('viewRange', $request->input('viewRange')); // forget session values: session()->forget('start'); session()->forget('end'); @@ -264,6 +265,7 @@ final class PreferencesController extends Controller // notification settings, cannot be set by the demo user. if (!auth()->user()->hasRole('demo')) { $variables = ['slack_webhook_url', 'pushover_app_token', 'pushover_user_token', 'ntfy_server', 'ntfy_topic', 'ntfy_user', 'ntfy_pass']; + $all = $request->only($variables); foreach ($variables as $variable) { if ('' === $all[$variable]) { Preferences::delete($variable); @@ -274,9 +276,10 @@ final class PreferencesController extends Controller } Preferences::set('ntfy_auth', $all['ntfy_auth'] ?? false); } + unset($all); // convert primary - $convertToPrimary = 1 === (int) $request->get('convertToPrimary'); + $convertToPrimary = 1 === (int)$request->input('convertToPrimary'); if ($convertToPrimary && !$this->convertToPrimary) { // set to true! Log::debug('User sets convertToPrimary to true.'); @@ -288,9 +291,9 @@ final class PreferencesController extends Controller Preferences::set('convert_to_primary', $convertToPrimary); // custom fiscal year - $customFiscalYear = 1 === (int) $request->get('customFiscalYear'); + $customFiscalYear = 1 === (int)$request->input('customFiscalYear'); Preferences::set('customFiscalYear', $customFiscalYear); - $fiscalYearString = (string) $request->get('fiscalYearStart'); + $fiscalYearString = (string)$request->input('fiscalYearStart'); if ('' !== $fiscalYearString) { $fiscalYearStart = Carbon::parse($fiscalYearString, config('app.timezone'))->format('m-d'); Preferences::set('fiscalYearStart', $fiscalYearStart); @@ -298,15 +301,15 @@ final class PreferencesController extends Controller // save page size: Preferences::set('listPageSize', 50); - $listPageSize = (int) $request->get('listPageSize'); + $listPageSize = (int)$request->input('listPageSize'); if ($listPageSize > 0 && $listPageSize < 1337) { Preferences::set('listPageSize', $listPageSize); } // language: /** @var Preference $currentLang */ - $currentLang = Preferences::get('language', 'en_US'); - $lang = $request->get('language'); + $currentLang = Preferences::get('language', 'en_US'); + $lang = $request->input('language'); if (array_key_exists($lang, config('firefly.languages'))) { Preferences::set('language', $lang); } @@ -317,14 +320,14 @@ final class PreferencesController extends Controller // same for locale: if (!auth()->user()->hasRole('demo')) { - $locale = (string) $request->get('locale'); + $locale = (string)$request->input('locale'); $locale = '' === $locale ? null : $locale; Preferences::set('locale', $locale); } // optional fields for transactions: - $setOptions = $request->get('tj') ?? []; - $optionalTj = [ + $setOptions = $request->input('tj') ?? []; + $optionalTj = [ 'interest_date' => array_key_exists('interest_date', $setOptions), 'book_date' => array_key_exists('book_date', $setOptions), 'process_date' => array_key_exists('process_date', $setOptions), @@ -341,17 +344,17 @@ final class PreferencesController extends Controller Preferences::set('transaction_journal_optional_fields', $optionalTj); // dark mode - $darkMode = $request->get('darkMode') ?? 'browser'; + $darkMode = $request->input('darkMode') ?? 'browser'; if (in_array($darkMode, config('firefly.available_dark_modes'), true)) { Preferences::set('darkMode', $darkMode); } // anonymous amounts? - $anonymous = '1' === $request->get('anonymous'); + $anonymous = '1' === $request->input('anonymous'); Preferences::set('anonymous', $anonymous); // save and continue - session()->flash('success', (string) trans('firefly.saved_preferences')); + session()->flash('success', (string)trans('firefly.saved_preferences')); Preferences::mark(); Log::debug('Done saving settings.'); @@ -360,12 +363,12 @@ final class PreferencesController extends Controller public function testNotification(Request $request): mixed { - $all = $request->all(); + $all = $request->only(['channel']); $channel = $all['channel'] ?? ''; switch ($channel) { default: - session()->flash('error', (string) trans('firefly.notification_test_failed', ['channel' => $channel])); + session()->flash('error', (string)trans('firefly.notification_test_failed', ['channel' => $channel])); break; @@ -377,7 +380,7 @@ final class PreferencesController extends Controller $user = auth()->user(); Log::debug(sprintf('Now in testNotification("%s") controller.', $channel)); event(new UserTestsNotificationChannel($channel, $user)); - session()->flash('success', (string) trans('firefly.notification_test_executed', ['channel' => $channel])); + session()->flash('success', (string)trans('firefly.notification_test_executed', ['channel' => $channel])); } return ''; diff --git a/app/Http/Controllers/Transaction/ConvertController.php b/app/Http/Controllers/Transaction/ConvertController.php index 29be4fda26..c4f150960b 100644 --- a/app/Http/Controllers/Transaction/ConvertController.php +++ b/app/Http/Controllers/Transaction/ConvertController.php @@ -153,7 +153,7 @@ final class ConvertController extends Controller foreach ($group->transactionJournals as $journal) { // catch FF exception. try { - $this->convertJournal($journal, $destinationType, $request->all()); + $this->convertJournal($journal, $destinationType, $request->only(['source_id', 'source_name', 'destination_id', 'destination_name',])); } catch (FireflyException $e) { session()->flash('error', $e->getMessage()); diff --git a/app/Http/Controllers/UserGroup/EditController.php b/app/Http/Controllers/UserGroup/EditController.php index 123fb1b612..83a1527abe 100644 --- a/app/Http/Controllers/UserGroup/EditController.php +++ b/app/Http/Controllers/UserGroup/EditController.php @@ -43,6 +43,6 @@ final class EditController extends Controller $mainTitleIcon = 'fa-book'; Log::debug(sprintf('Now at %s', __METHOD__)); - return view('administrations.edit')->with(['title' => $title, 'subTitle' => $subTitle, 'mainTitleIcon' => $mainTitleIcon]); + return view('administrations.edit', ['title' => $title, 'subTitle' => $subTitle, 'mainTitleIcon' => $mainTitleIcon]); } } diff --git a/mago.toml b/mago.toml index 2a1706ee06..69fb60c2b1 100644 --- a/mago.toml +++ b/mago.toml @@ -24,18 +24,12 @@ integrations = ["symfony", "laravel", "phpunit"] excludes = ["app/Providers/AppServiceProvider.php"] # Additionally excluded from linter only [linter.rules] -ambiguous-function-call = { enabled = false } -literal-named-argument = { enabled = false } -halstead = { effort-threshold = 7000, enabled = false } -prefer-early-continue = { enabled = false } -cyclomatic-complexity = { enabled = false } -middleware-in-routes = { enabled = false } -too-many-methods = { enabled = false } -kan-defect = { enabled = false } -no-request-all = { enabled = false } -tagged-todo = { enabled = false } -too-many-properties = { enabled = false } -prefer-view-array = { enabled = false } +halstead = { effort-threshold = 250000, volume-threshold = 200000, difficulty-threshold = 1000, enabled = true } # way too high +cyclomatic-complexity = { enabled = true, threshold = 255 } # way too high +kan-defect = { enabled = true, threshold = 20 } # way too high +too-many-properties = { enabled = true, threshold = 25 } # way too high +no-request-all = { enabled = true } +prefer-view-array = { enabled = false } # to be continued. assertion-style = { enabled = false } no-boolean-flag-parameter = { enabled = false } prefer-static-closure = { enabled = false } @@ -45,6 +39,12 @@ tagged-fixme = { enabled = false } excessive-parameter-list = { enabled = false } prefer-first-class-callable={ enabled = false } prefer-arrow-function = { enabled = false } +ambiguous-function-call = { enabled = false } # very specific rule. +literal-named-argument = { enabled = false } # insanely specific. +prefer-early-continue = { enabled = false } # does NOT improve readability. +middleware-in-routes = { enabled = false } # I like to do this. +too-many-methods = { enabled = false, threshold = 100 } # arbitrary nonsense. +tagged-todo = { enabled = false } # weird rule