fix: phpstan level 5 errors #84 #100
Conversation
github-actions
bot
added
documentation
dermatz
removed
documentation
github-actions
bot
added
documentation
dermatz
removed
documentation
There was a problem hiding this comment.
Pull request overview
This pull request addresses PHPStan level 5 static analysis errors by refactoring code to use modern PHP 8+ features, improve type safety, and fix type-related issues. The changes focus on constructor property promotion, better type documentation, and removing redundant checks.
Changes:
- Lowered PHPStan level from 6 to 5 and removed continue-on-error from CI workflow to enforce static analysis
- Refactored multiple classes to use PHP 8+ constructor property promotion (readonly properties)
- Improved PHPDoc type annotations and removed redundant type checks
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| phpstan.neon | Lowered PHPStan analysis level from 6 to 5 |
| .github/workflows/phpstan.yml | Changed Magento installation method and removed continue-on-error flag |
| src/Service/ThemeBuilder/BuilderPool.php | Applied constructor property promotion |
| src/Service/StaticContentCleaner.php | Simplified return logic to check only cleaned files |
| src/Service/Hyva/CompatibilityChecker.php | Removed unused IncompatibilityDetector dependency |
| src/Model/ThemePath.php | Removed unused ThemeList and ThemeCollection dependencies |
| src/Model/TemplateEngine/Plugin/InspectorHints.php | Applied constructor property promotion and quoted @SuppressWarnings annotation |
| src/Model/TemplateEngine/Decorator/InspectorHintsFactory.php | Deleted factory class |
| src/Model/TemplateEngine/Decorator/InspectorHints.php | Applied constructor property promotion, made Random required, added guard clause, improved instanceof checks |
| src/Console/Command/Theme/TokensCommand.php | Changed return type from int to null for validation methods |
| src/Console/Command/Theme/CleanCommand.php | Removed redundant is_string check on typed parameter |
| src/Console/Command/Theme/BuildCommand.php | Removed redundant is_string check on typed parameter |
| src/Console/Command/System/VersionCommand.php | Removed unused PACKAGE_NAME constant |
| src/Console/Command/System/CheckCommand.php | Improved PHPDoc type annotations and ini_get handling |
| src/Console/Command/Dev/InspectorCommand.php | Applied constructor property promotion and removed unreachable default case |
| src/Block/Inspector.php | Applied constructor property promotion |
| private readonly ScopeConfigInterface $scopeConfig, | ||
| private readonly StoreManagerInterface $storeManager, | ||
| private readonly DevHelper $devHelper, | ||
| private readonly InspectorHintsFactory $inspectorHintsFactory, |
There was a problem hiding this comment.
The InspectorHintsFactory class was deleted but it's still being used in this file. The factory is injected as a dependency on line 29 and used on line 63 to create InspectorHints instances. This will cause a runtime error when the plugin tries to instantiate the factory. Either the factory needs to be restored, or this code needs to be refactored to not use the factory pattern.
| $cleanedPreprocessed = $this->themeCleaner->cleanViewPreprocessed($themeCode, $io, false, $isVerbose); | ||
|
|
||
| return ($cleanedStatic > 0 || $cleanedPreprocessed > 0) || !$this->themeCleaner->hasStaticFiles($themeCode); | ||
| return ($cleanedStatic > 0 || $cleanedPreprocessed > 0); |
There was a problem hiding this comment.
The return logic has been simplified to only return true if files were actually cleaned. However, this removes a safety check that verified no static files remain after cleaning. If hasStaticFiles returns true but the clean operations return 0 (e.g., files removed between check and clean, or clean operations fail silently), this will now return false and cause builds to fail. Consider restoring the check for whether static files remain after cleaning, or add explicit error handling in the clean methods to ensure they report failures rather than returning 0.
This pull request includes a variety of improvements and refactorings across the codebase, focusing on constructor property promotion, type hinting, and code simplification. The most significant changes are grouped below.
Constructor Property Promotion and Dependency Handling:
Inspector,InspectorCommand,InspectorHints,InspectorHintsdecorator, andBuilderPool) to use PHP's constructor property promotion withprivate readonlyfor dependencies, reducing boilerplate and improving immutability. [1] [2] [3] [4] [5]ThemePathandCompatibilityChecker. [1] [2]Codebase Simplification and Cleanup:
InspectorHintsFactoryclass, streamlining the template engine decorator logic.Type Hinting and Documentation Improvements:
System/CheckCommand.php, enhancing code clarity and IDE support. [1] [2] [3] [4] [5] [6]Workflow and Build Improvements:
composer create-projectfor Magento installation instead ofgit clone, improving reliability and mirroring production setups.continue-on-errorflag and redundant module enablement.Minor Fixes and Consistency:
These changes collectively modernize the codebase, improve maintainability, and enhance type safety.