Code maintenance and fixes

6 files changed, 285 insertions, 123 deletions

diff --git a/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguagePragmaHandlers.java b/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguagePragmaHandlers.java
index 3eaf6b0..a4d6e54 100644
--- a/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguagePragmaHandlers.java
+++ b/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguagePragmaHandlers.java
@@ -77,5 +77,4 @@ public class DiceASTLanguagePragmaHandlers {
 							+ boundExpression.toString()));
 		}
 	}
-
-}
+}
\ No newline at end of file
diff --git a/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguageTest.java b/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguageTest.java
index a21ed66..6624231 100644
--- a/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguageTest.java
+++ b/dice-lang/src/examples/java/bjc/dicelang/examples/DiceASTLanguageTest.java
@@ -6,6 +6,7 @@ import java.util.Scanner;
 import java.util.function.BiConsumer;
 
 import bjc.dicelang.IDiceExpression;
+import bjc.dicelang.ast.DiceASTDefinedChecker;
 import bjc.dicelang.ast.DiceASTExpression;
 import bjc.dicelang.ast.DiceASTFreezer;
 import bjc.dicelang.ast.DiceASTParser;
@@ -15,6 +16,7 @@ import bjc.dicelang.ast.IDiceASTNode;
 import static bjc.dicelang.examples.DiceASTLanguagePragmaHandlers.*;
 
 import bjc.utils.data.GenHolder;
+import bjc.utils.data.IHolder;
 import bjc.utils.funcdata.FunctionalMap;
 import bjc.utils.funcdata.bst.ITreePart.TreeLinearizationMethod;
 import bjc.utils.parserutils.AST;
@@ -182,43 +184,61 @@ public class DiceASTLanguageTest {
 			} else {
 
 				// Build an AST from the string expression
-				AST<IDiceASTNode> builtAST =
-						astParser.buildAST(currentLine);
+				AST<IDiceASTNode> builtAST;
+
+				try {
+					builtAST = astParser.buildAST(currentLine);
+				} catch (IllegalStateException isex) {
+					System.out.println(
+							"ERROR: " + isex.getLocalizedMessage());
+
+					currentLine =
+							getNextCommand(inputSource, commandNumber);
+
+					continue;
+				}
 
 				// Build a rollable expression from the AST
 				DiceASTExpression expression =
 						new DiceASTExpression(builtAST, enviroment);
 
+				int sampleRoll;
+
+				try {
+					sampleRoll = expression.roll();
+				} catch (UnsupportedOperationException usex) {
+					System.out.println(
+							"ERROR: " + usex.getLocalizedMessage());
+
+					currentLine =
+							getNextCommand(inputSource, commandNumber);
+
+					continue;
+				}
+
+				if (checkUndefined(enviroment, expression)) {
+					System.out.println(
+							"ERROR: Expression contains undefined variables."
+									+ " Problematic expression: \n\t"
+									+ expression);
+
+					currentLine =
+							getNextCommand(inputSource, commandNumber);
+
+					continue;
+				}
+
+				expression = handleUpdateLast(enviroment, expression);
+
 				// Print out results
 				System.out.println("\tParsed: " + expression.toString());
-				System.out
-						.println("\t\tSample Roll: " + expression.roll());
-
-				// Assume we can update last by default
-				GenHolder<Boolean> canUpdateLast = new GenHolder<>(true);
-
-				// Check that no node references last
-				expression.getAst().traverse(
-						TreeLinearizationMethod.PREORDER,
-						new DiceASTReferenceChecker(canUpdateLast,
-								"last"));
-
-				// Update last if we can
-				if (canUpdateLast.unwrap((flag) -> flag)) {
-					enviroment.put("last", expression);
-				} else {
-					// We need to freeze out references to last
-					enviroment.put("last",
-							freezeOutLast(enviroment, builtAST));
-				}
+				System.out.println("\t\tSample Roll: " + sampleRoll);
 			}
 
 			// Increase the number of commands
 			commandNumber++;
 
-			// Read a new command
-			System.out.print("dice-lang-" + commandNumber + "> ");
-			currentLine = inputSource.nextLine();
+			currentLine = getNextCommand(inputSource, commandNumber);
 		}
 
 		System.out.println("Bye.");
@@ -227,6 +247,49 @@ public class DiceASTLanguageTest {
 		inputSource.close();
 	}
 
+	private static DiceASTExpression handleUpdateLast(
+			Map<String, DiceASTExpression> enviroment,
+			DiceASTExpression expression) {
+		IHolder<Boolean> canUpdateLast = new GenHolder<>(true);
+
+		// Check that no node references last
+		expression.getAst().traverse(TreeLinearizationMethod.PREORDER,
+				new DiceASTReferenceChecker(canUpdateLast, "last"));
+
+		// Update last if we can
+		if (canUpdateLast.unwrap((flag) -> flag)) {
+			enviroment.put("last", expression);
+		} else {
+			// We need to freeze out references to last
+			expression = freezeOutLast(enviroment, expression.getAst());
+
+			enviroment.put("last", expression);
+		}
+
+		return expression;
+	}
+
+	private static boolean checkUndefined(
+			Map<String, DiceASTExpression> enviroment,
+			DiceASTExpression expression) {
+		IHolder<Boolean> containsUndefined = new GenHolder<>(false);
+
+		// Check that no node references last
+		expression.getAst().traverse(TreeLinearizationMethod.PREORDER,
+				new DiceASTDefinedChecker(containsUndefined, enviroment));
+
+		return containsUndefined.unwrap((bool) -> bool);
+	}
+
+	private static String getNextCommand(Scanner inputSource,
+			int commandNumber) {
+		String currentLine;
+		// Read a new command
+		System.out.print("dice-lang-" + commandNumber + "> ");
+		currentLine = inputSource.nextLine();
+		return currentLine;
+	}
+
 	private static DiceASTExpression freezeOutLast(
 			Map<String, DiceASTExpression> enviroment,
 			AST<IDiceASTNode> builtAST) {
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));
 	}
 
 	/**