Cleanup

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());
-		$engine->setSerializer($serializer)->setUnserializer($unserializer)->setCompiler(new \App\Engine\Cfnpp\Compiler($options = new \App\Engine\Cfnpp\Options()));
+		$options = new \App\Engine\Cfnpp\Options();
 
 		$output = $engine->process($this->argument('in_file'), $options);
 

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;
 	}
 

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);
 

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->solve();
-		$docs = $graph->flatten();
 
 		// 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);
 

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.
  *

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;
 }

app/Util/DependencyGraph.php

@@ -4,9 +4,17 @@ declare(strict_types=1);
 
 namespace App\Util;
 
+/**
+ * Rudimentary dependency graph solver.
+ *
+ * @author Adam Pippin <hello@adampippin.ca>
+ */
 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 <hello@adampippin.ca>
+ */
 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;
-	}
 }