Code maintenance and fixes

4 files changed, 196 insertions, 96 deletions

diff --git a/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTDefinedChecker.java b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTDefinedChecker.java
new file mode 100644
index 0000000..247054a
--- /dev/null
+++ b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTDefinedChecker.java
@@ -0,0 +1,58 @@
+package bjc.dicelang.ast;
+
+import java.util.Map;
+import java.util.function.Consumer;
+
+import bjc.utils.data.IHolder;
+
+/**
+ * Check if the specified node references a particular variable
+ * 
+ * @author ben
+ *
+ */
+public final class DiceASTDefinedChecker
+		implements Consumer<IDiceASTNode> {
+	/**
+	 * This is true if the specified node references the set variable
+	 */
+	private IHolder<Boolean>				referencesVariable;
+
+	private Map<String, DiceASTExpression>	enviroment;
+
+	/**
+	 * Create a new reference checker
+	 * 
+	 * @param referencesVar
+	 *            The holder of whether the variable is referenced or not
+	 * @param env
+	 *            The enviroment to check undefinedness against
+	 */
+	public DiceASTDefinedChecker(IHolder<Boolean> referencesVar,
+			Map<String, DiceASTExpression> env) {
+		this.referencesVariable = referencesVar;
+		this.enviroment = env;
+	}
+
+	@Override
+	public void accept(IDiceASTNode astNode) {
+		referencesVariable.transform((bool) -> checkUndefined(astNode));
+	}
+
+	/**
+	 * Check if a given AST node references an undefined variable
+	 * 
+	 * @param astNode
+	 *            The node to check
+	 * @return Whether or not the node directly the variable
+	 */
+	private boolean checkUndefined(IDiceASTNode astNode) {
+		if (astNode.getType() == DiceASTType.VARIABLE) {
+			VariableDiceNode node = (VariableDiceNode) astNode;
+
+			return !enviroment.containsKey(node.getVariable());
+		} else {
+			return false;
+		}
+	}
+}
\ No newline at end of file
diff --git a/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTExpression.java b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTExpression.java
index eaadc53..5fec31a 100644
--- a/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTExpression.java
+++ b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTExpression.java
@@ -9,8 +9,9 @@ import org.apache.commons.lang3.StringUtils;
 import bjc.dicelang.ComplexDice;
 import bjc.dicelang.CompoundDice;
 import bjc.dicelang.IDiceExpression;
-
+import bjc.utils.data.GenHolder;
 import bjc.utils.data.Pair;
