Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use Ray.Aop's new binding injector _setBindings #297

Closed
wants to merge 10 commits into from
7 changes: 4 additions & 3 deletions composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"koriym/attributes": "^1.0.4",
"koriym/null-object": "^1.0",
"koriym/param-reader": "^1.0",
"ray/aop": "^2.16",
"ray/aop": "^2.x-dev",
Copy link

@coderabbitai coderabbitai bot Feb 22, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Verification agent

🧩 Analysis chain

Verify the stability implications of using dev version.

Using ^2.x-dev for ray/aop could introduce instability. Consider using a stable version that supports _setBindings.


🌐 Web query:

What is the latest stable version of ray/aop that supports _setBindings method?

💡 Result:

The latest stable version of Ray.Aop that supports the _setBindings method is version 2.16 "Tanabata", as indicated by its release notes[10]. However, this version introduces significant API simplifications and shifts toward using bind() instead of _setBindings for aspect configuration[1][10].

Key points:

  • Version 2.16 explicitly mentions API changes aimed at simplifying usage[10].
  • Earlier versions (like 2.10.x) still support _setBindings, as seen in Laravel integration examples requiring ray/aop ^2.10.1[4].
  • The newer bind() method (demonstrated in examples[1]) replaces _setBindings in modern implementations:
    $aspect->bind((new Matcher())->any(), ..., [new Interceptor()]);

For projects requiring _setBindings, use versions ≤2.15, but consider migrating to bind() for future compatibility[10]. Always verify method availability in specific releases via official documentation[1][10].

Citations:


Stable Dependency Recommendation: Update ray/aop Version

  • The use of "ray/aop": "^2.x-dev" poses a potential risk due to instability from a dev release.
  • If your project depends on the legacy _setBindings method, note that while version 2.16 "Tanabata" is the latest stable release, it shifts to using bind(), which may introduce breaking API changes.
  • To preserve compatibility with _setBindings, consider pinning the dependency to a stable version prior to 2.16 (e.g., ^2.15.0 or an earlier 2.x release).
  • Alternatively, if you prefer to upgrade, update your codebase to switch from _setBindings to the new bind() method as per the API changes in 2.16.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@coderabbitai The next version still not released.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@koriym Understood, thank you for the clarification about the unreleased version.


✏️ Learnings added
Learnt from: koriym
PR: ray-di/Ray.Di#297
File: composer.json:17-17
Timestamp: 2025-02-22T08:34:22.538Z
Learning: The use of "ray/aop": "^2.x-dev" in composer.json is intentional as it depends on features from the upcoming version that hasn't been released yet.

Note: Learnings are effective only in the context of similar code segments. To apply general coding guidelines, please configure review instructions. You can manage existing learnings in the app.

"ray/compiler": "^1.10.3"
},
"require-dev": {
Expand Down Expand Up @@ -47,10 +47,11 @@
"cs": ["phpcs --standard=./phpcs.xml src tests"],
"cs-fix": ["phpcbf src tests"],
"clean": ["phpstan clear-result-cache", "psalm --clear-cache", "rm -rf tests/tmp/*.php"],
"sa": ["psalm -m -c psalm.xml --show-info=true", "phpstan analyse -c phpstan.neon --no-progress "],
"sa": ["psalm -m -c psalm.xml --show-info=false", "phpstan analyse -c phpstan.neon --no-progress "],
"metrics": ["@test", "phpmetrics --report-html=build/metrics --exclude=Exception --log-junit=build/junit.xml --junit=build/junit.xml src"],
"phpmd": ["phpmd src/di text ./phpmd.xml"],
"build": ["@cs", "@sa", "@pcov", "@metrics"]
"build": ["@cs", "@sa", "@pcov", "@metrics"],
"baseline": ["phpstan analyse -c phpstan.neon --generate-baseline", "psalm --set-baseline=psalm-baseline.xml"]
},
"extra": {
"bamarni-bin": {
Expand Down
7 changes: 7 additions & 0 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
parameters:
ignoreErrors:
-
message: '#^Negated boolean expression is always false\.$#'
identifier: booleanNot.alwaysFalse
count: 1
path: src/di/AnnotatedClassMethods.php
33 changes: 33 additions & 0 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,33 @@
<?xml version="1.0" encoding="UTF-8"?>
<files psalm-version="4.30.0@d0bc6e25d89f649e4f36a534f330f8bb4643dd69">
<file src="src/di/AnnotatedClassMethods.php">
<DocblockTypeContradiction occurrences="2">
<code>! $constructor</code>
<code>$constructor</code>
</DocblockTypeContradiction>
</file>
<file src="src/di/AspectBind.php">
<MixedArrayOffset occurrences="1">
<code>$instantiatedBindings[$methodName]</code>
</MixedArrayOffset>
<MixedAssignment occurrences="3">
<code>$interceptorClassName</code>
<code>$interceptorClassNames</code>
<code>$methodName</code>
</MixedAssignment>
<MixedReturnTypeCoercion occurrences="2">
<code>$instantiatedBindings</code>
<code>array&lt;non-empty-string, list&lt;MethodInterceptor&gt;&gt;</code>
</MixedReturnTypeCoercion>
</file>
<file src="src/di/Dependency.php">
<InvalidArgument occurrences="1">
<code>$pointcuts</code>
</InvalidArgument>
</file>
<file src="src/di/NewInstance.php">
<InvalidArgument occurrences="1">
<code>$this-&gt;bind-&gt;inject($container)</code>
</InvalidArgument>
</file>
</files>
1 change: 1 addition & 0 deletions psalm.xml
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xmlns="https://getpsalm.org/schema/config"
xsi:schemaLocation="https://getpsalm.org/schema/config https://psalm.dev/schema/config"
errorBaseline="psalm-baseline.xml"
>
<projectFiles>
<directory name="src/di"/>
Expand Down
3 changes: 3 additions & 0 deletions src/di/AnnotatedClassMethods.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
use Ray\Di\Di\InjectInterface;
use Ray\Di\Di\Named;

use function assert;

use const PHP_VERSION_ID;

final class AnnotatedClassMethods
Expand Down Expand Up @@ -38,6 +40,7 @@ public function getConstructorName(ReflectionClass $class): Name
}
}

