Added commutative simplification for + and *

app/imports/parser/parseTree/operator.js

@@ -1,8 +1,12 @@
 import resolve, { toString, traverse, map } from '../resolve.js';
 import constant from './constant.js';
 
+// Which operators can be considered commutative by the parser
+// i.e. 1 + 2 + 3 === 2 + 3 + 1
+const commutativeOperators = ['+', '*']
+
 const operator = {
-  create({left, right, operator, fn}) {
+  create({ left, right, operator, fn }) {
     return {
       parseType: 'operator',
       left,
@@ -11,17 +15,21 @@ const operator = {
       fn
     };
   },
-  resolve(fn, node, scope, context){
+  resolve(fn, node, scope, context) {
-    const {result: leftNode} = resolve(fn, node.left, scope, context);
+    const { result: leftNode } = resolve(fn, node.left, scope, context);
-    const {result: rightNode} = resolve(fn, node.right, scope, context);
+    const { result: rightNode } = resolve(fn, node.right, scope, context);
     let left, right;
-    if (leftNode.parseType !== 'constant' || rightNode.parseType !== 'constant'){
+
+    // If commutation is possible, do it and return that result
+    const commutatedResult = reorderCommutativeOperations(node, leftNode, rightNode);
+    if (commutatedResult) return { result: commutatedResult, context };
+
+    if (leftNode.parseType !== 'constant' || rightNode.parseType !== 'constant') {
       return {
         result: operator.create({
           left: leftNode,
           right: rightNode,
           operator: node.operator,
-          fn: node.fn,
         }),
         context,
       };
@@ -29,28 +37,7 @@ const operator = {
       left = leftNode.value;
       right = rightNode.value;
     }
-    let result;
+    const result = applyOperator(node.operator, left, right);
-    switch(node.operator){
-      case '+': result = left + right; break;
-      case '-': result = left - right; break;
-      case '*': result = left * right; break;
-      case '/': result = left / right; break;
-      case '^': result = Math.pow(left, right); break;
-      case '%': result = left % right; break;
-      case '&':
-      case '&&': result = left && right; break;
-      case '|':
-      case '||': result = left || right; break;
-      case '=':
-      case '==':  result = left == right; break;
-      case '===': result = left === right; break;
-      case '!=':  result = left != right; break;
-      case '!==': result = left !== right; break;
-      case '>': result = left > right; break;
-      case '<': result = left < right; break;
-      case '>=': result = left >= right; break;
-      case '<=': result = left <= right; break;
-    }
     return {
       result: constant.create({
         value: result,
@@ -58,22 +45,22 @@ const operator = {
       context,
     };
   },
-  toString(node){
+  toString(node) {
-    let {left, right, operator} = node;
+    let { left, right, operator } = node;
     // special case of adding a negative number
-    if (operator === '+' && right.valueType === 'number' && right.value < 0){
+    if (operator === '+' && right.valueType === 'number' && right.value < 0) {
       return `${toString(left)} - ${-right.value}`
     }
     return `${toString(left)} ${operator} ${toString(right)}`;
   },
-  traverse(node, fn){
+  traverse(node, fn) {
     fn(node);
     traverse(node.left, fn);
     traverse(node.right, fn);
   },
-  map(node, fn){
+  map(node, fn) {
     const resultingNode = fn(node);
-    if (resultingNode === node){
+    if (resultingNode === node) {
       node.left = map(node.left, fn);
       node.right = map(node.right, fn);
     }
@@ -81,4 +68,77 @@ const operator = {
   },
 }
 
+function applyOperator(operator, left, right) {
+  let result;
+  switch (operator) {
+    case '+': result = left + right; break;
+    case '-': result = left - right; break;
+    case '*': result = left * right; break;
+    case '/': result = left / right; break;
+    case '^': result = Math.pow(left, right); break;
+    case '%': result = left % right; break;
+    case '&':
+    case '&&': result = left && right; break;
+    case '|':
+    case '||': result = left || right; break;
+    case '=':
+    case '==': result = left == right; break;
+    case '===': result = left === right; break;
+    case '!=': result = left != right; break;
+    case '!==': result = left !== right; break;
+    case '>': result = left > right; break;
+    case '<': result = left < right; break;
+    case '>=': result = left >= right; break;
+    case '<=': result = left <= right; break;
+  }
+  return result;
+}
+
+function reorderCommutativeOperations(node, leftNode, rightNode) {
+  // Make sure the operator is commutative
+  if (!commutativeOperators.includes(node.operator)) return;
+
+  // Find the case where one side is constant and the other is an identical operator
+  if (leftNode.parseType === 'constant' && rightNode.parseType === 'constant') return;
+  let constantNode, operatorNode, opConstant, opOther;
+  if (
+    rightNode.parseType == 'constant'
+    && leftNode.parseType === 'operator'
+    && leftNode.operator === node.operator
+  ) {
+    constantNode = rightNode;
+    operatorNode = leftNode;
+  } else if (
+    leftNode.parseType == 'constant'
+    && rightNode.parseType === 'operator'
+    && rightNode.operator === node.operator
+  ) {
+    constantNode = leftNode;
+    operatorNode = rightNode;
+  } else {
+    return;
+  }
+
+  // One of the sub nodes of the operator side must be constant
+  if (operatorNode.left.parseType === 'constant') {
+    opConstant = operatorNode.left;
+    opOther = operatorNode.right;
+  } else if (operatorNode.right.parseType === 'constant') {
+    opConstant = operatorNode.right;
+    opOther = operatorNode.left;
+  } else {
+    return;
+  }
+
+  // Apply the operator to the two constant nodes
+  const result = applyOperator(node.operator, constantNode.value, opConstant.value);
+  return operator.create({
+    left: opOther,
+    right: constant.create({
+      value: result,
+    }),
+    operator: node.operator,
+  });
+}
+
 export default operator;

app/imports/parser/parser.test.js

@@ -1,11 +1,26 @@
 import { parse } from './parser';
+import resolve, { toString } from './resolve';
 import { assert } from 'chai';
 
-describe('Parser', function(){
+describe('Parser', function () {
-  it('parses valid text without error', function(){
+  it('parses valid text without error', function () {
     assert.typeOf(parse('1'), 'object');
   });
-  it('parses various operations', function(){
+  it('parses various operations', function () {
     assert.typeOf(parse('1 + 2 * 3 / 4 * 1d8'), 'object');
   });
+  it('simplifies basic addition and multiplication', function () {
+    let add = parse('1 + 3 + 3 + 4');
+    ({ result: add } = resolve('compile', add));
+    assert.equal(toString(add), '11');
+
+    let mul = parse('2 * 3 * 4');
+    ({ result: mul } = resolve('compile', mul));
+    assert.equal(toString(mul), '24');
+  });
+  it('simplifies addition when possible, even if a roll is in the way', function () {
+    let { result } = resolve('compile', parse('1 + 3 + d12 + 3 + 4'));
+    console.log(result);
+    assert.equal(toString(result), 'd12 + 11');
+  });
 });

app/imports/parser/resolve.js

@@ -23,8 +23,11 @@ export default function resolve(fn, node, scope, context = new Context()) {
 
 export function toString(node) {
   if (!node) return '';
+  if (!node.parseType) {
+    throw new Meteor.Error(`Node does not have a parseType defined, node is type ${typeof node} with parseType ${node.parseType}`)
+  }
   let type = nodeTypeIndex[node.parseType];
-  if (!type.toString) {
+  if (!type?.toString) {
     throw new Meteor.Error('toString not implemented on ' + node.parseType);
   }
   return type.toString(node);