Skip to content

Commit

Permalink
fix fatal error when passing invalid class string as factory
Browse files Browse the repository at this point in the history
  • Loading branch information
filecage committed Jun 1, 2023
1 parent 0808cd1 commit 9ee0bd9
Show file tree
Hide file tree
Showing 2 changed files with 9 additions and 2 deletions.
4 changes: 2 additions & 2 deletions lib/InvokableFactory.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ class InvokableFactory extends Invokable {
private $factoryCreatable;

/**
* @param $factory
* @param class-string|string|callable|Factory $factory
*
* @return Invokable|InvokableFactory
* @throws InvalidFactory
Expand All @@ -27,7 +27,7 @@ static function createFromAnyFactory ($factory) {
} elseif (is_string($factory) && class_exists($factory)) {
$factoryCreatable = Creatable::createFromClassName($factory);
if (!$factoryCreatable->getReflectionClass()->implementsInterface(Factory::class)) {
throw new InvalidFactory(get_class($factory));
throw new InvalidFactory($factory, null, 'Factory does not implement ' . Factory::class . ' interface');

This comment has been minimized.

Copy link
@pleinx

pleinx Jun 1, 2023

@filecage maybe adding the className of factory without the right interface in the message?

'Factory {classNameOfGivenFactory} does not implement ' . Factory::class . ' interface'

This comment has been minimized.

Copy link
@filecage

filecage Jun 1, 2023

Author Owner

That's a part of the InvalidFactory exception message already. The full exception message would be:

Trying to register unsupported factory type Foo\Bar\MyFactoryForWhateverClass for class Foo\Bar\WhateverClass (Factory does not implement Creator\Interfaces\Factory interface)

It's just that the exception hierarchy is a bit clunky, so I added this as a second (optional) parameter to keep the previous hierarchy and messages the same.

This comment has been minimized.

Copy link
@pleinx

pleinx Jun 2, 2023

I got it, thank you :-)

}

$invokable = new InvokableFactory($factoryCreatable);
Expand Down
7 changes: 7 additions & 0 deletions tests/ExceptionsTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Creator\Exceptions\InvalidFactory;
use Creator\Exceptions\Unresolvable;
use Creator\Exceptions\UnresolvableDependency;
use Creator\Tests\Mocks\AnotherSimpleClass;
use Creator\Tests\Mocks\ExtendedClass;
use Creator\Tests\Mocks\InvalidClass;
use Creator\Tests\Mocks\InvalidNestedRequirementClass;
Expand Down Expand Up @@ -71,6 +72,12 @@ function testShouldThrowInvalidFactoryExceptionWhenRegisteringNullToInjectedRegi
$this->creator->createInjected(SimpleClass::class)->withFactory(null, SimpleClass::class);
}

function testShouldThrowInvalidFactoryWhenRegisteringFactoryThatDoesNotImplementFactoryInterface () {
$this->expectException(InvalidFactory::class);
$this->expectExceptionMessage('Trying to register unsupported factory type `Creator\Tests\Mocks\AnotherSimpleClass` for class `Creator\Tests\Mocks\SimpleClass` (Factory does not implement Creator\Interfaces\Factory interface)');
$this->creator->createInjected(ExtendedClass::class)->withFactory(AnotherSimpleClass::class, SimpleClass::class);
}

function testShouldThrowUnresolvableExceptionWhenUsingUnionTypes () {
if (version_compare(PHP_VERSION, '8.0.0', '<')) {
$this->markTestSkipped('Union Types are only relevant for PHP versions >= 8.0.0');
Expand Down

0 comments on commit 9ee0bd9

Please sign in to comment.