Move DiCompileModule from src-deprecated to src#122
Conversation
DiCompileModule and CompilerModule serve different purposes: - DiCompileModule: Controls compilation on/off via bool parameter - CompilerModule: Always enables compilation and configures script directory DiCompileModule is still actively needed for cases where compilation needs to be conditionally disabled (e.g., dev mode vs prod mode). Changes: - Remove incorrect @deprecated annotation - Move from src-deprecated/ to src/ - Make class final - Add #[Override] attribute 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
Reviewer's guide (collapsed on small PRs)Reviewer's GuideThis PR un-deprecates and moves DiCompileModule into the main src directory, applies PSalm-driven updates (final class, Override attribute import), and ensures all tests and static analysis checks pass. Class diagram for updated DiCompileModuleclassDiagram
class DiCompileModule {
- bool doCompile
+ __construct(bool doCompile, AbstractModule module = null)
+ configure() void
}
DiCompileModule --|> AbstractModule
class DiCompileModule {
<<final>>
<<Override>>
}
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
|
Caution Review failedThe pull request is closed. WalkthroughMakes Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #122 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 130 133 +3
===========================================
Files 13 14 +1
Lines 282 288 +6
===========================================
+ Hits 282 288 +6 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/DiCompileModule.php(2 hunks)
🧰 Additional context used
📓 Path-based instructions (1)
**/*.php
📄 CodeRabbit inference engine (CLAUDE.md)
Code must support PHP 7.2+ or 8.0+; usage of PHP 8 attributes (#[...]) should remain compatible with supported versions
Files:
src/DiCompileModule.php
🧬 Code graph analysis (1)
src/DiCompileModule.php (2)
src/AbstractInjectorContext.php (1)
Override(30-31)src/InjectionPoint.php (4)
Override(48-52)Override(57-67)Override(72-79)Override(88-92)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: ci / PHPUnit - PHP 8.5 (windows-latest, highest)
- GitHub Check: Sourcery review
🔇 Additional comments (2)
src/DiCompileModule.php (2)
26-30: Correct usage of #[Override] attribute.The
#[Override]attribute is semantically correct here, asconfigure()does overrideAbstractModule::configure(). The method implementation correctly binds the$doCompileboolean value to the@Compileannotation.
11-11: No breaking change concern identified in the codebase.All usages of
DiCompileModulefollow composition patterns (instantiation,install(),override()), never inheritance. No code in tests, examples, or production extends this class. Thefinalmodifier aligns with the intended design.
Summary
DiCompileModuleis not deprecated. It serves a different purpose thanCompilerModuleand is still actively needed.Problem
The
@deprecated Use CompilerModuleannotation is incorrect because:Different Purposes
boolparameternew DiCompileModule(true)→ enable compilationnew DiCompileModule(false)→ disable compilation$scriptDirparameter@ScriptDirannotationInjectorInterfacetoCompiledInjectorUse Cases
DiCompileModule(false)is commonly used in dev/test contexts where compilation should be disabledCompilerModulecannot replace this use case since it always sets@CompiletotrueChanges
@deprecatedannotationsrc-deprecated/tosrc/(it's not deprecated)final(Psalm requirement)#[Override]attribute (Psalm requirement)🤖 Generated with Claude Code
Summary by Sourcery
Reinstate DiCompileModule by removing its deprecated status, moving it into the active source tree, and updating its class definition to satisfy Psalm requirements
Enhancements:
Chores:
Summary by CodeRabbit
Refactor
Chores