phpstan config updates and remove another private unused function#2658
Conversation
WalkthroughReplaced broad PHPStan global ignores with many file-scoped ignore rules in Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
phpstan.neon (4)
31-31: Tightened “expects …” ignore is fine, but the regex is a bit sloppy.
(...|bool)+allows repeated tokens and can overmatch; consider removing the trailing+(and using a non-capturing group) to keep this predictable.- - '#expects (array|string|int|callable|object|stdClass|float|bool)+#' + - '#expects (?:array|string|int|callable|object|stdClass|float|bool)#'
169-176: “is unused” ignore: make the regex more exact to avoid accidental matches.
#is unused.#treats.as “any char”. If the intent is the literal period, escape it and optionally anchor end-of-string.- message: '#is unused.#' + message: '#is unused\.\s*$#'
180-182:fwriteignore looks appropriately scoped; consider switching to identifier-based ignores (if supported).Message-based ignores tend to break on PHPStan wording changes. If phpstan/phpstan 1.4.0 in this repo supports
identifier:for ignoreErrors, it’s typically more stable than matching message text.
183-280: The#should return#suppression is very broad; consider narrowing or using identifiers/baseline tooling.Given
reportUnmatchedIgnoredErrors: false(Line 3), any mistyped/stale ignores here will silently linger. Two options to reduce long-term drift:
- Prefer
identifier:-based ignores (if supported by your phpstan/phpstan version).- Or ensure CI runs with unmatched-ignored reporting enabled at least periodically (even if you keep it off locally).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
phpstan.neon(2 hunks)square/controllers/FrmSquareLiteSettingsController.php(0 hunks)
💤 Files with no reviewable changes (1)
- square/controllers/FrmSquareLiteSettingsController.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (6)
- GitHub Check: Cypress
- GitHub Check: PHP 7.4 tests in WP trunk
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: Cypress
- GitHub Check: PHP 8 tests in WP trunk
- GitHub Check: PHP 7.4 tests in WP trunk
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
phpstan.neon (1)
182-280: Large path list documents outstanding return type issues.The extensive list of 99 files with
#should return#ignores effectively documents which files have outstanding return type issues. While this is consistent with the PR's goal of narrowing exceptions to specific files, consider tracking these as technical debt to address incrementally. This makes the configuration more maintainable long-term.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
phpstan.neon(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Cypress
🔇 Additional comments (3)
phpstan.neon (3)
31-31: Stricter type checking for resource types.Removing
resourcefrom the global expects ignore and handling it with a targeted path-specific ignore (line 179-180) improves type safety across the codebase.
176-177: Regex escaping is now correct.The past review concern about escaping the
??operator has been resolved. The pattern\?\?on line 176 correctly escapes both question marks to match the literal null coalescing operator.
168-180: Well-structured path-specific ignores.Moving from global ignores to targeted, path-specific suppressions is a best practice. This allows PHPStan to enforce rules more strictly across the codebase while making necessary exceptions only for specific files.
phpstan config updates and remove another private unused function
This update:
get_default_settings_attsin the Square Settings controller.