diff --git a/app/Commands/Stack/Compile.php b/app/Commands/Stack/Compile.php index 53f5148..c305a57 100644 --- a/app/Commands/Stack/Compile.php +++ b/app/Commands/Stack/Compile.php @@ -39,8 +39,8 @@ class Compile extends Command $serializer = $this->getSerializer(); $unserializer = $this->getUnserializer(); - $options = null; - $engine->setSerializer($serializer)->setUnserializer($unserializer)->setCompiler(new \App\Engine\Cfnpp\Compiler($options = new \App\Engine\Cfnpp\Options())); + $engine->setSerializer($serializer)->setUnserializer($unserializer)->setCompiler(new \App\Engine\Cfnpp\Compiler()); + $options = new \App\Engine\Cfnpp\Options(); $output = $engine->process($this->argument('in_file'), $options); diff --git a/app/Dom/Document.php b/app/Dom/Document.php index 1d180f6..19f6139 100644 --- a/app/Dom/Document.php +++ b/app/Dom/Document.php @@ -14,7 +14,7 @@ class Document extends Node /** * The original name of this document, as they will relate to each other * via include/stack refs, etc. - * @var string + * @var ?string */ protected $document_name; @@ -31,6 +31,10 @@ class Document extends Node */ public function getDocumentName(): string { + if (!isset($this->document_name)) + { + throw new \Exception('No document name set'); + } return $this->document_name; } diff --git a/app/Dom/Node.php b/app/Dom/Node.php index 376c303..26112ff 100644 --- a/app/Dom/Node.php +++ b/app/Dom/Node.php @@ -350,6 +350,12 @@ class Node implements \ArrayAccess, \Iterator $this->getParent()->removeChild($this); } + /** + * Convert a node tree containing basic data into native PHP data types. + * + * @param Node $node + * @return array|scalar + */ public static function toPhp(Node $node) { if ($node instanceof NodeValue) @@ -378,6 +384,12 @@ class Node implements \ArrayAccess, \Iterator throw new \Exception('Cannot convert from DOM node'); } + /** + * Convert native PHP data types into a node tree. + * + * @param array|scalar $value + * @return Node + */ public static function fromPhp($value): Node { if (is_scalar($value)) @@ -386,7 +398,7 @@ class Node implements \ArrayAccess, \Iterator } elseif (is_array($value)) { - $contains_only_numeric_keys = array_reduce(array_keys($value), static function($carry, $item) { + $contains_only_numeric_keys = array_reduce(array_keys($value), static function(bool $carry, string $item): bool { return $carry && is_numeric($item); }, true); diff --git a/app/Engine/Cfnpp/Compiler.php b/app/Engine/Cfnpp/Compiler.php index 9dd680e..0fe68a7 100644 --- a/app/Engine/Cfnpp/Compiler.php +++ b/app/Engine/Cfnpp/Compiler.php @@ -69,7 +69,7 @@ class Compiler implements \App\Engine\ICompile * * @param Document[] $documents * @param IOptions $options - * @return void + * @return Document */ public function compile(array $documents, IOptions $options): Document { @@ -126,8 +126,7 @@ class Compiler implements \App\Engine\ICompile $graph->add($doc->getDocumentName(), $stack); } - $graph->link(); - $docs = $graph->flatten(); + $docs = $graph->solve(); // Reorder all our documents in whatever order the dependency solver // says we should process them in. @@ -353,7 +352,7 @@ class Compiler implements \App\Engine\ICompile protected function getShortestCommonPrefix(array $strings): string { // Find longest common prefix - $shortest_string_length = array_reduce($strings, static function(int $carry, string $item) { + $shortest_string_length = array_reduce($strings, static function(int $carry, string $item): int { return min($carry, strlen($item)); }, PHP_INT_MAX); diff --git a/app/Engine/Cfnpp/Options.php b/app/Engine/Cfnpp/Options.php index 584c7af..81495ca 100644 --- a/app/Engine/Cfnpp/Options.php +++ b/app/Engine/Cfnpp/Options.php @@ -4,8 +4,6 @@ declare(strict_types=1); namespace App\Engine\Cfnpp; -use \App\Engine\IOptions; - /** * Options for controlling the Cfnpp compilation process. * diff --git a/app/Engine/ICompile.php b/app/Engine/ICompile.php index d4e6868..da26ddb 100644 --- a/app/Engine/ICompile.php +++ b/app/Engine/ICompile.php @@ -16,7 +16,7 @@ interface ICompile * * @param \App\Dom\Document[] $documents documents to compile * @param IOptions $options - * @return void + * @return \App\Dom\Document */ public function compile(array $documents, IOptions $options): \App\Dom\Document; } diff --git a/app/Util/DependencyGraph.php b/app/Util/DependencyGraph.php index d589ed3..f88e24d 100644 --- a/app/Util/DependencyGraph.php +++ b/app/Util/DependencyGraph.php @@ -4,9 +4,17 @@ declare(strict_types=1); namespace App\Util; +/** + * Rudimentary dependency graph solver. + * + * @author Adam Pippin + */ class DependencyGraph { - /** @var DependencyGraphNode[] */ + /** + * All nodes added to this solver. + * @var DependencyGraphNode[] + */ protected $nodes; public function __construct() @@ -14,6 +22,13 @@ class DependencyGraph $this->nodes = []; } + /** + * Add a node to this solver. + * + * @param string $name name of the node + * @param string[] $depends_on names of all nodes this node depends on + * @return void + */ public function add(string $name, array $depends_on = []): void { if (!isset($this->nodes[$name])) @@ -27,11 +42,32 @@ class DependencyGraph // Probably unnecessary -- if the same node is added multiple times // with different dependencies something's probably way messed up, // but... we'll try our best?! - $this->nodes[$name] = array_unique(array_merge($this->nodes[$name]->DependsOn, $depends_on)); + $this->nodes[$name]->DependsOn = array_unique(array_merge($this->nodes[$name]->DependsOn, $depends_on)); } } - public function link(): void + /** + * Link all nodes, solve dependencies, and flatten the graph into a single + * ordered list representing one possible solution to the processing order + * guaranteeing that no node will be processed _before_ its dependencies. + * + * @return string[] node names + */ + public function solve(): array + { + $this->link(); + $nodes = $this->flatten(); + return array_values(array_unique(array_map(static function(DependencyGraphNode $node): string { return $node->Name; }, $nodes))); + } + + /** + * Go through nodes, finding ones that depend on other nodes, and marking + * those nodes as being depended on by the original node. Actually building + * out a bi-directed graph from the single direction we started with. + * + * @return void + */ + protected function link(): void { foreach ($this->nodes as $node) { @@ -50,7 +86,13 @@ class DependencyGraph } } - public function flatten(): array + /** + * Given the list of nodes, flatten them into one possible solution on + * processing order and return an ordered list of nodes. + * + * @return DependencyGraphNode[] + */ + protected function flatten(): array { $state = []; @@ -82,27 +124,60 @@ class DependencyGraph --$i; } + // Reset all nodes in case we run again + foreach ($this->nodes as $node) + { + $node->Processed = false; + } + // Remove all but the first appearance of a node. // array_unique guarantees that the _first_ element will be retained, // so this satisfies what we're trying to do. - return array_unique(array_map(static function($node) { return $node->Name; }, $state)); + return $state; } } +/** + * Represents a single node in our dependency graph. + * + * @internal + * @author Adam Pippin + */ class DependencyGraphNode { - /** @var string */ + /** + * Name of this node. + * @var string + */ public $Name; - /** @var string */ + /** + * List of nodes this node depends on. + * @var string[] + */ public $DependsOn; - /** @var string */ + /** + * List of nodes that depend on this node. + * @var string[] + */ public $DependedOnBy; - /** @var bool */ + /** + * Whether this node has already had its dependencies included into the solver + * state. + * + * Remember how this whole class is @internal? Yep. + * + * @var bool + */ public $Processed; + /** + * Build a new, empty dependency graph node. + * + * @param string $name name of the node + */ public function __construct(string $name) { $this->Name = $name; @@ -110,14 +185,4 @@ class DependencyGraphNode $this->DependedOnBy = []; $this->Processed = false; } - - public function dependsOn(string $node): void - { - $this->DependsOn[] = $node; - } - - public function dependedOnBy(string $node): void - { - $this->DependedOnBy[] = $node; - } }