+import bjc.utils.funcdata.bst.ITreePart.TreeLinearizationMethod;
 import bjc.utils.parserutils.AST;
 
 /**
@@ -100,67 +101,102 @@ public class DiceASTExpression implements IDiceExpression {
 				});
 
 		operatorCollapsers.put(OperatorDiceNode.ASSIGN, (left, right) -> {
-			return left.merge((leftValue, leftAST) -> {
-				return right.merge((rightValue, rightAST) -> {
-					String variableName = leftAST.collapse(
-							new VariableRetriever(), (operator) -> {
-								throw new UnsupportedOperationException(
-										"Can only assign to plain variable names. The problem operator is "
-												+ operator);
-							}, (returnedAST) -> returnedAST);
+			return parseBinding(enviroment, left, right);
+		});
+
+		operatorCollapsers.put(OperatorDiceNode.COMPOUND,
+				DiceASTExpression::parseCompound);
+
+		operatorCollapsers.put(OperatorDiceNode.GROUP,
+				DiceASTExpression::parseGroup);
+
+		return operatorCollapsers;
+	}
+
+	private static Pair<Integer, AST<IDiceASTNode>> parseBinding(
+			Map<String, DiceASTExpression> enviroment,
+			Pair<Integer, AST<IDiceASTNode>> left,
+			Pair<Integer, AST<IDiceASTNode>> right) {
+		return left.merge((leftValue, leftAST) -> {
+			return right.merge((rightValue, rightAST) -> {
+				String variableName = leftAST
+						.collapse(new VariableRetriever(), (operator) -> {
+							throw new UnsupportedOperationException(
+									"Can only assign to plain variable names. The problem operator is "
+											+ operator);
+						}, (returnedAST) -> returnedAST);
+
+				GenHolder<Boolean> selfReference = new GenHolder<>(false);
+
+				DiceASTReferenceChecker refChecker =
+						new DiceASTReferenceChecker(selfReference,
+								variableName);
+
+				rightAST.traverse(TreeLinearizationMethod.PREORDER,
+						refChecker);
+
+				// Ignore meta-variable that'll be auto-frozen to restore
+				// definition sanity
+				if (selfReference.unwrap((bool) -> bool)
+						&& !variableName.equals("last")) {
+					throw new UnsupportedOperationException(
+							"Variable '" + variableName
+									+ "' references itself. Problematic definition: \n\t"
+									+ rightAST);
+				}
 
+				if (!variableName.equals("last")) {
 					enviroment.put(variableName,
 							new DiceASTExpression(rightAST, enviroment));
+				} else {
+					// Do nothing, last is a auto-handled meta-variable
+				}
 
-					return new Pair<>(rightValue, new AST<>(
-							OperatorDiceNode.ASSIGN, leftAST, rightAST));
-				});
+				return new Pair<>(rightValue, new AST<>(
+						OperatorDiceNode.ASSIGN, leftAST, rightAST));
 			});
 		});
+	}
 
-		operatorCollapsers.put(OperatorDiceNode.COMPOUND,
-				(leftNode, rightNode) -> {
-					return leftNode.merge((leftValue, leftAST) -> {
-						return rightNode.merge((rightValue, rightAST) -> {
-							int compoundValue = Integer.parseInt(
-									Integer.toString(leftValue) + Integer
-											.toString(rightValue));
-
-							return new Pair<>(compoundValue,
-									new AST<>(OperatorDiceNode.COMPOUND,
-											leftAST, rightAST));
-						});
-					});
-				});
-
-		operatorCollapsers.put(OperatorDiceNode.GROUP,
-				(leftNode, rightNode) -> {
-					return leftNode.merge((leftValue, leftAST) -> {
-						return rightNode.merge((rightValue, rightAST) -> {
-							if (leftValue < 0) {
-								throw new UnsupportedOperationException(
-										"Can't attempt to roll a negative number of dice."
-												+ " The problematic AST is "
-												+ leftAST);
-							} else if (rightValue < 1) {
-								throw new UnsupportedOperationException(
-										"Can't roll dice with less than one side."
-												+ " The problematic AST is "
-												+ rightAST);
-							}
+	private static Pair<Integer, AST<IDiceASTNode>> parseCompound(
+			Pair<Integer, AST<IDiceASTNode>> leftNode,
+			Pair<Integer, AST<IDiceASTNode>> rightNode) {
+		return leftNode.merge((leftValue, leftAST) -> {
+			return rightNode.merge((rightValue, rightAST) -> {
+				int compoundValue =
+						Integer.parseInt(Integer.toString(leftValue)
+								+ Integer.toString(rightValue));
+
+				return new Pair<>(compoundValue, new AST<>(
+						OperatorDiceNode.COMPOUND, leftAST, rightAST));
+			});
+		});
+	}
 
-							int rolledValue =
-									new ComplexDice(leftValue, rightValue)
-											.roll();
+	private static Pair<Integer, AST<IDiceASTNode>> parseGroup(
+			Pair<Integer, AST<IDiceASTNode>> leftNode,
+			Pair<Integer, AST<IDiceASTNode>> rightNode) {
+		return leftNode.merge((leftValue, leftAST) -> {
+			return rightNode.merge((rightValue, rightAST) -> {
+				if (leftValue < 0) {
+					throw new UnsupportedOperationException(
+							"Can't attempt to roll a negative number of dice."
+									+ " The problematic AST is "
+									+ leftAST);
+				} else if (rightValue < 1) {
+					throw new UnsupportedOperationException(
+							"Can't roll dice with less than one side."
+									+ " The problematic AST is "
+									+ rightAST);
+				}
 
-							return new Pair<>(rolledValue,
-									new AST<>(OperatorDiceNode.GROUP,
-											leftAST, rightAST));
-						});
-					});
-				});
+				int rolledValue =
+						new ComplexDice(leftValue, rightValue).roll();
 
-		return operatorCollapsers;
+				return new Pair<>(rolledValue, new AST<>(
+						OperatorDiceNode.GROUP, leftAST, rightAST));
+			});
+		});
 	}
 
 	/**
@@ -197,49 +233,58 @@ public class DiceASTExpression implements IDiceExpression {
 	 */
 	private Pair<Integer, AST<IDiceASTNode>> evalLeaf(IDiceASTNode tokn) {
 		if (tokn.getType() == DiceASTType.VARIABLE) {
-			String varName = ((VariableDiceNode) tokn).getVariable();
-
-			if (env.containsKey(varName)) {
-				return new Pair<>(env.get(varName).roll(),
-						new AST<>(tokn));
-			} else {
-				// Handle special case for defining variables
-				return new Pair<>(0, new AST<>(tokn));
-			}
+			return parseVariable(tokn);
 		} else if (tokn.getType() == DiceASTType.LITERAL) {
-			LiteralDiceNode literalNode = (LiteralDiceNode) tokn;
-			String dat = literalNode.getData();
-
-			if (StringUtils.countMatches(dat, 'c') == 1
-					&& !dat.equalsIgnoreCase("c")
-					&& !dat.startsWith("c")) {
-				String[] strangs = dat.split("c");
-
-				return new Pair<>(new CompoundDice(strangs).roll(),
-						new AST<>(tokn));
-			} else if (StringUtils.countMatches(dat, 'd') == 1
-					&& !dat.equalsIgnoreCase("d")
-					&& !dat.startsWith("d")) {
-				/*
-				 * Handle dice groups
-				 */
-				return new Pair<>(ComplexDice.fromString(dat).roll(),
-						new AST<>(tokn));
-			} else {
-				try {
-					return new Pair<>(Integer.parseInt(dat),
-							new AST<>(tokn));
-				} catch (NumberFormatException nfex) {
-					throw new UnsupportedOperationException(
-							"Found malformed leaf token " + tokn);
-				}
-			}
+			return parseLiteral(tokn);
 		} else {
 			throw new UnsupportedOperationException("Found leaf operator "
 					+ tokn + ". These aren't supported.");
 		}
 	}
 
