Update Container to not inject null for unset nullable parameters#286
Merged
Update Container to not inject null for unset nullable parameters#286
Container to not inject null for unset nullable parameters#286Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the Container to enforce explicit configuration for nullable parameters without default values, instead of automatically injecting null. This is a breaking change that requires developers to use explicit default values if they want nullable behavior.
- Removes automatic
nullinjection for unset nullable parameters in Container dependency resolution - Updates error handling to throw exceptions for nullable parameters without explicit defaults or container configuration
- Adds comprehensive test coverage for the new behavior with nullable, union, and mixed types
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| src/Container.php | Removes logic that auto-injects null for nullable parameters; now requires explicit defaults or container configuration |
| tests/ContainerTest.php | Removes tests for old null-injection behavior; adds tests verifying errors for unset nullable parameters and proper handling of explicit defaults |
| tests/Io/RouteHandlerTest.php | Removes test that verified nullable constructor autowiring with null injection |
| docs/best-practices/controllers.md | Removes documentation example showing nullable parameter behavior; streamlines docs to emphasize explicit default values |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
9c6235c to
7fa20c4
Compare
This was referenced Dec 17, 2025
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.
This changeset updates the
Containerto not injectnullvalues for any unset nullable parameters. With these changes applied, nullable arguments without explicit container configuration or a default value will cause an error if they are not set instead of being silently set tonull. You can still use an explicit default value to restore the old behavior:Note that this is a BC break with a very real chance to affect a number of projects relying on the existing behavior. The main motivation for this changes is to reduce complexity to introduce support for union types in a follow-up PR. Also, given the above upgrade path, upgrading should be fairly straightforward.
This includes a number of updated tests to verify correct behavior and avoid introducing any potential regressions, so this has 100% code coverage and should be safe to apply. On top of this, I've updated/simplified the documentation slightly to emphasize assigning default values instead.
Builds on top of #284, #184, #181, #92, and others