Integrate PHP-DI for dependency injection #2285
Open
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
Introduce PHP-DI as a dependency and refactor core classes (Builder, Steps, Generators, Converter, Renderer) to use dependency injection for improved modularity and testability. Add ContainerFactory and dependencies configuration, update constructors to support DI, and ensure services are resolved via the container. Update composer dependencies accordingly.
Replaces constructor-based dependency injection with PHP-DI attribute injection (#[Inject]) in several classes for improved clarity and reduced boilerplate. Adds a comprehensive documentation file on dependency injection usage and best practices. Updates the DI container factory to enable attribute support, adds convenience methods for cache instantiation, and improves dependency configuration for services like Twig and Cache.
Added copyright and license information headers to dependencies.php, ContainerFactory.php, and TwigFactory.php for improved code documentation and compliance.
Replaced all direct instantiations of Cache with calls to Builder::getCache() for consistency and to follow best practices. Updated documentation to reflect this change and removed exceptions for contextual instances.
Updated dependency injection configuration to use fully qualified class names with leading backslashes. This change improves clarity and consistency, reducing ambiguity and potential issues with class resolution.
There was a problem hiding this comment.
Pull request overview
This pull request introduces PHP-DI as the dependency injection framework for Cecil, significantly refactoring the architecture to improve modularity, testability, and maintainability. The changes include adding a DI container, updating constructors to support dependency injection, and providing comprehensive documentation.
Changes:
- Introduced PHP-DI 7.0 as a new dependency with ContainerFactory and dependencies configuration
- Refactored core classes (AbstractStep, AbstractGenerator, Converter, Parsedown) to support flexible constructors with optional DI parameters
- Replaced all
new Cache()instantiations with centralizedBuilder::getCache()method for better consistency - Added comprehensive documentation explaining DI architecture, usage patterns, and best practices
Reviewed changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| composer.json | Added php-di/php-di ^7.0 dependency |
| composer.lock | Updated with PHP-DI and its dependencies (php-di/invoker, laravel/serializable-closure) |
| src/Container/ContainerFactory.php | New factory class to create and configure the DI container with compilation caching |
| config/dependencies.php | New configuration file defining service autowiring rules and dependencies |
| src/Builder.php | Integrated DI container, added getCache() and get() helper methods, updated step instantiation with fallback |
| src/Step/StepInterface.php | Removed constructor signature requirement from interface |
| src/Step/AbstractStep.php | Updated constructor to support optional Config and Logger parameters for DI compatibility |
| src/Step/Pages/Generate.php | Uses injected GeneratorManager, implements fallback for generator instantiation |
| src/Step/Pages/Convert.php | Uses injected Converter instead of manual instantiation |
| src/Step/Pages/Render.php | Uses injected TwigFactory, updated logger usage |
| src/Generator/GeneratorInterface.php | Removed constructor signature requirement from interface |
| src/Generator/AbstractGenerator.php | Updated constructor to support optional Config and Logger parameters |
| src/Generator/GeneratorManager.php | Updated constructor to accept Config and Logger directly |
| src/Generator/ExternalBody.php | Uses injected Converter |
| src/Converter/Converter.php | Refactored to accept Parsedown dependency instead of Builder |
| src/Converter/Parsedown.php | Updated constructor signature to accept Config as explicit parameter |
| src/Renderer/Twig/TwigFactory.php | New factory class for creating Twig renderer instances |
| src/Renderer/Extension/Core.php | Updated Parsedown instantiations to include Config parameter |
| src/Step/Assets/Save.php | Replaced new Cache() with Builder::getCache() |
| src/Step/Optimize/AbstractOptimize.php | Replaced new Cache() with Builder::getCache() |
| src/Asset.php | Replaced all new Cache() with Builder::getCache() |
| docs/di/README.md | Comprehensive documentation covering architecture, usage, examples, and best practices |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
This comment was marked as resolved.
This comment was marked as resolved.
* Document logger parameter for future use Co-authored-by: ArnaudLigny <80580+ArnaudLigny@users.noreply.github.com> * Apply fixes from StyleCI (#2288) --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Arnaud Ligny <arnaud@ligny.fr>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This comment was marked as resolved.
This comment was marked as resolved.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
#2289) * Improve DI fallback: catch specific exception and add logging * Fix Parsedown DI config to explicitly inject Builder * Address code review feedback: improve comments and remove unnecessary namespace prefix Co-authored-by: ArnaudLigny <80580+ArnaudLigny@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: ArnaudLigny <80580+ArnaudLigny@users.noreply.github.com>
Updated the docblock for the logger property to better explain its purpose and future use, emphasizing constructor signature consistency and potential for future logging integration.
This comment was marked as resolved.
This comment was marked as resolved.
#2290) * Add comprehensive DI container integration tests * Fix environment variable cleanup in test * Remove unused imports from test file * Address code review feedback: remove redundant assertion and fix env var handling * Improve documentation comments in tests * Fix comment clarity and remove redundant assertion * Update tests/ContainerFactoryTest.php * Update tests/ContainerFactoryTest.php * Update tests/ContainerFactoryTest.php * Address code review feedback: add tearDown, fix env cleanup, improve cache test --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: Arnaud Ligny <arnaud@ligny.fr>
Introduces a comprehensive guide for GitHub Copilot usage and development conventions in the Cecil PHP static site generator. Covers architecture, dependency injection migration, build pipeline, collections, template system, workflows, configuration, namespace structure, error handling, integrations, and common development tasks.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Introduce PHP-DI as a dependency and refactor core classes (Builder, Steps, Generators, Converter, Renderer) to use dependency injection for improved modularity and testability. Add ContainerFactory and dependencies configuration, update constructors to support DI, and ensure services are resolved via the container. Update composer dependencies accordingly.