Skip to content

Discover providers from the project itself. #84

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.

Sign up for GitHub

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

Merged
GautierDele merged 2 commits into from
Jul 31, 2026
Merged

Discover providers from the project itself. #84

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5611b17
Discover providers from the project itself.
sukei Jul 8, 2026
8841a31
Add unit tests for the project provider discovery
GautierDele Jul 31, 2026
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
7 changes: 4 additions & 3 deletions src/Manifests/ContainerMixinManifest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,8 +120,8 @@ public function build(array $extensionMethods, array $extensions)

$tags[] = new Method(
$methodName,
returnType: $reflectionMethod->hasReturnType() ? (new TypeResolver())->resolve($reflectionMethod->getReturnType()) : new Mixed_(),
parameters: $parameters,
returnType: $reflectionMethod->hasReturnType() ? (new TypeResolver())->resolve($reflectionMethod->getReturnType()) : new Mixed_(),
Comment thread
GautierDele marked this conversation as resolved.
);
}

Expand All @@ -142,8 +142,9 @@ public function build(array $extensionMethods, array $extensions)
public function shouldRecompile(): bool
{
return !is_file($this->containerMixinPath) ||
// We check here if the manifest has been generated before changing the installed.json composer file
filemtime($this->containerMixinPath) <= filemtime($this->vendorPath.'/composer/installed.json');
// We check here if the manifest has been generated before changing the installed.json composer file or the project composer.json
filemtime($this->containerMixinPath) <= filemtime($this->vendorPath.'/composer/installed.json') ||
filemtime($this->containerMixinPath) <= filemtime($this->basePath.'/composer.json');
}

/**
Expand Down
13 changes: 11 additions & 2 deletions src/Manifests/PackageManifest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,14 @@ public function build()
}
}

if (is_file($path = $this->basePath.'/composer.json')) {
$package = json_decode(file_get_contents($path), true);

if (isset($package['extra']['faker'])) {
$packagesToProvide[$package['name'] ?? 'root'] = $package['extra']['faker'];
}
}

$this->write(
$packagesToProvide
);
Expand All @@ -133,8 +141,9 @@ public function build()
public function shouldRecompile(): bool
{
return !is_file($this->manifestPath) ||
// We check here if the manifest has been generated before changing the installed.json composer file
filemtime($this->manifestPath) <= filemtime($this->vendorPath.'/composer/installed.json');
// We check here if the manifest has been generated before changing the installed.json composer file or the project composer.json
filemtime($this->manifestPath) <= filemtime($this->vendorPath.'/composer/installed.json') ||
filemtime($this->manifestPath) <= filemtime($this->basePath.'/composer.json');
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.

/**
Expand Down
65 changes: 65 additions & 0 deletions tests/Support/Concerns/CreatesTemporaryProjects.php
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,65 @@
<?php

namespace Xefi\Faker\Tests\Support\Concerns;

trait CreatesTemporaryProjects
{
/**
* The temporary project paths created during the test.
*
* @var array
*/
private array $temporaryProjectPaths = [];

/**
* Create a temporary project directory holding an installed.json and,
* when given, a root composer.json.
*
* @param ?array $composer
* @param array $installedPackages
*
* @return string
*/
private function createTemporaryProject(?array $composer = null, array $installedPackages = []): string
{
$projectPath = sys_get_temp_dir().'/faker-php-'.uniqid();

mkdir($projectPath.'/vendor/composer', 0777, true);

file_put_contents(
$projectPath.'/vendor/composer/installed.json',
json_encode(['packages' => $installedPackages])
);

if ($composer !== null) {
file_put_contents($projectPath.'/composer.json', json_encode($composer));
}

$this->temporaryProjectPaths[] = $projectPath;

return $projectPath;
}

/**
* Remove every temporary project created during the test.
*
* @return void
*/
private function deleteTemporaryProjects(): void
{
foreach ($this->temporaryProjectPaths as $projectPath) {
$files = new \RecursiveIteratorIterator(
new \RecursiveDirectoryIterator($projectPath, \FilesystemIterator::SKIP_DOTS),
\RecursiveIteratorIterator::CHILD_FIRST
);

foreach ($files as $file) {
$file->isDir() ? rmdir($file->getPathname()) : unlink($file->getPathname());
}

rmdir($projectPath);
}

$this->temporaryProjectPaths = [];
}
}
3 changes: 3 additions & 0 deletions tests/Support/composer.json
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
{
"name": "xefi/faker-php-tests-support"
}
Comment thread
GautierDele marked this conversation as resolved.
32 changes: 32 additions & 0 deletions tests/Unit/ContainerMixinManifestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,12 @@

