Add PHP 8.4 Support #67
Conversation
WalkthroughThe codebase updates method signatures across several helper classes, removing nullable array type hints in favor of untyped parameters to address PHP 8.4 deprecation. The continuous integration workflow is also updated to test against PHP 8.3 and 8.4 and to use newer GitHub Actions versions. Changes
Sequence Diagram(s)sequenceDiagram
participant CI as GitHub Actions CI
participant PHP as PHP Interpreter
participant Code as Helper Classes
CI->>PHP: Run tests on PHP 8.3, 8.4
PHP->>Code: Invoke helper methods (with untyped parameters)
Code-->>PHP: Process attributes/specs (no type errors on null/array)
PHP-->>CI: Return test results
Assessment against linked issues
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (4)
src/Helper/AbstractList.php (1)
56-62: Add runtime type casting for consistency with other helper classes.While the type hint removal addresses PHP 8.4 compatibility, this implementation lacks the runtime type safety that other helper classes in this PR provide. The method still expects an array based on the PHPDoc comment (line 51) and property initialization (line 27).
Consider adding runtime type casting for consistency:
public function __invoke($attr = null) { if ($attr !== null) { - $this->attr = $attr; + $this->attr = (array) $attr; } return $this; }This approach matches the pattern used in
src/Helper/Styles.phpand maintains type safety while preserving PHP 8.4 compatibility.src/Helper/Input/AbstractInput.php (1)
77-83: Inconsistent type handling with theprepmethod.The
__invokemethod now accepts any type, but theprepmethod it calls (line 80) still has an array type hint (line 101). This inconsistency could cause runtime errors when non-array values are passed.Consider adding runtime type casting to ensure consistency:
public function __invoke($spec = null) { if ($spec !== null) { - $this->prep($spec); + $this->prep((array) $spec); } return $this; }This ensures that
prepreceives an array as expected while maintaining the flexible parameter type for__invoke.src/Helper/Input/Select.php (1)
75-85: Same inconsistency issue as AbstractInput - type mismatch withprepmethod.Similar to
AbstractInput.php, this method now accepts any type but callsprep(line 78) which expects an array type. This could cause runtime errors.Apply the same fix as suggested for
AbstractInput.php:public function __invoke($spec = null) { if ($spec !== null) { - $this->prep($spec); + $this->prep((array) $spec); $this->attribs($this->attribs); $this->options($this->options); $this->selected($this->value); } return $this; }src/Helper/Input.php (1)
31-47: Add runtime type casting to prevent array access errors.The method removes the array type hint but still accesses
$specas an array (lines 37, 41, 42, 43) without runtime type casting. This could cause errors if non-array values are passed.Add runtime type casting for safety:
public function __invoke($spec = null) { if ($spec === null) { return $this; } + $spec = (array) $spec; + if (empty($spec['type'])) { $spec['type'] = 'text'; } if (empty($spec['attribs']['name'])) { $spec['attribs']['name'] = $spec['name']; } $input = $this->get($spec['type']); return $input($spec); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
.github/workflows/continuous-integration.yml(3 hunks)src/Helper/AbstractList.php(1 hunks)src/Helper/Input.php(1 hunks)src/Helper/Input/AbstractInput.php(1 hunks)src/Helper/Input/Select.php(1 hunks)src/Helper/Styles.php(9 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (4)
src/Helper/Input/Select.php (2)
src/Helper/Input.php (1)
__invoke(31-47)src/Helper/Input/AbstractInput.php (1)
__invoke(77-83)
src/Helper/Input.php (5)
src/Helper/AbstractList.php (1)
__invoke(56-62)src/Helper/Input/AbstractInput.php (1)
__invoke(77-83)src/Escaper/AbstractEscaper.php (1)
__invoke(73-73)src/Helper/Input/Select.php (1)
__invoke(75-85)src/Helper/Title.php (1)
__invoke(39-45)
src/Helper/Input/AbstractInput.php (5)
src/Helper/Input.php (1)
__invoke(31-47)src/Helper/AbstractList.php (1)
__invoke(56-62)src/Helper/Input/Select.php (1)
__invoke(75-85)src/Helper/Title.php (1)
__invoke(39-45)src/Helper/Label.php (1)
__invoke(68-79)
src/Helper/Styles.php (4)
src/Helper/Scripts.php (7)
add(42-49)attr(184-191)addCond(67-75)addInternal(88-94)addCondInternal(108-116)beginInternal(129-133)beginCondInternal(146-150)src/Helper/Links.php (1)
add(31-35)src/Helper/Metas.php (1)
add(31-35)src/Escaper.php (2)
attr(176-179)css(190-193)
🪛 actionlint (1.7.7)
.github/workflows/continuous-integration.yml
48-48: the runner of "actions/cache@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
61-61: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (3)
.github/workflows/continuous-integration.yml (2)
29-30: Double-check ecosystem readiness for PHP 8.3/8.4Great to see the new versions in the matrix, but two moving parts may still lag behind:
composer:2.2was released long before 8.4 existed – verify it bootstraps correctly.xdebugbuilds for bleeding-edge PHP sometimes trail GA releases.Please run the workflow once on a fork or use
shivammathur/setup-php@v2’scoverage: noneflag for 8.4 until binaries are available.
33-33: Checkout action bumped to v4 – nice catch
Latest major brings Node 20 and improved performance. No further action needed.src/Helper/Styles.php (1)
42-49: Excellent implementation of PHP 8.4 compatibility fix.This file demonstrates the correct approach for addressing PHP 8.4 compatibility:
- Type hint removal: All
arraytype hints are removed from$attrparameters- Runtime type casting: Proper use of
(array) $attrcasting maintains type safety- Consistent pattern: Applied consistently across all affected methods
This approach successfully resolves the PHP 8.4 null parameter issue while preserving backward compatibility and type safety.
This implementation should serve as the reference pattern for the other helper classes in this PR.
Also applies to: 67-76, 88-93, 106-111, 129-137, 149-157, 170-179, 218-231, 244-259
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
.github/workflows/continuous-integration.yml (1)
61-61: Move tocodecov-action@v4for Node 20 runners
Staying on v3 (Node 16) already triggers the “runner too old” warning flagged by actionlint. Updating is a one-liner:- uses: codecov/codecov-action@v3 + uses: codecov/codecov-action@v4
🧹 Nitpick comments (1)
.github/workflows/continuous-integration.yml (1)
92-94: Pin PHPStan incomposer.jsoninstead of installing ad-hoc in CIInstalling PHPStan with
composer requireat runtime mutatescomposer.lock, loses the version pin, and costs an extra network round-trip on every run. Prefer committing the dev-dependency and dropping this step:- - name: Install PHPStan - run: composer require --dev phpstan/phpstan --with-all-dependenciesAdd
"phpstan/phpstan": "^1.11"(or whatever version you need) underrequire-devincomposer.json, thencomposer installwill bring it in for both local devs and CI.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
.github/workflows/continuous-integration.yml(3 hunks)composer.json(1 hunks)src/Escaper.php(3 hunks)src/Escaper/AbstractEscaper.php(1 hunks)src/Escaper/HtmlEscaper.php(1 hunks)src/EscaperFactory.php(0 hunks)src/Helper/AbstractHelper.php(1 hunks)src/Helper/AbstractList.php(4 hunks)src/Helper/AbstractSeries.php(1 hunks)src/Helper/Input.php(1 hunks)src/Helper/Input/AbstractChecked.php(2 hunks)src/Helper/Input/AbstractInput.php(3 hunks)src/Helper/Input/Select.php(2 hunks)src/Helper/Scripts.php(3 hunks)src/Helper/Styles.php(9 hunks)src/HelperLocator.php(1 hunks)
💤 Files with no reviewable changes (1)
- src/EscaperFactory.php
✅ Files skipped from review due to trivial changes (8)
- src/HelperLocator.php
- src/Helper/Input/AbstractChecked.php
- composer.json
- src/Helper/AbstractHelper.php
- src/Escaper/AbstractEscaper.php
- src/Helper/AbstractSeries.php
- src/Escaper/HtmlEscaper.php
- src/Escaper.php
🚧 Files skipped from review as they are similar to previous changes (5)
- src/Helper/AbstractList.php
- src/Helper/Input/Select.php
- src/Helper/Input.php
- src/Helper/Input/AbstractInput.php
- src/Helper/Styles.php
🧰 Additional context used
🪛 actionlint (1.7.7)
.github/workflows/continuous-integration.yml
61-61: the runner of "codecov/codecov-action@v3" action is too old to run on GitHub Actions. update the action's version to fix this issue
(action)
🔇 Additional comments (4)
src/Helper/Scripts.php (3)
60-60: PHPDoc parameter type correction looks good.The parameter type annotation has been correctly updated from
stringtointto match the actual usage pattern where$posdefaults to 100 (an integer) and represents a numeric position in the stack.
125-125: PHPDoc return type correction is accurate.The return type annotation has been correctly updated from
nulltovoidsince thebeginInternalmethod doesn't explicitly return any value - it only performs setup operations for output buffering.
142-142: PHPDoc return type correction is accurate.The return type annotation has been correctly updated from
nulltovoidsince thebeginCondInternalmethod doesn't explicitly return any value - it only performs setup operations for conditional output buffering..github/workflows/continuous-integration.yml (1)
29-30: 👍 8.3 / 8.4 matrix entries look goodAdding PHP 8.3 and 8.4 keeps the test matrix current without dropping legacy coverage. No issues spotted.
harikt
left a comment
harikt
left a comment
There was a problem hiding this comment.
I want to check again, if I missed the details.
| * | ||
| * @return self|string | ||
| */ | ||
| public function __invoke(array $spec = null) |
There was a problem hiding this comment.
Bit confused, can we do array $spec = [] to fix this ? I didnot noticed where the casting was done . (array) $attr mentioned. May be I missed.
There was a problem hiding this comment.
Thanks for the comment, @harikt!
I kept the $spec = null as-is to avoid any behavior changes in legacy code — this method has been around since the PHP 5.x days. Instead of relying on a type hint, I tried to compensate with a more precise PHPDoc type so modern static analysis tools can still verify correctness.
The idea is to preserve runtime compatibility while improving type clarity at the tooling level.
Happy to add an explicit cast if you think it's safer. Let me know what you think!
2 participants
Close #66
The main compatibility issue addressed is with parameter type hints when null is passed. By removing explicit array type hints and relying on runtime type checking with (array) $attr, the code becomes more flexible while maintaining the same functionality.
Two approaches were considered for PHP 8.4 compatibility:
We chose approach 2 for the following reasons:
This approach keeps the library accessible to projects still using older PHP versions while ensuring compatibility with PHP 8.4's stricter type checking, without requiring a major version release.
Thoughts?
Summary by CodeRabbit