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

Fix: Require to pass name to ContainerBuilder::add() #702

Merged
merged 1 commit into from
Sep 3, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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: 2 additions & 0 deletions roave-bc-check.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -16,3 +16,5 @@ parameters:
- '#\[BC\] REMOVED: Class Faker\\Extension\\Container has been deleted#'
- '#\[BC\] CHANGED: The parameter \$container of Faker\\Generator\#\_\_construct\(\) changed from Psr\\Container\\ContainerInterface\|null to a non-contravariant Faker\\Container\\ContainerInterface\|null#'
- '#\[BC\] CHANGED: The parameter \$container of Faker\\Generator\#\_\_construct\(\) changed from Psr\\Container\\ContainerInterface\|null to Faker\\Container\\ContainerInterface\|null#'
- '#\[BC\] CHANGED: The number of required arguments for Faker\\Container\\ContainerBuilder\#add\(\) increased from 1 to 2#'
- '#\[BC\] CHANGED: The parameter \$name of Faker\\Container\\ContainerBuilder\#add\(\) changed from string\|null to a non-contravariant string#'
bram-pkg marked this conversation as resolved.
Show resolved Hide resolved
15 changes: 1 addition & 14 deletions src/Faker/Container/ContainerBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ final class ContainerBuilder
*
* @throws \InvalidArgumentException
*/
public function add($value, string $name = null): self
public function add($value, string $name): self
{
if (!is_string($value) && !is_callable($value) && !is_object($value)) {
throw new \InvalidArgumentException(sprintf(
Expand All @@ -38,19 +38,6 @@ public function add($value, string $name = null): self
));
}

if ($name === null) {
if (is_string($value)) {
$name = $value;
} elseif (is_object($value)) {
$name = get_class($value);
} else {
throw new \InvalidArgumentException(sprintf(
'Second argument to "%s::add()" is required not passing a string or object as first argument',
self::class,
));
}
}
bram-pkg marked this conversation as resolved.
Show resolved Hide resolved

$this->definitions[$name] = $value;

return $this;
Expand Down
49 changes: 4 additions & 45 deletions test/Faker/Extension/ContainerBuilderTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,6 @@
use Faker\Container\ContainerBuilder;
use Faker\Container\ContainerInterface;
use Faker\Core\File;
use Faker\Extension\Extension;
use PHPUnit\Framework\TestCase;

/**
Expand All @@ -30,7 +29,7 @@ public function testAddRejectsInvalidValue($value): void
ContainerBuilder::class,
));

$containerBuilder->add($value);
$containerBuilder->add($value, 'foo');
}

/**
Expand Down Expand Up @@ -59,46 +58,6 @@ public function provideInvalidValue(): \Generator
}
}

public function testAddRejectsNameWhenValueIsCallableAndNameIsNull(): void
{
$value = [
new class() {
public static function create(): Extension
{
return new class() implements Extension {
};
}
},
'create',
];

$containerBuilder = new ContainerBuilder();

$this->expectException(\InvalidArgumentException::class);
$this->expectExceptionMessage(sprintf(
'Second argument to "%s::add()" is required not passing a string or object as first argument',
ContainerBuilder::class,
));

$containerBuilder->add($value);
}

public function testAddAcceptsValueWhenItIsAnObjectAndNameIsNull(): void
{
$value = new class() implements Extension {};

$name = get_class($value);

$containerBuilder = new ContainerBuilder();

$containerBuilder->add($value);

$container = $containerBuilder->build();

self::assertTrue($container->has($name));
self::assertSame($value, $container->get($name));
}

public function testBuildEmpty(): void
{
$builder = new ContainerBuilder();
Expand All @@ -112,7 +71,7 @@ public function testBuild(): void
{
$builder = new ContainerBuilder();

$builder->add(File::class);
$builder->add(File::class, 'foo');

$container = $builder->build();

Expand All @@ -123,8 +82,8 @@ public function testBuildWithDuplicates(): void
{
$builder = new ContainerBuilder();

$builder->add(File::class);
$builder->add(File::class);
$builder->add(File::class, 'foo');
$builder->add(File::class, 'foo');

$container = $builder->build();

Expand Down
2 changes: 1 addition & 1 deletion test/Faker/GeneratorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -145,7 +145,7 @@ public function testFormatterCallsGenerator(): void
public function testFormatterCallsExtension(): void
{
$builder = new ContainerBuilder();
$builder->add(Blood::class);
$builder->add(Blood::class, Blood::class);
$faker = new Generator($builder->build());

$output = $faker->format('Faker\Core\Blood->bloodType');
Expand Down
Loading