Remove container ArrayAccess and dynamic service properties - #23
Closed
binaryfire wants to merge 29 commits into
Closed
Remove container ArrayAccess and dynamic service properties#23binaryfire wants to merge 29 commits into
binaryfire wants to merge 29 commits into
Conversation
Use concurrency.default as the sole source of the selected driver and stop reading or writing the legacy scalar concurrency.driver key. This keeps the per-driver configuration map intact when the default changes and lets missing defaults fail at the typed configuration boundary. Add regression coverage that changes the default instance while preserving the configured driver options.
Replace foundation service reads and binding checks with make(), bound(), and explicit instance registration. This makes resolution visible in bootstrapping, console, routing, exception, and HTTP paths before array access is removed from the container. Register the detected environment as the shared env instance instead of relying on offset assignment's implicit transient binding behavior, and cover the resolved environment through the public application API.
Clear temporary middleware instances with forgetInstance() when middleware is re-enabled. This restores any original binding and lifecycle instead of deleting the registration through container offset unsetting. Add coverage for a middleware binding that cannot be reconstructed without its original factory, while retaining the existing global and formerly-unbound cases.
Resolve auth, session, database, event, and console services through explicit container methods in the foundation testing concerns. Register fixed test doubles as instances so their intended shared lifetime is clear. Update the matching authentication and database truncation coverage to use the same named registration and resolution surface.
Resolve configuration, connection, migration, filesystem, and event services through explicit container methods across the database manager, provider, migrator, and console commands. Migrate database integration setup to explicit instance and make calls, preserving fixed-value lifetimes and existing driver coverage while removing reliance on container offset syntax.
Resolve queue, cache, event, and command services through make() and express queue test fixtures with explicit instance lifetimes. This removes implicit resolution and registration behavior from queue managers, sync dispatch, and worker commands. Clear one-time payload fixtures with forgetInstance() so the tests continue to exercise stale callback behavior without depending on destructive offset unsetting.
Replace the facade layer's ArrayAccess-shaped application slot with the nullable Hypervel container contract. Resolve facade roots through make() and use PSR has()/get() only where the caller is intentionally contract-shaped. Keep existing fail-fast behavior at unset application boundaries, retain tolerant nested filesystem configuration, and update the facade test containers to count real make() resolutions.
Resolve service-provider configuration through make() while preserving the existing tolerant behavior for missing or non-array view paths. Replace support test setup and maintenance-mode config access with explicit instance registration and repository resolution. This keeps configuration semantics unchanged while removing the support package's dependency on container offsets and dynamic service properties.
Resolve vendor-link, event, filesystem, and cloned-application services through make(), removing the unreachable fallback for the guaranteed vendor symlink flag. Convert Testbench fixtures and application setup to explicit instance registration and named resolution. Clear one-time payload values with forgetInstance() so cleanup restores the container lifecycle without deleting registrations.
Resolve cache and view services through make() in the shared testing concerns. Register PendingCommand's console output mock as the exact temporary instance and remove only that instance during cleanup. Update the matching utility and parallel-test coverage to use explicit binding checks, resolutions, and fixed-value registrations.
Resolve validator and application path services through the command application's make() method. Update console integration setup to register fixed dependencies explicitly and resolve services through named methods. The command lifecycle remains unchanged while the console package no longer depends on array-shaped application access.
Resolve the event dispatcher through the command application's make() method and register cache test doubles through explicit instances. Update Redis cache lock and funnel coverage to use named container resolution without changing the tested locking or throttling behavior.
Resolve event dispatchers through make() in the context provider and log manager. Convert logging and queued-context tests to explicit service resolution and fixed-value instance registration. This preserves logger construction, event handling, and context propagation while removing implicit container offset behavior.
Resolve the configuration repository through make() and read Sentry's guaranteed package config with the typed array getter. Invalid or missing root configuration now fails at the configuration boundary instead of being hidden by an empty fallback. Update Sentry providers, integrations, and feature tests to register application services explicitly and use named resolution throughout.
Use the container's make() method when the filesystem manager resolves the URL generator. This preserves lazy URL generation while removing the manager's last dependency on container offset access.
Check optional validation dependencies with bound() and resolve translator and presence-verifier services through make(). This keeps the provider's conditional behavior intact without relying on offset existence or reads.
Replace foundation test setup, service reads, and fixed application values with explicit bind(), instance(), make(), and bound() calls. Add native void return types to the touched test methods while retaining their existing bootstrap, console, helper, Vite, and static-state assertions. These conversions make each test's intended container lifecycle visible before array access is removed.
Migrate route caching, exception handling, provider registration, and health-route fixtures to explicit container registration and resolution. Fixed objects and flags are installed with instance(), while services under test are resolved through make(). Keep the integration behavior and failure assertions unchanged while removing implicit array-shaped application access.
Register environment values, configuration repositories, and command dependencies through explicit container instances in the key-generation and source-generator suites. Resolve application services with make() and add native void return types to touched tests. The generated output and command behavior remain the same; only the container interaction is made explicit.
Convert authentication, Fortify, and Inertia test setup from container offsets to explicit instance registration and named resolution. This makes fixed request and service fixtures shared by intent while preserving the existing authentication and component assertions. Add native void return types to the test methods touched by the migration.
Register request, middleware, route, and URL dependencies explicitly and resolve application services through make() across the HTTP middleware and routing integration suites. Preserve request-forgery, CORS, compiled-route, binding, precognition, and signed-URL behavior while removing implicit container access from their fixtures.
Convert notification, session, translation, and view integration fixtures to explicit application registrations and named service resolution. Fixed config and service values now use instance(), while tested services are obtained through make(). Retain all locale, persistence, rendering, and delivery assertions and add native void return types to the methods touched by the conversion.
Replace Horizon controller and supervisor fixture offsets with explicit instance registration and named application resolution. Memory monitors, clear commands, dashboard statistics, and batch endpoints keep their existing behavior while their service lifetimes become visible in setup. Add native void return types to the touched Horizon test methods.
Migrate Reverb application setup, event dispatch, protocol handlers, channel fixtures, and server lifecycle tests to explicit bindings, instances, and named resolution. Preserve the existing websocket, shutdown, and protocol behavior while removing container offset syntax and adding native void return types to the methods touched by the migration.
Register Telescope test services as explicit instances and resolve the Reverb watcher dependencies through named container methods. The feature base and watcher assertions remain unchanged while no longer depending on array-shaped application access.
Drop ArrayAccess from the container contract and implementation, and remove offset and dynamic service-property methods from the concrete container. Named methods now define the complete registration, lookup, existence, and temporary-instance lifecycle surface. Keep a concise source marker for future Laravel ports, migrate extender coverage to bind() and forgetExtenders(), and consolidate lifecycle assertions around bound() and forgetInstance() without adding an incomplete arbitrary binding-removal API.
Describe the intentional Laravel difference in the container package and Laravel porting guide, including direct mappings for resolution, existence checks, factories, fixed instances, and temporary override cleanup. Narrow broader compatibility claims in the container and release documentation so they acknowledge deliberate public API omissions without duplicating the detailed migration guidance.
Record container ArrayAccess and dynamic service properties as unsupported porting surfaces, and direct Laravel source and tests toward named container methods. Clarify that typed configuration reads must not duplicate defaults already defined by framework or package config, so missing and misspelled keys fail at the configuration boundary instead of silently falling back.
Record the final public contract, migration rules, lifecycle decisions, facade typing, configuration cleanup, documentation requirements, and verification gates for the container API change. Keep the plan as the authoritative design reference for the completed refactor and future review of intentional Laravel differences.
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
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 |
Greptile SummaryThis PR intentionally removes container ArrayAccess and dynamic service-property access, migrating framework and test consumers to explicit named APIs.
Confidence Score: 5/5The PR appears safe to merge; no concrete changed-code failure remains after tracing the affected container, facade, configuration, and temporary-override paths. Named container access preserves the prior resolution behavior at migrated call sites, lifecycle-specific cleanup matches how temporary overrides are installed, and configuration changes are either backed by guaranteed defaults or explicitly intentional compatibility removals.
|
| Filename | Overview |
|---|---|
| src/container/src/Container.php | Removes ArrayAccess and dynamic service access while retaining named resolution, binding, and lifecycle APIs. |
| src/contracts/src/Container/Container.php | Narrows the public container contract to PSR-11 and Hypervel's named methods. |
| src/support/src/Facades/Facade.php | Types the facade application as the Hypervel container contract and resolves roots through make(). |
| src/foundation/src/Testing/Concerns/MakesHttpRequests.php | Cleans up instance-based middleware overrides with forgetInstance(), allowing underlying bindings to resolve again. |
| src/concurrency/src/ConcurrencyManager.php | Uses typed configuration access and intentionally removes the legacy concurrency.driver scalar compatibility path. |
| src/sentry/src/SentryServiceProvider.php | Resolves package configuration through a typed array getter after provider registration merges its defaults. |
| src/foundation/src/Application.php | Migrates environment and event access to named APIs and registers the detected environment as a shared instance. |
Reviews (1): Last reviewed commit: "Add the container named API implementati..." | Re-trigger Greptile
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
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.
Summary
This PR removes
ArrayAccessand dynamic service-property access from Hypervel's container. Container registration, lookup, existence checks, and temporary overrides now use named methods throughout the framework and test suite.This is an intentional Laravel compatibility break for Hypervel 0.4. Laravel still supports these forms on its concrete container, but they hide behavior that Hypervel should make explicit:
$app['service']performs service resolution.$app['service'] = $valuesilently selects transient binding behavior.unset($app['service'])removes only part of the container's lifecycle state.$app->servicemakes a resolved service look like a declared property.ArrayAccessremains appropriate for map-like objects elsewhere in the framework. The problem is specifically using array and dynamic-property syntax for a dependency injection container.Container API
The container contract now extends only PSR-11's
ContainerInterface, and the concrete container no longer implementsArrayAccess. The four offset methods and the__get/__setservice accessors have been removed.The supported replacements are:
$app['events']or$app->events$app->make('events')isset($app['events'])$app->bound('events')$container->get()and$container->has()$app['service'] = fn ($app) => ...$app->bind('service', fn ($app) => ...)$app['service'] = $service$app->instance('service', $service)$app->forgetInstance('service')No compatibility shim or custom error layer is included. Unsupported access fails at the call site through native PHP behavior.
This PR also does not add arbitrary binding removal. The old offset-unset operation cleared selected binding, resolution, instance, scoped, and alias state while leaving other related container state behind.
forgetInstance()is the correct operation for temporary overrides because it removes the override and allows the original registration and lifecycle to resolve again.Framework migration
All active framework and test consumers now use named APIs. Each conversion keeps the intended lifetime explicit:
make().bound(), orhas()for deliberately PSR-11-shaped callers.instance().bind().forgetInstance().The facade application slot is now typed as the nullable Hypervel container contract instead of accepting an untyped
ArrayAccessvalue. Facade roots resolve throughmake(), while the existing fail-fast behavior when no application is configured is preserved.Lifecycle and configuration fixes
The migration exposed several places where the old syntax obscured the real behavior:
envinstance instead of an implicit transient binding.concurrency.defaultexclusively for the selected driver. It no longer reads or writes a legacy scalarconcurrency.drivervalue that conflicts with the per-driver configuration map.view.pathsvalues.Documentation and porting
The container package README and framework documentation now describe the named-only API and the intentional difference from Laravel. The Laravel porting guide includes concise conversion mappings and keeps the existing lifecycle guidance as the source of truth.
The contribution guide now treats container ArrayAccess and dynamic service properties as unsupported porting surfaces. It also clarifies that code should not duplicate defaults already supplied by framework or package configuration, so missing and misspelled keys fail loudly.
Verification
The implementation was verified with focused container, facade, foundation testing, concurrency, command, queue, Sentry, and Testbench coverage. The full formatter, static analysis, parallel framework suite, Testbench package suite, and dogfood package checks pass. The latest
0.4request-binding coverage also passes after the branch update.