namespace Xefi\Faker\Tests\Unit;

use Xefi\Faker\Tests\Support\Concerns\CreatesTemporaryProjects;

final class ContainerMixinManifestTest extends TestCase
{
use CreatesTemporaryProjects;

protected function setUp(): void
{
parent::setUp();
Expand All @@ -17,6 +21,13 @@ protected function setUp(): void
]);
}

protected function tearDown(): void
{
$this->deleteTemporaryProjects();

parent::tearDown();
}

public function testContainerMixinBuild()
{
@unlink('/tmp/ContainerMixin.php');
Expand Down Expand Up @@ -111,6 +122,27 @@ public function testShouldRecompile()
unlink('/tmp/ContainerMixin.php');
}

public function testShouldRecompileWhenTheProjectComposerFileChanged()
{
$projectPath = $this->createTemporaryProject(['name' => 'xefi/my-project']);

$container = new \Xefi\Faker\Container\Container(shouldBuildContainerMixin: false);
$manifest = new \Xefi\Faker\Manifests\ContainerMixinManifest($projectPath, $projectPath.'/ContainerMixin.php');
$manifest->build($container->getExtensionMethods(), $container->getExtensions());
touch($projectPath.'/vendor/composer/installed.json', time() - 1);
touch($projectPath.'/composer.json', time() - 1);

$this->assertFalse($manifest->shouldRecompile());

// Test on current time
touch($projectPath.'/composer.json');
$this->assertTrue($manifest->shouldRecompile());

// Test on future
touch($projectPath.'/composer.json', time() + 1);
$this->assertTrue($manifest->shouldRecompile());
}

public function testNoExtension()
{
@unlink('/tmp/ContainerMixin.php');
Expand Down
203 changes: 203 additions & 0 deletions tests/Unit/PackageManifestTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,9 +5,20 @@
namespace Xefi\Faker\Tests\Unit;

use Xefi\Faker\Manifests\PackageManifest;
use Xefi\Faker\Tests\Support\Concerns\CreatesTemporaryProjects;
use Xefi\Faker\Tests\Support\TestServiceProvider;

final class PackageManifestTest extends TestCase
{
use CreatesTemporaryProjects;

protected function tearDown(): void
{
$this->deleteTemporaryProjects();

parent::tearDown();
}

public function testAssetLoading()
{
@unlink('/tmp/packages.php');
Expand Down Expand Up @@ -41,4 +52,196 @@ public function testShouldRecompile()

unlink('/tmp/packages.php');
}

public function testProjectProvidersAreDiscovered()
{
$projectPath = $this->createTemporaryProject([
'name' => 'xefi/my-project',
'extra' => [
'faker' => [
'providers' => [TestServiceProvider::class],
],
],
]);

$manifest = new PackageManifest($projectPath, $projectPath.'/packages.php');

$this->assertEquals(
[
'xefi/my-project' => [TestServiceProvider::class],
],
$manifest->providers()
);
}

public function testProjectProvidersAreMergedWithTheInstalledPackagesOnes()
{
$projectPath = $this->createTemporaryProject(
[
'name' => 'xefi/my-project',
'extra' => [
'faker' => [
'providers' => ['Xefi\Faker\Tests\Support\ProjectServiceProvider'],
],
],
],
[
[
'name' => 'xefi/faker-number',
'extra' => [
'faker' => [
'providers' => [TestServiceProvider::class],
],
],
],
]
);

$manifest = new PackageManifest($projectPath, $projectPath.'/packages.php');

$this->assertEquals(
[
'xefi/faker-number' => [TestServiceProvider::class],
'xefi/my-project' => ['Xefi\Faker\Tests\Support\ProjectServiceProvider'],
],
$manifest->providers()
);
}

public function testProjectProvidersAreKeyedByRootWhenTheProjectHasNoName()
{
$projectPath = $this->createTemporaryProject([
'extra' => [
'faker' => [
'providers' => [TestServiceProvider::class],
],
],
]);

$manifest = new PackageManifest($projectPath, $projectPath.'/packages.php');

$this->assertEquals(
[
'root' => [TestServiceProvider::class],
],
$manifest->providers()
);
}

public function testProjectWithoutFakerConfigurationIsIgnored()
{
$projectPath = $this->createTemporaryProject(
[
'name' => 'xefi/my-project',
'extra' => [
'branch-alias' => ['dev-master' => '2.0.x-dev'],
],
],
[
[
'name' => 'xefi/faker-number',
'extra' => [
'faker' => [
'providers' => [TestServiceProvider::class],
],
],
],
]
);

$manifest = new PackageManifest($projectPath, $projectPath.'/packages.php');

$this->assertEquals(
[
'xefi/faker-number' => [TestServiceProvider::class],
],
$manifest->providers()
);
}

public function testProjectTakesPrecedenceOverAnInstalledPackageOfTheSameName()
{
$projectPath = $this->createTemporaryProject(
[
'name' => 'xefi/faker-number',
'extra' => [
'faker' => [
'providers' => ['Xefi\Faker\Tests\Support\ProjectServiceProvider'],
],
],
],
[
[
'name' => 'xefi/faker-number',
'extra' => [
'faker' => [
'providers' => [TestServiceProvider::class],
],
],
],
]
);

$manifest = new PackageManifest($projectPath, $projectPath.'/packages.php');

$this->assertEquals(
[
'xefi/faker-number' => ['Xefi\Faker\Tests\Support\ProjectServiceProvider'],
],
$manifest->providers()
);
}

public function testProjectWithoutComposerFileIsIgnored()
{
$projectPath = $this->createTemporaryProject(null, [
[
'name' => 'xefi/faker-number',
'extra' => [
'faker' => [
'providers' => [TestServiceProvider::class],
],
],
],
]);

$manifest = new PackageManifest($projectPath, $projectPath.'/packages.php');
$manifest->build();

$this->assertEquals(
[
'xefi/faker-number' => [
'providers' => [TestServiceProvider::class],
],
],
require $projectPath.'/packages.php'
);
}
Comment on lines +195 to +219

@coderabbitai coderabbitai Bot Jul 31, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🩺 Stability & Availability | 🟠 Major | ⚡ Quick win

Handle a missing root composer.json in shouldRecompile().

This test declares that a project without composer.json is supported, but it only calls build(). PackageManifest::shouldRecompile() unconditionally calls filemtime($this->basePath.'/composer.json'), which emits a warning when this file is absent.

Guard that timestamp check with is_file(). Add a shouldRecompile() assertion here after build().

🧰 Tools
🪛 OpenGrep (1.26.0)

[ERROR] 217-217: Dynamic file path passed to include/require. This can lead to local or remote file inclusion. Use a fixed allowlist of paths.

(coderabbit.file-inclusion.php-dynamic-include)

🤖 Prompt for AI Agents 
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@tests/Unit/PackageManifestTest.php` around lines 195 - 219, The
PackageManifest test must cover projects without a root composer.json during
recompilation checks. In PackageManifest::shouldRecompile(), guard the
composer.json filemtime lookup with is_file() so missing files do not emit
warnings, then update testProjectWithoutComposerFileIsIgnored() to assert the
expected shouldRecompile() result after build().


public function testShouldRecompileWhenTheProjectComposerFileChanged()
{
$projectPath = $this->createTemporaryProject([
'name' => 'xefi/my-project',
'extra' => [
'faker' => [
'providers' => [TestServiceProvider::class],
],
],
]);

$manifest = new PackageManifest($projectPath, $projectPath.'/packages.php');
$manifest->build();
touch($projectPath.'/vendor/composer/installed.json', time() - 1);
touch($projectPath.'/composer.json', time() - 1);

$this->assertFalse($manifest->shouldRecompile());

// Test on current time
touch($projectPath.'/composer.json');
$this->assertTrue($manifest->shouldRecompile());

// Test on future
touch($projectPath.'/composer.json', time() + 1);
$this->assertTrue($manifest->shouldRecompile());
}
}
Loading