Сomprehensive codebase modernization, quality-tooling integration, and generator enhancements #225
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…n, and generator enhancements; feat: added PHPStan configuration and workflow; feat: added PHP_CodeSniffer configuration and workflow; feat: added PHPMD configuration, baseline, and updated workflow; feat: added Rector configuration and workflow feat: added Composer Unused configuration and composer-quality workflow feat: added Composer security audit workflow style: applied declare(strict_types=1) across codebase feat: introduced UIGeneratorTrait for UI component generation feat: added custom Rector rules for PHPUnit assertions and mock objects feat(Seeder): added WITH_TRANSACTIONS option for seeder execution feat(Generator): added example state method to factory stub feat(Generator): automatically name routes in generated route stubs feat(Generator): standardized date formatting for Seeder and Migration filenames feat(Generator): Covers the FORMAT_TIME constant and getDate() helper in Migration/Seeder generators refactor: updated Hashed ID decoding logic in Repositories, Covers the rewrite of decodeSearchQueryString and related helpers in Repository.php refactor: improved pagination handling and limit setting in Repositories, Covers changes to setPaginationLimit, wantsToSkipPagination, canSkipPagination in Repository.php refactor: updated Response class to use Symfony HTTP status constants refactor(Generator): updated string and parameter handling in Generator core. Covers changes to removeSpecialChars, getInput, checkParameterOr methods and removal of \Safe\ prefixes for preg_replace refactor(Generator): updated various generator stubs and commands logic refactor(User): moved model casts to dedicated casts() method refactor: standardized exception catching to \Throwable. Covers changes in CreateBookTask.php and Repository.php (create method) refactor(Foundation): updated glob usage in Configuration classes. Covers ApplicationBuilder.php and Apiato.php changing from \Safe\glob to glob refactor(Routing): improved API version resolution and prefixing. Covers changes in src/Foundation/Configuration/Routing.php fix(Action): ensured correct return type for transactional run closure fix(Generator): ensured correct parameter type hints in Generator class methods. Specifically for checkParameterOrAsk if the default value change was a fix fix(Generator): ensure relations are unloaded in generated 'create' event stubs fix(MacroServiceProvider): ensure correct this context for Config::unset macro fix(Foundation): improved base path and shared path resolution in Apiato class. Covers inferBasePath and sharedPath logic changes fix(Http): corrected RequestRelation method signatures and type hints. Covers return type change of isValidRelationOf and closure type hint chore: updated composer.json package name to dan-wolf-at/apiato-core chore: updated composer.json dependencies and script configurations chore: removed legacy static analysis configurations (PHPStan.dist, Psalm, old PHPMD) chore: updated GitHub Actions test matrix and CI workflow configurations. Covers general updates to tests.yaml, phpmd.yaml and matrix_includes.json not related to adding new tools fix: update minimum PHP version to 8.2 in Rector classes and adjust related messages; feat: updated PHP CodeSniffer and Rector configurations;
… for compatibility with version 13.x;
Contributor
There was a problem hiding this comment.
Pull Request Overview
This PR implements a comprehensive codebase modernization by enforcing strict typing across all PHP files, integrating static analysis tools (PHPStan, PHP_CodeSniffer, PHPMD, Rector), and refactoring core components for improved reliability and maintainability. The changes establish stricter coding standards and reduce technical debt throughout the project.
Key Changes:
- Added
declare(strict_types=1);to all PHP files for stricter type checking - Refactored repository search query decoding logic and pagination handling
- Enhanced code generators with new UIGeneratorTrait and automatic route naming
- Updated test files to use static assertions and improved type hints
Reviewed Changes
Copilot reviewed 299 out of 392 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| workbench/app/Containers/MySection/Author/* | Added strict types declaration to route files, helpers, events, seeders, and actions |
| workbench/app/Containers/Identity/User/* | Added strict types, modernized casts to method format, updated factory to use fake() helper, added parameter types to migrations |
| tests/* | Added strict types across all test files, improved closure type hints, renamed variables for clarity, added return types |
| src/Support/* | Added strict types, improved type hints with null |
| src/Http/* | Added strict types, moved methods for better organization, added exception documentation, improved null checks |
| src/Generator/* | Added strict types, removed FormatterTrait, added UIGeneratorTrait, improved type safety and return types throughout |
| src/Foundation/* | Added strict types, improved configuration class organization, refactored seeder transaction handling |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+1
to
+9
| <?php | ||
| declare(strict_types=1); | ||
| ?> | ||
| <h1> | ||
| This is a fake mail view for testing purposes. | ||
| </h1> | ||
| <?php |
There was a problem hiding this comment.
Adding PHP strict types declaration to a Blade template file is unusual and unnecessary. Blade templates are primarily for HTML markup with minimal PHP logic. The strict_types declaration should only be in pure PHP files. Additionally, the closing <?php tag on line 9 appears incomplete or unnecessary.
Suggested change
| <?php | |
| declare(strict_types=1); | |
| ?> | |
| <h1> | |
| This is a fake mail view for testing purposes. | |
| </h1> | |
| <?php | |
| <h1> | |
| This is a fake mail view for testing purposes. | |
| </h1> |
Comment on lines
+27
to
32
| public function name(): self | ||
| { | ||
| return $this->state(fn (array $attributes): array => [ | ||
| 'name' => fake()->name(), | ||
| ]); | ||
| } |
There was a problem hiding this comment.
The name() method in the factory stub duplicates the default 'name' field from the definition() method. This creates redundant state methods that don't add value. Consider removing this method or making it demonstrate a different state pattern.
Suggested change
| public function name(): self | |
| { | |
| return $this->state(fn (array $attributes): array => [ | |
| 'name' => fake()->name(), | |
| ]); | |
| } |
| * @var Seeder $this | ||
| */ | ||
| collect($classes)->each(fn (string $class) => $class::WITH_TRANSACTIONS | ||
| ? DB::transaction(fn (string $class) => $this->call($class)) |
There was a problem hiding this comment.
The closure parameter shadows the outer $class variable. The inner closure fn (string $class) on line 22 should be fn () => since it doesn't use the parameter and would incorrectly reference a different variable. This will cause the wrong class to be called within the transaction.
Suggested change
| ? DB::transaction(fn (string $class) => $this->call($class)) | |
| ? DB::transaction(fn () => $this->call($class)) |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This pull request introduces a comprehensive overhaul of the project's codebase, focused on modernization, quality assurance, and improved developer experience. It addresses technical debt and establishes a solid foundation for future development.
The primary motivation for this extensive update is to elevate the project's code quality, maintainability, and reliability by integrating a modern suite of static analysis tools and applying strict coding standards across the entire codebase. This proactive approach aims to catch bugs earlier, ensure consistency, and simplify the process of contributing to the project.
Key changes include:
declare(strict_types=1);has been applied to all PHP files to enforce stricter type checking.UIGeneratorTraitto reduce code duplication and the automatic naming of routes.WITH_TRANSACTIONSoption has been added to Seeders for more efficient database seeding.This pull request does not fix a single issue but rather represents a foundational upgrade for the entire project.
Dependencies:
This change introduces several new
require-devdependencies for static analysis and code quality tooling. It does not add any new runtime dependencies.Type of change
BREAKING CHANGE DETAILS:
declare(strict_types=1);may cause issues in downstream projects that relied on PHP's loose type coercion.