assert($constructor instanceof ReflectionMethod);
$named = $constructor->getAnnotation(Named::class);
if ($named instanceof Named) {
return new Name($named->value);
Expand Down
2 changes: 1 addition & 1 deletion src/di/AspectBind.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ public function __construct(AopBind $bind)
/**
* Instantiate interceptors
*
* @return array<string, list<MethodInterceptor>>
* @return array<non-empty-string, list<MethodInterceptor>>
*/
public function inject(Container $container): array
{
Expand Down
2 changes: 1 addition & 1 deletion src/di/BindValidator.php
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ public function to(string $interface, string $class): ReflectionClass
throw new InvalidType("[{$class}] is no implemented [{$interface}] interface");
}

return new ReflectionClass($class); // @phpstan-ignore-line
return new ReflectionClass($class);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Fix type safety issue in return type.

The method promises to return ReflectionClass<T> but returns ReflectionClass<object>, which could lead to type safety issues.

Apply this diff to fix the type safety issue:

-        return new ReflectionClass($class);
+        /** @var ReflectionClass<T> */
+        $reflectionClass = new ReflectionClass($class);
+        return $reflectionClass;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
return new ReflectionClass($class);
/** @var ReflectionClass<T> */
$reflectionClass = new ReflectionClass($class);
return $reflectionClass;
🧰 Tools
🪛 PHPStan (2.0.3)

45-45: Method Ray\Di\BindValidator::to() should return Ray\Aop\ReflectionClass but returns Ray\Aop\ReflectionClass.

(return.type)

}

/**
Expand Down
18 changes: 13 additions & 5 deletions src/di/NewInstance.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
namespace Ray\Di;

use Ray\Aop\Bind as AopBind;
use Ray\Aop\WeavedInterface;
use ReflectionClass;
use ReflectionException;

Expand Down Expand Up @@ -94,15 +95,22 @@ public function accept(VisitorInterface $visitor): void

private function postNewInstance(Container $container, object $instance): object
{
// bind dependency injected interceptors
if ($this->bind instanceof AspectBind) {
assert(isset($instance->bindings));
$instance->bindings = $this->bind->inject($container);
}
$this->enableAop($instance, $container);

// setter injection
($this->setterMethods)($instance, $container);

return $instance;
}

public function enableAop(object $instance, Container $container): void
{
if (! $this->bind instanceof AspectBind) {
return;
}

assert($instance instanceof WeavedInterface);

$instance->_setBindings($this->bind->inject($container)); // Ray.Aop ^2.18
}
}
2 changes: 1 addition & 1 deletion tests/di/AnnotatedClassTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ protected function setUp(): void

public function testInvoke(): void
{
$newInstance = $this->annotatedClass->getNewInstance(new ReflectionClass(FakeCar::class));
$newInstance = $this->annotatedClass->getNewInstance(new ReflectionClass(FakeCar::class)); // @phpstan-ignore-line
$this->assertInstanceOf(NewInstance::class, $newInstance);
$container = new Container();
(new Bind($container, FakeTyreInterface::class))->to(FakeTyre::class);
Expand Down
9 changes: 7 additions & 2 deletions tests/di/ModuleTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@
use PHPUnit\Framework\TestCase;
use Ray\Di\Exception\NotFound;

use function str_replace;

class ModuleTest extends TestCase
{
public function testNew(): void
Expand Down Expand Up @@ -50,14 +52,17 @@ public function testActivate(): void
public function testtoString(): void
{
$string = (string) new FakeLogStringModule();
$this->assertSame('-array => (array)
$normalize = static function (string $str): string {
return str_replace(["\r\n", "\r"], "\n", $str);
};
$this->assertSame($normalize('-array => (array)
-bool => (boolean) 1
-int => (integer) 1
-null => (NULL)
-object => (object) stdClass
-string => (string) 1
Ray\Di\FakeAopInterface- => (dependency) Ray\Di\FakeAop (aop) +returnSame(Ray\Di\FakeDoubleInterceptor)
Ray\Di\FakeDoubleInterceptor- => (dependency) Ray\Di\FakeDoubleInterceptor
Ray\Di\FakeRobotInterface- => (provider) (dependency) Ray\Di\FakeRobotProvider', $string);
Ray\Di\FakeRobotInterface- => (provider) (dependency) Ray\Di\FakeRobotProvider'), $normalize($string));
}
}
Loading
Loading