Bump PHP to ^8.3 #38
Bump PHP to ^8.3 #38
Conversation
Resolves #33
WalkthroughThe changes update PHP compatibility across the project to require PHP 8.3 or higher. The composer configuration, GitHub Actions workflow, source, and test files are revised to enforce strict types and modern PHP syntax. Test autoloading is also configured in composer for development. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant Composer
participant CI
participant PHP
Developer->>Composer: Update composer.json (require ^8.3)
Composer->>PHP: Enforce PHP >= 8.3 on install
CI->>PHP: Run tests on PHP 8.3 and 8.4 only
CI->>Composer: Use autoload-dev for tests
Possibly related issues
Poem
✨ Finishing Touches
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
🧹 Nitpick comments (2)
src/IncludeQCompiler.php (1)
57-57: Add type hints to the publicexecute()API as well
validateArguments()was upgraded to: void; consider doing the same forexecute:- public function execute($args, Smarty_Internal_SmartyTemplateCompiler $compiler) + public function execute(array $args, Smarty_Internal_SmartyTemplateCompiler $compiler): stringThis removes the mixed
$argsparameter, propagates strictness, and documents the generated PHP string return value..github/workflows/on_pull_request.yml (1)
15-18: Nightly 8.4 builds can be flaky
shivammathur/setup-phpmaps8.4to the current nightly snapshot. CI might break on upstream changes.
If stability is critical, pin to8.4snapshotor remove it until 8.4 is released.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
.github/workflows/on_pull_request.yml(1 hunks)composer.json(2 hunks)src/IncludeQCompiler.php(2 hunks)tests/IncludeQCompilerTest.php(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/IncludeQCompilerTest.php (1)
src/IncludeQCompiler.php (2)
IncludeQCompiler(15-145)getName(141-144)
🔇 Additional comments (4)
src/IncludeQCompiler.php (1)
2-2: 👍 Strict-types declaration is a welcome hardening step
Enabling strict types project-wide reduces accidental scalar coercions and aligns with the PHP-8.3 baseline.tests/IncludeQCompilerTest.php (2)
2-2: Strict-types in tests keep runtime behaviour consistent
Good to see the test-suite compiled in the same mode as production code.
27-36: Method signatures updated – good, but PHPUnit 8 may not parse them on PHP 8.3
composer.jsonstill allowsphpunit/phpunit^8.0, yet PHPUnit 8 is unmaintained and not tested on PHP 8.3.
Bump to^10.0(or at minimum^9.6) to avoid unexpected runtime issues.composer.json (1)
21-25: 👍 PSR-4 dev autoload improves IDE discovery
Addingautoload-devfor tests makes runningcomposer dump-autoloadsufficient for PHPUnit to find test classes.
| * @var Smarty | ||
| */ | ||
| private $smarty; | ||
| private Smarty $smarty; |
There was a problem hiding this comment.
Missing typed property triggers deprecation in PHP 8.3
$plugin is created dynamically in setUp(). Dynamic properties raise E_DEPRECATED since PHP 8.2 and will become fatal in a future release.
class IncludeQCompilerTest extends TestCase
{
- private Smarty $smarty;
+ private Smarty $smarty;
+ private IncludeQCompiler $plugin;
}Add the declaration (and corresponding use if you prefer a fully-qualified name) to silence the warning and future-proof the test.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| private Smarty $smarty; | |
| class IncludeQCompilerTest extends TestCase | |
| { | |
| private Smarty $smarty; | |
| private IncludeQCompiler $plugin; | |
| // … rest of the test class … | |
| } |
🤖 Prompt for AI Agents
In tests/IncludeQCompilerTest.php at line 10, the property $plugin is used
dynamically in setUp() but not declared, causing a deprecation warning in PHP
8.3. To fix this, declare the $plugin property explicitly in the class with the
appropriate type hint, adding a use statement if needed for the type. This will
prevent the dynamic property warning and future-proof the code.
| "type": "library", | ||
| "require": { | ||
| "php": "^7.3 || ^8.0", | ||
| "php": "^8.3", |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Composer PHP constraint updated – remember PHPUnit compatibility
Now that the package requires PHP 8.3+, ensure require-dev lists a PHPUnit version that officially supports 8.3 (≥ 9.6 / 10.x).
- "require-dev": {
- "phpunit/phpunit": "^8.0 || ^9.0"
+ "require-dev": {
+ "phpunit/phpunit": "^9.6 || ^10.0"This prevents composer from installing an outdated PHPUnit build during CI.
🤖 Prompt for AI Agents
In composer.json at line 6, the PHP version constraint was updated to "^8.3". To
maintain compatibility, update the require-dev section to specify a PHPUnit
version that supports PHP 8.3, such as version 9.6 or 10.x. Modify the PHPUnit
version constraint accordingly to prevent composer from installing an
incompatible or outdated PHPUnit version during continuous integration.
1 participant
Resolves #33
Summary by CodeRabbit