Add binding diagnostics and module composition contracts#15
Closed
koriym wants to merge 230 commits into
Closed
Conversation
Serializable provider provider
Removed unnecessary type suppression comments and refined type annotations to enhance code clarity across multiple classes. Updated several development dependencies in the composer.lock to their latest versions, ensuring compatibility and consistency. This update primarily affects internal documentation and tooling without altering functionality.
Add a check in the Injector class to verify that the temporary directory is writable before proceeding with the class directory initialization. Introduced a new exception, DirectoryNotWritable, to handle cases where the directory is not writable, enhancing error handling and code robustness.
Handle missing dependencies in setter methods with @Inject(optional=true)
The continuous integration workflow now supports PHP 8.4 as the current stable version. PHP 8.3 has been moved to the old_stable versions list to ensure compatibility continues with earlier versions.
Upgraded PHPStan and PHPStan-PHPUnit to version 2.0 to ensure compatibility with the latest features and improvements. Additionally, Symfony components have been updated to version 7.2.0 to benefit from bug fixes and performance enhancements. This update maintains alignment with the supported PHP versions and related dependencies.
This commit eliminates redundant assert statements to streamline the code and improve readability. Additionally, it uses `@phpstan-ignore-line` annotations to suppress specific warnings, enhancing compatibility with static analysis tools.
Removed unnecessary assertions such as `assert` and `property_exists` to make the tests cleaner and more efficient. Adjusted array handling by casting `bindings` to an array for better compatibility and added necessary code comments to suppress static analysis warnings where applicable. These changes improve the maintainability and readability of the codebase without affecting the functionality.
The PHPUnit version constraint in composer.json has been updated to "^8.5.40 || ^9.5" from "^8.5.24 || ^9.5" to ensure compatibility with recent bug fixes and improvements. This change will help maintain the stability and reliability of the development and testing environment.
This change updates the "ray/aop" package to version 2.16 in the composer.json file. The update aims to incorporate enhancements, bug fixes, or security improvements that come with the new version, ensuring better performance or compliance with the latest standards.
Update CI to support PHP 8.4 and include PHP 8.3 in old_stable
Add @psalm-pure and @psalm-immutable annotations to various classes to improve static analysis and enforce immutability. These changes ensure better code quality and maintainability by providing more explicit type information for tools like Psalm.
Cleaned up code by removing the default value for `Named::$value` and `Inject::$optional`, ensuring they must be explicitly set upon instantiation. The `psalm-pure` annotation was also removed from the `getStringName` function, reflecting the function's possible side effects or method calls. These changes improve code clarity and enforce proper initialization practices.
Corrected the spelling of "LazyInteterface" to "LazyInterface" in all relevant files to ensure consistent naming and prevent potential interface reference errors. This change impacts class implementations and annotations that depended on the misspelled interface name.
Enhance code quality with Psalm annotations
Update license copyright year(s)
The "baseline" script generates static analysis baselines for PHPStan and Psalm. This simplifies managing analysis results and helps suppress existing issues for future incremental improvements.
Introduced baseline files for PHPStan and Psalm to suppress known issues temporarily, improving focus on new errors. Added references to these baselines in the respective configuration files to aid static analysis setup.
Moved Aspect-Oriented Programming (AOP) binding logic to a new `enableAop` method for better modularity and clarity. Replaced direct assertions with a safer method call (`__setBindings`) to streamline the binding process and improve maintainability.
Switched the Ray/AOP dependency from a stable version to the ^2.x-dev branch. This allows for using the latest development changes while maintaining compatibility with the project.
Added stricter type assertion in `AnnotatedClassMethods` with `assert` to ensure constructor type safety. Removed unnecessary suppression comments and improved type annotations for better code reliability and maintainability.
Upgraded multiple packages to ensure compatibility and receive the latest bug fixes and improvements. Adjusted metadata such as branch aliases and autoload paths where necessary. This helps maintain system stability and prepares for future development.
Disabled the `--show-info` output in the `sa` script for psalm to reduce unnecessary verbosity. This ensures the analysis output remains focused on essential issues.
Added a normalization function to standardize line endings to '\n' in string assertions for consistent behavior across environments. Updated the test to apply this normalization to both expected and actual strings.
Upgraded `vimeo/psalm` to `^6.8` and adjusted `phpmetrics` to support version `^2.7 || v3.0.0rc8`. Removed `psalm/plugin-phpunit` as it is no longer required.
…guation Disambiguate source links for shared PSR-4 prefixes
Add explicit bindings snapshot API
rename() rewires a binding made before this module existed: the module
wraps another, moves the binding it is about to replace aside, binds its
own implementation over it, and injects the moved one by its new name.
That shape is the whole reason rename() exists -- `BEAR\Package\Context\
CliModule` composes every BEAR.Sunday console application with it, and
BEAR.Defer (TransferInterface) and BEAR.DevTools (RenderInterface) do the
same -- yet nothing pinned it. The 2.21 redesign passed the whole suite,
including the composition contract layer, while breaking all three.
The subject of a rename is always the wrapped module's container. That is
what keeps the ordering rule a single sentence: rename() reaches the past,
never the bindings this module is declaring right now. All four failures
below are that sentence being violated, each in its own way:
testDecoratesWrappedBinding
Unbound: 'Ray\Di\FakeRobotInterface-'
the decorator was renamed away from the index it was bound to
testMovedBindingIsInjectableByItsNewName
CircularDependency: 'Ray\Di\FakeRobotInterface-original
-> Ray\Di\FakeRobotInterface-original'
...onto its own inner name, so it injects itself
testMissingSourceInWrappedModuleThrows
Failed asserting that exception of type Unbound is thrown
a wrapped module without the binding is a configuration error, but
construction succeeds and leaves the container silently broken
testRenamingWithNothingWrappedIsNoOp
Unbound: 'Ray\Di\FakeRobotInterface-'
with nothing wrapped there is no past to rewire, yet an install()ed
binding this module made itself is moved out from under it
The spec deliberately does not pin renaming a binding this module composed
itself (own bind()/install()/override()). rename() has never done that in
any 2.x release.
rename() moves a binding of the module this one wraps, and nothing else.
That is the semantics every 2.x release shipped and every real caller was
built on, restored verbatim:
if ($this->lastModule instanceof self) {
$this->lastModule->getContainer()->move(...);
}
The 2.21 redesign replaced the wrapped module with "this module's own
container" as the subject. That reads more general, but the subject of a
rename is by definition a binding made before this module existed, and the
own container cannot hold it when rename() runs: configure() completes
before the wrapped module merges, precisely so this module's declarations
win. So the redesign had to defer the rename past the merge -- by which
point the decorator occupied the source index and was renamed onto its own
inner name. Every mechanism added to contain that (pendingRenames,
isConfiguring, hasBinding, applyPendingRenamesTo, discardPendingRenames)
is removed here; what remained of it was the wrapped module's container
reached through a deferral, which is what lastModule already was.
The capability the redesign added on the way -- renaming a binding this
module composed itself, via bind()/install()/override() -- is dropped. No
2.x release ever had it, and the three callers that exercise rename() in
the wild (BEAR.Package CliModule, BEAR.Defer, BEAR.DevTools) all wrap.
Its specs went with it: testRenamesBindingIntroducedByInstall and
testRenamesBindingAfterOverride pinned the new capability rather than any
2.x behavior. The rest of RenameTest keeps its intent against a wrapped
source, and BindingLogTest's rename spec holds unchanged -- the move is
recorded on the wrapped container's log and merge() carries the event and
its provenance across, so a rename still says the moved binding belongs to
the module that bound it.
$lastModule is no longer deprecated: it is the subject of the operation,
not a leftover. BEAR.Package reached for it directly to work around 2.21+
and can drop that workaround.
AbstractModule: 34 lines, 11 methods, 100% covered. 283 tests green,
cs/sa clean.
The 2.21.0 entry credits an `AbstractModule::renameBinding()` API. No release ships it: the API existed only between ray-di#319's first design, which added it alongside an unchanged rename(), and the tag, by which point it had been folded into rename() itself — the change that broke composition.
Restore rename() to its wrapper transformation
Nothing in the changelog claims a rename change: 2.21.0's entry credited a `renameBinding()` API that no release shipped, and that line is already corrected. So an entry restoring rename() explains the undoing of a change the changelog never mentions -- and for anyone who can install Ray.Di it is no change at all, since every version carrying it was withdrawn and 2.22.1's rename() behaves exactly as 2.20.0's. What happened is recorded where it belongs: the commits on this branch and ray-di#333. The changelog says what a reader upgrading from 2.20.0 gets.
The changelog began in the 2.21 cycle, so every entry in it described a version that was withdrawn before anyone installed it. Listing them told a reader about releases they never had, and the entries contradicted each other once read as a set: 2.21.0 added automatic `bindings.md` generation and 2.22.0 removed it, 2.21.0 cached a generation step that no longer exists, 2.22.0 kept `Ray\Di\BindingsMarkdown` as an adapter for a name no release ever shipped. 2.22.1 is what a reader can install, so it now carries what they get, described as it stands rather than as it was arrived at.
Use ReflectionClass::isInstantiable() instead of !isAbstract() when deciding untargeted/JIT eligibility in Container::unbound and Bind. Private and other non-instantiable concrete classes now fail with Unbound instead of attempting construction (which became a bare Error after NewInstance switched to language-level new).
Reject non-instantiable classes from just-in-time binding
Drop the Ray\Bindings namespace (Bindings, BindingsMarkdown, BindingsHtml, the bindings-html CLI and its viewer assets) and the deprecated Ray\Di\BindingsMarkdown shim. Rich visualization moves to the ray/object-visual-grapher package; composition provenance (BindingLog) stays in core. Ray\Di\ModuleVisitorInterface replaces Ray\Bindings\ModuleVisitorInterface as the stable route for tools to visit a module's composed container via AbstractModule::accept().
getInstance() on an unbound instantiable class must throw Untargeted (an Unbound) instead of binding the class just in time, and a named request must fail as a plain Unbound preserving its name. The old pins of runtime JIT behavior (auto-bind, single construction, on-demand AOP weaving, post-unserialize JIT provenance) are retired with the contract they pinned. Red on purpose: this commit documents the new contract; the fix that turns it green follows.
The Injector no longer catches Untargeted to bind an unbound concrete class on the fly: getInstance() resolves the container as composed and Untargeted propagates to the caller. Container::unbound() returns Untargeted only for Name::ANY requests, so a named miss is reported as a plain Unbound that preserves the requested name. Resolution no longer mutates the container, keeping the composed bindings race-free and identical to what CompiledInjector sees. Tests that leaned on JIT now bind their concrete classes explicitly, and the single-dependency weaveAspect() path is covered directly at the Container level.
An Injector created with an explicit tmpDir writes the composed bindings
to {tmpDir}/bindings.md: a deterministic, human- and agent-readable
snapshot of every resolved binding, the modules that composed them, and
the binding provenance. The shared sys_get_temp_dir() fallback is not
application-owned, so no file is written there.
Ray\Di\BindingsMarkdown keeps the 2.22 API (__invoke(Container, string)
and render(Container)) as the real implementation instead of a shim.
The writer is best-effort and atomic (tmp file + rename), and the file
carries its own signature trailer: an unchanged binding surface skips
rendering and writing, resolution performs no diagnostics I/O, and a
container revived with an empty provenance log keeps the richer
snapshot written at composition time.
SpyCompiler now names interceptor instances by their class so a
pointcut bound to an interceptor object renders instead of silently
skipping the write.
Architecture decision: record + publish + text artifact = core, exploration UI = external. Two-tier necessity: build time (fatal), runtime (best-effort). Lifecycle, risks, alternatives documented. Release order: Ray.Di 2.23.0 → ObjectGrapher 1.1.0 → Ray.Compiler.
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
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 @@
## 2.x #15 +/- ##
============================================
Coverage 100.00% 100.00%
- Complexity 351 411 +60
============================================
Files 47 50 +3
Lines 911 1062 +151
============================================
+ Hits 911 1062 +151 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| UnusedCode | 2 minor |
🟢 Metrics 85 complexity · 0 duplication
Metric Results Complexity 85 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
Owner
Author
|
Superseded by ray-di#336, which targets the correct upstream |
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
3 participants
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 introduces comprehensive binding diagnostics infrastructure and pins module composition semantics through contract tests. It establishes Ray.Di's commitment to observable, stable dependency injection behavior by recording binding provenance and generating human-readable binding documentation.
Key Changes
Binding Diagnostics Infrastructure
bindings.mddocumentation of all composed bindings with checksums for change detectionModule Composition Contracts
bind()overwrites (last write wins)install()and constructor chaining merge with+=(existing entries win)override()target always winsException Hierarchy Improvements
CircularDependency,NoHint,RenameTargetAlreadyBound,ReadOnlyMapAccess,UncallableSetterMethod,DirectoryNotWritable,InvalidAssistedInjectMatch,SpyCompilerNotInstantiablesrc-deprecated/di/Exception/Unboundexception with better error chain reportingBackward Compatibility Layer
src-deprecated/Type System & Documentation
Test Infrastructure
tests/di/README.mdDemo & Documentation
Notable Implementation Details
bindings.mdis always generated alongside proxy classesExceptionInterfacefor consistent error handlinghttps://claude.ai/code/session_017TRx6YLVCgL1wY9SuyfRuu