feat: param interface

[loks0n]

Summary by CodeRabbit

• Refactor

  • Restructured dependency resolution with enhanced dependency tracking and instance caching mechanisms.

• Removed

  • Removed Dependency class utility.

• Tests

  • Expanded test coverage for container operations, including caching behavior, hierarchical resolution, and dependency chaining scenarios.

[coderabbitai]

Warning

Rate limit exceeded

@loks0n has exceeded the limit for the number of commits that can be reviewed per hour. Please wait 18 minutes and 28 seconds before requesting another review.

⌛ How to resolve this issue?

After the wait time has elapsed, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout.

Please see our FAQ for further information.

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: bba7985e-1817-48c9-ae09-e60d962909da

📥 Commits

Reviewing files that changed from the base of the PR and between b5e51f7 and ae2b5d7.

⛔ Files ignored due to path filters (1)

• tools/rector/composer.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• src/DI/Container.php
• src/DI/Dependency.php
• tests/ContainerTest.php

Walkthrough

This pull request refactors the DI container's dependency resolution system. It introduces explicit dependency tracking per ID alongside factory callables, replacing the old resolution flow that used a resolving stack. A new private build() method constructs instances by resolving their dependencies and invoking their factories. The Container class exposes dependencies and factories as public properties while caching built instances in concrete. The set() method now accepts a dependencies parameter. The Dependency class is removed. Tests are updated to cover the new behavior with additional test methods.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

🚥 Pre-merge checks | ✅ 1 | ❌ 2

❌ Failed checks (2 warnings)

Check name	Status	Explanation	Resolution
Title check	⚠️ Warning	The title 'feat: param interface' does not clearly describe the actual changes, which involve a major refactoring of the Container class dependency resolution system, removal of the Dependency class, and comprehensive test updates.	Revise the title to reflect the primary changes, such as 'refactor: reimplement Container dependency resolution with explicit build flow' or 'refactor: consolidate Container dependency handling and remove Dependency class'.
Docstring Coverage	⚠️ Warning	Docstring coverage is 10.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (1 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Post copyable unit tests in a comment
• Commit unit tests in branch feat/param-interface

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 4

🧹 Nitpick comments (1)

src/DI/Container.php (1)

12-24: Consider making $dependencies and $factories protected or private.

Exposing these as public allows external code to mutate the container's internal state directly, bypassing set(). This could lead to inconsistent state (e.g., adding to $factories without corresponding $dependencies).

Proposed change to restrict access

-    public array $dependencies = [];
+    protected array $dependencies = [];
     ...
-    public array $factories = [];
+    protected array $factories = [];

If external read access is needed, consider adding getter methods instead.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/DI/Container.php` around lines 12 - 24, Make $dependencies and $factories
non-public to prevent external mutation: change their visibility from public to
protected or private in the Container class and update any internal uses
accordingly (e.g., code referencing $this->dependencies or $this->factories
inside Container methods remains valid). Add explicit getter methods (e.g.,
getDependencies() and getFactories()) if external read access is required, and
ensure any external callers currently accessing the properties are updated to
use those getters; keep the existing set() factory registration flow as the only
sanctioned mutator.

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@src/DI/Container.php`:
- Around line 93-101: In build() fix the dependency accumulation and invocation:
in the foreach over $this->dependencies[$id] append each resolved dependency to
the $dependencies array (e.g., $dependencies[] = $this->get($dependency))
instead of overwriting it, and invoke the factory stored in
$this->factories[$id] by spreading the accumulated array so each dependency
becomes a separate argument (i.e., call the factory with the spread operator
...$dependencies).
- Around line 44-50: The PHPDoc for the method (the docblock above the method
that registers dependencies—look for the method named register or the method
that accepts parameters ($id, $factory, $dependencies)) contains a redundant
"@return static" tag; remove that "@return static" line from the docblock so the
docblock no longer duplicates the method return type declared in the signature
and the Rector dry-run error is resolved.
- Around line 71-73: The parent type check in the get method currently uses
"instanceof Container" which is inconsistent with the constructor accepting
"?ContainerInterface" and can ignore valid parents that implement
ContainerInterface; change the check in the get method to "instanceof
ContainerInterface" so the method delegates to any parent implementing the
interface (Container::get -> parent->get), or if the intent was to restrict
parents to your concrete Container class instead, update the constructor
signature to "?Container" to match; adjust the code in the constructor or the
get method accordingly to keep the types consistent.

In `@tests/ContainerTest.php`:
- Around line 107-114: In testFactoryReceivesResolvedDependency update the arrow
function passed to $container->set('message', ...) so it has an explicit return
type; change the factory from fn($greeting) => "$greeting, World!" to include a
return type (e.g., fn($greeting): string => "$greeting, World!"), leaving the
rest of the test and the 'greeting' factory unchanged.

---

Nitpick comments:
In `@src/DI/Container.php`:
- Around line 12-24: Make $dependencies and $factories non-public to prevent
external mutation: change their visibility from public to protected or private
in the Container class and update any internal uses accordingly (e.g., code
referencing $this->dependencies or $this->factories inside Container methods
remains valid). Add explicit getter methods (e.g., getDependencies() and
getFactories()) if external read access is required, and ensure any external
callers currently accessing the properties are updated to use those getters;
keep the existing set() factory registration flow as the only sanctioned
mutator.

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 7e351d9b-b63f-4ce6-8cc9-118677d90d97

📥 Commits

Reviewing files that changed from the base of the PR and between 6a91359 and b5e51f7.

⛔ Files ignored due to path filters (1)

• tools/rector/composer.lock is excluded by !**/*.lock

📒 Files selected for processing (3)

• src/DI/Container.php
• src/DI/Dependency.php
• tests/ContainerTest.php

💤 Files with no reviewable changes (1)

• src/DI/Dependency.php