Skip to content

chore(phpstan): enable level 4 for lib/ helpers, Pages/ and installer - #1883

Merged
JohnVillalovos merged 1 commit into
developfrom
jlvillal/phpstan
Nov 9, 2026
Merged

JohnVillalovos merged 1 commit into
developfrom
jlvillal/phpstan

Conversation

@JohnVillalovos

Copy link
Copy Markdown
Collaborator

Fix the level 4 findings in lib/Common, lib/Config, lib/Database, lib/Email, Pages/ and Web/install, and remove them from the phpstan_next exclude list. None of them changes behaviour:

  • Drop properties that are written but never read: SqlCommand's parameter name and value copies and ReservationWaitlistPage's save flag (the waitlist presenter never reports a save result).
  • Drop the primaryResource parameter of ReservationEmailTemplateContext and the userRepository parameter of ReservationCreatedEmailAdmin and ReservationRequiresApprovalEmailAdmin. Both became unused when the email context switched to AllResources() in 0ae98b6.
  • Remove checks that can never fail: is_array() on debug_backtrace(), is_null() on a Date parameter, method_exists() on ConfigKeys methods that AbstractConfigKeys always defines, and duplicate '' / false comparisons.
  • Make ExceptionHandler::$handler nullable instead of relying on isset(), and declare the real return types of the installer permission check and the rich text attribute sanitizer.
  • Document that legacy config files fill in $conf through require, so PHPStan no longer treats the 'settings' check as impossible.

Assisted-by: Claude:claude-opus-5-5

Fix the level 4 findings in lib/Common, lib/Config, lib/Database,
lib/Email, Pages/ and Web/install, and remove them from the
phpstan_next exclude list. None of them changes behaviour:

- Drop properties that are written but never read: SqlCommand's
  parameter name and value copies and ReservationWaitlistPage's save
  flag (the waitlist presenter never reports a save result).
- Drop the primaryResource parameter of ReservationEmailTemplateContext
  and the userRepository parameter of ReservationCreatedEmailAdmin and
  ReservationRequiresApprovalEmailAdmin. Both became unused when the
  email context switched to AllResources() in 0ae98b6.
- Remove checks that can never fail: is_array() on debug_backtrace(),
  is_null() on a Date parameter, method_exists() on ConfigKeys methods
  that AbstractConfigKeys always defines, and duplicate '' / false
  comparisons.
- Make ExceptionHandler::$handler nullable instead of relying on
  isset(), and declare the real return types of the installer
  permission check and the rich text attribute sanitizer.
- Document that legacy config files fill in $conf through require, so
  PHPStan no longer treats the 'settings' check as impossible.

Assisted-by: Claude:claude-opus-5-5

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟢 Approval recommended

No active review issues remain, with the reported PHPStan findings resolved and corresponding exclusions removed across the affected code and tests.

No open issues 🎉

What changed in this PR

This pull request enables PHPStan level 4 for selected helpers, pages, email components, and installer code by resolving reported findings and removing the corresponding exclusions.

Changes:

  • Removes unused properties and constructor parameters.
  • Simplifies redundant or unreachable checks.
  • Adds explicit return and nullable type declarations.
  • Documents legacy configuration mutation through require.
  • Updates affected tests and PHPStan exclusions.
File Description
Web/​install/​index.php Declares the installer permission helper’s boolean return type.
tests/​lib/​Email/​Messages/​ReservationEmailMessageTest.php Updates email construction after removing an unused dependency.
tests/​Application/​Reservation/​ReservationEmailTemplateContextTest.php Updates context construction after removing an unused parameter.
tests/​Application/​Reservation/​AdminEmailNotificationTest.php Updates expected admin email construction for simplified constructors.
phpstan_next.neon Removes resolved paths from the PHPStan exclusion list.
Pages/​SchedulePage.php Removes a redundant empty-string comparison.
Pages/​Ajax/​ReservationWaitlistPage.php Removes unused save-state storage and keeps the setter as a no-op.
lib/​Email/​Messages/​ReservationRequiresApprovalEmailAdmin.php Removes an unused user repository parameter.
lib/​Email/​Messages/​ReservationEmailTemplateContext.php Removes an unused primary-resource parameter.
lib/​Email/​Messages/​ReservationEmailMessage.php Adjusts context construction for the simplified signature.
lib/​Email/​Messages/​ReservationCreatedEmailAdmin.php Removes unused repository state and constructor input.
lib/​Database/​SqlCommand.php Removes unused parameter copies and a redundant comparison.
lib/​Config/​MigrateConfig.php Documents legacy configuration mutation through require.
lib/​Config/​Configurator.php Documents legacy config loading and simplifies guaranteed method calls.
lib/​Config/​Configuration.php Documents legacy configuration mutation through require.
lib/​Common/​Security/​RichTextHtmlSanitizer.php Adds concrete array return types to sanitizer methods.
lib/​Common/​Logging/​Log.php Removes an impossible debug_backtrace() array check.
lib/​Common/​Logging/​ExceptionHandler.php Makes the static handler explicitly nullable.
lib/​Common/​Date.php Removes an impossible null branch for a non-nullable date parameter.
lib/​Application/​Reservation/​Notification/​AdminEmailNotification.php Updates notification message construction for simplified email constructors.

Review effort: Lite

@JohnVillalovos
JohnVillalovos merged commit 2e2a871 into develop Nov 9, 2026
19 checks passed
@JohnVillalovos
JohnVillalovos deleted the jlvillal/phpstan branch November 9, 2026 21:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants