Skip to content

Commit

Permalink
Compiler circular dependency detection (switch to DFS).
Browse files Browse the repository at this point in the history
  • Loading branch information
@serge-kvashnin
serge-kvashnin committed May 26, 2024
1 parent fe0b73a commit 3167c5b
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 85 deletions.
102 changes: 56 additions & 46 deletions src/Compiler/Compiler.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
use Norvica\Container\Definition\Run;
use Norvica\Container\Definition\Val;
use Norvica\Container\Exception\ContainerException;
use Norvica\Container\Visitor;
use PhpParser\Node;
use PhpParser\Node\Arg;
use PhpParser\Node\Expr;
Expand Down Expand Up @@ -53,49 +54,47 @@ final class Compiler
{
private readonly Definitions $definitions;
private readonly Parser $parser;
private array $ast;
private array $map;
private array $postponed;
private readonly Visitor $visitor;
private array $body;
private array $hashes;

public function __construct(
Definitions $definitions,
) {
$ids = array_keys($definitions->all());
$hashes = array_map('md5', $ids);
$this->map = array_combine($ids, $hashes);
$this->hashes = array_combine($ids, $hashes);
$this->definitions = $definitions;
$this->postponed = [];
$this->parser = (new ParserFactory())->createForNewestSupportedVersion();
$this->visitor = new Visitor();
}

public function compile(string $class = 'Container'): string
{
$this->ast = $this->parser->parse(file_get_contents(__DIR__ . '/template.php'));
// echo (new \PhpParser\NodeDumper())->dump($this->ast)."\n";die;
$ast = $this->parser->parse(file_get_contents(__DIR__ . '/template.php'));

$this->ast[1]->name = new Identifier(name: $class);
$body = &$this->ast[1]->stmts;
$map = &$body[0]->consts[0]->value->items;
$ast[1]->name = new Identifier(name: $class);
$this->body = &$ast[1]->stmts;
$map = &$this->body[0]->consts[0]->value->items;

foreach ($this->map as $id => $hash) {
$definition = $this->definitions->get($id);
$body[] = $this->method($id, $hash, $this->definition($definition, $id));
}
foreach ($this->hashes as $id => $hash) {
// skip entries processed by DFS
if (isset($this->body[$id])) {
continue;
}

while ($this->postponed) {
$id = array_shift($this->postponed);
$body[] = $this->method($id, $this->map[$id], $this->definition(new Obj($id), $id));
$definition = $this->definitions->get($id);
$this->method($id, $hash, $definition);
}
// $this->definition(new Obj($id), $id)

foreach ($this->map as $id => $hash) {
foreach ($this->hashes as $id => $hash) {
$map[] = new ArrayItem(
value: new String_(value: $hash),
key: new String_(value: $id),
);
}

return (new Standard())->prettyPrintFile($this->ast);
return (new Standard())->prettyPrintFile($ast);
}

private function definition(
Expand All @@ -106,18 +105,6 @@ private function definition(
return $this->scalar($definition);
}

if (is_array($definition)) {
$items = [];
foreach ($definition as $key => $item) {
$items[] = new ArrayItem(
value: $this->definition($item),
key: is_int($key) ? new Int_(value: $key): new String_(value: $key),
);
}

return new Array_(items: $items);
}

if ($definition instanceof Val) {
return $this->val($definition);
}
Expand All @@ -138,7 +125,24 @@ private function definition(
return $this->run($definition, $id);
}

throw new ContainerException(); // TODO: message
if (is_array($definition)) {
$items = [];
foreach ($definition as $key => $item) {
$items[] = new ArrayItem(
value: $this->definition($item),
key: is_int($key) ? new Int_(value: $key) : new String_(value: $key),
);
}

return new Array_(items: $items);
}

throw new ContainerException(
sprintf(
"Expected definition, got '%s'.",
get_debug_type($definition),
)
);
}

private function val(Val $definition): Expr
Expand Down Expand Up @@ -171,6 +175,15 @@ private function env(Env $definition): Expr

private function ref(Ref $definition): Expr
{
if (!isset($this->hashes[$definition->id])) {
// autowiring
$this->hashes[$definition->id] = md5($definition->id);
$this->method($definition->id, $this->hashes[$definition->id], new Obj($definition->id));
} elseif (!isset($this->body[$definition->id])) {
// DFS
$this->method($definition->id, $this->hashes[$definition->id], $this->definitions->get($definition->id));
}

return new Coalesce(
left: new ArrayDimFetch(
var: new PropertyFetch(
Expand All @@ -181,7 +194,7 @@ private function ref(Ref $definition): Expr
),
right: new StaticCall(
class: new Name(name: 'self'),
name: "_{$this->map[$definition->id]}",
name: "_{$this->hashes[$definition->id]}",
args: [
new Arg(
value: new Variable(name: 'container'),
Expand Down Expand Up @@ -494,20 +507,15 @@ private function autowire(ReflectionParameter $rp): Expr
throw new ContainerException("Cannot autowire parameter {$reference} based on built-in type '{$rt->getName()}'.");
}

$id = $rt->getName();
if (isset($this->map[$id])) {
return $this->definition(new Ref($id));
}

$this->map[$id] = md5($id);
$this->postponed[] = $id;

return $this->definition(new Ref($id));
return $this->definition(new Ref($rt->getName()));
}

private function method(string $id, string $hash, Expr $definition): ClassMethod
private function method(string $id, string $hash, mixed $definition): void
{
return new ClassMethod(
$this->visitor->enter($id);
$expr = $this->definition($definition, $id);

$this->body[$id] = new ClassMethod(
name: "_{$hash}",
subNodes: [
'flags' => 12,
Expand All @@ -527,11 +535,13 @@ private function method(string $id, string $hash, Expr $definition): ClassMethod
),
dim: new String_(value: $id),
),
expr: $definition,
expr: $expr,
),
),
],
],
);

$this->visitor->exit($id);
}
}
44 changes: 6 additions & 38 deletions src/Container.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
use Norvica\Container\Definition\Ref;
use Norvica\Container\Definition\Run;
use Norvica\Container\Definition\Val;
use Norvica\Container\Exception\CircularDependencyException;
use Norvica\Container\Exception\ContainerException;
use Norvica\Container\Exception\NotFoundException;
use Psr\Container\ContainerExceptionInterface;
Expand All @@ -33,16 +32,14 @@ final class Container implements ContainerInterface
*/
private array $resolved = [];

/**
* @var string[]
*/
private array $resolving = [];
private Visitor $visitor;

public function __construct(
private readonly Definitions $definitions,
private readonly ContainerInterface|null $compiled = null,
private bool $autowiring = true,
private readonly bool $autowiring = true,
) {
$this->visitor = new Visitor();
}

/**
Expand All @@ -65,16 +62,7 @@ public function get(string $id): mixed
return $this->compiled->get($id);
}

if ($this->inProgress($id)) {
throw new CircularDependencyException(
sprintf(
"Circular dependency detected when resolving the following chain: '%s' → '{$id}'.",
implode("' → '", $this->resolving),
)
);
}

$this->resolvingStarted($id);
$this->visitor->enter($id);

// if ID is a class name, try to construct it, even if it's not registered explicitly
if (!$this->definitions->has($id) && $this->autowiring) {
Expand All @@ -84,14 +72,14 @@ public function get(string $id): mixed

$resolved = $this->resolve(new Obj($id));
$this->resolved[$id] = $resolved;
$this->resolvingFinished($id);
$this->visitor->exit($id);

return $resolved;
}

$resolved = $this->resolve($this->definitions->get($id));
$this->resolved[$id] = $resolved;
$this->resolvingFinished($id);
$this->visitor->exit($id);

return $resolved;
}
Expand Down Expand Up @@ -248,24 +236,4 @@ private function guess(ReflectionParameter $rp): mixed

return $this->get($rt->getName());
}

private function inProgress(string $id): bool
{
return in_array($id, $this->resolving, true);
}

private function resolvingStarted(string $id): void
{
$this->resolving[] = $id;
}

private function resolvingFinished(string $id): void
{
$index = array_search($id, $this->resolving, true);
if ($index === false) {
throw new ContainerException("Tried to finish resolving for '{$id}' that hasn't been started.");
}

unset($this->resolving[$index]);
}
}
48 changes: 48 additions & 0 deletions src/Visitor.php
View file
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
<?php

declare(strict_types=1);

namespace Norvica\Container;

use Norvica\Container\Exception\CircularDependencyException;
use Norvica\Container\Exception\ContainerException;

/**
* @internal
*/
final class Visitor
{
/**
* @var string[]
*/
private array $visiting = [];

public function enter(string $id): void
{
if ($this->visiting($id)) {
throw new CircularDependencyException(
sprintf(
"Circular dependency detected when resolving the following chain: '%s' → '{$id}'.",
implode("' → '", $this->visiting),
)
);
}

$this->visiting[] = $id;
}

public function exit(string $id): void
{
$index = array_search($id, $this->visiting, true);
if ($index === false) {
throw new ContainerException("Tried to exit node '{$id}' that hasn't been entered.");
}

unset($this->visiting[$index]);
}

private function visiting(string $id): bool
{
return in_array($id, $this->visiting, true);
}
}
11 changes: 10 additions & 1 deletion tests/Integration/CircularDependencyTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -37,11 +37,20 @@ public static function configuration(): Generator
}

#[DataProvider('configuration')]
public function test(array $configuration, string $id): void
public function testCold(array $configuration, string $id): void
{
$this->expectException(CircularDependencyException::class);
$container = $this->container($configuration);

$container->get($id);
}

#[DataProvider('configuration')]
public function testCompiled(array $configuration, string $id): void
{
$this->expectException(CircularDependencyException::class);
$container = $this->compiled($configuration);

$container->get($id);
}
}

0 comments on commit 3167c5b

Please sign in to comment.