+	private static Pair<Integer, AST<IDiceASTNode>>
+			parseLiteral(IDiceASTNode tokn) {
+		LiteralDiceNode literalNode = (LiteralDiceNode) tokn;
+		String dat = literalNode.getData();
+
+		if (isValidInfixOperator(dat, "c")) {
+			String[] strangs = dat.split("c");
+
+			return new Pair<>(new CompoundDice(strangs).roll(),
+					new AST<>(tokn));
+		} else if (isValidInfixOperator(dat, "d")) {
+			/*
+			 * Handle dice groups
+			 */
+			return new Pair<>(ComplexDice.fromString(dat).roll(),
+					new AST<>(tokn));
+		} else {
+			try {
+				return new Pair<>(Integer.parseInt(dat), new AST<>(tokn));
+			} catch (NumberFormatException nfex) {
+				throw new UnsupportedOperationException(
+						"Found malformed leaf token " + tokn);
+			}
+		}
+	}
+
+	private static boolean isValidInfixOperator(String dat, String op) {
+		return StringUtils.countMatches(dat, op) == 1
+				&& !dat.equalsIgnoreCase(op) && !dat.startsWith(op);
+	}
+
+	private Pair<Integer, AST<IDiceASTNode>>
+			parseVariable(IDiceASTNode tokn) {
+		String varName = ((VariableDiceNode) tokn).getVariable();
+
+		if (env.containsKey(varName)) {
+			return new Pair<>(env.get(varName).roll(), new AST<>(tokn));
+		} else {
+			// Handle special case for defining variables
+			return new Pair<>(0, new AST<>(tokn));
+		}
+	}
+
 	/**
 	 * Get the AST bound to this expression
 	 * 
diff --git a/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTFreezer.java b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTFreezer.java
index bad24f8..90b4d11 100644
--- a/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTFreezer.java
+++ b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTFreezer.java
@@ -53,6 +53,7 @@ public class DiceASTFreezer {
 				FunctionalMap<String, AST<IDiceASTNode>> env,
 				FunctionalList<String> varNames) {
 			super(env);
+			
 			variableNames = varNames;
 		}
 
diff --git a/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTReferenceChecker.java b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTReferenceChecker.java
index a74d61e..09d3d13 100644
--- a/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTReferenceChecker.java
+++ b/dice-lang/src/main/java/bjc/dicelang/ast/DiceASTReferenceChecker.java
@@ -2,7 +2,7 @@ package bjc.dicelang.ast;
 
 import java.util.function.Consumer;
 
-import bjc.utils.data.GenHolder;
+import bjc.utils.data.IHolder;
 
 /**
  * Check if the specified node references a particular variable
@@ -15,7 +15,7 @@ public final class DiceASTReferenceChecker
 	/**
 	 * This is true if the specified node references the set variable
 	 */
-	private GenHolder<Boolean>	referencesVariable;
+	private IHolder<Boolean>	referencesVariable;
 
 	private String				varName;
 
@@ -27,7 +27,7 @@ public final class DiceASTReferenceChecker
 	 * @param varName
 	 *            The variable to check for references in
 	 */
-	public DiceASTReferenceChecker(GenHolder<Boolean> referencesVar,
+	public DiceASTReferenceChecker(IHolder<Boolean> referencesVar,
 			String varName) {
 		this.referencesVariable = referencesVar;
 		this.varName = varName;
@@ -35,11 +35,7 @@ public final class DiceASTReferenceChecker
 
 	@Override
 	public void accept(IDiceASTNode astNode) {
-		if (!referencesVariable.unwrap(bool -> bool)) {
-			if (isDirectReference(astNode)) {
-				referencesVariable.transform((bool) -> false);
-			}
-		}
+		referencesVariable.transform((bool) -> isDirectReference(astNode));
 	}
 
 	/**