Inject routineLookup, increase coverage to 100%, remove dead code#182
Merged
alganet merged 1 commit intoRespect:masterfrom Mar 29, 2026
Merged
Inject routineLookup, increase coverage to 100%, remove dead code#182alganet merged 1 commit intoRespect:masterfrom
alganet merged 1 commit intoRespect:masterfrom
Conversation
Member
alganet
commented
Mar 28, 2026
- Make NamespaceLookup a constructor dependency injected from Router into all routes and handlers, removing duplicate creation and the setRoutineLookup()/withRoutineNamespace() post-construction API
- Add 16 targeted tests reaching 100% class/method coverage; remove two unreachable code paths (ControllerRoute::getTargetMethod HEAD fallback, Router::process empty-dispatch guard)
- Remove unused CallbackList methods (hasKey, filterKeysContain, filterKeysNotContain, executeCallback) and their tests
- Merge Responder::finalize() dual null-check blocks into one
- Make compareOcurrences private and fix typo -> compareOccurrences
- Collapse Router::__call() callable branch into single call
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #182 +/- ##
============================================
+ Coverage 96.35% 98.80% +2.44%
+ Complexity 398 394 -4
============================================
Files 31 31
Lines 1016 1004 -12
============================================
+ Hits 979 992 +13
+ Misses 37 12 -25 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
- Make NamespaceLookup a constructor dependency injected from Router into all routes and handlers, removing duplicate creation and the setRoutineLookup()/withRoutineNamespace() post-construction API - Add 16 targeted tests reaching 100% class/method coverage; remove two unreachable code paths (ControllerRoute::getTargetMethod HEAD fallback, Router::process empty-dispatch guard) - Remove unused CallbackList methods (hasKey, filterKeysContain, filterKeysNotContain, executeCallback) and their tests - Merge Responder::finalize() dual null-check blocks into one - Make compareOcurrences private and fix typo -> compareOccurrences - Collapse Router::__call() callable branch into single call
There was a problem hiding this comment.
Pull request overview
This PR refactors routing/routine resolution to inject a shared NamespaceLookup into routes/handlers at construction time, removes several dead/unreachable code paths, and adds targeted tests to reach full class/method coverage.
Changes:
- Inject
NamespaceLookupfromRouterinto allAbstractRoutesubclasses and handlers; remove the post-construction routine-lookup API and introducesetBasePath(). - Remove dead/unreachable code and simplify logic (e.g., HEAD target-method fallback logic,
Router::processguard,Router::__callcallable branch,Responder::finalizenull-check structure). - Remove unused
CallbackListmethods and associated tests; add new tests/stubs to cover edge cases and reach 100% coverage.
Reviewed changes
Copilot reviewed 27 out of 27 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| src/Routes/AbstractRoute.php | Makes routine lookup a constructor dependency; adds setBasePath; tightens mutability via private(set) properties. |
| src/Routes/Callback.php | Updates constructor to accept injected routine lookup; tightens arguments mutability. |
| src/Routes/ClassName.php | Updates constructor to accept injected routine lookup; tightens promoted properties. |
| src/Routes/Factory.php | Updates constructor to accept injected routine lookup; tightens promoted properties. |
| src/Routes/Instance.php | Updates constructor to accept injected routine lookup; tightens class mutability. |
| src/Routes/StaticValue.php | Updates constructor to accept injected routine lookup. |
| src/Routes/ControllerRoute.php | Simplifies HEAD target method selection logic. |
| src/Router.php | Injects/propagates routine lookup via constructors; removes withRoutineNamespace; simplifies process and __call; makes compare helper private and fixes typo. |
| src/Routines/CallbackList.php | Removes unused helper methods and related import. |
| src/Responder.php | Consolidates status override logic inside the existing draft-response block. |
| src/Handlers/StatusHandler.php | Passes routine lookup into handler route construction. |
| src/Handlers/ExceptionHandler.php | Encapsulates exception state and adds helper methods for matching/capture/reset; passes routine lookup. |
| src/Handlers/ErrorHandler.php | Encapsulates error collection state; passes routine lookup. |
| src/DispatchContext.php | Uses new handler APIs (addError/hasErrors/clear*, matches/capture/clearException) instead of direct property writes. |
| tests/Routines/CallbackListTest.php | Removes tests for deleted CallbackList APIs; adds constructor validation test. |
| tests/Routines/AcceptTest.php | Adds coverage for skipping empty Accept segments and by() returning null. |
| tests/RoutinePipelineTest.php | Updates route creation to pass injected routine lookup. |
| tests/Routes/StaticValueTest.php | Updates route creation to pass injected routine lookup. |
| tests/Routes/InstanceTest.php | Updates route creation to pass injected routine lookup; adds non-routable instance assertion. |
| tests/Routes/FactoryTest.php | Updates route creation to pass injected routine lookup; adds non-routable factory return assertion. |
| tests/Routes/ClassNameTest.php | Updates route creation to pass injected routine lookup. |
| tests/DispatchEngineTest.php | Updates route creation to pass injected routine lookup. |
| tests/DispatchContextTest.php | Updates mocked route constructor args to include injected routine lookup; adds coverage for unknown container ID and empty string conversion. |
| tests/RouterTest.php | Adds new coverage for file-extension negotiation, array-path binding, catchall sorting, PSR-15 process delegation, explicit HEAD handling, exception-handler filtering, and magic __call controllers. |
| tests/Stubs/MagicMethodController.php | Adds stub controller using __call for dispatch testing without reflection. |
| tests/Stubs/HeadWithExplicitHead.php | Adds stub controller with explicit head() method. |
| tests/Stubs/FunkyCallbackList.php | Removes stub method that depended on deleted CallbackList::executeCallback(). |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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
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.