Skip to content

[13.x] RFC: Lazy Services - New #[Lazy] Attribute #55645

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

Draft
wants to merge 2 commits into
base: master
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
}
],
"require": {
"php": "^8.3",
"php": "^8.4",
Copy link
Member

Choose a reason for hiding this comment

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

Laravel 13 will depends on PHP 8.3 as minimum.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alrighty, in that case we can consider version 14.x, but any potential merging should wait until 13.x is released - is that correct? :)

"ext-ctype": "*",
"ext-filter": "*",
"ext-hash": "*",
Expand Down
11 changes: 11 additions & 0 deletions src/Illuminate/Container/Attributes/Lazy.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
<?php

namespace Illuminate\Container\Attributes;

use Attribute;

#[Attribute]
class Lazy
{
//
}
11 changes: 10 additions & 1 deletion src/Illuminate/Container/Container.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use ArrayAccess;
use Closure;
use Exception;
use Illuminate\Container\Attributes\Lazy;
use Illuminate\Contracts\Container\BindingResolutionException;
use Illuminate\Contracts\Container\CircularDependencyException;
use Illuminate\Contracts\Container\Container as ContainerContract;
Expand Down Expand Up @@ -1058,8 +1059,16 @@ public function build($concrete)

array_pop($this->buildStack);

if (!empty($reflector->getAttributes(Lazy::class))) {
Copy link
Contributor

Choose a reason for hiding this comment

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

You could just directly check this

Suggested change
if (!empty($reflector->getAttributes(Lazy::class))) {
if ($reflector->getAttributes(Lazy::class) !== []) {

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does this change bring anything beyond syntax? I’m just asking out of curiosity.

Copy link
Contributor

Choose a reason for hiding this comment

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

Not much. Technically, a function call is more expensive than a comparison, but this should mostly be optimized away in practice. One slight advantage would be a clear communication that we are working with an array here and don't have to check any structure's emptiness.

Choose a reason for hiding this comment

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

@olekjs empty function is a trap for future bugs.
We have a rule in place to never use empty function because of that.

empty('0') is true for example.

$instance = $reflector->newLazyProxy(function () use ($concrete, $instances) {
return new $concrete(...$instances);
});
Comment on lines +1069 to +1071
Copy link
Contributor

Choose a reason for hiding this comment

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

You could do just

Suggested change
$instance = $reflector->newLazyProxy(function () use ($concrete, $instances) {
return new $concrete(...$instances);
});
$instance = $reflector->newLazyProxy(fn () => new $concrete(...$instances));

} else {
$instance = $reflector->newInstanceArgs($instances);
}

$this->fireAfterResolvingAttributeCallbacks(
$reflector->getAttributes(), $instance = $reflector->newInstanceArgs($instances)
$reflector->getAttributes(), $instance
);

return $instance;
Expand Down
133 changes: 133 additions & 0 deletions tests/Container/ContainerLazyTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
<?php

namespace Illuminate\Tests\Container;

use Illuminate\Container\Attributes\Lazy;
use Illuminate\Container\Container;
use PHPUnit\Framework\TestCase;

class ContainerLazyTest extends TestCase
{
protected function tearDown(): void
{
Container::setInstance();
}

public function testConcreteDoesNotThrowsExceptionWithAttribute()
{
$container = new Container;
$lazy = $container->make(LazyWithAttributeStub::class);

// No RuntimeException has occurred
// LazyWithAttributeStub behaves like a Lazy Object, but this is not obvious from its type
$this->assertInstanceOf(LazyWithAttributeStub::class, $lazy);
}

public function testConcreteThrowsExceptionWithoutAttribute()
{
$this->expectException(\RuntimeException::class);
$this->expectExceptionMessage('Lazy call');

$container = new Container;
$container->make(LazyWithoutAttributeStub::class);
}

public function testConcreteDoesNotThrowsExceptionWithNoLogicConstructor()
{
$container = new Container;
$lazy = $container->make(LazyWithAttributeStub::class);

$this->assertInstanceOf(LazyWithAttributeStub::class, $lazy);

$this->assertSame('work', $lazy->work());
}

public function testConcreteDoesThrowsExceptionWithConstructorWithLogic()
{
$container = new Container;
$lazy = $container->make(LazyWithAttributeLogicStub::class);

// No RuntimeException has occurred so far
$this->assertInstanceOf(LazyWithAttributeLogicStub::class, $lazy);

$this->expectException(\RuntimeException::class);
$this->expectExceptionMessage('Lazy call');

// Only the call to number() causes a RuntimeException
$lazy->number();
}

public function testConcreteThrowsExceptionButNotLazyDependency()
{
$this->expectException(\RuntimeException::class);
$this->expectExceptionMessage('Parent call');

$container = new Container;
$container->make(LazyDependencyWithAttributeStub::class);
}

public function testConcreteNotLazyDependencyThrowsException()
{
$this->expectException(\RuntimeException::class);
$this->expectExceptionMessage('Lazy call');

$container = new Container;
$container->make(LazyDependencyWithoutAttributeStub::class);
}
}

#[Lazy]
class LazyWithAttributeStub
{
public function __construct()
{
throw new \RuntimeException('Lazy call');
}

public function work()
{
return 'work';
}
}

#[Lazy]
class LazyWithAttributeLogicStub
{
public $number;

public function __construct()
{
$this->number = 10;

throw new \RuntimeException('Lazy call');
}

public function number()
{
$this->number += 10;
}
}

class LazyWithoutAttributeStub
{
public function __construct()
{
throw new \RuntimeException('Lazy call');
}
}

class LazyDependencyWithAttributeStub
{
public function __construct(LazyWithAttributeStub $stub)
{
throw new \RuntimeException('Parent call');
}
}

class LazyDependencyWithoutAttributeStub
{
public function __construct(LazyWithoutAttributeStub $stub)
{
throw new \RuntimeException('Parent call');
}
}
Loading